-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[flang] Schedule InlineHLFIRAssign after BufferizeHLFIR. #121863
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
Conversation
This helps to get rid of *some* calls to AssignTemporary runtime that are appearing due to temporary_lhs hlfir.assign produced in BufferizeHLFIR. I only tested it on `tonto`, and did not see any performance changes. I will run more performance testing before merging this.
@llvm/pr-subscribers-flang-fir-hlfir Author: Slava Zakharin (vzakhari) ChangesThis helps to get rid of some calls to AssignTemporary runtime Full diff: https://github.com/llvm/llvm-project/pull/121863.diff 3 Files Affected:
diff --git a/flang/lib/Optimizer/Passes/Pipelines.cpp b/flang/lib/Optimizer/Passes/Pipelines.cpp
index 20e4599587c4b2..e1d7376ec3805d 100644
--- a/flang/lib/Optimizer/Passes/Pipelines.cpp
+++ b/flang/lib/Optimizer/Passes/Pipelines.cpp
@@ -240,6 +240,16 @@ void createHLFIRToFIRPassPipeline(mlir::PassManager &pm, bool enableOpenMP,
pm.addPass(hlfir::createLowerHLFIROrderedAssignments());
pm.addPass(hlfir::createLowerHLFIRIntrinsics());
pm.addPass(hlfir::createBufferizeHLFIR());
+ // Run hlfir.assign inlining again after BufferizeHLFIR,
+ // because the latter may introduce new hlfir.assign operations,
+ // e.g. for copying an array into a temporary due to
+ // hlfir.associate.
+ // TODO: we can remove the previous InlineHLFIRAssign, when
+ // FIR AliasAnalysis is good enough to say that a temporary
+ // array does not alias with any user object.
+ if (optLevel.isOptimizingForSpeed())
+ addNestedPassToAllTopLevelOperations<PassConstructor>(
+ pm, hlfir::createInlineHLFIRAssign);
pm.addPass(hlfir::createConvertHLFIRtoFIR());
if (enableOpenMP)
pm.addPass(flangomp::createLowerWorkshare());
diff --git a/flang/test/Driver/mlir-pass-pipeline.f90 b/flang/test/Driver/mlir-pass-pipeline.f90
index 9655afce96d927..55e86da2dfdf14 100644
--- a/flang/test/Driver/mlir-pass-pipeline.f90
+++ b/flang/test/Driver/mlir-pass-pipeline.f90
@@ -49,6 +49,15 @@
! ALL: LowerHLFIROrderedAssignments
! ALL-NEXT: LowerHLFIRIntrinsics
! ALL-NEXT: BufferizeHLFIR
+! O2-NEXT: Pipeline Collection : ['fir.global', 'func.func', 'omp.declare_reduction', 'omp.private']
+! O2-NEXT: 'fir.global' Pipeline
+! O2-NEXT: InlineHLFIRAssign
+! O2-NEXT: 'func.func' Pipeline
+! O2-NEXT: InlineHLFIRAssign
+! O2-NEXT: 'omp.declare_reduction' Pipeline
+! O2-NEXT: InlineHLFIRAssign
+! O2-NEXT: 'omp.private' Pipeline
+! O2-NEXT: InlineHLFIRAssign
! ALL-NEXT: ConvertHLFIRtoFIR
! ALL-NEXT: CSE
! Ideally, we need an output with only the pass names, but
diff --git a/flang/test/Fir/basic-program.fir b/flang/test/Fir/basic-program.fir
index 620882ebbed2a9..29a0f661579710 100644
--- a/flang/test/Fir/basic-program.fir
+++ b/flang/test/Fir/basic-program.fir
@@ -50,6 +50,15 @@ func.func @_QQmain() {
// PASSES-NEXT: LowerHLFIROrderedAssignments
// PASSES-NEXT: LowerHLFIRIntrinsics
// PASSES-NEXT: BufferizeHLFIR
+// PASSES-NEXT: Pipeline Collection : ['fir.global', 'func.func', 'omp.declare_reduction', 'omp.private']
+// PASSES-NEXT: 'fir.global' Pipeline
+// PASSES-NEXT: InlineHLFIRAssign
+// PASSES-NEXT: 'func.func' Pipeline
+// PASSES-NEXT: InlineHLFIRAssign
+// PASSES-NEXT: 'omp.declare_reduction' Pipeline
+// PASSES-NEXT: InlineHLFIRAssign
+// PASSES-NEXT: 'omp.private' Pipeline
+// PASSES-NEXT: InlineHLFIRAssign
// PASSES-NEXT: ConvertHLFIRtoFIR
// PASSES-NEXT: LowerWorkshare
// PASSES-NEXT: CSE
|
@llvm/pr-subscribers-flang-driver Author: Slava Zakharin (vzakhari) ChangesThis helps to get rid of some calls to AssignTemporary runtime Full diff: https://github.com/llvm/llvm-project/pull/121863.diff 3 Files Affected:
diff --git a/flang/lib/Optimizer/Passes/Pipelines.cpp b/flang/lib/Optimizer/Passes/Pipelines.cpp
index 20e4599587c4b2..e1d7376ec3805d 100644
--- a/flang/lib/Optimizer/Passes/Pipelines.cpp
+++ b/flang/lib/Optimizer/Passes/Pipelines.cpp
@@ -240,6 +240,16 @@ void createHLFIRToFIRPassPipeline(mlir::PassManager &pm, bool enableOpenMP,
pm.addPass(hlfir::createLowerHLFIROrderedAssignments());
pm.addPass(hlfir::createLowerHLFIRIntrinsics());
pm.addPass(hlfir::createBufferizeHLFIR());
+ // Run hlfir.assign inlining again after BufferizeHLFIR,
+ // because the latter may introduce new hlfir.assign operations,
+ // e.g. for copying an array into a temporary due to
+ // hlfir.associate.
+ // TODO: we can remove the previous InlineHLFIRAssign, when
+ // FIR AliasAnalysis is good enough to say that a temporary
+ // array does not alias with any user object.
+ if (optLevel.isOptimizingForSpeed())
+ addNestedPassToAllTopLevelOperations<PassConstructor>(
+ pm, hlfir::createInlineHLFIRAssign);
pm.addPass(hlfir::createConvertHLFIRtoFIR());
if (enableOpenMP)
pm.addPass(flangomp::createLowerWorkshare());
diff --git a/flang/test/Driver/mlir-pass-pipeline.f90 b/flang/test/Driver/mlir-pass-pipeline.f90
index 9655afce96d927..55e86da2dfdf14 100644
--- a/flang/test/Driver/mlir-pass-pipeline.f90
+++ b/flang/test/Driver/mlir-pass-pipeline.f90
@@ -49,6 +49,15 @@
! ALL: LowerHLFIROrderedAssignments
! ALL-NEXT: LowerHLFIRIntrinsics
! ALL-NEXT: BufferizeHLFIR
+! O2-NEXT: Pipeline Collection : ['fir.global', 'func.func', 'omp.declare_reduction', 'omp.private']
+! O2-NEXT: 'fir.global' Pipeline
+! O2-NEXT: InlineHLFIRAssign
+! O2-NEXT: 'func.func' Pipeline
+! O2-NEXT: InlineHLFIRAssign
+! O2-NEXT: 'omp.declare_reduction' Pipeline
+! O2-NEXT: InlineHLFIRAssign
+! O2-NEXT: 'omp.private' Pipeline
+! O2-NEXT: InlineHLFIRAssign
! ALL-NEXT: ConvertHLFIRtoFIR
! ALL-NEXT: CSE
! Ideally, we need an output with only the pass names, but
diff --git a/flang/test/Fir/basic-program.fir b/flang/test/Fir/basic-program.fir
index 620882ebbed2a9..29a0f661579710 100644
--- a/flang/test/Fir/basic-program.fir
+++ b/flang/test/Fir/basic-program.fir
@@ -50,6 +50,15 @@ func.func @_QQmain() {
// PASSES-NEXT: LowerHLFIROrderedAssignments
// PASSES-NEXT: LowerHLFIRIntrinsics
// PASSES-NEXT: BufferizeHLFIR
+// PASSES-NEXT: Pipeline Collection : ['fir.global', 'func.func', 'omp.declare_reduction', 'omp.private']
+// PASSES-NEXT: 'fir.global' Pipeline
+// PASSES-NEXT: InlineHLFIRAssign
+// PASSES-NEXT: 'func.func' Pipeline
+// PASSES-NEXT: InlineHLFIRAssign
+// PASSES-NEXT: 'omp.declare_reduction' Pipeline
+// PASSES-NEXT: InlineHLFIRAssign
+// PASSES-NEXT: 'omp.private' Pipeline
+// PASSES-NEXT: InlineHLFIRAssign
// PASSES-NEXT: ConvertHLFIRtoFIR
// PASSES-NEXT: LowerWorkshare
// PASSES-NEXT: CSE
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM, especially if you think we can rely on this one only in the future after alias analysis improvement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This helps to get rid of some calls to AssignTemporary runtime
that are appearing due to temporary_lhs hlfir.assign produced
in BufferizeHLFIR. I only tested it on
tonto
, and did not seeany performance changes. I will run more performance testing
before merging this.