Skip to content

Commit 36844e9

Browse files
lukel97lukel97
committed
[SimpleLoopUnswitch] Don't use BlockFrequencyInfo to skip cold loops
In https://reviews.llvm.org/D129599, non-trivial switching was disabled for cold loops in the interest of code size. This added a dependency on BlockFrequencyInfo, but in loop passes this is only available on a lossy basis: see https://reviews.llvm.org/D86156 LICM moved away from BFI and as of today SimpleLoopUnswitch is the only remaining loop pass that uses BFI, for the sole reason to prevent code size increases in PGO builds. It doesn't use BFI if there's no profile summary available. However just before the BFI check, we also check to see if the function is marked as OptSize: https://reviews.llvm.org/D94559 And coincidentally sometime after the initial BFI patch PGOForceFunctionAttrsPass was added which will automatically annotate cold functions with OptSize: https://reviews.llvm.org/D149800 I think using PGOForceFunctionAttrs to add the OptSize is probably a more accurate and generalized way to prevent unwanted code size increases. So this patch proposes to remove the BFI check in SimpleLoopUnswitch. This isn't 100% the same behaviour since the previous behaviour checked for coldness at the loop level and this is now at the function level, but I believe the benefits outweigh this: - It allows us to remove BFI from the function to loop pass adapter, which was only enabled for certain stages in the LTO pipeline - We no longer have to worry about lossy analysis results - Which in turn means the decision to avoid non-trivial switching will be more accurate - It brings the behaviour inline with other passes that respect OptSize - It respects the -pgo-cold-func-opt flag so users can control the behaviour - It prevents the need to run OuterAnalysisManagerProxy as often which is probably good for compile time
1 parent 72596b3 commit 36844e9

16 files changed

+38
-134
lines changed

