[llvm] r230827 - [RewriteStatepointsForGC] Fix iterator invalidation bug

[Philip Reames]

Author: reames
Date: Fri Feb 27 18:47:50 2015
New Revision: 230827

URL: http://llvm.org/viewvc/llvm-project?rev=230827&view=rev
Log:
[RewriteStatepointsForGC] Fix iterator invalidation bug

Inserting into a DenseMap you're iterating over is not well defined.  This is unfortunate since this is well defined on a std::map.

"cleanup per llvm code style standards" bug #2




Modified:
    llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp?rev=230827&r1=230826&r2=230827&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp Fri Feb 27 18:47:50 2015
@@ -684,12 +684,19 @@ static Value *findBasePointer(Value *I,
   states[def] = PhiState();
   // Recursively fill in all phis & selects reachable from the initial one
   // for which we don't already know a definite base value for
-  // PERF: Yes, this is as horribly inefficient as it looks.
+  // TODO: This should be rewritten with a worklist
   bool done = false;
   while (!done) {
     done = true;
+    // Since we're adding elements to 'states' as we run, we can't keep
+    // iterators into the set.
+    SmallVector<Value*, 16> Keys;
+    Keys.reserve(states.size());
     for (auto Pair : states) {
-      Value *v = Pair.first;
+      Value *V = Pair.first;
+      Keys.push_back(V);
+    }
+    for (Value *v : Keys) {
       assert(!isKnownBaseResult(v) && "why did it get added?");
       if (PHINode *phi = dyn_cast<PHINode>(v)) {
         assert(phi->getNumIncomingValues() > 0 &&
@@ -734,6 +741,7 @@ static Value *findBasePointer(Value *I,
   while (progress) {
     oldSize = states.size();
     progress = false;
+    // We're only changing keys in this loop, thus safe to keep iterators
     for (auto Pair : states) {
       MeetPhiStates calculateMeet(states);
       Value *v = Pair.first;
@@ -768,6 +776,7 @@ static Value *findBasePointer(Value *I,
   }
 
   // Insert Phis for all conflicts
+  // Only changing keys in 'states', thus safe to keep iterators
   for (auto Pair : states) {
     Instruction *v = cast<Instruction>(Pair.first);
     PhiState state = Pair.second;