[MLIR][ODS] Re-enable direct implementation of type interfaces with method bodies

[andidr]

Since commit 842622b adding support for overloading interface methods, a using directive is emitted for any interface method that does not require emission of a trait method, including for methods that define a method body.

However, methods directly specifying a body (e.g., via the methodBody parameter of InterfaceMethod) are implemented directly in the interface class and are therefore not present in the associated trait. The generated using directive then referes to a non-existent method of the trait, resulting in an error upon compilation of the generated code.

This patch changes DefGen::emitTraitMethods(), such that genTraitMethodUsingDecl() is not invoked for interface methods with a body anymore.

[llvmbot]

@llvm/pr-subscribers-mlir-core

@llvm/pr-subscribers-mlir

Author: Andi Drebes (andidr)

Changes

Since commit 842622b adding support for overloading interface methods, a using directive is emitted for any interface method that does not require emission of a trait method, including for methods that define a method body.

However, methods directly specifying a body (e.g., via the methodBody parameter of InterfaceMethod) are implemented directly in the interface class and are therefore not present in the associated trait. The generated using directive then referes to a non-existent method of the trait, resulting in an error upon compilation of the generated code.

This patch changes DefGen::emitTraitMethods(), such that genTraitMethodUsingDecl() is not invoked for interface methods with a body anymore.

---

Full diff: https://github.com/llvm/llvm-project/pull/166335.diff

2 Files Affected:

• (modified) mlir/test/lib/Dialect/Test/TestTypeDefs.td (+7)
• (modified) mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp (+6-3)

diff --git a/mlir/test/lib/Dialect/Test/TestTypeDefs.td b/mlir/test/lib/Dialect/Test/TestTypeDefs.td
index ea20597231d58..9859bd06cb526 100644
--- a/mlir/test/lib/Dialect/Test/TestTypeDefs.td
+++ b/mlir/test/lib/Dialect/Test/TestTypeDefs.td
@@ -470,4 +470,11 @@ def TestMemrefType : Test_Type<"TestMemref",
   }];
 }
 
+// Test implementation of an interface with methods specifying a
+// method body
+def TestBaseBody : Test_Type<"TestBaseBody",
+    [DeclareTypeInterfaceMethods<TestBaseTypeInterfacePrintTypeA>]> {
+  let mnemonic = "test_base_body";
+}
+
 #endif // TEST_TYPEDEFS
diff --git a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
index 8ec2e03095423..b87a3247d2965 100644
--- a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
+++ b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
@@ -637,12 +637,15 @@ void DefGen::emitTraitMethods(const InterfaceTrait &trait) {
   for (auto &method : iface.getMethods()) {
     // Don't declare if the method has a body. Or if the method has a default
     // implementation and the def didn't request that it always be declared.
-    if (method.getBody() || (method.getDefaultImplementation() &&
-                             !alwaysDeclared.count(method.getName()))) {
+    if (method.getBody()) {
+      continue;
+    } else if (method.getDefaultImplementation() &&
+               !alwaysDeclared.count(method.getName())) {
       genTraitMethodUsingDecl(trait, method);
       continue;
+    } else {
+      emitTraitMethod(method);
     }
-    emitTraitMethod(method);
   }
 }
 

[jpienaar]

Nice, thanks