[llvm] 91332c4 - [CGSCC][NewPM] Fix adding mutually recursive new functions

[Arthur Eubanks via llvm-commits]

Author: Arthur Eubanks
Date: 2020-09-15T16:44:08-07:00
New Revision: 91332c4dbb033f7d1ffa1a9632012d88b08661c4

URL: https://github.com/llvm/llvm-project/commit/91332c4dbb033f7d1ffa1a9632012d88b08661c4
DIFF: https://github.com/llvm/llvm-project/commit/91332c4dbb033f7d1ffa1a9632012d88b08661c4.diff

LOG: [CGSCC][NewPM] Fix adding mutually recursive new functions

When adding a new function via addNewFunctionIntoRefSCC(), it creates a
new node and immediately populates the edges. Since populateSlow() calls
G->get() on all referenced functions, it will create a node (but not
populate it) for functions that haven't yet been added. If we add two
mutually recursive functions, the assert that the node should never have
been created will fire when the second function is added. So here we
remove that assert since the node may have already been created (but not
yet populated).

createNode() is only called from addNewFunctionInto{,Ref}SCC().

https://bugs.llvm.org/show_bug.cgi?id=47502

Reviewed By: jdoerfert

Differential Revision: https://reviews.llvm.org/D87623

Added: 
    

Modified: 
    llvm/lib/Analysis/LazyCallGraph.cpp
    llvm/unittests/Analysis/CGSCCPassManagerTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/LazyCallGraph.cpp b/llvm/lib/Analysis/LazyCallGraph.cpp
index efded17cef4e..b3658999e7fe 100644
--- a/llvm/lib/Analysis/LazyCallGraph.cpp
+++ b/llvm/lib/Analysis/LazyCallGraph.cpp
@@ -1595,8 +1595,6 @@ void LazyCallGraph::updateGraphPtrs() {
 }
 
 LazyCallGraph::Node &LazyCallGraph::createNode(Function &F) {
-  assert(!lookup(F) && "node already exists");
-
   Node &N = get(F);
   NodeMap[&F] = &N;
   N.DFSNumber = N.LowLink = -1;

diff  --git a/llvm/unittests/Analysis/CGSCCPassManagerTest.cpp b/llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
index 2dad605395c3..e0ff4e891ab6 100644
--- a/llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
+++ b/llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
@@ -1766,5 +1766,60 @@ TEST_F(CGSCCPassManagerTest, TestInsertionOfNewRefSCC) {
   MPM.run(*M, MAM);
 }
 
+TEST_F(CGSCCPassManagerTest, TestInsertionOfNewRefSCCMutuallyRecursive) {
+  std::unique_ptr<Module> M = parseIR("define void @f() {\n"
+                                      "entry:\n"
+                                      "  ret void\n"
+                                      "}\n");
+
+  CGSCCPassManager CGPM(/*DebugLogging*/ true);
+  CGPM.addPass(LambdaSCCPassNoPreserve([&](LazyCallGraph::SCC &C,
+                                           CGSCCAnalysisManager &AM,
+                                           LazyCallGraph &CG,
+                                           CGSCCUpdateResult &UR) {
+    auto &FAM =
+        AM.getResult<FunctionAnalysisManagerCGSCCProxy>(C, CG).getManager();
+
+    for (auto &N : C) {
+      auto &F = N.getFunction();
+      if (F.getName() != "f")
+        continue;
+
+      // Create mutually recursive functions (ref only) 'h1' and 'h2'.
+      auto *H1 = Function::Create(F.getFunctionType(), F.getLinkage(),
+                                  F.getAddressSpace(), "h1", F.getParent());
+      auto *H2 = Function::Create(F.getFunctionType(), F.getLinkage(),
+                                  F.getAddressSpace(), "h2", F.getParent());
+      BasicBlock *H1BB =
+          BasicBlock::Create(F.getParent()->getContext(), "entry", H1);
+      BasicBlock *H2BB =
+          BasicBlock::Create(F.getParent()->getContext(), "entry", H2);
+      (void)CastInst::CreatePointerCast(H2, Type::getInt8PtrTy(F.getContext()),
+                                        "h2.ref", H1BB);
+      (void)ReturnInst::Create(H1->getContext(), H1BB);
+      (void)CastInst::CreatePointerCast(H1, Type::getInt8PtrTy(F.getContext()),
+                                        "h1.ref", H2BB);
+      (void)ReturnInst::Create(H2->getContext(), H2BB);
+
+      // Add 'f -> h1' ref edge.
+      (void)CastInst::CreatePointerCast(H1, Type::getInt8PtrTy(F.getContext()),
+                                        "h.ref", &F.getEntryBlock().front());
+
+      CG.addNewFunctionIntoRefSCC(*H1, C.getOuterRefSCC());
+      CG.addNewFunctionIntoRefSCC(*H2, C.getOuterRefSCC());
+
+      ASSERT_NO_FATAL_FAILURE(
+          updateCGAndAnalysisManagerForCGSCCPass(CG, C, N, AM, UR, FAM))
+          << "Updating the call graph with a demoted, self-referential "
+             "call edge 'f -> f', a newly inserted ref edge 'f -> g', and "
+             "mutually recursive h1 <-> h2 caused a fatal failure";
+    }
+  }));
+
+  ModulePassManager MPM(/*DebugLogging*/ true);
+  MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM)));
+  MPM.run(*M, MAM);
+}
+
 #endif
 } // namespace