[llvm] r290645 - [LCG] Teach the ref edge removal to handle a ref edge that is trivial

[Chandler Carruth via llvm-commits]

Author: chandlerc
Date: Tue Dec 27 20:24:58 2016
New Revision: 290645

URL: http://llvm.org/viewvc/llvm-project?rev=290645&view=rev
Log:
[LCG] Teach the ref edge removal to handle a ref edge that is trivial
due to a call cycle.

This actually crashed the ref removal before.

I've added a unittest that covers this kind of interesting graph
structure and mutation.

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

Modified: llvm/trunk/lib/Analysis/LazyCallGraph.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/LazyCallGraph.cpp?rev=290645&r1=290644&r2=290645&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/LazyCallGraph.cpp (original)
+++ llvm/trunk/lib/Analysis/LazyCallGraph.cpp Tue Dec 27 20:24:58 2016
@@ -1117,6 +1117,13 @@ LazyCallGraph::RefSCC::removeInternalRef
   if (&SourceN == &TargetN)
     return Result;
 
+  // If this ref edge is within an SCC then there are sufficient other edges to
+  // form a cycle without this edge so removing it is a no-op.
+  SCC &SourceC = *G->lookupSCC(SourceN);
+  SCC &TargetC = *G->lookupSCC(TargetN);
+  if (&SourceC == &TargetC)
+    return Result;
+
   // We build somewhat synthetic new RefSCCs by providing a postorder mapping
   // for each inner SCC. We also store these associated with *nodes* rather
   // than SCCs because this saves a round-trip through the node->SCC map and in
@@ -1139,7 +1146,6 @@ LazyCallGraph::RefSCC::removeInternalRef
   // and handle participants in that cycle without walking all the edges that
   // form the connections, and instead by relying on the fundamental guarantee
   // coming into this operation.
-  SCC &TargetC = *G->lookupSCC(TargetN);
   for (Node &N : TargetC)
     PostOrderMapping[&N] = RootPostOrderNumber;
 

Modified: llvm/trunk/unittests/Analysis/LazyCallGraphTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Analysis/LazyCallGraphTest.cpp?rev=290645&r1=290644&r2=290645&view=diff
==============================================================================
--- llvm/trunk/unittests/Analysis/LazyCallGraphTest.cpp (original)
+++ llvm/trunk/unittests/Analysis/LazyCallGraphTest.cpp Tue Dec 27 20:24:58 2016
@@ -1600,6 +1600,84 @@ TEST(LazyCallGraphTest, InternalEdgeRemo
   EXPECT_EQ(E, J);
 }
 
+TEST(LazyCallGraphTest, InternalNoOpEdgeRemoval) {
+  LLVMContext Context;
+  // A graph with a single cycle formed both from call and reference edges
+  // which makes the reference edges trivial to delete. The graph looks like:
+  //
+  // Reference edges: a -> b -> c -> a
+  //      Call edges: a -> c -> b -> a
+  std::unique_ptr<Module> M = parseAssembly(
+      Context, "define void @a(i8** %ptr) {\n"
+               "entry:\n"
+               "  call void @b(i8** %ptr)\n"
+               "  store i8* bitcast (void(i8**)* @c to i8*), i8** %ptr\n"
+               "  ret void\n"
+               "}\n"
+               "define void @b(i8** %ptr) {\n"
+               "entry:\n"
+               "  store i8* bitcast (void(i8**)* @a to i8*), i8** %ptr\n"
+               "  call void @c(i8** %ptr)\n"
+               "  ret void\n"
+               "}\n"
+               "define void @c(i8** %ptr) {\n"
+               "entry:\n"
+               "  call void @a(i8** %ptr)\n"
+               "  store i8* bitcast (void(i8**)* @b to i8*), i8** %ptr\n"
+               "  ret void\n"
+               "}\n");
+  LazyCallGraph CG(*M);
+
+  // Force the graph to be fully expanded.
+  auto I = CG.postorder_ref_scc_begin(), E = CG.postorder_ref_scc_end();
+  LazyCallGraph::RefSCC &RC = *I;
+  EXPECT_EQ(E, std::next(I));
+
+  LazyCallGraph::SCC &C = *RC.begin();
+  EXPECT_EQ(RC.end(), std::next(RC.begin()));
+
+  LazyCallGraph::Node &AN = *CG.lookup(lookupFunction(*M, "a"));
+  LazyCallGraph::Node &BN = *CG.lookup(lookupFunction(*M, "b"));
+  LazyCallGraph::Node &CN = *CG.lookup(lookupFunction(*M, "c"));
+  EXPECT_EQ(&RC, CG.lookupRefSCC(AN));
+  EXPECT_EQ(&RC, CG.lookupRefSCC(BN));
+  EXPECT_EQ(&RC, CG.lookupRefSCC(CN));
+  EXPECT_EQ(&C, CG.lookupSCC(AN));
+  EXPECT_EQ(&C, CG.lookupSCC(BN));
+  EXPECT_EQ(&C, CG.lookupSCC(CN));
+
+  // Remove the edge from a -> c which doesn't change anything.
+  SmallVector<LazyCallGraph::RefSCC *, 1> NewRCs =
+      RC.removeInternalRefEdge(AN, CN);
+  EXPECT_EQ(0u, NewRCs.size());
+  EXPECT_EQ(&RC, CG.lookupRefSCC(AN));
+  EXPECT_EQ(&RC, CG.lookupRefSCC(BN));
+  EXPECT_EQ(&RC, CG.lookupRefSCC(CN));
+  EXPECT_EQ(&C, CG.lookupSCC(AN));
+  EXPECT_EQ(&C, CG.lookupSCC(BN));
+  EXPECT_EQ(&C, CG.lookupSCC(CN));
+  auto J = CG.postorder_ref_scc_begin();
+  EXPECT_EQ(I, J);
+  EXPECT_EQ(&RC, &*J);
+  EXPECT_EQ(E, std::next(J));
+
+  // Remove the edge from b -> a and c -> b; again this doesn't change
+  // anything.
+  NewRCs = RC.removeInternalRefEdge(BN, AN);
+  NewRCs = RC.removeInternalRefEdge(CN, BN);
+  EXPECT_EQ(0u, NewRCs.size());
+  EXPECT_EQ(&RC, CG.lookupRefSCC(AN));
+  EXPECT_EQ(&RC, CG.lookupRefSCC(BN));
+  EXPECT_EQ(&RC, CG.lookupRefSCC(CN));
+  EXPECT_EQ(&C, CG.lookupSCC(AN));
+  EXPECT_EQ(&C, CG.lookupSCC(BN));
+  EXPECT_EQ(&C, CG.lookupSCC(CN));
+  J = CG.postorder_ref_scc_begin();
+  EXPECT_EQ(I, J);
+  EXPECT_EQ(&RC, &*J);
+  EXPECT_EQ(E, std::next(J));
+}
+
 TEST(LazyCallGraphTest, InternalCallEdgeToRef) {
   LLVMContext Context;
   // A nice fully connected (including self-edges) SCC (and RefSCC)