[llvm] Revert "[Coroutines][LazyCallGraph] addSplitRefRecursiveFunctions allows spurious ref edges between new functions." (PR #127285)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-analysis

Author: Arthur Eubanks (aeubanks)

<details>
<summary>Changes</summary>

Reverts llvm/llvm-project#<!-- -->116285

Breaks expensive checks build, e.g. https://lab.llvm.org/buildbot/#/builders/16/builds/13821

---
Full diff: https://github.com/llvm/llvm-project/pull/127285.diff


3 Files Affected:

- (modified) llvm/include/llvm/Analysis/LazyCallGraph.h (+4-9) 
- (modified) llvm/lib/Analysis/LazyCallGraph.cpp (+2-4) 
- (modified) llvm/unittests/Analysis/LazyCallGraphTest.cpp (-54) 


``````````diff
diff --git a/llvm/include/llvm/Analysis/LazyCallGraph.h b/llvm/include/llvm/Analysis/LazyCallGraph.h
index a69ed91b0465e..289e9c3990bcc 100644
--- a/llvm/include/llvm/Analysis/LazyCallGraph.h
+++ b/llvm/include/llvm/Analysis/LazyCallGraph.h
@@ -1082,16 +1082,11 @@ 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 the original function. New
-  /// functions must not call other new functions or the original function.
+  /// function did. The new functions may reference (not call) the original
+  /// function.
   ///
-  /// 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.
+  /// The original function must reference (not call) all new functions.
+  /// All new functions must reference (not call) each other.
   void addSplitRefRecursiveFunctions(Function &OriginalFunction,
                                      ArrayRef<Function *> NewFunctions);
 
diff --git a/llvm/lib/Analysis/LazyCallGraph.cpp b/llvm/lib/Analysis/LazyCallGraph.cpp
index eb91b7673afee..5aa36bfc36d46 100644
--- a/llvm/lib/Analysis/LazyCallGraph.cpp
+++ b/llvm/lib/Analysis/LazyCallGraph.cpp
@@ -1720,7 +1720,6 @@ 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
@@ -1776,9 +1775,8 @@ void LazyCallGraph::addSplitRefRecursiveFunctions(
       if (F1 == F2)
         continue;
       Node &N2 = get(*F2);
-      assert(!N1->lookup(N2) ||
-             (!N1->lookup(N2)->isCall() &&
-              "Edges between new functions must be ref edges"));
+      assert(!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 311c5d470f2cc..6ca233a50f31f 100644
--- a/llvm/unittests/Analysis/LazyCallGraphTest.cpp
+++ b/llvm/unittests/Analysis/LazyCallGraphTest.cpp
@@ -3027,58 +3027,4 @@ 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);
-}
 }

``````````

</details>


https://github.com/llvm/llvm-project/pull/127285