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

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: lib/Transforms/Scalar/GVN.cpp:2210
+    if (!IsSafeToSpeculativelyExecute && FirstImplicitControlFlowInsts.count(P))
+      return false;
     // We're not interested in PRE where blocks with predecessors that are
----------------
efriedma wrote:
> Err, wait, sorry, I wasn't reading carefully enough.
> 
> What are you trying to check for here?  I can't see why it matters if "P" has an implicit control flow instruction; we insert the new instruction at the end of the predecessor, so implicit control flow isn't a problem unless the terminator is an invoke.
You're right, I misinterpreted this part. Invoke is also not a problem because if the terminator was an invoke, then we have a critical edge, and we don't PRE across critical edges without splitting them. This check is obsolete, I'll remove it.


https://reviews.llvm.org/D38619