tejohnson added a comment.
Overall I like this approach.
================
Comment at: llvm/lib/Passes/PassBuilder.cpp:246
-static bool isOptimizingForSize(PassBuilder::OptimizationLevel Level) {
- switch (Level) {
- case PassBuilder::O0:
- case PassBuilder::O1:
- case PassBuilder::O2:
- case PassBuilder::O3:
- return false;
-
- case PassBuilder::Os:
- case PassBuilder::Oz:
- return true;
- }
- llvm_unreachable("Invalid optimization level!");
-}
+const PassBuilder::OptimizationLevel PassBuilder::OptimizationLevel::O0 = {0,
+ 0};
----------------
Nit, it would be good to document the constant parameters, e.g.
{/*SpeedLevel*/0, /*SizeLevel*/0}
================
Comment at: llvm/lib/Passes/PassBuilder.cpp:407
// Hoisting of scalars and load expressions.
- if (Level > O1) {
+ if (Level.getSpeedupLevel() >= 2) {
if (EnableGVNHoist)
----------------
Nit, all similar checks below are Level.getSpeedupLevel() > 1, make this one
consistent.
================
Comment at: llvm/lib/Passes/PassBuilder.cpp:487
PTO.LoopUnrolling)
- LPM2.addPass(LoopFullUnrollPass(Level, /*OnlyWhenForced=*/false,
+ LPM2.addPass(LoopFullUnrollPass(Level.getSpeedupLevel(),
+ /*OnlyWhenForced=*/false,
----------------
This results in a behavior, change, right? I.e. we used to inadvertently get
the O3 threshold for full unrolling with Oz/Os but no longer will. If so, make
sure you note this in the patch summary. Also add a test.
================
Comment at: llvm/lib/Passes/PassBuilder.cpp:973
if (EnableUnrollAndJam && PTO.LoopUnrolling) {
- OptimizePM.addPass(LoopUnrollAndJamPass(Level));
+ OptimizePM.addPass(LoopUnrollAndJamPass(Level.getSpeedupLevel()));
}
----------------
This one and the loop unrolling pass below will also get a change in behavior
for Os/Oz, correct? That seems reasonable, but needs to be noted in summary and
tested.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72547/new/
https://reviews.llvm.org/D72547
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
