[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:2351
+void GVN::fillImplicitControlFlowInfo(Function &F) {
+  ReversePostOrderTraversal<Function *> RPOT(&F);
+  for (BasicBlock *BB : RPOT)
----------------
This is expensive to compute, you should pass it in.


================
Comment at: lib/Transforms/Scalar/GVN.cpp:2365
+    // must be removed once isGuaranteedToTransferExecutionToSuccessor is fixed.
+    for (BasicBlock::iterator BI = BB->begin(), BE = BB->end(); BI != BE; ++BI)
+      if (!isGuaranteedToTransferExecutionToSuccessor(&*BI) &&
----------------
for (auto &I : BB)

(the &*BI will just be &I below)


================
Comment at: lib/Transforms/Scalar/GVN.cpp:2368
+          !isa<LoadInst>(&*BI) && !isa<StoreInst>(&*BI)) {
+        FirstImplicitControlFlowInsts[BB] = &*BI;
+        break;
----------------
This map could be const *BasicBlock->const *Instruction, FWIW.



https://reviews.llvm.org/D37460