Skip to content

Commit 0d4a709

Browse files
aeubanksaeubanks
committed
[Pipeline] Adjust PostOrderFunctionAttrs placement in simplification pipeline
We can infer more attribute information once functions are fully simplified, so move the PostOrderFunctionAttrs pass after the function simplification pipeline. However, just doing this can impact simplification of recursive functions since function simplification takes advantage of function attributes of callees (some LLVM tests are actually impacted by this), so keep a copy of PostOrderFunctionAttrs before the function simplification pipeline that only runs on recursive functions. For example, this fixes the small regression noticed in https://reviews.llvm.org/D128830. This requires some restructuring of the CGSCC NoRerun feature. We need to cache the ShouldNotRunFunctionPassesAnalysis analysis after the simplification is done, which now is after the second PostOrderFunctionAttrs run, rather than after the function simplification pipeline. Compile time impact: https://llvm-compile-time-tracker.com/compare.php?from=33cf40122279342b50f92a3a53f5c185390b6018&to=1bb2a07875634e508a6bdf2ca1b130f55510f060&stat=instructions:u Compile time increase from unconditionally running the first PostOrderFunctionAttrs: https://llvm-compile-time-tracker.com/compare.php?from=1bb2a07875634e508a6bdf2ca1b130f55510f060&to=f4f87e89cc7a35c64e3a103a8036192a84ae002b&stat=instructions:u Reviewed By: nikic Differential Revision: https://reviews.llvm.org/D145210
1 parent edd0213 commit 0d4a709

15 files changed

+83
-21
lines changed

