[Mlir-commits] [mlir] 0815281 - [MLIR] Fix autogenerated pass constructors linkage

Michele Scuttari llvmlistbot at llvm.org
Mon Aug 29 11:54:07 PDT 2022


Author: Michele Scuttari
Date: 2022-08-29T20:53:42+02:00
New Revision: 0815281ff286def360589a97611fecab2f64ff3d

URL: https://github.com/llvm/llvm-project/commit/0815281ff286def360589a97611fecab2f64ff3d
DIFF: https://github.com/llvm/llvm-project/commit/0815281ff286def360589a97611fecab2f64ff3d.diff

LOG: [MLIR] Fix autogenerated pass constructors linkage

The patch addresses the linkage of the new autogenerated pass constructors, which, being declared as friend functions, resulted in having an inline nature and thus their implementations not being exported.

Reviewd By: mehdi_amini, rriddle

Differential Revision: https://reviews.llvm.org/D132572

Added: 
    

Modified: 
    mlir/docs/PassManagement.md
    mlir/tools/mlir-tblgen/PassGen.cpp
    mlir/unittests/TableGen/PassGenTest.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/docs/PassManagement.md b/mlir/docs/PassManagement.md
index a09ca328c4d02..e92de3cb592c4 100644
--- a/mlir/docs/PassManagement.md
+++ b/mlir/docs/PassManagement.md
@@ -810,7 +810,8 @@ def MyPass : Pass<"my-pass", "ModuleOp"> {
   }];
 
   // A constructor must be provided to specify how to create a default instance
-  // of MyPass.
+  // of MyPass. It can be skipped for this specific example, because both the
+  // constructor and the registration methods live in the same namespace.
   let constructor = "foo::createMyPass()";
 
   // Specify any options.
@@ -842,17 +843,23 @@ generates a `registerGroupPasses`, where `Group` is the tag provided via the
 `-name` input parameter, that registers all of the passes present.
 
 ```c++
-// gen-pass-decls -name="Example"
+// Tablegen options: -gen-pass-decls -name="Example"
 
+// Passes.h
+
+namespace foo {
 #define GEN_PASS_REGISTRATION
 #include "Passes.h.inc"
+} // namespace foo
 
 void registerMyPasses() {
   // Register all of the passes.
-  registerExamplePasses();
+  foo::registerExamplePasses();
+  
+  // Or
 
   // Register `MyPass` specifically.
-  registerMyPass();
+  foo::registerMyPass();
 }
 ```
 
@@ -900,20 +907,22 @@ implementation.
 
 It generates a base class for each of the passes, containing most of the boiler
 plate related to pass definitions. These classes are named in the form of
