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

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added inline comments.


================
Comment at: lib/Transforms/Scalar/GVN.cpp:2070
+        !isa<LoadInst>(&*BI) && !isa<StoreInst>(&*BI))
+      BlocksWithImplicitControlFlow.insert(BB);
+
----------------
It's worth noting:

The RPO iteration order only guarantees that checking straight line predecessors is safe[1]
It would not be safe to check other predecessors unless the program is loop free
(IE such a thing would have to be computed separately from this loop).
 

[1] I can prove this if you like - A visit order dfs walk of the dominator tree is a valid RPO order [2]. Any straight line single predecessor must be your idom, because there is no path around it to your node.  Because the dominator tree is acylic, and a valid RPO order, RPO order must guarantee you visit this node before you.    
The only way something else can be visited first is if there was another path to your node (which would in turn prove that that your idom is not really your idom)
, 
[2] LLVM does not happen to keep it in the sort order, but you can see that, for example, NewGVN sorts it back into the "right" order. 


https://reviews.llvm.org/D37460