‎llvm/include/llvm/Analysis/LoopAnalysisManager.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ namespace llvm {
3636

3737
class AAResults;
3838
class AssumptionCache;
39-
class BlockFrequencyInfo;
4039
class DominatorTree;
4140
class Function;
4241
class Loop;
@@ -58,7 +57,6 @@ struct LoopStandardAnalysisResults {
5857
ScalarEvolution &SE;
5958
TargetLibraryInfo &TLI;
6059
TargetTransformInfo &TTI;
61-
BlockFrequencyInfo *BFI;
6260
MemorySSA *MSSA;
6361
};
6462

‎llvm/include/llvm/Transforms/Scalar/LoopPassManager.h‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -404,10 +404,8 @@ class FunctionToLoopPassAdaptor
404404

405405
explicit FunctionToLoopPassAdaptor(std::unique_ptr<PassConceptT> Pass,
406406
bool UseMemorySSA = false,
407-
bool UseBlockFrequencyInfo = false,
408407
bool LoopNestMode = false)
409408
: Pass(std::move(Pass)), UseMemorySSA(UseMemorySSA),
410-
UseBlockFrequencyInfo(UseBlockFrequencyInfo),
411409
LoopNestMode(LoopNestMode) {
412410
LoopCanonicalizationFPM.addPass(LoopSimplifyPass());
413411
LoopCanonicalizationFPM.addPass(LCSSAPass());
@@ -429,7 +427,6 @@ class FunctionToLoopPassAdaptor
429427
FunctionPassManager LoopCanonicalizationFPM;
430428

431429
bool UseMemorySSA = false;
432-
bool UseBlockFrequencyInfo = false;
433430
const bool LoopNestMode;
434431
};
435432

@@ -442,8 +439,7 @@ class FunctionToLoopPassAdaptor
442439
/// \c LoopPassManager and the returned adaptor will be in loop-nest mode.
443440
template <typename LoopPassT>
444441
inline FunctionToLoopPassAdaptor
445-
createFunctionToLoopPassAdaptor(LoopPassT &&Pass, bool UseMemorySSA = false,
446-
bool UseBlockFrequencyInfo = false) {
442+
createFunctionToLoopPassAdaptor(LoopPassT &&Pass, bool UseMemorySSA = false) {
447443
if constexpr (is_detected<HasRunOnLoopT, LoopPassT>::value) {
448444
using PassModelT =
449445
detail::PassModel<Loop, LoopPassT, LoopAnalysisManager,
@@ -453,7 +449,7 @@ createFunctionToLoopPassAdaptor(LoopPassT &&Pass, bool UseMemorySSA = false,
453449
return FunctionToLoopPassAdaptor(
454450
std::unique_ptr<FunctionToLoopPassAdaptor::PassConceptT>(
455451
new PassModelT(std::forward<LoopPassT>(Pass))),
456-
UseMemorySSA, UseBlockFrequencyInfo, false);
452+
UseMemorySSA, false);
457453
} else {
458454
LoopPassManager LPM;
459455
LPM.addPass(std::forward<LoopPassT>(Pass));
@@ -465,7 +461,7 @@ createFunctionToLoopPassAdaptor(LoopPassT &&Pass, bool UseMemorySSA = false,
465461
return FunctionToLoopPassAdaptor(
466462
std::unique_ptr<FunctionToLoopPassAdaptor::PassConceptT>(
467463
new PassModelT(std::move(LPM))),
468-
UseMemorySSA, UseBlockFrequencyInfo, true);
464+
UseMemorySSA, true);
469465
}
470466
}
471467

@@ -474,8 +470,7 @@ createFunctionToLoopPassAdaptor(LoopPassT &&Pass, bool UseMemorySSA = false,
474470
template <>
475471
inline FunctionToLoopPassAdaptor
476472
createFunctionToLoopPassAdaptor<LoopPassManager>(LoopPassManager &&LPM,
477-
bool UseMemorySSA,
478-
bool UseBlockFrequencyInfo) {
473+
bool UseMemorySSA) {
479474
// Check if LPM contains any loop pass and if it does not, returns an adaptor
480475
// in loop-nest mode.
481476
using PassModelT =
@@ -487,7 +482,7 @@ createFunctionToLoopPassAdaptor<LoopPassManager>(LoopPassManager &&LPM,
487482
return FunctionToLoopPassAdaptor(
488483
std::unique_ptr<FunctionToLoopPassAdaptor::PassConceptT>(
489484
new PassModelT(std::move(LPM))),
490-
UseMemorySSA, UseBlockFrequencyInfo, LoopNestMode);
485+
UseMemorySSA, LoopNestMode);
491486
}
492487

493488
/// Pass for printing a loop's contents as textual IR.

‎llvm/lib/Passes/PassBuilder.cpp‎

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1931,23 +1931,22 @@ Error PassBuilder::parseModulePass(ModulePassManager &MPM,
19311931
#define LOOPNEST_PASS(NAME, CREATE_PASS) \
19321932
if (Name == NAME) { \
19331933
MPM.addPass(createModuleToFunctionPassAdaptor( \
1934-
createFunctionToLoopPassAdaptor(CREATE_PASS, false, false))); \
1934+
createFunctionToLoopPassAdaptor(CREATE_PASS, false))); \
19351935
return Error::success(); \
19361936
}
19371937
#define LOOP_PASS(NAME, CREATE_PASS) \
19381938
if (Name == NAME) { \
19391939
MPM.addPass(createModuleToFunctionPassAdaptor( \
1940-
createFunctionToLoopPassAdaptor(CREATE_PASS, false, false))); \
1940+
createFunctionToLoopPassAdaptor(CREATE_PASS, false))); \
19411941
return Error::success(); \
19421942
}
19431943
#define LOOP_PASS_WITH_PARAMS(NAME, CLASS, CREATE_PASS, PARSER, PARAMS) \
19441944
if (checkParametrizedPassName(Name, NAME)) { \
19451945
auto Params = parsePassParameters(PARSER, Name, NAME); \
19461946
if (!Params) \
19471947
return Params.takeError(); \
1948-
MPM.addPass( \
1949-
createModuleToFunctionPassAdaptor(createFunctionToLoopPassAdaptor( \
1950-
CREATE_PASS(Params.get()), false, false))); \
1948+
MPM.addPass(createModuleToFunctionPassAdaptor( \
1949+
createFunctionToLoopPassAdaptor(CREATE_PASS(Params.get()), false))); \
19511950
return Error::success(); \
19521951
}
19531952
#include "PassRegistry.def"
@@ -2046,23 +2045,22 @@ Error PassBuilder::parseCGSCCPass(CGSCCPassManager &CGPM,
20462045
#define LOOPNEST_PASS(NAME, CREATE_PASS) \
20472046
if (Name == NAME) { \
20482047
CGPM.addPass(createCGSCCToFunctionPassAdaptor( \
2049-
createFunctionToLoopPassAdaptor(CREATE_PASS, false, false))); \
2048+
createFunctionToLoopPassAdaptor(CREATE_PASS, false))); \
20502049
return Error::success(); \
20512050
}
20522051
#define LOOP_PASS(NAME, CREATE_PASS) \
20532052
if (Name == NAME) { \
20542053
CGPM.addPass(createCGSCCToFunctionPassAdaptor( \
2055-
createFunctionToLoopPassAdaptor(CREATE_PASS, false, false))); \
2054+
createFunctionToLoopPassAdaptor(CREATE_PASS, false))); \
20562055
return Error::success(); \
20572056
}
20582057
#define LOOP_PASS_WITH_PARAMS(NAME, CLASS, CREATE_PASS, PARSER, PARAMS) \
20592058
if (checkParametrizedPassName(Name, NAME)) { \
20602059
auto Params = parsePassParameters(PARSER, Name, NAME); \
20612060
if (!Params) \
20622061
return Params.takeError(); \
2063-
CGPM.addPass( \
2064-
createCGSCCToFunctionPassAdaptor(createFunctionToLoopPassAdaptor( \
2065-
CREATE_PASS(Params.get()), false, false))); \
2062+
CGPM.addPass(createCGSCCToFunctionPassAdaptor( \
2063+
createFunctionToLoopPassAdaptor(CREATE_PASS(Params.get()), false))); \
20662064
return Error::success(); \
20672065
}
20682066
#include "PassRegistry.def"
@@ -2095,11 +2093,8 @@ Error PassBuilder::parseFunctionPass(FunctionPassManager &FPM,
20952093
return Err;
20962094
// Add the nested pass manager with the appropriate adaptor.
20972095
bool UseMemorySSA = (Name == "loop-mssa");
2098-
bool UseBFI = llvm::any_of(InnerPipeline, [](auto Pipeline) {
2099-
return Pipeline.Name.contains("simple-loop-unswitch");
2100-
});
2101-
FPM.addPass(createFunctionToLoopPassAdaptor(std::move(LPM), UseMemorySSA,
2102-
UseBFI));
2096+
FPM.addPass(
2097+
createFunctionToLoopPassAdaptor(std::move(LPM), UseMemorySSA));
21032098
return Error::success();
21042099
}
21052100
if (Name == "machine-function") {
@@ -2152,21 +2147,21 @@ Error PassBuilder::parseFunctionPass(FunctionPassManager &FPM,
21522147
// The risk is that it may become obsolete if we're not careful.
21532148
#define LOOPNEST_PASS(NAME, CREATE_PASS) \
21542149
if (Name == NAME) { \
2155-
FPM.addPass(createFunctionToLoopPassAdaptor(CREATE_PASS, false, false)); \
2150+
FPM.addPass(createFunctionToLoopPassAdaptor(CREATE_PASS, false)); \
21562151
return Error::success(); \
21572152
}
21582153
#define LOOP_PASS(NAME, CREATE_PASS) \
21592154
if (Name == NAME) { \
2160-
FPM.addPass(createFunctionToLoopPassAdaptor(CREATE_PASS, false, false)); \
2155+
FPM.addPass(createFunctionToLoopPassAdaptor(CREATE_PASS, false)); \
21612156
return Error::success(); \
21622157
}
21632158
#define LOOP_PASS_WITH_PARAMS(NAME, CLASS, CREATE_PASS, PARSER, PARAMS) \
21642159
if (checkParametrizedPassName(Name, NAME)) { \
21652160
auto Params = parsePassParameters(PARSER, Name, NAME); \
21662161
if (!Params) \
21672162
return Params.takeError(); \
2168-
FPM.addPass(createFunctionToLoopPassAdaptor(CREATE_PASS(Params.get()), \
2169-
false, false)); \
2163+
FPM.addPass( \
2164+
createFunctionToLoopPassAdaptor(CREATE_PASS(Params.get()), false)); \
21702165
return Error::success(); \
21712166
}
21722167
#include "PassRegistry.def"

‎llvm/lib/Passes/PassBuilderPipelines.cpp‎

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -517,16 +517,14 @@ PassBuilder::buildO1FunctionSimplificationPipeline(OptimizationLevel Level,
517517
invokeLoopOptimizerEndEPCallbacks(LPM2, Level);
518518

519519
FPM.addPass(createFunctionToLoopPassAdaptor(std::move(LPM1),
520-
/*UseMemorySSA=*/true,
521-
/*UseBlockFrequencyInfo=*/true));
520+
/*UseMemorySSA=*/true));
522521
FPM.addPass(
523522
SimplifyCFGPass(SimplifyCFGOptions().convertSwitchRangeToICmp(true)));
524523
FPM.addPass(InstCombinePass());
525524
// The loop passes in LPM2 (LoopFullUnrollPass) do not preserve MemorySSA.
526525
// *All* loop passes must preserve it, in order to be able to use it.
527526
FPM.addPass(createFunctionToLoopPassAdaptor(std::move(LPM2),
528-
/*UseMemorySSA=*/false,
529-
/*UseBlockFrequencyInfo=*/false));
527+
/*UseMemorySSA=*/false));
530528

