[llvm] r288795 - [LCG] Add some much needed asserts and verify runs to uncover

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 6 02:29:24 PST 2016


Author: chandlerc
Date: Tue Dec  6 04:29:23 2016
New Revision: 288795

URL: http://llvm.org/viewvc/llvm-project?rev=288795&view=rev
Log:
[LCG] Add some much needed asserts and verify runs to uncover
a hilarious bug and fix it.

We somehow were never verifying the RefSCCs newly formed when
splitting an existing one apart, and when verifying them we weren't
really checking the SCC indices mapping effectively.

If we had been, it would have been blindingly obvious that right after
putting something int `RC.SCCs` we should update `RC.SCCIndices` instead
of `SCCIndices` which we were about to clear and rebuild anyways. =[

Anyways, this is thoroughly covered by existing tests now that we
actually verify things properly.

Modified:
    llvm/trunk/lib/Analysis/LazyCallGraph.cpp

Modified: llvm/trunk/lib/Analysis/LazyCallGraph.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/LazyCallGraph.cpp?rev=288795&r1=288794&r2=288795&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/LazyCallGraph.cpp (original)
+++ llvm/trunk/lib/Analysis/LazyCallGraph.cpp Tue Dec  6 04:29:23 2016
@@ -258,6 +258,9 @@ void LazyCallGraph::RefSCC::verify() {
            "SCC doesn't think it is inside this RefSCC!");
     bool Inserted = SCCSet.insert(C).second;
     assert(Inserted && "Found a duplicate SCC!");
+    auto IndexIt = SCCIndices.find(C);
+    assert(IndexIt != SCCIndices.end() &&
+           "Found an SCC that doesn't have an index!");
   }
 
   // Check that our indices map correctly.
@@ -1203,9 +1206,8 @@ LazyCallGraph::RefSCC::removeInternalRef
           }
 
           // If this child isn't currently in this RefSCC, no need to process
-          // it.
-          // However, we do need to remove this RefSCC from its RefSCC's parent
-          // set.
+          // it. However, we do need to remove this RefSCC from its RefSCC's
+          // parent set.
           RefSCC &ChildRC = *G->lookupRefSCC(ChildN);
           ChildRC.Parents.erase(this);
           ++I;
@@ -1305,7 +1307,7 @@ LazyCallGraph::RefSCC::removeInternalRef
     RefSCC &RC = *Result[SCCNumber - 1];
     int SCCIndex = RC.SCCs.size();
     RC.SCCs.push_back(C);
-    SCCIndices[C] = SCCIndex;
+    RC.SCCIndices[C] = SCCIndex;
     C->OuterRefSCC = &RC;
   }
 
@@ -1376,6 +1378,12 @@ LazyCallGraph::RefSCC::removeInternalRef
         std::remove(G->LeafRefSCCs.begin(), G->LeafRefSCCs.end(), this),
         G->LeafRefSCCs.end());
 
+#ifndef NDEBUG
+  // Verify all of the new RefSCCs.
+  for (RefSCC *RC : Result)
+    RC->verify();
+#endif
+
   // Return the new list of SCCs.
   return Result;
 }




More information about the llvm-commits mailing list