Skip to content

[flang][CodeGen][NFC] Reduce boilerplatre for ExternalNameConversion #94474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

tblah
Copy link
Contributor

@tblah tblah commented Jun 5, 2024

Use tablegen to generate the pass constructor.

I removed the duplicated pass option handling. I don't understand why the manual instantiation of the pass needs its own duplicate of the pass options in the (automatically generated) base class (even with the option to ignore the pass options in the base class).

This pass doesn't need changes to support other top level operations.

Use tablegen to generate the pass constructor.

I removed the duplicated pass option handling. I don't understand why
the manual instantiation of the pass needs its own duplicate of the pass
options in the (automatically generated) base class (even with the
option to ignore the pass options in the base class).

This pass doesn't need changes to support other top level operations.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Jun 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 5, 2024

@llvm/pr-subscribers-flang-fir-hlfir

Author: Tom Eccles (tblah)

Changes

Use tablegen to generate the pass constructor.

I removed the duplicated pass option handling. I don't understand why the manual instantiation of the pass needs its own duplicate of the pass options in the (automatically generated) base class (even with the option to ignore the pass options in the base class).

This pass doesn't need changes to support other top level operations.


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

4 Files Affected:

  • (modified) flang/include/flang/Optimizer/Transforms/Passes.h (-3)
  • (modified) flang/include/flang/Optimizer/Transforms/Passes.td (-1)
  • (modified) flang/include/flang/Tools/CLOptions.inc (+2-3)
  • (modified) flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp (+4-19)
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.h b/flang/include/flang/Optimizer/Transforms/Passes.h
index a7ba704fdb39b..2d43f4d4c55b6 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.h
+++ b/flang/include/flang/Optimizer/Transforms/Passes.h
@@ -60,9 +60,6 @@ std::unique_ptr<mlir::Pass> createAffineDemotionPass();
 std::unique_ptr<mlir::Pass>
 createArrayValueCopyPass(fir::ArrayValueCopyOptions options = {});
 std::unique_ptr<mlir::Pass> createCFGConversionPassWithNSW();
-std::unique_ptr<mlir::Pass> createExternalNameConversionPass();
-std::unique_ptr<mlir::Pass>
-createExternalNameConversionPass(bool appendUnderscore);
 std::unique_ptr<mlir::Pass> createMemDataFlowOptPass();
 std::unique_ptr<mlir::Pass> createPromoteToAffinePass();
 std::unique_ptr<mlir::Pass>
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.td b/flang/include/flang/Optimizer/Transforms/Passes.td
index 82638200e5e20..cac590a8da003 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.td
+++ b/flang/include/flang/Optimizer/Transforms/Passes.td
@@ -163,7 +163,6 @@ def ExternalNameConversion : Pass<"external-name-interop", "mlir::ModuleOp"> {
   let description = [{
     Demangle FIR internal name and mangle them for external interoperability.
   }];
-  let constructor = "::fir::createExternalNameConversionPass()";
   let options = [
     Option<"appendUnderscoreOpt", "append-underscore",
            "bool", /*default=*/"true",
diff --git a/flang/include/flang/Tools/CLOptions.inc b/flang/include/flang/Tools/CLOptions.inc
index c5c35e9a6a33f..d0399d65f5655 100644
--- a/flang/include/flang/Tools/CLOptions.inc
+++ b/flang/include/flang/Tools/CLOptions.inc
@@ -233,9 +233,8 @@ inline void addBoxedProcedurePass(mlir::PassManager &pm) {
 
 inline void addExternalNameConversionPass(
     mlir::PassManager &pm, bool appendUnderscore = true) {
-  addPassConditionally(pm, disableExternalNameConversion, [&]() {
-    return fir::createExternalNameConversionPass(appendUnderscore);
-  });
+  addPassConditionally(pm, disableExternalNameConversion,
+      [&]() { return fir::createExternalNameConversion({appendUnderscore}); });
 }
 
 // Use inliner extension point callback to register the default inliner pass.
diff --git a/flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp b/flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp
index b265c74c33dd5..648628fd1c9af 100644
--- a/flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp
+++ b/flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp
@@ -45,17 +45,11 @@ namespace {
 class ExternalNameConversionPass
     : public fir::impl::ExternalNameConversionBase<ExternalNameConversionPass> {
 public:
-  ExternalNameConversionPass(bool appendUnderscoring)
-      : appendUnderscores(appendUnderscoring) {}
-
-  ExternalNameConversionPass() { usePassOpt = true; }
+  using ExternalNameConversionBase<
+      ExternalNameConversionPass>::ExternalNameConversionBase;
 
   mlir::ModuleOp getModule() { return getOperation(); }
   void runOnOperation() override;
-
-private:
-  bool appendUnderscores;
-  bool usePassOpt = false;
 };
 } // namespace
 
@@ -63,7 +57,6 @@ void ExternalNameConversionPass::runOnOperation() {
   auto op = getOperation();
   auto *context = &getContext();
 
-  appendUnderscores = (usePassOpt) ? appendUnderscoreOpt : appendUnderscores;
   llvm::DenseMap<mlir::StringAttr, mlir::FlatSymbolRefAttr> remappings;
   // Update names of external Fortran functions and names of Common Block
   // globals.
@@ -74,7 +67,8 @@ void ExternalNameConversionPass::runOnOperation() {
           mlir::SymbolTable::getSymbolAttrName());
       auto deconstructedName = fir::NameUniquer::deconstruct(symName);
       if (fir::NameUniquer::isExternalFacingUniquedName(deconstructedName)) {
-        auto newName = mangleExternalName(deconstructedName, appendUnderscores);
+        auto newName =
+            mangleExternalName(deconstructedName, appendUnderscoreOpt);
         auto newAttr = mlir::StringAttr::get(context, newName);
         mlir::SymbolTable::setSymbolName(&funcOrGlobal, newAttr);
         auto newSymRef = mlir::FlatSymbolRefAttr::get(newAttr);
@@ -101,12 +95,3 @@ void ExternalNameConversionPass::runOnOperation() {
       nestedOp->setAttr(update.first, update.second);
   });
 }
-
-std::unique_ptr<mlir::Pass> fir::createExternalNameConversionPass() {
-  return std::make_unique<ExternalNameConversionPass>();
-}
-
-std::unique_ptr<mlir::Pass>
-fir::createExternalNameConversionPass(bool appendUnderscoring) {
-  return std::make_unique<ExternalNameConversionPass>(appendUnderscoring);
-}

Copy link
Contributor

@jeanPerier jeanPerier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Copy link
Contributor

@clementval clementval left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tblah tblah merged commit fdcdc3d into llvm:main Jun 6, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants