CoroSplit: Replace ad-hoc implementation of reachability with API from CFG.h

[aprantl]

Summary

The current ad-hoc implementation used to determine whether a basic block is unreachable doesn't work correctly in the general case (for example it won't detect successors of unreachable blocks as unreachable). This patch replaces it with the correct API that uses a DominatorTree to answer the question correctly and quickly.

rdar://77181156

[ChuanqiXu]

It looks good to me with an unused variables to remove.

llvm/lib/Transforms/Coroutines/CoroSplit.cpp

664

It looks like UnreachableCache doesn't be used.

llvm/lib/Transforms/Coroutines/CoroSplit.cpp

 #include "llvm/Transforms/Coroutines/CoroSplit.h"
 #include "CoroInstr.h"
 #include "CoroInternal.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Analysis/CFG.h"
 #include "llvm/Analysis/CallGraph.h"
 #include "llvm/Analysis/CallGraphSCCPass.h"
 #include "llvm/Analysis/LazyCallGraph.h"
 #include "llvm/IR/Argument.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/CallingConv.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DerivedTypes.h"
+#include "llvm/IR/Dominators.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InstIterator.h"
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
for (auto &BB : *NewF)
     for (auto &I : BB)
       if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&I))
         Worklist.push_back(DVI);
   for (DbgVariableIntrinsic *DVI : Worklist)
     coro::salvageDebugInfo(DbgPtrAllocaCache, DVI, Shape.ReuseFrameSlot);
 
   // Remove all salvaged dbg.declare intrinsics that became
   // either unreachable or stale due to the CoroSplit transformation.
