[llvm] [SimplifyCFG] Add predecessor threshold for TryToSimplifyUncondBranchFromEmptyBlock optimization. (PR #110715)

[Amara Emerson via llvm-commits]

aemerson wrote:

> Could you please explain in more detail where the compile-time issue comes from? Just many predecessors is not a problem by itself, something must be causing quadratic complexity if this is an issue.
> 
> 128 is really not particularly high in this context, that's something you'll easily see with a large switch statement.
> 
> llvm-opt-benchmark results indicate that this does improve compile-time in some cases, but it also makes it _much_ worse in others:
> 
> ```
> 
> Top 5 improvements:
>   cvc5/kind.cpp.ll 600352836 -> 215689291 -64.07%
>   php/parse_date.ll 46278212535 -> 21177805250 -54.24%
>   llvm/X86EncodingOptimization.cpp.ll 908626738 -> 436539081 -51.96%
>   llvm/TokenKinds.cpp.ll 319626734 -> 156038687 -51.18%
>   libquic/net_errors.cc.ll 234434780 -> 126031135 -46.24%
> Top 5 regressions:
>   php/pcre2_match.ll 10618116865 -> 997582429963 +9295.10%
>   llvm/X86FixupInstTuning.cpp.ll 1011415693 -> 1822547362 +80.20%
>   tokenizers-rs/4hn9gefsll13qr1r.ll 18284264366 -> 28850854248 +57.79%
>   cpython/socketmodule.ll 1538073290 -> 1868852500 +21.51%
>   openusd/write.c.ll 1139618620 -> 1299783905 +14.05%
> ```

Thanks for the data, I didn't know that benchmark suite existed, very cool!

Our pathological case is quite dumb, it's not particularly any quadratic behavior I think (unless I'm mistaken). The issue was introduced by fc6bdb8549842613da51b9d570b29e27cc709f69 which added an unconditional initialization of the successor pointer set. In our test case that pointer set ends up being filled hundreds of thousands of times with 50k elements each time.

In any case, I realized that this initial fix was the wrong thing to do anyway because we should only be trying to fix the new regression introduced by that change, the optimization before fc6bdb8549842613da51b9d570b29e27cc709f69 was fine by itself. I've changed the fix to instead try to sink the initialization of that set as late as possible and to re-use the predecessor walk done by `CanRedirectPredsOfEmptyBBToSucc()`. It makes the implementation messier unfortunately, but it's now an NFC change.

https://github.com/llvm/llvm-project/pull/110715