[PATCH] D46211: [LICM] Compute a must execute property for the prefix of the header as we go

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev accepted this revision.
mkazantsev added a comment.
This revision is now accepted and ready to land.

LGTM once comments are addressed.



================
Comment at: lib/Transforms/Scalar/LICM.cpp:490
         Product->insertAfter(&I);
         I.replaceAllUsesWith(Product);
         I.eraseFromParent();
----------------
Here we create some instructions. I think all of them are fine, but doesn't it make sense to assert that each of them transfers execution to successor? Just in case this logic ever changes.


================
Comment at: lib/Transforms/Scalar/LICM.cpp:508
+               CurLoop->getLoopPreheader()->getTerminator())))
         Changed |= hoist(I, DT, CurLoop, SafetyInfo, ORE);
+
----------------
If hoist was successful, we don't actually need to make `isGuaranteedToTransferExecutionToSuccessor` check on the instruction that is no longer in this block. Maybe replace with assert in this case?


Repository:
  rL LLVM

https://reviews.llvm.org/D46211