[llvm] c662103 - [Coroutines][LazyCallGraph] addSplitRefRecursiveFunctions allows spurious ref edges between new functions. (#116285)

[via llvm-commits]

Author: Tyler Nowicki
Date: 2025-02-14T16:55:06-05:00
New Revision: c662103c6ead3bbfad9bc24d7770552163dbb92b

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

LOG: [Coroutines][LazyCallGraph] addSplitRefRecursiveFunctions allows spurious ref edges between new functions. (#116285)

The addSplitRefRecursiveFunctions LazyCallGraph helper should not
require a reference between every new function. Spurious ref edges
between the new functions are allowed and the new function are
considered to be a RefSCC. This change clarifies that this is the case
in the method's description and its DEBUG mode verifier.

Added: 
    

Modified: 
    llvm/include/llvm/Analysis/LazyCallGraph.h
    llvm/lib/Analysis/LazyCallGraph.cpp
    llvm/unittests/Analysis/LazyCallGraphTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Analysis/LazyCallGraph.h b/llvm/include/llvm/Analysis/LazyCallGraph.h
index 289e9c3990bcc..a69ed91b0465e 100644
--- a/llvm/include/llvm/Analysis/LazyCallGraph.h
+++ b/llvm/include/llvm/Analysis/LazyCallGraph.h
@@ -1082,11 +1082,16 @@ class LazyCallGraph {
   /// Add new ref-recursive functions split/outlined from an existing function.
   ///
   /// The new functions may only reference other functions that the original
-  /// function did. The new functions may reference (not call) the original
-  /// function.
+  /// function did. The new functions may reference the original function. New
+  /// functions must not call other new functions or the original function.
   ///
-  /// The original function must reference (not call) all new functions.
-  /// All new functions must reference (not call) each other.
+  /// Marks the original function as referencing all new functions.
+  ///
+  /// It is not necessary for each new function to reference all other new
+  /// functions. Spurious/missing ref edges are allowed. The new functions
+  /// are considered to be a RefSCC. If any new function references the
+  /// original function they are all considered to be part of the original
+  /// function's RefSCC.
   void addSplitRefRecursiveFunctions(Function &OriginalFunction,
                                      ArrayRef<Function *> NewFunctions);
 

diff  --git a/llvm/lib/Analysis/LazyCallGraph.cpp b/llvm/lib/Analysis/LazyCallGraph.cpp
index 5aa36bfc36d46..eb91b7673afee 100644
--- a/llvm/lib/Analysis/LazyCallGraph.cpp
+++ b/llvm/lib/Analysis/LazyCallGraph.cpp
@@ -1720,6 +1720,7 @@ void LazyCallGraph::addSplitRefRecursiveFunctions(
   for (Function *NewFunction : NewFunctions) {
     Node &NewN = initNode(*NewFunction);
 
+    // Make the original function reference each new function
     OriginalN->insertEdgeInternal(NewN, Edge::Kind::Ref);
 
     // Check if there is any edge from any new function back to any function in
@@ -1775,8 +1776,9 @@ void LazyCallGraph::addSplitRefRecursiveFunctions(
       if (F1 == F2)
         continue;
       Node &N2 = get(*F2);
-      assert(!N1->lookup(N2)->isCall() &&
-             "Edges between new functions must be ref edges");
+      assert(!N1->lookup(N2) ||
+             (!N1->lookup(N2)->isCall() &&
+              "Edges between new functions must be ref edges"));
     }
   }
 #endif

diff  --git a/llvm/unittests/Analysis/LazyCallGraphTest.cpp b/llvm/unittests/Analysis/LazyCallGraphTest.cpp
index 6ca233a50f31f..311c5d470f2cc 100644
--- a/llvm/unittests/Analysis/LazyCallGraphTest.cpp
+++ b/llvm/unittests/Analysis/LazyCallGraphTest.cpp
@@ -3027,4 +3027,58 @@ TEST(LazyCallGraphTest, AddSplitFunctions5) {
   EXPECT_EQ(RC, CG.lookupRefSCC(F2N));
   EXPECT_EQ(CG.postorder_ref_scc_end(), I);
 }
+
+TEST(LazyCallGraphTest, AddSplitFunctions6) {
+  LLVMContext Context;
+  std::unique_ptr<Module> M = parseAssembly(Context, "define void @f() {\n"
+                                                     "  ret void\n"
+                                                     "}\n");
+  LazyCallGraph CG = buildCG(*M);
+
+  Function &F = lookupFunction(*M, "f");
+  LazyCallGraph::Node &FN = CG.get(F);
+
+  // Force the graph to be fully expanded.
+  CG.buildRefSCCs();
+  auto I = CG.postorder_ref_scc_begin();
+  LazyCallGraph::RefSCC *ORC = &*I++;
+  EXPECT_EQ(CG.postorder_ref_scc_end(), I);
+
+  auto *G1 = Function::Create(F.getFunctionType(), F.getLinkage(),
+                              F.getAddressSpace(), "g1", F.getParent());
+  auto *G2 = Function::Create(F.getFunctionType(), F.getLinkage(),
+                              F.getAddressSpace(), "g2", F.getParent());
+  BasicBlock *G1BB = BasicBlock::Create(Context, "", G1);
+  BasicBlock *G2BB = BasicBlock::Create(Context, "", G2);
+  // Create g1 -ref-> g2 and g2 has no references.
+  (void)CastInst::CreatePointerCast(G2, PointerType::getUnqual(Context), "",
+                                    G1BB);
+  (void)ReturnInst::Create(Context, G1BB);
+  (void)ReturnInst::Create(Context, G2BB);
+
+  // Create f -ref-> g1 and f -ref-> g2.
+  (void)CastInst::CreatePointerCast(G1, PointerType::getUnqual(Context), "",
+                                    F.getEntryBlock().begin());
+  (void)CastInst::CreatePointerCast(G2, PointerType::getUnqual(Context), "",
+                                    F.getEntryBlock().begin());
+
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+
+  CG.addSplitRefRecursiveFunctions(F, SmallVector<Function *, 1>({G1, G2}));
+
+  LazyCallGraph::Node *G1N = CG.lookup(*G1);
+  EXPECT_TRUE(G1N);
+  LazyCallGraph::Node *G2N = CG.lookup(*G2);
+  EXPECT_TRUE(G2N);
+
+  I = CG.postorder_ref_scc_begin();
+  LazyCallGraph::RefSCC *RC1 = &*I++;
+  EXPECT_EQ(2, RC1->size());
+  EXPECT_EQ(RC1, CG.lookupRefSCC(*G1N));
+  EXPECT_EQ(RC1, CG.lookupRefSCC(*G2N));
+  LazyCallGraph::RefSCC *RC2 = &*I++;
+  EXPECT_EQ(RC2, ORC);
+  EXPECT_EQ(RC2, CG.lookupRefSCC(FN));
+  EXPECT_EQ(CG.postorder_ref_scc_end(), I);
+}
 }