-`MyPassBase`, where `MyPass` is the name of the pass definition in tablegen. We
-can update the original C++ pass definition as so:
+`MyPassBase` and are declared inside the `impl` namespace, where `MyPass` is
+the name of the pass definition in tablegen. We can update the original C++
+pass definition as so:
 
 ```c++
+// MyPass.cpp
+
 /// Include the generated base pass class definitions.
+namespace foo {
 #define GEN_PASS_DEF_MYPASS
 #include "Passes.cpp.inc"
+}
 
 /// Define the main class as deriving from the generated base class.
-struct MyPass : MyPassBase<MyPass> {
-  /// The explicit constructor is no longer explicitly necessary when defining
-  /// pass options and statistics, the base class takes care of that
-  /// automatically.
-  ...
+struct MyPass : foo::impl::MyPassBase<MyPass> {
+  using MyPassBase::MyPassBase;
 
   /// The definitions of the options and statistics are now generated within
   /// the base class, but are accessible in the same way.

diff  --git a/mlir/tools/mlir-tblgen/PassGen.cpp b/mlir/tools/mlir-tblgen/PassGen.cpp
index f4a9d5b9993bc..65159559206eb 100644
--- a/mlir/tools/mlir-tblgen/PassGen.cpp
+++ b/mlir/tools/mlir-tblgen/PassGen.cpp
@@ -192,7 +192,7 @@ static void emitDecls(const llvm::RecordKeeper &recordKeeper, raw_ostream &os) {
 /// {1}: The base class for the pass.
 /// {2): The command line argument for the pass.
 /// {3}: The dependent dialects registration.
-const char *const passDeclBegin = R"(
+const char *const baseClassBegin = R"(
 template <typename DerivedT>
 class {0}Base : public {1} {
 public:
@@ -243,18 +243,42 @@ const char *const dialectRegistrationTemplate = R"(
   registry.insert<{0}>();
 )";
 
-const char *const friendDefaultConstructorTemplate = R"(
+const char *const friendDefaultConstructorDeclTemplate = R"(
+namespace impl {{
+  std::unique_ptr<::mlir::Pass> create{0}();
+} // namespace impl
+)";
+
+const char *const friendDefaultConstructorWithOptionsDeclTemplate = R"(
+namespace impl {{
+  std::unique_ptr<::mlir::Pass> create{0}(const {0}Options &options);
+} // namespace impl
+)";
+
+const char *const friendDefaultConstructorDefTemplate = R"(
   friend std::unique_ptr<::mlir::Pass> create{0}() {{
     return std::make_unique<DerivedT>();
   }
 )";
 
-const char *const friendDefaultConstructorWithOptionsTemplate = R"(
+const char *const friendDefaultConstructorWithOptionsDefTemplate = R"(
   friend std::unique_ptr<::mlir::Pass> create{0}(const {0}Options &options) {{
     return std::make_unique<DerivedT>(options);
   }
 )";
 
+const char *const defaultConstructorDefTemplate = R"(
+std::unique_ptr<::mlir::Pass> create{0}() {{
+  return impl::create{0}();
+}
+)";
+
+const char *const defaultConstructorWithOptionsDefTemplate = R"(
+std::unique_ptr<::mlir::Pass> create{0}(const {0}Options &options) {{
+  return impl::create{0}(options);
+}
+)";
+
 /// Emit the declarations for each of the pass options.
 static void emitPassOptionDecls(const Pass &pass, raw_ostream &os) {
   for (const PassOption &opt : pass.getOptions()) {
@@ -285,10 +309,20 @@ static void emitPassStatisticDecls(const Pass &pass, raw_ostream &os) {
 static void emitPassDefs(const Pass &pass, raw_ostream &os) {
   StringRef passName = pass.getDef()->getName();
   std::string enableVarName = "GEN_PASS_DEF_" + passName.upper();
+  bool emitDefaultConstructors = pass.getConstructor().empty();
+  bool emitDefaultConstructorWithOptions = !pass.getOptions().empty();
 
   os << "#ifdef " << enableVarName << "\n";
   os << llvm::formatv(passHeader, passName);
 
+  if (emitDefaultConstructors) {
+    os << llvm::formatv(friendDefaultConstructorDeclTemplate, passName);
+
+    if (emitDefaultConstructorWithOptions)
+      os << llvm::formatv(friendDefaultConstructorWithOptionsDeclTemplate,
+                          passName);
+  }
+
   std::string dependentDialectRegistrations;
   {
     llvm::raw_string_ostream dialectsOs(dependentDialectRegistrations);
@@ -297,7 +331,8 @@ static void emitPassDefs(const Pass &pass, raw_ostream &os) {
                                   dependentDialect);
   }
 
-  os << llvm::formatv(passDeclBegin, passName, pass.getBaseClass(),
+  os << "namespace impl {\n";
+  os << llvm::formatv(baseClassBegin, passName, pass.getBaseClass(),
                       pass.getArgument(), pass.getSummary(),
                       dependentDialectRegistrations);
 
@@ -320,15 +355,23 @@ static void emitPassDefs(const Pass &pass, raw_ostream &os) {
   // Private content
   os << "private:\n";
 
-  if (pass.getConstructor().empty()) {
-    os << llvm::formatv(friendDefaultConstructorTemplate, passName);
+  if (emitDefaultConstructors) {
+    os << llvm::formatv(friendDefaultConstructorDefTemplate, passName);
 
     if (!pass.getOptions().empty())
-      os << llvm::formatv(friendDefaultConstructorWithOptionsTemplate,
+      os << llvm::formatv(friendDefaultConstructorWithOptionsDefTemplate,
                           passName);
   }
 
   os << "};\n";
+  os << "} // namespace impl\n";
+
+  if (emitDefaultConstructors) {
+    os << llvm::formatv(defaultConstructorDefTemplate, passName);
+
+    if (emitDefaultConstructorWithOptions)
+      os << llvm::formatv(defaultConstructorWithOptionsDefTemplate, passName);
+  }
 
   os << "#undef " << enableVarName << "\n";
   os << "#endif // " << enableVarName << "\n";

diff  --git a/mlir/unittests/TableGen/PassGenTest.cpp b/mlir/unittests/TableGen/PassGenTest.cpp
index 7de17b5ec4b11..9b752fc6d7616 100644
--- a/mlir/unittests/TableGen/PassGenTest.cpp
+++ b/mlir/unittests/TableGen/PassGenTest.cpp
@@ -24,7 +24,7 @@ std::unique_ptr<mlir::Pass> createTestPassWithCustomConstructor(int v = 0);
 #define GEN_PASS_DEF_TESTPASSWITHCUSTOMCONSTRUCTOR
 #include "PassGenTest.cpp.inc"
 
-struct TestPass : public TestPassBase<TestPass> {
+struct TestPass : public impl::TestPassBase<TestPass> {
   using TestPassBase::TestPassBase;
 
   void runOnOperation() override {}
@@ -54,7 +54,7 @@ TEST(PassGenTest, PassClone) {
 }
 
 struct TestPassWithOptions
-    : public TestPassWithOptionsBase<TestPassWithOptions> {
+    : public impl::TestPassWithOptionsBase<TestPassWithOptions> {
   using TestPassWithOptionsBase::TestPassWithOptionsBase;
 
   void runOnOperation() override {}
@@ -89,7 +89,8 @@ TEST(PassGenTest, PassOptions) {
 }
 
 struct TestPassWithCustomConstructor
-    : public TestPassWithCustomConstructorBase<TestPassWithCustomConstructor> {
+    : public impl::TestPassWithCustomConstructorBase<
+          TestPassWithCustomConstructor> {
   explicit TestPassWithCustomConstructor(int v) : extraVal(v) {}
 
   void runOnOperation() override {}


        


More information about the Mlir-commits mailing list