[llvm] [SimplifyCFG] Enable MergeBlockIntoPredecessor with two successors (PR #143766)

[Nikita Popov via llvm-commits]

https://github.com/nikic requested changes to this pull request.

It looks like you modified just MergeBlockIntoPredecessor without actually using the PredecessorWithTwoSuccessors mode anywhere in SimplifyCFG, so I don't see how your test could possibly work.

If I understand what you're trying to do, this is effectively speculating blocks into predecessors. Checking for side effects is incorrect there, you need to check speculability. For example, in your test case speculation is *not* legal because the loads are non-speculatable. On top of that, speculating instructions already requires cost modelling, you can't just dump a block of arbitrary size into the predecessor, even though it may not be executed at all.

SimplifyCFG *already* has various transforms that do this correctly and with cost modelling.

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