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

[Michael Kuperstein via Phabricator via llvm-commits]

mkuper added a comment.

I'm a bit confused about the distinction between "legal" and "safe". I'd assume them to mean the same thing.



================
Comment at: lib/Transforms/Scalar/LICM.cpp:422
               I, DT, CurLoop, SafetyInfo,
               CurLoop->getLoopPreheader()->getTerminator()))
         Changed |= hoist(I, DT, CurLoop, SafetyInfo);
----------------
danielcdh wrote:
> isSafeToExecuteUnconditionally was called twice (without the patch):
> 
> * once in canSinkOrHoistInst, with CtxI=nullptr
> * once here, with Ctxl=CurLoop->getLoopPreheader()->getTerminator()
> 
> I'm not quite familiar with isSafeToExecuteUnconditionally, but looks to me different CtxI may have different meaning to check? Or am I missing something?
Passing CtxI == nullptr is conservative w.r.t any other CtxI, so that looks like it should be fine.


https://reviews.llvm.org/D28241