[SimplifyCFG] FoldBranchToCommonDest(): don't deal with unconditional branches
authorRoman Lebedev <lebedev.ri@gmail.com>
Thu, 21 Jan 2021 16:45:41 +0000 (19:45 +0300)
committerRoman Lebedev <lebedev.ri@gmail.com>
Fri, 22 Jan 2021 14:22:49 +0000 (17:22 +0300)
commit0895b836d74ed333468ddece2102140494eb33b6
treecfa8e1e0676fc73b63e5e74e0eb8e99fca3b9f4e
parent98a8344895a8e1f2cfa98b664b50fb7b864afa52
[SimplifyCFG] FoldBranchToCommonDest(): don't deal with unconditional branches

The case where BB ends with an unconditional branch,
and has a single predecessor w/ conditional branch
to BB and a single successor of BB is exactly the pattern
SpeculativelyExecuteBB() transform deals with.
(and in this case they both allow speculating only a single instruction)

Well, or FoldTwoEntryPHINode(), if the final block
has only those two predecessors.

Here, in FoldBranchToCommonDest(), only a weird subset of that
transform is supported, and it's glued on the side in a weird way.
  In particular, it took me a bit to understand that the Cond
isn't actually a branch condition in that case, but just the value
we allow to speculate (otherwise it reads as a miscompile to me).
  Additionally, this only supports for the speculated instruction
to be an ICmp.

So let's just unclutter FoldBranchToCommonDest(), and leave
this transform up to SpeculativelyExecuteBB(). As far as i can tell,
this shouldn't really impact optimization potential, but if it does,
improving SpeculativelyExecuteBB() will be more beneficial anyways.

Notably, this only affects a single test,
but EarlyCSE should have run beforehand in the pipeline,
and then FoldTwoEntryPHINode() would have caught it.

This reverts commit rL158392 / commit d33f4efbfdef6ffccf212ab3e40a7673589085fd.
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/test/Transforms/SimplifyCFG/X86/fold-branch-debuginvariant.ll [deleted file]
llvm/test/Transforms/SimplifyCFG/branch-fold.ll