chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.
Minor nits around redundant predicates for SROA. With thouse fixed, LGTM.
I'd really love to find a way to make TCO debuggable so that we don't lose
that. I'm particularly worried about code that relies on it to not run out of
stack. Not sure what the best thing to do here is though. Anyways, not relevant
for this iteration. I mostly feel bad for a potential future re-churn of all
the tests. ;]
================
Comment at: llvm/lib/Passes/PassBuilder.cpp:395
// scalars.
- FPM.addPass(SROA());
+ if (Level >= O1)
+ FPM.addPass(SROA());
----------------
We know `O0` isn't used here, so this should be a no-op.
================
Comment at: llvm/lib/Transforms/IPO/PassManagerBuilder.cpp:263
FPM.add(createCFGSimplificationPass());
- FPM.add(createSROAPass());
+ if (OptLevel >= 1)
+ FPM.add(createSROAPass());
----------------
We early exit at `O0` above, so this is a no-op.
================
Comment at: llvm/lib/Transforms/IPO/PassManagerBuilder.cpp:294
MPM.add(createFunctionInliningPass(IP));
- MPM.add(createSROAPass());
+ if (OptLevel >= 1)
+ MPM.add(createSROAPass());
----------------
We only reach here if `OptLevel > 0` so this should be redundant?
================
Comment at: llvm/lib/Transforms/IPO/PassManagerBuilder.cpp:325
// Break up aggregate allocas, using SSAUpdater.
- MPM.add(createSROAPass());
+ if (OptLevel >= 1)
+ MPM.add(createSROAPass());
----------------
This doesn't have the assert, but I believe this is only used above `O0` as
well. Maybe just add the assert?
================
Comment at: llvm/test/Other/new-pm-defaults.ll:253
+; CHECK-Os-NEXT: Finished llvm::Function pass manager run.
+; CHECK-Oz-NEXT: Finished llvm::Function pass manager run.
; CHECK-EP-SCALAR-LATE-NEXT: Running pass: NoOpFunctionPass
----------------
mehdi_amini wrote:
> Just a drive-by idea: you could have simplified the changes here with a new
> prefix: "CHECK-O23sz" that you could add to the non-O1 invocations.
>
Good idea!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65410/new/
https://reviews.llvm.org/D65410
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
