[PATCH] D116692: [SimplifyCFG] Tail-merging all blocks with `unreachable` terminator, final take

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D116692#3249159 <https://reviews.llvm.org/D116692#3249159>, @lebedev.ri wrote:

>> I should probably test how this impacts rust code (which has a lot of unreachable terminators in release builds due to bounds checks), though that requires applying this patch on top of LLVM 13.
>
> Were you able to to so?

Sorry for the delay. I tested this together with your recent commit removing sinking limitations for unreachable blocks. The result was close to no change in either compile-time or run-time (where "run-time" here is non-LLVM compile-time, so a fairly narrow workload). All sub-1% and mostly below the significance threshold. Unfortunately I wasn't able to get code size information because the necessary infrastructure was broken at the time. I did look at some rustc artifacts as a sanity check and the only larger increase I spotted was rustdoc by +0.4%.

I played with the patch a bit, and found that this approach has one major limitation as far as rust code is concerned: It only works if you have a single assert/panic/whatever function. All unreachable terminators are merged together and we can then only sink if the predecessors have the same call. Rust has a bunch of different panic functions depending on the situation. So if you have a bunch of array accesses that all call panic_bounds_check and then add a single assert, then the tail merging stops working. I expect that significantly limits the cases where the optimization applies.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116692/new/

https://reviews.llvm.org/D116692