531529
// Delete small array after loop unroll.
532530
FPM.addPass(SROAPass(SROAOptions::ModifyCFG));
@@ -706,17 +704,15 @@ PassBuilder::buildFunctionSimplificationPipeline(OptimizationLevel Level,
706704
invokeLoopOptimizerEndEPCallbacks(LPM2, Level);
707705

708706
FPM.addPass(createFunctionToLoopPassAdaptor(std::move(LPM1),
709-
/*UseMemorySSA=*/true,
710-
/*UseBlockFrequencyInfo=*/true));
707+
/*UseMemorySSA=*/true));
711708
FPM.addPass(
712709
SimplifyCFGPass(SimplifyCFGOptions().convertSwitchRangeToICmp(true)));
713710
FPM.addPass(InstCombinePass());
714711
// The loop passes in LPM2 (LoopIdiomRecognizePass, IndVarSimplifyPass,
715712
// LoopDeletionPass and LoopFullUnrollPass) do not preserve MemorySSA.
716713
// *All* loop passes must preserve it, in order to be able to use it.
717714
FPM.addPass(createFunctionToLoopPassAdaptor(std::move(LPM2),
718-
/*UseMemorySSA=*/false,
719-
/*UseBlockFrequencyInfo=*/false));
715+
/*UseMemorySSA=*/false));
720716

