[flang] Adapt PolymorphicOpConversion to run on all top level ops #90597
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We might use polymorphic ops in top-level operations other than functions some time in the future. We need to ensure that these operations can be lowered. See RFC: https://discourse.llvm.org/t/rfc-add-an-interface-for-top-level-container-operations Some of the changes are from moving declaration and definition of the constructor function into tablegen (as requested in code review when altering another pass).
llvmbot
added
flang:driver
flang
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-driver Author: Tom Eccles (tblah) ChangesWe might use polymorphic ops in top-level operations other than functions some time in the future. We need to ensure that these operations can be lowered. See RFC: Some of the changes are from moving declaration and definition of the constructor function into tablegen (as requested in code review when altering another pass). Full diff: https://github.com/llvm/llvm-project/pull/90597.diff 8 Files Affected:
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.h b/flang/include/flang/Optimizer/Transforms/Passes.h
index 547fe742967a4f..470ed8a125ac4a 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.h
+++ b/flang/include/flang/Optimizer/Transforms/Passes.h
@@ -67,7 +67,6 @@ std::unique_ptr<mlir::Pass> createAnnotateConstantOperandsPass();
std::unique_ptr<mlir::Pass> createAlgebraicSimplificationPass();
std::unique_ptr<mlir::Pass>
createAlgebraicSimplificationPass(const mlir::GreedyRewriteConfig &config);
-std::unique_ptr<mlir::Pass> createPolymorphicOpConversionPass();
std::unique_ptr<mlir::Pass> createOMPDescriptorMapInfoGenPass();
std::unique_ptr<mlir::Pass> createOMPFunctionFilteringPass();
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.td b/flang/include/flang/Optimizer/Transforms/Passes.td
index 020b8a6b64a9e0..dcb7037e2991be 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.td
+++ b/flang/include/flang/Optimizer/Transforms/Passes.td
@@ -298,14 +298,13 @@ def AlgebraicSimplification : Pass<"flang-algebraic-simplification"> {
let constructor = "::fir::createAlgebraicSimplificationPass()";
}
-def PolymorphicOpConversion : Pass<"fir-polymorphic-op", "::mlir::func::FuncOp"> {
+def PolymorphicOpConversion : Pass<"fir-polymorphic-op"> {
let summary =
"Simplify operations on polymorphic types";
let description = [{
This pass breaks up the lowering of operations on polymorphic types by
introducing an intermediate FIR level that simplifies code geneation.
}];
- let constructor = "::fir::createPolymorphicOpConversionPass()";
let dependentDialects = [
"fir::FIROpsDialect", "mlir::func::FuncDialect"
];
diff --git a/flang/include/flang/Tools/CLOptions.inc b/flang/include/flang/Tools/CLOptions.inc
index 34af9f1c21f8d8..bd60c66b61ad24 100644
--- a/flang/include/flang/Tools/CLOptions.inc
+++ b/flang/include/flang/Tools/CLOptions.inc
@@ -271,7 +271,7 @@ inline void createDefaultFIROptimizerPassPipeline(
pm.addPass(mlir::createCSEPass());
// Polymorphic types
- pm.addPass(fir::createPolymorphicOpConversionPass());
+ addNestedPassToAllTopLevelOperations(pm, fir::createPolymorphicOpConversion);
if (pc.AliasAnalysis && !disableFirAliasTags && !useOldAliasTags)
pm.addPass(fir::createAliasTagsPass());
diff --git a/flang/lib/Optimizer/Transforms/PolymorphicOpConversion.cpp b/flang/lib/Optimizer/Transforms/PolymorphicOpConversion.cpp
index d933dc58f3757f..0f5c43882ee30a 100644
--- a/flang/lib/Optimizer/Transforms/PolymorphicOpConversion.cpp
+++ b/flang/lib/Optimizer/Transforms/PolymorphicOpConversion.cpp
@@ -229,7 +229,9 @@ class PolymorphicOpConversion
void runOnOperation() override {
auto *context = &getContext();
- auto mod = getOperation()->getParentOfType<ModuleOp>();
+ auto mod = mlir::dyn_cast_or_null<mlir::ModuleOp>(getOperation());
+ if (!mod)
+ mod = getOperation()->getParentOfType<ModuleOp>();
mlir::RewritePatternSet patterns(context);
BindingTables bindingTables;
@@ -471,7 +473,3 @@ SelectTypeConv::collectAncestors(fir::TypeInfoOp dt, mlir::ModuleOp mod) const {
}
return ancestors;
}
-
-std::unique_ptr<mlir::Pass> fir::createPolymorphicOpConversionPass() {
- return std::make_unique<PolymorphicOpConversion>();
-}
diff --git a/flang/test/Driver/bbc-mlir-pass-pipeline.f90 b/flang/test/Driver/bbc-mlir-pass-pipeline.f90
index caa86e66e62bc0..07b68bfe03b336 100644
--- a/flang/test/Driver/bbc-mlir-pass-pipeline.f90
+++ b/flang/test/Driver/bbc-mlir-pass-pipeline.f90
@@ -47,13 +47,16 @@
! CHECK-NEXT: Pipeline Collection : ['fir.global', 'func.func', 'omp.declare_reduction', 'omp.private']
! CHECK-NEXT: 'fir.global' Pipeline
+! CHECK-NEXT: PolymorphicOpConversion
! CHECK-NEXT: CFGConversion
! CHECK-NEXT: 'func.func' Pipeline
! CHECK-NEXT: PolymorphicOpConversion
! CHECK-NEXT: CFGConversion
! CHECK-NEXT: 'omp.declare_reduction' Pipeline
+! CHECK-NEXT: PolymorphicOpConversion
! CHECK-NEXT: CFGConversion
! CHECK-NEXT: 'omp.private' Pipeline
+! CHECK-NEXT: PolymorphicOpConversion
! CHECK-NEXT: CFGConversion
! CHECK-NEXT: SCFToControlFlow
diff --git a/flang/test/Driver/mlir-debug-pass-pipeline.f90 b/flang/test/Driver/mlir-debug-pass-pipeline.f90
index 2c81441e7ec9bf..cad7415a3b528d 100644
--- a/flang/test/Driver/mlir-debug-pass-pipeline.f90
+++ b/flang/test/Driver/mlir-debug-pass-pipeline.f90
@@ -67,13 +67,16 @@
! ALL-NEXT: Pipeline Collection : ['fir.global', 'func.func', 'omp.declare_reduction', 'omp.private']
! ALL-NEXT: 'fir.global' Pipeline
+! ALL-NEXT: PolymorphicOpConversion
! ALL-NEXT: CFGConversion
! ALL-NEXT: 'func.func' Pipeline
! ALL-NEXT: PolymorphicOpConversion
! ALL-NEXT: CFGConversion
! ALL-NEXT: 'omp.declare_reduction' Pipeline
+! ALL-NEXT: PolymorphicOpConversion
! ALL-NEXT: CFGConversion
! ALL-NEXT: 'omp.private' Pipeline
+! ALL-NEXT: PolymorphicOpConversion
! ALL-NEXT: CFGConversion
! ALL-NEXT: SCFToControlFlow
! ALL-NEXT: Canonicalizer
diff --git a/flang/test/Driver/mlir-pass-pipeline.f90 b/flang/test/Driver/mlir-pass-pipeline.f90
index 320467a2ac2a74..7f63f946c2fbd9 100644
--- a/flang/test/Driver/mlir-pass-pipeline.f90
+++ b/flang/test/Driver/mlir-pass-pipeline.f90
@@ -56,18 +56,28 @@
! ALL-NEXT: (S) 0 num-cse'd - Number of operations CSE'd
! ALL-NEXT: (S) 0 num-dce'd - Number of operations DCE'd
+! O2-NEXT: Pipeline Collection : ['fir.global', 'func.func', 'omp.declare_reduction', 'omp.private']
+! O2-NEXT: 'fir.global' Pipeline
+! O2-NEXT: PolymorphicOpConversion
! O2-NEXT: 'func.func' Pipeline
! O2-NEXT: PolymorphicOpConversion
+! O2-NEXT: 'omp.declare_reduction' Pipeline
+! O2-NEXT: PolymorphicOpConversion
+! O2-NEXT: 'omp.private' Pipeline
+! O2-NEXT: PolymorphicOpConversion
! O2-NEXT: AddAliasTags
! ALL-NEXT: Pipeline Collection : ['fir.global', 'func.func', 'omp.declare_reduction', 'omp.private']
! ALL-NEXT: 'fir.global' Pipeline
+! NOTO2-NEXT: PolymorphicOpConversion
! ALL-NEXT: CFGConversion
! ALL-NEXT: 'func.func' Pipeline
! NOTO2-NEXT: PolymorphicOpConversion
! ALL-NEXT: CFGConversion
! ALL-NEXT: 'omp.declare_reduction' Pipeline
+! NOTO2-NEXT: PolymorphicOpConversion
! ALL-NEXT: CFGConversion
! ALL-NEXT: 'omp.private' Pipeline
+! NOTO2-NEXT: PolymorphicOpConversion
! ALL-NEXT: CFGConversion
! ALL-NEXT: SCFToControlFlow
diff --git a/flang/test/Fir/basic-program.fir b/flang/test/Fir/basic-program.fir
index d54b0895cc3330..67a9c56ed9acb5 100644
--- a/flang/test/Fir/basic-program.fir
+++ b/flang/test/Fir/basic-program.fir
@@ -62,8 +62,15 @@ func.func @_QQmain() {
// PASSES-NEXT: (S) 0 num-cse'd - Number of operations CSE'd
// PASSES-NEXT: (S) 0 num-dce'd - Number of operations DCE'd
+// PASSES-NEXT: Pipeline Collection : ['fir.global', 'func.func', 'omp.declare_reduction', 'omp.private']
+// PASSES-NEXT: 'fir.global' Pipeline
+// PASSES-NEXT: PolymorphicOpConversion
// PASSES-NEXT: 'func.func' Pipeline
// PASSES-NEXT: PolymorphicOpConversion
+// PASSES-NEXT: 'omp.declare_reduction' Pipeline
+// PASSES-NEXT: PolymorphicOpConversion
+// PASSES-NEXT: 'omp.private' Pipeline
+// PASSES-NEXT: PolymorphicOpConversion
// PASSES-NEXT: AddAliasTags
|
Renaud-K
reviewed
Apr 30, 2024
tblah
added a commit
to tblah/llvm-project
that referenced
this pull request
May 1, 2024
It was pointed out in post commit review of llvm#90597 that the pass should never have been run in parallel over all functions (and now other top level operations) in the first place. The mutex used in the pass was ineffective at preventing races since each instance of the pass would have a different mutex.
tblah
added a commit
that referenced
this pull request
May 1, 2024
It was pointed out in post commit review of #90597 that the pass should never have been run in parallel over all functions (and now other top level operations) in the first place. The mutex used in the pass was ineffective at preventing races since each instance of the pass would have a different mutex.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We might use polymorphic ops in top-level operations other than functions some time in the future. We need to ensure that these operations can be lowered.
See RFC:
https://discourse.llvm.org/t/rfc-add-an-interface-for-top-level-container-operations
Some of the changes are from moving declaration and definition of the constructor function into tablegen (as requested in code review when altering another pass).