[SimplifyCFG][PhaseOrdering] Defer lowering switch into an integer range comparison and branch until after at least the IPSCCP #132
Conversation
Based on original tests from D119839. See llvm#53853
…nge comparison and branch until after at least the IPSCCP That transformation is lossy, as discussed in llvm#53853 and rust-lang/rust#85133 (comment) This is an alternative to D119839, which would add a limited IPSCCP into SimplifyCFG. Unlike lowering switch to lookup, we still want this transformation to happen relatively early, but after giving a chance for the things like CVP to do their thing. It seems like deferring it just until the IPSCCP is enough for the tests at hand, but perhaps we need to be more aggressive and disable it until CVP. Fixes llvm#53853 Refs. rust-lang/rust#85133 Reviewed By: nikic Differential Revision: https://reviews.llvm.org/D119854
|
Is this really something we must backport? I don't recall us doing so all that often, except in the cases where it fixes a serious regression or something of the similar nature. |
|
I'd rather not wait another 6 months until LLVM 15 to get this optimization, but if you feel strongly about it then I'm happy to close this. |
|
There's an upstream backport request for this change. If you can do a perf run to sanity-check this change, I'll approve the upstream backport, so this will be part of LLVM 14 and we don't have to diverge. |
|
Perf run started in rust-lang/rust#94424. I'd also like to have an answer to rust-lang/rust#85133 (comment), so I can check whether this actually fixes the problem it is supposed to fix. |
|
As this doesn't actually address the original issue, I don't think it makes sense to backport this. I have some patched pending that do (together with this) fix it, but I don't think we'll want to backport the whole stack. |
That transformation is lossy, as discussed in
llvm#53853
and rust-lang/rust#85133 (comment)
This is an alternative to D119839,
which would add a limited IPSCCP into SimplifyCFG.
Unlike lowering switch to lookup, we still want this transformation
to happen relatively early, but after giving a chance for the things
like CVP to do their thing. It seems like deferring it just until
the IPSCCP is enough for the tests at hand, but perhaps we need to
be more aggressive and disable it until CVP.
Fixes llvm#53853
Refs. rust-lang/rust#85133
Reviewed By: nikic
Differential Revision: reviews.llvm.org/D119854