721717
// Delete small array after loop unroll.
722718
FPM.addPass(SROAPass(SROAOptions::ModifyCFG));
@@ -769,7 +765,7 @@ PassBuilder::buildFunctionSimplificationPipeline(OptimizationLevel Level,
769765
FPM.addPass(createFunctionToLoopPassAdaptor(
770766
LICMPass(PTO.LicmMssaOptCap, PTO.LicmMssaNoAccForPromotionCap,
771767
/*AllowSpeculation=*/true),
772-
/*UseMemorySSA=*/true, /*UseBlockFrequencyInfo=*/false));
768+
/*UseMemorySSA=*/true));
773769

774770
FPM.addPass(CoroElidePass());
775771

@@ -837,8 +833,7 @@ void PassBuilder::addPostPGOLoopRotation(ModulePassManager &MPM,
837833
createFunctionToLoopPassAdaptor(
838834
LoopRotatePass(EnableLoopHeaderDuplication ||
839835
Level != OptimizationLevel::Oz),
840-
/*UseMemorySSA=*/false,
841-
/*UseBlockFrequencyInfo=*/false),
836+
/*UseMemorySSA=*/false),
842837
PTO.EagerlyInvalidateAnalyses));
843838
}
844839
}
@@ -1354,8 +1349,7 @@ void PassBuilder::addVectorPasses(OptimizationLevel Level,
13541349
LPM.addPass(SimpleLoopUnswitchPass(/* NonTrivial */ Level ==
13551350
OptimizationLevel::O3));
13561351
ExtraPasses.addPass(
1357-
createFunctionToLoopPassAdaptor(std::move(LPM), /*UseMemorySSA=*/true,
1358-
/*UseBlockFrequencyInfo=*/true));
1352+
createFunctionToLoopPassAdaptor(std::move(LPM), /*UseMemorySSA=*/true));
13591353
ExtraPasses.addPass(
13601354
SimplifyCFGPass(SimplifyCFGOptions().convertSwitchRangeToICmp(true)));
13611355
ExtraPasses.addPass(InstCombinePass());
@@ -1433,7 +1427,7 @@ void PassBuilder::addVectorPasses(OptimizationLevel Level,
14331427
FPM.addPass(createFunctionToLoopPassAdaptor(
14341428
LICMPass(PTO.LicmMssaOptCap, PTO.LicmMssaNoAccForPromotionCap,
14351429
/*AllowSpeculation=*/true),
1436-
/*UseMemorySSA=*/true, /*UseBlockFrequencyInfo=*/false));
1430+
/*UseMemorySSA=*/true));
14371431

