Adjust the pipeline slightly to move ConstraintElim just before the loop
simplification pipeline. This increases the number of cases where SCEV
should can preserved in the future. It also allows additional
simplification after SimplifyCFG converts switches to regular compares.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I can see how ConstraintElimination may benefit from dealing with regular compares, but wouldn't CE provide a more amenable IR for SimplifyCFG to work with? I just wonder whether we might risk to miss potential SimplifyCFG opportunities if we postpone this. Running InstCombine, AggressiveIC, and CE together early in the pipeline looks beneficial too for CFG simplification purposes, AFAICT.
To be clear: if the same previous opportunities would continue being caught by having SimplifyCFG run before, I see the rationale. Not sure if there's, e.g., some threshold on the number of cases to regular compares simplification for which that would not happen otherwise. Maybe would be nice to have some case that proves the benefit of this.
I think CFG simplifications based on simplified conditions by ConstraintElimination should still happen before other optimizations; now we run the loop optimization pipeline LPM1 directly after ConstraintElimination, which runs LoopSimplifyCFG early. This should take care of simplifying branches based on simplified conditions for loops, which is the scope the other passes in the pipeline operate on.
Straight after LPM1, we run SimplifyCFGPass again, which should simplify branches in non-loop blocks, before other optimizations.
Yeah I had one somewhere. let me put one together.
Rebased on top of phase-ordering test showing impact (added in e6ebd2890f0e). Also depends on https://github.com/llvm/llvm-project/pull/67061
llvm/lib/Passes/PassBuilderPipelines.cpp | ||
---|---|---|
580 | unrelated but can we remove the flag now? |
llvm/lib/Passes/PassBuilderPipelines.cpp | ||
---|---|---|
580 | Yep definitely. Would do as a follow up. |
LGTM
Could you please add a few words to the patch description on how this improves optimization of the phase ordering test? It's not super obvious looking at just the diff.
Thanks! I extended the description, it's not obvious and a side effect of SimplifyCFG adjusting the CFG so it allows us to add additional conditions. This seems to lead to a few improvements overall which is nice, but the main benefit is to slightly reduce the compile-time when using SCEV in the pass for D152730
unrelated but can we remove the flag now?