[PATCH] D37460: [GVN] Prevent LoadPRE from hoisting across instructions that don't pass control flow to successors

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 3 11:24:33 PDT 2017


efriedma added inline comments.


================
Comment at: lib/Transforms/Scalar/GVN.cpp:2406
+      if (!isGuaranteedToTransferExecutionToSuccessor(&I) &&
+          !isa<LoadInst>(&I) && !isa<StoreInst>(&I)) {
+        FirstImplicitControlFlowInsts[BB] = &I;
----------------
mkazantsev wrote:
> reames wrote:
> > This is scarily permissive.  It (for instance) allows ordered loads and stores which (I think) is valid.  It'd be much safer to add a special matcher for volatile-only loads/stores here.
> Will do.
Hoisting a load across an Acquire operation is often not allowed, yes... but alias analysis will catch that; we don't need to handle it here.


https://reviews.llvm.org/D37460





More information about the llvm-commits mailing list