[PATCH] D28147: [LICM] Allow promotion of some stores that are not guaranteed to execute

[Michael Kuperstein via Phabricator via llvm-commits]

mkuper added inline comments.


================
Comment at: lib/Transforms/Scalar/LICM.cpp:1019
+            return DT->dominates(Store->getParent(), Exit);
+          });
+
----------------
efriedma wrote:
> mkuper wrote:
> > efriedma wrote:
> > > This check is similar to llvm::isGuaranteedToExecute.  Maybe worth noting the parallel here.
> > > 
> > > If the loop throws, you're depending on the earlier check that the memory location is an alloca; you should explicitly note that here.
> > I'm not depending on it being an alloca. I'm only depending on it being dereferenceable - which is the DereferenceableInPH check and is documented. So I'm not sure how this can be more explicit.
> The implicit unwind edges from the loop aren't listed in ExitBlocks, so you aren't checking them.  If the memory is an alloca, this doesn't matter because the unwind edge will end the lifetime of the alloca; if we somehow extended LICM so it could make implicit unwind edges explicit, the missing check might matter.
Oh, got it, you mean the check on line 951. Right, will add a comment.


https://reviews.llvm.org/D28147