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

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 4 15:19:55 PST 2017


efriedma added inline comments.


================
Comment at: lib/Transforms/Scalar/LICM.cpp:1019
+            return DT->dominates(Store->getParent(), Exit);
+          });
+
----------------
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.


https://reviews.llvm.org/D28147





More information about the llvm-commits mailing list