14381432
// Now that we've vectorized and unrolled loops, we may have more refined
14391433
// alignment information, try to re-derive it here.
@@ -1510,7 +1504,7 @@ PassBuilder::buildModuleOptimizationPipeline(OptimizationLevel Level,
15101504
OptimizePM.addPass(createFunctionToLoopPassAdaptor(
15111505
LICMPass(PTO.LicmMssaOptCap, PTO.LicmMssaNoAccForPromotionCap,
15121506
/*AllowSpeculation=*/true),
1513-
/*USeMemorySSA=*/true, /*UseBlockFrequencyInfo=*/false));
1507+
/*USeMemorySSA=*/true));
15141508
}
15151509

15161510
OptimizePM.addPass(Float2IntPass());
@@ -1550,8 +1544,8 @@ PassBuilder::buildModuleOptimizationPipeline(OptimizationLevel Level,
15501544
if (PTO.LoopInterchange)
15511545
LPM.addPass(LoopInterchangePass());
15521546

1553-
OptimizePM.addPass(createFunctionToLoopPassAdaptor(
1554-
std::move(LPM), /*UseMemorySSA=*/false, /*UseBlockFrequencyInfo=*/false));
1547+
OptimizePM.addPass(
1548+
createFunctionToLoopPassAdaptor(std::move(LPM), /*UseMemorySSA=*/false));
15551549

15561550
// FIXME: This may not be the right place in the pipeline.
15571551
// We need to have the data to support the right place.
@@ -2100,7 +2094,7 @@ PassBuilder::buildLTODefaultPipeline(OptimizationLevel Level,
21002094
MainFPM.addPass(createFunctionToLoopPassAdaptor(
21012095
LICMPass(PTO.LicmMssaOptCap, PTO.LicmMssaNoAccForPromotionCap,
21022096
/*AllowSpeculation=*/true),
2103-
/*USeMemorySSA=*/true, /*UseBlockFrequencyInfo=*/false));
2097+
/*USeMemorySSA=*/true));
21042098

21052099
if (RunNewGVN)
21062100
MainFPM.addPass(NewGVNPass());
@@ -2130,8 +2124,8 @@ PassBuilder::buildLTODefaultPipeline(OptimizationLevel Level,
21302124
PTO.ForgetAllSCEVInLoopUnroll));
21312125
// The loop passes in LPM (LoopFullUnrollPass) do not preserve MemorySSA.
21322126
// *All* loop passes must preserve it, in order to be able to use it.
2133-
MainFPM.addPass(createFunctionToLoopPassAdaptor(
2134-
std::move(LPM), /*UseMemorySSA=*/false, /*UseBlockFrequencyInfo=*/true));
2127+
MainFPM.addPass(
2128+
createFunctionToLoopPassAdaptor(std::move(LPM), /*UseMemorySSA=*/false));
21352129

21362130
MainFPM.addPass(LoopDistributePass());
21372131

‎llvm/lib/Transforms/Scalar/LoopPassManager.cpp‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
#include "llvm/Transforms/Scalar/LoopPassManager.h"
1010
#include "llvm/Analysis/AssumptionCache.h"
11-
#include "llvm/Analysis/BlockFrequencyInfo.h"
1211
#include "llvm/Analysis/MemorySSA.h"
1312
#include "llvm/Analysis/ScalarEvolution.h"
1413
#include "llvm/Analysis/TargetLibraryInfo.h"
@@ -219,17 +218,13 @@ PreservedAnalyses FunctionToLoopPassAdaptor::run(Function &F,
219218
// Get the analysis results needed by loop passes.
220219
MemorySSA *MSSA =
221220
UseMemorySSA ? (&AM.getResult<MemorySSAAnalysis>(F).getMSSA()) : nullptr;
222-
BlockFrequencyInfo *BFI = UseBlockFrequencyInfo && F.hasProfileData()
223-
? (&AM.getResult<BlockFrequencyAnalysis>(F))
224-
: nullptr;
225221
LoopStandardAnalysisResults LAR = {AM.getResult<AAManager>(F),
226222
AM.getResult<AssumptionAnalysis>(F),
227223
AM.getResult<DominatorTreeAnalysis>(F),
228224
AM.getResult<LoopAnalysis>(F),
229225
AM.getResult<ScalarEvolutionAnalysis>(F),
230226
AM.getResult<TargetLibraryAnalysis>(F),
231227
AM.getResult<TargetIRAnalysis>(F),
232-
BFI,
233228
MSSA};
234229

235230
// Setup the loop analysis manager from its proxy. It is important that

0 commit comments

Comments
 (0)