[NewPM][Inliner] Check if deleted function is in current SCC

[aeubanks]

Summary

In weird cases, the inliner will inline internal recursive functions,
sometimes causing them to have no more uses, in which case the
inliner will mark the function to be deleted. The function is
actually deleted after the call to
updateCGAndAnalysisManagerForCGSCCPass(). In
updateCGAndAnalysisManagerForCGSCCPass(), UR.UpdatedC may be set to
the SCC containing the function to be deleted. Then the inliner calls
CG.removeDeadFunction() which can cause that SCC to be deleted, even
though it's still stored in UR.UpdatedC.

We could potentially check in the wrappers/pass managers if UR.UpdatedC
is in UR.InvalidatedSCCs before doing anything with it, but it's safer
to do this as close to possible to the call to CG.removeDeadFunction()
to avoid issues with allocating a new SCC in the same address as
the deleted one.

It's hard to find a small test case since we need to have recursive
internal functions be reachable from non-internal functions, yet they
need to become non-recursive and not referenced by other functions when
inlined.

Similar to https://reviews.llvm.org/D106306.

Fixes PR50788.

[asbirlea]

LGTM, thank you!

llvm/lib/Transforms/IPO/Inliner.cpp

for (Function *DeadF : DeadFunctions) {
     auto &DeadRC = DeadC.getOuterRefSCC();
     CG.removeDeadFunction(*DeadF);
 
     // Mark the relevant parts of the call graph as invalid so we don't visit
     // them.
     UR.InvalidatedSCCs.insert(&DeadC);
     UR.InvalidatedRefSCCs.insert(&DeadRC);
 
+    // If the updated SCC was the one containing the deleted function, clear it.
+    if (&DeadC == UR.UpdatedC)
+      UR.UpdatedC = nullptr;
+
     // And delete the actual function from the module.
     // The Advisor may use Function pointers to efficiently index various
     // internal maps, e.g. for memoization. Function cleanup passes like
     // argument promotion create new functions. It is possible for a new
     // function to be allocated at the address of a deleted function. We could
     // index using names, but that's inefficient. Alternatively, we let the
     // Advisor free the functions when it sees fit.
     DeadF->getBasicBlockList().clear();

llvm/test/Transforms/Inline/deleted-scc.ll

+; RUN: opt -passes=inline < %s -S | FileCheck %s
+
+; CHECK-NOT: define
+; CHECK: define void @e()
+; CHECK-NOT: define
+
+@b = external local_unnamed_addr global i32, align 4
+
+define void @e() local_unnamed_addr {
+entry:
+  call fastcc void @d()
+  ret void
+}
+
+define internal fastcc void @f() unnamed_addr {
+entry:
+  call fastcc void @d()
+  ret void
+}
+
+define internal fastcc void @d() unnamed_addr {
+entry:
+  br label %L
+
+L:                                                ; preds = %cleanup9, %entry
+  %cleanup.dest.slot.0 = phi i32 [ undef, %entry ], [ %cleanup.dest.slot.3, %cleanup9 ]
+  store i32 0, i32* @b, align 4
+  %tobool.not = icmp eq i32 0, 0
+  br i1 %tobool.not, label %if.then, label %while.cond
+
+while.cond:                                       ; preds = %cleanup9, %L
+  %cleanup.dest.slot.2 = phi i32 [ %cleanup.dest.slot.0, %L ], [ 0, %cleanup9 ]
+  %0 = load i32, i32* @b, align 4
+  %tobool3.not = icmp eq i32 %0, 0
+  br i1 %tobool3.not, label %cleanup9, label %while.body4
+
+while.body4:                                      ; preds = %while.cond
+  call fastcc void @f()
+  br label %cleanup9
+
+cleanup9:                                         ; preds = %while.cond, %while.body4
+  %cleanup.dest.slot.3 = phi i32 [ %cleanup.dest.slot.2, %while.body4 ], [ 0, %while.cond ]
+  switch i32 %cleanup.dest.slot.3, label %common.ret [
+    i32 0, label %while.cond
+    i32 2, label %L
+  ]
+
+common.ret:                                       ; preds = %cleanup9, %if.then
+  ret void
+
+if.then:                                          ; preds = %L
+  call void @e()
+  br label %common.ret
+}