‎llvm/include/llvm/Transforms/IPO/FunctionAttrs.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,16 @@ bool thinLTOPropagateFunctionAttrs(
4848
/// attribute. It also discovers function arguments that are not captured by
4949
/// the function and marks them with the nocapture attribute.
5050
struct PostOrderFunctionAttrsPass : PassInfoMixin<PostOrderFunctionAttrsPass> {
51+
PostOrderFunctionAttrsPass(bool SkipNonRecursive = false)
52+
: SkipNonRecursive(SkipNonRecursive) {}
5153
PreservedAnalyses run(LazyCallGraph::SCC &C, CGSCCAnalysisManager &AM,
5254
LazyCallGraph &CG, CGSCCUpdateResult &UR);
55+
56+
void printPipeline(raw_ostream &OS,
57+
function_ref<StringRef(StringRef)> MapClassName2PassName);
58+
59+
private:
60+
bool SkipNonRecursive;
5361
};
5462

5563
/// A pass to do RPO deduction and propagation of function attributes.

‎llvm/lib/Analysis/CGSCCPassManager.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -545,8 +545,6 @@ PreservedAnalyses CGSCCToFunctionPassAdaptor::run(LazyCallGraph::SCC &C,
545545
// function's analyses (that's the contract of a function pass), so
546546
// directly handle the function analysis manager's invalidation here.
547547
FAM.invalidate(F, EagerlyInvalidate ? PreservedAnalyses::none() : PassPA);
548-
if (NoRerun)
549-
(void)FAM.getResult<ShouldNotRunFunctionPassesAnalysis>(F);
550548

551549
// Then intersect the preserved set so that invalidation of module
552550
// analyses will eventually occur when the module pass completes.

‎llvm/lib/Passes/PassBuilder.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -657,6 +657,11 @@ Expected<bool> parseCoroSplitPassOptions(StringRef Params) {
657657
return parseSinglePassOption(Params, "reuse-storage", "CoroSplitPass");
658658
}
659659

660+
Expected<bool> parsePostOrderFunctionAttrsPassOptions(StringRef Params) {
661+
return parseSinglePassOption(Params, "skip-non-recursive",
662+
"PostOrderFunctionAttrs");
663+
}
664+
660665
Expected<bool> parseEarlyCSEPassOptions(StringRef Params) {
661666
return parseSinglePassOption(Params, "memssa", "EarlyCSE");
662667
}

‎llvm/lib/Passes/PassBuilderPipelines.cpp‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -831,8 +831,11 @@ PassBuilder::buildInlinerPipeline(OptimizationLevel Level,
831831
if (AttributorRun & AttributorRunOption::CGSCC)
832832
MainCGPipeline.addPass(AttributorCGSCCPass());
833833

834-
// Now deduce any function attributes based in the current code.
835-
MainCGPipeline.addPass(PostOrderFunctionAttrsPass());
834+
// Deduce function attributes. We do another run of this after the function
835+
// simplification pipeline, so this only needs to run when it could affect the
836+
// function simplification pipeline, which is only the case with recursive
837+
// functions.
838+
MainCGPipeline.addPass(PostOrderFunctionAttrsPass(/*SkipNonRecursive*/ true));
836839

837840
// When at O3 add argument promotion to the pass pipeline.
838841
// FIXME: It isn't at all clear why this should be limited to O3.
@@ -847,14 +850,25 @@ PassBuilder::buildInlinerPipeline(OptimizationLevel Level,
847850
for (auto &C : CGSCCOptimizerLateEPCallbacks)
848851
C(MainCGPipeline, Level);
849852

850-
// Lastly, add the core function simplification pipeline nested inside the
853+
// Add the core function simplification pipeline nested inside the
851854
// CGSCC walk.
852855
MainCGPipeline.addPass(createCGSCCToFunctionPassAdaptor(
853856
buildFunctionSimplificationPipeline(Level, Phase),
854857
PTO.EagerlyInvalidateAnalyses, /*NoRerun=*/true));
855858

859+
// Finally, deduce any function attributes based on the fully simplified
860+
// function.
861+
MainCGPipeline.addPass(PostOrderFunctionAttrsPass());
862+
863+
// Mark that the function is fully simplified and that it shouldn't be
864+
// simplified again if we somehow revisit it due to CGSCC mutations unless
865+
// it's been modified since.
866+
MainCGPipeline.addPass(createCGSCCToFunctionPassAdaptor(
867+
RequireAnalysisPass<ShouldNotRunFunctionPassesAnalysis, Function>()));
868+
856869
MainCGPipeline.addPass(CoroSplitPass(Level != OptimizationLevel::O0));
857870

871+
// Make sure we don't affect potential future NoRerun CGSCC adaptors.
858872
MIWP.addLateModulePass(createModuleToFunctionPassAdaptor(
859873
InvalidateAnalysisPass<ShouldNotRunFunctionPassesAnalysis>()));
860874

‎llvm/lib/Passes/PassRegistry.def‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,6 @@ CGSCC_ANALYSIS("pass-instrumentation", PassInstrumentationAnalysis(PIC))
184184
#endif
185185
CGSCC_PASS("argpromotion", ArgumentPromotionPass())
186186
CGSCC_PASS("invalidate<all>", InvalidateAllAnalysesPass())
187-
CGSCC_PASS("function-attrs", PostOrderFunctionAttrsPass())
188187
CGSCC_PASS("attributor-cgscc", AttributorCGSCCPass())
189188
CGSCC_PASS("openmp-opt-cgscc", OpenMPOptCGSCCPass())
190189
CGSCC_PASS("no-op-cgscc", NoOpCGSCCPass())
@@ -207,6 +206,13 @@ CGSCC_PASS_WITH_PARAMS("coro-split",
207206
},
208207
parseCoroSplitPassOptions,
209208
"reuse-storage")
209+
CGSCC_PASS_WITH_PARAMS("function-attrs",
210+
"PostOrderFunctionAttrsPass",
211+
[](bool SkipNonRecursive) {
212+
return PostOrderFunctionAttrsPass(SkipNonRecursive);
213+
},
214+
parsePostOrderFunctionAttrsPassOptions,
215+
"skip-non-recursive")
210216
#undef CGSCC_PASS_WITH_PARAMS
211217

212218
#ifndef FUNCTION_ANALYSIS

‎llvm/lib/Transforms/IPO/FunctionAttrs.cpp‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1763,6 +1763,13 @@ PreservedAnalyses PostOrderFunctionAttrsPass::run(LazyCallGraph::SCC &C,
17631763
CGSCCAnalysisManager &AM,
17641764
LazyCallGraph &CG,
17651765
CGSCCUpdateResult &) {
1766+
// Skip non-recursive functions if requested.
1767+
if (C.size() == 1 && SkipNonRecursive) {
1768+
LazyCallGraph::Node &N = *C.begin();
1769+
if (!N->lookup(N))
1770+
return PreservedAnalyses::all();
1771+
}
1772+
17661773
FunctionAnalysisManager &FAM =
17671774
AM.getResult<FunctionAnalysisManagerCGSCCProxy>(C, CG).getManager();
17681775

@@ -1808,6 +1815,14 @@ PreservedAnalyses PostOrderFunctionAttrsPass::run(LazyCallGraph::SCC &C,
18081815
return PA;
18091816
}
18101817

1818+
void PostOrderFunctionAttrsPass::printPipeline(
1819+
raw_ostream &OS, function_ref<StringRef(StringRef)> MapClassName2PassName) {
1820+
static_cast<PassInfoMixin<PostOrderFunctionAttrsPass> *>(this)->printPipeline(
1821+
OS, MapClassName2PassName);
1822+
if (SkipNonRecursive)
1823+
OS << "<skip-non-recursive>";
1824+
}
1825+
18111826
template <typename AARGetterT>
18121827
static bool runImpl(CallGraphSCC &SCC, AARGetterT AARGetter) {
18131828
SmallVector<Function *, 8> Functions;

‎llvm/test/Other/new-pm-defaults.ll‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,14 +139,14 @@
139139
; CHECK-O-NEXT: Running pass: InlinerPass
140140
; CHECK-O-NEXT: Running pass: InlinerPass
141141
; CHECK-O-NEXT: Running pass: PostOrderFunctionAttrsPass
142-
; CHECK-O-NEXT: Running analysis: AAManager
143142
; CHECK-O3-NEXT: Running pass: ArgumentPromotionPass
144143
; CHECK-O2-NEXT: Running pass: OpenMPOptCGSCCPass on (foo)
145144
; CHECK-O3-NEXT: Running pass: OpenMPOptCGSCCPass on (foo)
146145
; CHECK-EP-CGSCC-LATE-NEXT: Running pass: NoOpCGSCCPass
147146
; CHECK-O-NEXT: Running pass: SROAPass
148147
; CHECK-O-NEXT: Running pass: EarlyCSEPass
149148
; CHECK-O-NEXT: Running analysis: MemorySSAAnalysis
149+
; CHECK-O-NEXT: Running analysis: AAManager
150150
; CHECK-O23SZ-NEXT: Running pass: SpeculativeExecutionPass
151151
; CHECK-O23SZ-NEXT: Running pass: JumpThreadingPass
152152
; CHECK-O23SZ-NEXT: Running analysis: LazyValueAnalysis
@@ -214,6 +214,8 @@
214214
; CHECK-O-NEXT: Running pass: SimplifyCFGPass
215215
; CHECK-O-NEXT: Running pass: InstCombinePass
216216
; CHECK-EP-PEEPHOLE-NEXT: Running pass: NoOpFunctionPass
217+
; CHECK-O-NEXT: Running pass: PostOrderFunctionAttrsPass
218+
; CHECK-O-NEXT: Running pass: RequireAnalysisPass<{{.*}}ShouldNotRunFunctionPassesAnalysis
217219
; CHECK-O-NEXT: Running analysis: ShouldNotRunFunctionPassesAnalysis
218220
; CHECK-O-NEXT: Running pass: CoroSplitPass
219221
; CHECK-O-NEXT: Running pass: InvalidateAnalysisPass<{{.*}}ShouldNotRunFunctionPassesAnalysis

‎llvm/test/Other/new-pm-print-pipeline.ll‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,3 +97,7 @@
9797
;; Test InstCombine options - the first pass checks default settings, and the second checks customized options.
9898
; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='function(instcombine,instcombine<use-loop-info;max-iterations=42>)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-28
9999
; CHECK-28: function(instcombine<max-iterations=1000;no-use-loop-info>,instcombine<max-iterations=42;use-loop-info>)
100+
101+
;; Test function-attrs
102+
; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='cgscc(function-attrs<skip-non-recursive>)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-29
103+
; CHECK-29: cgscc(function-attrs<skip-non-recursive>)

‎llvm/test/Other/new-pm-thinlto-defaults.ll‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,13 +116,13 @@
116116
; CHECK-O-NEXT: Running pass: InlinerPass
117117
; CHECK-O-NEXT: Running pass: InlinerPass
118118
; CHECK-O-NEXT: Running pass: PostOrderFunctionAttrsPass
119-
; CHECK-O-NEXT: Running analysis: AAManager
120119
; CHECK-O3-NEXT: Running pass: ArgumentPromotionPass
121120
; CHECK-O2-NEXT: Running pass: OpenMPOptCGSCCPass on (foo)
122121
; CHECK-O3-NEXT: Running pass: OpenMPOptCGSCCPass on (foo)
123122
; CHECK-O-NEXT: Running pass: SROAPass
124123
; CHECK-O-NEXT: Running pass: EarlyCSEPass
125124
; CHECK-O-NEXT: Running analysis: MemorySSAAnalysis
125+
; CHECK-O-NEXT: Running analysis: AAManager
126126
; CHECK-O23SZ-NEXT: Running pass: SpeculativeExecutionPass
127127
; CHECK-O23SZ-NEXT: Running pass: JumpThreadingPass
128128
; CHECK-O23SZ-NEXT: Running analysis: LazyValueAnalysis
@@ -184,6 +184,8 @@
184184
; CHECK-O23SZ-NEXT: Running pass: CoroElidePass
185185
; CHECK-O-NEXT: Running pass: SimplifyCFGPass
186186
; CHECK-O-NEXT: Running pass: InstCombinePass
187+
; CHECK-O-NEXT: Running pass: PostOrderFunctionAttrsPass
188+
; CHECK-O-NEXT: Running pass: RequireAnalysisPass<{{.*}}ShouldNotRunFunctionPassesAnalysis
187189
; CHECK-O-NEXT: Running analysis: ShouldNotRunFunctionPassesAnalysis
188190
; CHECK-O-NEXT: Running pass: CoroSplitPass
189191
; CHECK-O-NEXT: Running pass: InvalidateAnalysisPass<{{.*}}ShouldNotRunFunctionPassesAnalysis

‎llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,13 @@
7979
; CHECK-O-NEXT: Running pass: InlinerPass
8080
; CHECK-O-NEXT: Running pass: InlinerPass
8181
; CHECK-O-NEXT: Running pass: PostOrderFunctionAttrsPass
82-
; CHECK-O-NEXT: Running analysis: AAManager
8382
; CHECK-O3-NEXT: Running pass: ArgumentPromotionPass
8483
; CHECK-O2-NEXT: Running pass: OpenMPOptCGSCCPass
8584
; CHECK-O3-NEXT: Running pass: OpenMPOptCGSCCPass
8685
; CHECK-O-NEXT: Running pass: SROAPass
8786
; CHECK-O-NEXT: Running pass: EarlyCSEPass
8887
; CHECK-O-NEXT: Running analysis: MemorySSAAnalysis
88+
; CHECK-O-NEXT: Running analysis: AAManager
8989
; CHECK-O23SZ-NEXT: Running pass: SpeculativeExecutionPass
9090
; CHECK-O23SZ-NEXT: Running pass: JumpThreadingPass
9191
; CHECK-O23SZ-NEXT: Running analysis: LazyValueAnalysis
@@ -145,6 +145,8 @@
145145
; CHECK-O23SZ-NEXT: Running pass: CoroElidePass
146146
; CHECK-O-NEXT: Running pass: SimplifyCFGPass
147147
; CHECK-O-NEXT: Running pass: InstCombinePass
148+
; CHECK-O-NEXT: Running pass: PostOrderFunctionAttrsPass
149+
; CHECK-O-NEXT: Running pass: RequireAnalysisPass<{{.*}}ShouldNotRunFunctionPassesAnalysis
148150
; CHECK-O-NEXT: Running analysis: ShouldNotRunFunctionPassesAnalysis
149151
; CHECK-O-NEXT: Running pass: CoroSplitPass
150152
; CHECK-O-NEXT: Running pass: InvalidateAnalysisPass<{{.*}}ShouldNotRunFunctionPassesAnalysis

0 commit comments

Comments
 (0)