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

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev marked 3 inline comments as done.
mkazantsev added inline comments.


================
Comment at: lib/Transforms/Scalar/GVN.cpp:2406
+      if (!isGuaranteedToTransferExecutionToSuccessor(&I) &&
+          !isa<LoadInst>(&I) && !isa<StoreInst>(&I)) {
+        FirstImplicitControlFlowInsts[BB] = &I;
----------------
efriedma wrote:
> 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.
Added assert that the said load and store are volatile. Otherwise `isGuaranteedToTransferExecutionToSuccessor` is true for them.


https://reviews.llvm.org/D37460