[PATCH] D80120: [InstCombine] Sink pure instructions down to return and unreachable blocks

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev marked an inline comment as done.
mkazantsev added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:3309
+    if (DestBlock->getUniquePredecessor() != I->getParent())
+      return false;
     for (BasicBlock::iterator Scan = I->getIterator(),
----------------
mkazantsev wrote:
> jdoerfert wrote:
> > I first thought:
> > This is more restrictive than you want it to be.
> > `DestBlock != I->getParent()->getUniqueSuccessor` is what u want.
> > (There is even a less strict restriction but that is more complicated.)
> > That allows `DestBlock` to have multiple predecessors (maybe add at test).
> > 
> > Then I realized this is also the test outside. Though, I still think the above is what we want here.
> This is not true. We don't want to sink from A to C in case:
>   A    B
>    \  /
>     C
> because BC may be a hot path and A is "nearly never executed" block.
Maybe the example above will be more clear like this:

    A
    | 
    C<-|
    |  |
    B--|

Though, we want to sink from A to either B or C in case
    A
   /  \
  B    C


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

https://reviews.llvm.org/D80120