Operation interfaces: can't name the interface method same as free function on op

[bondhugula]

With the way tablegen based operation interfaces are generated
https://github.com/tensorflow/mlir/blob/master/g3doc/OpDefinitions.md#operation-interfaces
it doesn't appear to be possible to use the same name for an interface method and a free function that it calls taking the concrete op as input. The latter would resolve to an internal op interface generated method from implicit conversion of the concrete op to Operation *, and thus lead to an infinite recursion. Normally, one would like to use the same name. For example, consider:

InterfaceMethod<"/*insert doc here*/",
      "unsigned", "computeSomeProperty", (ins), [{
        return computeSomeProperty(op);
    }]>,

computeSomeProperty(op) will resolve to an internally generated method:

unsigned computeSomeProperty(Operation *tablegen_opaque_op) final 

instead of the intended mlir::computeSomeProperty(ConcreteOpType opType);

[antiagainst]

Isn't this a C++ symbol resolution issue? Does using fully qualified symbol name help?

[bondhugula]

Isn't this a C++ symbol resolution issue? Does using fully qualified symbol name help?

The different ops that use the op interface typically reside in different dialects and sometimes in different namespaces (for eg. mlir::loop::ForOp and mlir::AffineForOp). There isn't a single fully qualified symbol name that can be used.

[River707]

This is c++ visibility rules, it has nothing to do with interfaces. What you are encountering is essentially:

unsigned computeSomeProperty(Operation *opaque_op) {
auto op = cast(opaque_op);
return computeSomeProperty(op);
}

You will have to qualify the body as you in c++. Why can't you just use mlir::computeSomeProperty if that is what you want? The place that the call is resolved is where ever the interface header is included(just like any other template).

[bondhugula]

This is c++ visibility rules, it has nothing to do with interfaces. What you are encountering is essentially:

unsigned computeSomeProperty(Operation *opaque_op) {
auto op = cast(opaque_op);
return computeSomeProperty(op);
}

You will have to qualify the body as you in c++. Why can't you just use mlir::computeSomeProperty if that is what you want? The place that the call is resolved is where ever the interface header is included(just like any other template).

Have you already seen post #3 above? computeSomeProperty may not be in the namespace mlir for all the ops that use this interface (for eg. ForOp and AffineForOp live in different namespaces.)

[River707]

I don't understand why that makes a difference? There is only one point of resolution for all ops if you define a body. Are you intending to rely on argument dependent lookup?

[bondhugula]

I don't understand why that makes a difference? There is only one point of resolution for all ops if you define a body. Are you intending to rely on argument dependent lookup?

If you use mlir::computeSomeProperty(op), it will not resolve for another dialect op which would have needed mlir::dialectB::computeSomeProperty(op). As a concrete example, consider mlir::getConstantTripCount(AffineForOp) and mlir::loop::getConstantTripCount(ForOp). Using an op interface method with name getConstantTripCount won't work, but changing its name to let's say getTripCountConstant will work because in the latter case, you won't need the fully qualified name when referencing in the body of the interface method, and it would correctly resolve for all ops at the point of use. Am I missing something?

[River707]

I just don't see how this is a problem with interfaces. This is a general c++(any programming language) visibility thing.

[bondhugula]

I just don't see how this is a problem with interfaces. This is a general c++(any programming language) visibility thing.

But the issue exists because the internal helper method generated by op interface gen has the same name as the user-specified interface method. A user is stuck if the ops using the interface lie in different namespaces (their free functions would be in different namespaces and so there is no unique fully qualified name to use). This issue is resolved for example if a suffix of _ is added to the internal auto generated helper (the one that takes "Operation *tablegen_opaque_op, ...") as args. There is no reason to specifically choose the same name as the user-specified ODS interface method name for the generated helper.

unsigned methodName_(Operation *tablegen_opaque_op)

As a result, one is able to now use a free function with the same name as the interface method name even when those free functions live in different namespaces for the various ops using the op interface.

This is just a two line fix I can include in the related PR. Perhaps the PR will make it clearer.