[PATCH] D28241: [LICM] Untangle some of the logic in LICM canSinkOrHoistInst

[Xin Tong via Phabricator via llvm-commits]

trentxintong added inline comments.


================
Comment at: lib/Transforms/Scalar/LICM.cpp:366
     if (isNotUsedInLoop(I, CurLoop, SafetyInfo) &&
-        canSinkOrHoistInst(I, AA, DT, CurLoop, CurAST, SafetyInfo)) {
+        canSinkOrHoistInst(I, AA, DT, CurLoop, CurAST)) {
       ++II;
----------------
danielcdh wrote:
> isSafeToExecuteUnconditionally will not be called in this function, is it safe?
I think it is safe. 

This is the way I see LICM in case of sinking. 

Say originally, the sunk instruction is InstructionX in BasicBlockX

Is there a way we sink the instruction to some exit blocks where the instruction is used and the exit block could get executed even the BBX does not. (If thats the case we may have a problem as its not safe to speculate the instruction then). From what I see, we cant, because if we use InstructionX in this exit block in a special _single incoming value_ phi node, BBX must dominate this exit block. This means by the time we execute the exit block, BBX must have been executed already. 

NOTE: There is the case of the block not dominating the exit block where the value is used, this is done by Phi nodes with multiple incoming value in the exit block. Then we would not sink this value. This is checked in isNotUsedInLoop.



https://reviews.llvm.org/D28241