[PATCH] D72226: Short-circuit SCC update for self-referential edge

[Brian Gesiak via Phabricator via llvm-commits]

modocache created this revision.
modocache added reviewers: jdoerfert, chandlerc, wenlei.
Herald added subscribers: hiraditya, mehdi_amini.
Herald added a project: LLVM.

Depends on https://reviews.llvm.org/D70927.

When switching a call egde to a ref edge within the same SCC, special
care is taken to update the SCC in case the strongly-connected cycle is
broken. However, for self referential edges, it is not possible for the
switch from call edge to ref edge to break a cycle -- all nodes form a
strongly-connected cycle with themselves, regardless of any
self-referential call or ref edges.

Adding short-circuiting behavior prevents unnecessary DFS in some cases.
In other cases, such as when using the CallGraphUpdater, the
short-circuit prevents an assertion within 'incorporateNewSCCRange' from
firing. In that case, the DFS update over the nodes in the graph
erroneously create new SCCs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72226

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


Index: llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
===================================================================
--- llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
+++ llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
@@ -1560,5 +1560,55 @@
   MPM.run(*M, MAM);
 }
 
+TEST_F(CGSCCPassManagerTest, TestDemotionOfSelfReferentialCallEdge) {
+  std::unique_ptr<Module> M = parseIR("define void @f() {\n"
+                                      "entry:\n"
+                                      "  call void @f()\n"
+                                      "  ret void\n"
+                                      "}\n");
+
+  CGSCCPassManager CGPM(/*DebugLogging*/ true);
+  CGPM.addPass(LambdaSCCPassNoPreserve(
+      [&](LazyCallGraph::SCC &C, CGSCCAnalysisManager &AM, LazyCallGraph &CG,
+          CGSCCUpdateResult &UR) {
+        CallGraphUpdater CGUpdater(CG, AM, UR);
+        for (auto &N : C) {
+          auto &F = N.getFunction();
+          if (F.getName() != "f")
+            continue;
+          auto *Call = dyn_cast<CallInst>(F.begin()->begin());
+          if (!Call || Call->getCalledFunction()->getName() != "f")
+            continue;
+
+          // "Demote" the 'f -> f' call egde to a ref edge.
+          // 1. Erase the call edge from 'f' to 'f'.
+          Call->eraseFromParent();
+          // 2. Insert a ref edge from 'f' to 'f'.
+          (void)CastInst::CreatePointerCast(&F,
+                                            Type::getInt8PtrTy(F.getContext()),
+                                            "f.ref", &*F.begin()->begin());
+
+          // Create a new function 'g' and use the CallGraphUpdater to insert
+          // a node for 'g' into the call graph. This also connects creates a
+          // ref edge 'f -> g'.
+          auto *G =
+              llvm::Function::Create(F.getFunctionType(), F.getLinkage(),
+                                     F.getAddressSpace(), "g", F.getParent());
+          llvm::BasicBlock::Create(F.getParent()->getContext(), "entry", G);
+          CGUpdater.registerOutlinedFunction(*G, &C);
+
+          ASSERT_NO_FATAL_FAILURE(
+              updateCGAndAnalysisManagerForCGSCCPass(CG, C, N, AM, UR))
+              << "Updating the call graph with a demoted, self-referential "
+                 "call edge 'f -> f', and a newly inserted ref edge 'f -> g', "
+                 "caused a fatal failure";
+        }
+      }));
+
+  ModulePassManager MPM(/*DebugLogging*/ true);
+  MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM)));
+  MPM.run(*M, MAM);
+}
+
 #endif
 } // namespace
Index: llvm/lib/Analysis/LazyCallGraph.cpp
===================================================================
--- llvm/lib/Analysis/LazyCallGraph.cpp
+++ llvm/lib/Analysis/LazyCallGraph.cpp
@@ -721,8 +721,14 @@
   // Set the edge kind.
   SourceN->setEdgeKind(TargetN, Edge::Ref);
 
-  // Otherwise we are removing a call edge from a single SCC. This may break
-  // the cycle. In order to compute the new set of SCCs, we need to do a small
+  // Otherwise, we are removing a call edge from a single SCC. This may break
+  // the cycle. However, one trivial case remains that we check for now: a
+  // node is always strongly connected to itself, so demoting an edge from a
+  // node to itself cannot break a cycle, and does not introduce any new SCCs.
+  if (&SourceN == &TargetN)
+    return range(iterator(), iterator());
+
+  // In order to compute the new set of SCCs, we need to do a small
   // DFS over the nodes within the SCC to form any sub-cycles that remain as
   // distinct SCCs and compute a postorder over the resulting SCCs.
   //


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72226.236238.patch
Type: text/x-patch
Size: 3637 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200105/9185a1e3/attachment.bin>