Reland "[LTO] Run Argument Promotion before IPSCCP" #111853
Merged
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
hazzlim
changed the title
hazzlim
requested review from
nikic,
mtrofin,
aeubanks,
davemgreen,
teresajohnson,
sjoerdmeijer and
david-arm
|
Polite ping on this :) |
nikic
reviewed
Oct 17, 2024
There was a problem hiding this comment.
Given how low the compile-time impact is, I'm inclined to accept this.
I see that there is a PostOrderFunctionAttrsPass run shortly after these passes -- would it be feasible to move it earlier rather than running twice?
In any case, this change needs a PhaseOrdering test.
Run ArgumentPromotion before IPSCCP in the LTO pipeline, to expose more constants to be propagated. We also run PostOrderFunctionAttrs to improve the information available to ArgumentPromotion's alias analysis, and SROA to clean up allocas.
hazzlim
force-pushed
the
reland-lto-argprom
branch
from
99ad4ee to
b776bb3
Compare
|
@llvm/pr-subscribers-llvm-transforms Author: Hari Limaye (hazzlim) ChangesRun ArgumentPromotion before IPSCCP in the LTO pipeline, to expose more constants to be propagated. We also run PostOrderFunctionAttrs to improve the information available to ArgumentPromotion's alias analysis, and SROA to clean up allocas. Relands #111163. Full diff: https://github.com/llvm/llvm-project/pull/111853.diff 3 Files Affected:
diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp
index 17710eb94b6ded..651a16e1f2ae6e 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -1828,6 +1828,15 @@ PassBuilder::buildLTODefaultPipeline(OptimizationLevel Level,
MPM.addPass(PGOIndirectCallPromotion(
true /* InLTO */, PGOOpt && PGOOpt->Action == PGOOptions::SampleUse));
+ // Promoting by-reference arguments to by-value exposes more constants to
+ // IPSCCP.
+ MPM.addPass(
+ createModuleToPostOrderCGSCCPassAdaptor(PostOrderFunctionAttrsPass()));
+ MPM.addPass(
+ createModuleToPostOrderCGSCCPassAdaptor(ArgumentPromotionPass()));
+ MPM.addPass(
+ createModuleToFunctionPassAdaptor(SROAPass(SROAOptions::ModifyCFG)));
+
// Propagate constants at call sites into the functions they call. This
// opens opportunities for globalopt (and inlining) by substituting function
// pointers passed as arguments to direct uses of functions.
@@ -1840,9 +1849,12 @@ PassBuilder::buildLTODefaultPipeline(OptimizationLevel Level,
MPM.addPass(CalledValuePropagationPass());
}
- // Now deduce any function attributes based in the current code.
- MPM.addPass(
- createModuleToPostOrderCGSCCPassAdaptor(PostOrderFunctionAttrsPass()));
+ // For higher optimization levels this Pass has just run, so don't repeat it.
+ if (Level.getSpeedupLevel() == 1) {
+ // Now deduce any function attributes based on the current code.
+ MPM.addPass(
+ createModuleToPostOrderCGSCCPassAdaptor(PostOrderFunctionAttrsPass()));
+ }
// Do RPO function attribute inference across the module to forward-propagate
// attributes where applicable.
diff --git a/llvm/test/Other/new-pm-lto-defaults.ll b/llvm/test/Other/new-pm-lto-defaults.ll
index 5543472df685b0..3a49903be429c8 100644
--- a/llvm/test/Other/new-pm-lto-defaults.ll
+++ b/llvm/test/Other/new-pm-lto-defaults.ll
@@ -41,9 +41,6 @@
; CHECK-O23SZ-NEXT: PGOIndirectCallPromotion
; CHECK-O23SZ-NEXT: Running analysis: ProfileSummaryAnalysis
; CHECK-O23SZ-NEXT: Running analysis: OptimizationRemarkEmitterAnalysis
-; CHECK-O23SZ-NEXT: Running pass: IPSCCPPass
-; CHECK-O23SZ-NEXT: Running analysis: AssumptionAnalysis on foo
-; CHECK-O23SZ-NEXT: Running pass: CalledValuePropagationPass
; CHECK-O-NEXT: Running analysis: InnerAnalysisManagerProxy<{{.*}}SCC
; CHECK-O-NEXT: Running analysis: LazyCallGraphAnalysis
; CHECK-O1-NEXT: Running analysis: TargetLibraryAnalysis
@@ -52,12 +49,16 @@
; CHECK-O-NEXT: Running pass: PostOrderFunctionAttrsPass
; CHECK-O-NEXT: Running analysis: AAManager
; CHECK-O-NEXT: Running analysis: BasicAA
-; CHECK-O1-NEXT: Running analysis: AssumptionAnalysis on foo
+; CHECK-O: Running analysis: AssumptionAnalysis on foo
; CHECK-O1-NEXT: Running analysis: TargetIRAnalysis
; CHECK-O1-NEXT: Running analysis: DominatorTreeAnalysis
; CHECK-O-NEXT: Running analysis: ScopedNoAliasAA
; CHECK-O-NEXT: Running analysis: TypeBasedAA
; CHECK-O-NEXT: Running analysis: OuterAnalysisManagerProxy
+; CHECK-O23SZ-NEXT: Running pass: ArgumentPromotionPass
+; CHECK-O23SZ-NEXT: Running pass: SROAPass
+; CHECK-O23SZ-NEXT: Running pass: IPSCCPPass
+; CHECK-O23SZ-NEXT: Running pass: CalledValuePropagationPass
; CHECK-O-NEXT: Running pass: ReversePostOrderFunctionAttrsPass
; CHECK-O-NEXT: Running pass: GlobalSplitPass
; CHECK-O-NEXT: Running pass: WholeProgramDevirtPass
diff --git a/llvm/test/Transforms/PhaseOrdering/lto-argpromotion-ipsccp.ll b/llvm/test/Transforms/PhaseOrdering/lto-argpromotion-ipsccp.ll
new file mode 100644
index 00000000000000..72921acba5969f
--- /dev/null
+++ b/llvm/test/Transforms/PhaseOrdering/lto-argpromotion-ipsccp.ll
@@ -0,0 +1,68 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -passes='lto<O3>' -S < %s | FileCheck %s
+
+; We should be able to propagate the constants from @parent to @child.
+
+define void @parent(ptr %p) {
+; CHECK-LABEL: define void @parent(
+; CHECK-SAME: ptr nocapture [[P:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+; CHECK-NEXT: tail call fastcc void @child(ptr [[P]])
+; CHECK-NEXT: ret void
+;
+ %c = alloca i32
+ store i32 5, ptr %c
+ %n = alloca i32
+ store i32 1024, ptr %n
+ call void @child(ptr %p, ptr %n, ptr %c)
+ ret void
+}
+
+define internal void @child(ptr %p, ptr %n, ptr %c) noinline {
+; CHECK-LABEL: define internal fastcc void @child(
+; CHECK-SAME: ptr nocapture [[P:%.*]]) unnamed_addr #[[ATTR1:[0-9]+]] {
+; CHECK-NEXT: [[ENTRY:.*]]:
+; CHECK-NEXT: br label %[[FOR_COND:.*]]
+; CHECK: [[FOR_COND]]:
+; CHECK-NEXT: [[I_0:%.*]] = phi i32 [ 0, %[[ENTRY]] ], [ [[INC:%.*]], %[[FOR_INC:.*]] ]
+; CHECK-NEXT: [[CMP_NOT:%.*]] = icmp eq i32 [[I_0]], 1024
+; CHECK-NEXT: br i1 [[CMP_NOT]], label %[[FOR_END:.*]], label %[[FOR_INC]]
+; CHECK: [[FOR_INC]]:
+; CHECK-NEXT: [[IDXPROM:%.*]] = zext nneg i32 [[I_0]] to i64
+; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds i32, ptr [[P]], i64 [[IDXPROM]]
+; CHECK-NEXT: [[TMP0:%.*]] = load i32, ptr [[ARRAYIDX]], align 4
+; CHECK-NEXT: [[MUL:%.*]] = mul i32 [[TMP0]], 5
+; CHECK-NEXT: store i32 [[MUL]], ptr [[ARRAYIDX]], align 4
+; CHECK-NEXT: [[INC]] = add nuw nsw i32 [[I_0]], 1
+; CHECK-NEXT: br label %[[FOR_COND]]
+; CHECK: [[FOR_END]]:
+; CHECK-NEXT: ret void
+;
+entry:
+ br label %for.cond
+
+for.cond:
+ %i.0 = phi i32 [ 0, %entry ], [ %inc, %for.inc ]
+ %n.val = load i32, ptr %n
+ %cmp = icmp ne i32 %i.0, %n.val
+ br i1 %cmp, label %for.body, label %for.cond.cleanup
+
+for.cond.cleanup:
+ br label %for.end
+
+for.body:
+ %idxprom = sext i32 %i.0 to i64
+ %arrayidx = getelementptr inbounds i32, ptr %p, i64 %idxprom
+ %0 = load i32, ptr %arrayidx, align 4
+ %c.val = load i32, ptr %c
+ %mul = mul i32 %0, %c.val
+ store i32 %mul, ptr %arrayidx, align 4
+ br label %for.inc
+
+for.inc:
+ %inc = add nsw i32 %i.0, 1
+ br label %for.cond
+
+for.end:
+ ret void
+}
+
|
Thanks for pointing this out - I've added a PhaseOrdering test.
Good point - I've moved the |
|
Polite ping :) |
nikic
reviewed
Nov 5, 2024
nikic
reviewed
Nov 5, 2024
|
Thank you for reviewing :) |
|
A MemorySanitizer buildbot (https://lab.llvm.org/buildbot/#/builders/164/builds/4320) started breaking around the time of this change; this change and #114964 are the only non-revert, non-MLIR changes. Could you please take a look? |
|
My bad, please ignore my above message. The failure was due to a revert (f548d39). |
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.
Run ArgumentPromotion before IPSCCP in the LTO pipeline, to expose more constants to be propagated. We also run PostOrderFunctionAttrs to improve the information available to ArgumentPromotion's alias analysis, and SROA to clean up allocas.
Relands #111163.