+  DominatorTree DomTree(*NewF);
+  SmallDenseMap<BasicBlock *, bool, 8> UnreachableCache;
   auto IsUnreachableBlock = [&](BasicBlock *BB) {
-    return BB->hasNPredecessors(0) && BB != &NewF->getEntryBlock();
+    return !isPotentiallyReachable(&NewF->getEntryBlock(), BB, nullptr,
+                                   &DomTree);
   };
   for (DbgVariableIntrinsic *DVI : Worklist) {
     if (IsUnreachableBlock(DVI->getParent()))
       DVI->eraseFromParent();
     else if (dyn_cast_or_null<AllocaInst>(DVI->getVariableLocationOp(0))) {
       // Count all non-debuginfo uses in reachable blocks.
       unsigned Uses = 0;
       for (auto *User : DVI->getVariableLocationOp(0)->users())

llvm/test/Transforms/Coroutines/coro-debug.ll

entry:
   %0 = call token @llvm.coro.id(i32 0, i8* null, i8* bitcast (i8* (i32)* @f to i8*), i8* null), !dbg !16
   %undef = bitcast i8* undef to [16 x i8]*
   %1 = call i64 @llvm.coro.size.i64(), !dbg !16
   %call = call i8* @malloc(i64 %1), !dbg !16
   %2 = call i8* @llvm.coro.begin(token %0, i8* %call) #7, !dbg !16
   store i8* %2, i8** %coro_hdl, align 8, !dbg !16
   %3 = call i8 @llvm.coro.suspend(token none, i1 false), !dbg !17
   %conv = sext i8 %3 to i32, !dbg !17
+  %late_local = alloca i32, align 4
   call void @coro.devirt.trigger(i8* null)
   switch i32 %conv, label %sw.default [
     i32 0, label %sw.bb
     i32 1, label %sw.bb1
   ], !dbg !17
 
 sw.bb:                                            ; preds = %entry
   %direct = load i32, i32* %x.addr, align 4, !dbg !14
   %gep = getelementptr inbounds [16 x i8], [16 x i8]* %undef, i32 %direct, !dbg !14
   call void @llvm.dbg.declare(metadata [16 x i8] *%gep, metadata !27, metadata !13), !dbg !14
   call void @llvm.dbg.declare(metadata i32 %conv, metadata !26, metadata !13), !dbg !14
   call void @llvm.dbg.declare(metadata i32 %direct, metadata !25, metadata !13), !dbg !14
   call void @llvm.dbg.declare(metadata i32* %x.addr, metadata !12, metadata !13), !dbg !14
   call void @llvm.dbg.declare(metadata i8** %coro_hdl, metadata !15, metadata !13), !dbg !16
   call void @llvm.dbg.declare(metadata i8* null, metadata !28, metadata !13), !dbg !16
+  call void @llvm.dbg.declare(metadata i32* %late_local, metadata !29, metadata !13), !dbg !16
+  br label %next, !dbg !18
+
+next:
   br label %sw.epilog, !dbg !18
 
 sw.bb1:                                           ; preds = %entry
   br label %coro_Cleanup, !dbg !18
 
 sw.default:                                       ; preds = %entry
   br label %coro_Suspend, !dbg !18
 
 sw.epilog:                                        ; preds = %sw.bb
   %4 = load i32, i32* %x.addr, align 4, !dbg !20
   %add = add nsw i32 %4, 1, !dbg !21
   store i32 %add, i32* %x.addr, align 4, !dbg !22
   br label %coro_Cleanup, !dbg !23
 
 coro_Cleanup:                                     ; preds = %sw.epilog, %sw.bb1
   %5 = load i8*, i8** %coro_hdl, align 8, !dbg !24
   %6 = call i8* @llvm.coro.free(token %0, i8* %5), !dbg !24
   call void @free(i8* %6), !dbg !24
   br label %coro_Suspend, !dbg !24
 
 coro_Suspend:                                     ; preds = %coro_Cleanup, %sw.default
   %7 = call i1 @llvm.coro.end(i8* null, i1 false) #7, !dbg !24
   %8 = load i8*, i8** %coro_hdl, align 8, !dbg !24
+  store i32 0, i32* %late_local, !dbg !24
   ret i8* %8, !dbg !24
 }
 
 ; Function Attrs: nounwind readnone speculatable
 declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
 
 ; Function Attrs: argmemonly nounwind readonly
 declare token @llvm.coro.id(i32, i8* readnone, i8* nocapture readonly, i8*) #2

 !22 = !DILocation(line: 59, column: 7, scope: !6)
 !23 = !DILocation(line: 59, column: 5, scope: !6)
 !24 = !DILocation(line: 62, column: 3, scope: !6)
 ; These variables were added manually.
 !25 = !DILocalVariable(name: "direct_mem", scope: !6, file: !7, line: 55, type: !11)
 !26 = !DILocalVariable(name: "direct_const", scope: !6, file: !7, line: 55, type: !11)
 !27 = !DILocalVariable(name: "undefined", scope: !6, file: !7, line: 55, type: !11)
 !28 = !DILocalVariable(name: "null", scope: !6, file: !7, line: 55, type: !11)
+!29 = !DILocalVariable(name: "partial_dead", scope: !6, file: !7, line: 55, type: !11)
 
 ; CHECK: define i8* @f(i32 %x) #0 !dbg ![[ORIG:[0-9]+]]
 ; CHECK: define internal fastcc void @f.resume(%f.Frame* noalias nonnull align 8 dereferenceable(32) %FramePtr) #0 !dbg ![[RESUME:[0-9]+]]
 ; CHECK: entry.resume:
 ; CHECK: %[[DBG_PTR:.*]] = alloca %f.Frame*
 ; CHECK: call void @llvm.dbg.declare(metadata %f.Frame** %[[DBG_PTR]], metadata ![[RESUME_COROHDL:[0-9]+]], metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst,
 ; CHECK: call void @llvm.dbg.declare(metadata %f.Frame** %[[DBG_PTR]], metadata ![[RESUME_X:[0-9]+]], metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst, [[EXPR_TAIL:.*]])
 ; CHECK: call void @llvm.dbg.declare(metadata %f.Frame** %[[DBG_PTR]], metadata ![[RESUME_DIRECT:[0-9]+]], metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst, [[EXPR_TAIL]])
 ; CHECK: store %f.Frame* {{.*}}, %f.Frame** %[[DBG_PTR]]
 ; CHECK-NOT: alloca %struct.test*
 ; CHECK: call void @llvm.dbg.declare(metadata i32 0, metadata ![[RESUME_CONST:[0-9]+]], metadata !DIExpression())
+; Note that keeping the undef value here could be acceptable, too.
+; CHECK-NOT: call void @llvm.dbg.declare(metadata i32* undef, metadata !{{[0-9]+}}, metadata !DIExpression())
 ; CHECK: call void @coro.devirt.trigger(i8* null)
 ; CHECK: define internal fastcc void @f.destroy(%f.Frame* noalias nonnull align 8 dereferenceable(32) %FramePtr) #0 !dbg ![[DESTROY:[0-9]+]]
 ; CHECK: define internal fastcc void @f.cleanup(%f.Frame* noalias nonnull align 8 dereferenceable(32) %FramePtr) #0 !dbg ![[CLEANUP:[0-9]+]]
 
 ; CHECK: ![[ORIG]] = distinct !DISubprogram(name: "f", linkageName: "flink"
 
 ; CHECK: ![[RESUME]] = distinct !DISubprogram(name: "f", linkageName: "flink"
 ; CHECK: ![[RESUME_COROHDL]] = !DILocalVariable(name: "coro_hdl", scope: ![[RESUME]]
 ; CHECK: ![[RESUME_X]] = !DILocalVariable(name: "x", arg: 1, scope: ![[RESUME]]
 ; CHECK: ![[RESUME_DIRECT]] = !DILocalVariable(name: "direct_mem", scope: ![[RESUME]]
 ; CHECK: ![[RESUME_CONST]] = !DILocalVariable(name: "direct_const", scope: ![[RESUME]]
 
 ; CHECK: ![[DESTROY]] = distinct !DISubprogram(name: "f", linkageName: "flink"
 
 ; CHECK: ![[CLEANUP]] = distinct !DISubprogram(name: "f", linkageName: "flink"