[TableGen] OpInterface inheritance duplicates bases

[darthscsi]

Summary

OpInterface inheritance will duplicate base interfaces, causing compilation failure. Unique the set of base interfaces.

[jdd]

This looks good to me, though I don't know much about the tblgen interface system.

[rriddle]

Looks good to me

mlir/lib/TableGen/Interfaces.cpp

88

I'd expect that to be the default.

[darthscsi]

StringSet change

mlir/lib/TableGen/Interfaces.cpp

 //===- Interfaces.cpp - Interface classes ---------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "mlir/TableGen/Interfaces.h"
 #include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 
 using namespace mlir;
 using namespace mlir::tblgen;
 
 //===----------------------------------------------------------------------===//
Interface::Interface(const llvm::Record *def) : def(def) {
   // Initialize the interface methods.
   auto *listInit = dyn_cast<llvm::ListInit>(def->getValueInit("methods"));
   for (llvm::Init *init : listInit->getValues())
     methods.emplace_back(cast<llvm::DefInit>(init)->getDef());
 
   // Initialize the interface base classes.
   auto *basesInit =
       dyn_cast<llvm::ListInit>(def->getValueInit("baseInterfaces"));
+  // Chained inheritance will produce duplicates in the base interface set.
+  StringSet<> basesAdded;
   llvm::unique_function<void(Interface)> addBaseInterfaceFn =
       [&](const Interface &baseInterface) {
         // Inherit any base interfaces.
         for (const auto &baseBaseInterface : baseInterface.getBaseInterfaces())
           addBaseInterfaceFn(baseBaseInterface);
 
         // Add the base interface.
+        if (basesAdded.contains(baseInterface.getName()))
+          return;
         baseInterfaces.push_back(std::make_unique<Interface>(baseInterface));
+        basesAdded.insert(baseInterface.getName());
       };
   for (llvm::Init *init : basesInit->getValues())
     addBaseInterfaceFn(Interface(cast<llvm::DefInit>(init)->getDef()));
 }
 
 // Return the name of this interface.
 StringRef Interface::getName() const {
   return def->getValueAsString("cppInterfaceName");

mlir/test/mlir-tblgen/op-interface.td

 // DECL-NEXT:   return (*this).someOtherMethod();
 // DECL-NEXT: }
 
 // DECL: struct ExtraShardDeclsInterfaceTrait
 // DECL:      bool sharedMethodDeclaration() {
 // DECL-NEXT:   return (*static_cast<ConcreteOp *>(this)).someOtherMethod();
 // DECL-NEXT: }
 
-def TestInheritanceBaseInterface : OpInterface<"TestInheritanceBaseInterface"> {
+def TestInheritanceMultiBaseInterface : OpInterface<"TestInheritanceMultiBaseInterface"> {
+  let methods = [
+    InterfaceMethod<
+      /*desc=*/[{some function comment}],
+      /*retTy=*/"int",
+      /*methodName=*/"baz",
+      /*args=*/(ins "int":$input)
+    >
+  ];
+}
+
+def TestInheritanceBaseInterface : OpInterface<"TestInheritanceBaseInterface", [TestInheritanceMultiBaseInterface]> {
   let methods = [
     InterfaceMethod<
       /*desc=*/[{some function comment}],
       /*retTy=*/"int",
       /*methodName=*/"foo",
       /*args=*/(ins "int":$input)
     >
   ];
let methods = [
     >
   ];
 }
 def TestInheritanceZDerivedInterface
   : OpInterface<"TestInheritanceZDerivedInterface", [TestInheritanceMiddleBaseInterface]>;
 
 // DECL: class TestInheritanceZDerivedInterface
 // DECL: struct Concept {
+// DECL:     const TestInheritanceMultiBaseInterface::Concept *implTestInheritanceMultiBaseInterface = nullptr;
+// DECL-NOT: const TestInheritanceMultiBaseInterface::Concept
 // DECL:     const TestInheritanceBaseInterface::Concept *implTestInheritanceBaseInterface = nullptr;
 // DECL:     const TestInheritanceMiddleBaseInterface::Concept *implTestInheritanceMiddleBaseInterface = nullptr;
 
 // DECL:     void initializeInterfaceConcept(::mlir::detail::InterfaceMap &interfaceMap) {
 // DECL:       implTestInheritanceBaseInterface = interfaceMap.lookup<TestInheritanceBaseInterface>();
 // DECL:       assert(implTestInheritanceBaseInterface && "`TestInheritanceZDerivedInterface` expected its base interface `TestInheritanceBaseInterface` to be registered");
 // DECL:       implTestInheritanceMiddleBaseInterface = interfaceMap.lookup<TestInheritanceMiddleBaseInterface>();
 // DECL:       assert(implTestInheritanceMiddleBaseInterface