[PATCH] D31084: [GVN] Fix accidental double storage of the function BasicBlock list in iterateOnFunction

[Phabricator via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL298191: [GVN] Fix accidental double storage of the function BasicBlock list in… (authored by ctopper).

Changed prior to commit:
  https://reviews.llvm.org/D31084?vs=92248&id=92251#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D31084

Files:
  llvm/trunk/include/llvm/ADT/PostOrderIterator.h
  llvm/trunk/lib/Transforms/Scalar/GVN.cpp


Index: llvm/trunk/include/llvm/ADT/PostOrderIterator.h
===================================================================
--- llvm/trunk/include/llvm/ADT/PostOrderIterator.h
+++ llvm/trunk/include/llvm/ADT/PostOrderIterator.h
@@ -268,6 +268,10 @@
 // with a postorder iterator to build the data structures).  The moral of this
 // story is: Don't create more ReversePostOrderTraversal classes than necessary.
 //
+// Because it does the traversal in its constructor, it won't invalidate when
+// BasicBlocks are removed, *but* it may contain erased blocks. Some places
+// rely on this behavior (i.e. GVN).
+//
 // This class should be used like this:
 // {
 //   ReversePostOrderTraversal<Function*> RPOT(FuncPtr); // Expensive to create
Index: llvm/trunk/lib/Transforms/Scalar/GVN.cpp
===================================================================
--- llvm/trunk/lib/Transforms/Scalar/GVN.cpp
+++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp
@@ -2155,21 +2155,12 @@
 
   // Top-down walk of the dominator tree
   bool Changed = false;
-  // Save the blocks this function have before transformation begins. GVN may
-  // split critical edge, and hence may invalidate the RPO/DT iterator.
-  //
-  std::vector<BasicBlock *> BBVect;
-  BBVect.reserve(256);
   // Needed for value numbering with phi construction to work.
+  // RPOT walks the graph in its constructor and will not be invalidated during
+  // processBlock.
   ReversePostOrderTraversal<Function *> RPOT(&F);
-  for (ReversePostOrderTraversal<Function *>::rpo_iterator RI = RPOT.begin(),
-                                                           RE = RPOT.end();
-       RI != RE; ++RI)
-    BBVect.push_back(*RI);
-
-  for (std::vector<BasicBlock *>::iterator I = BBVect.begin(), E = BBVect.end();
-       I != E; I++)
-    Changed |= processBlock(*I);
+  for (BasicBlock *BB : RPOT)
+    Changed |= processBlock(BB);
 
   return Changed;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D31084.92251.patch
Type: text/x-patch
Size: 1910 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170318/72bac02a/attachment.bin>