Commit 62140d94 authored by Adrian Prantl's avatar Adrian Prantl
Browse files

Better document the limitations of coro::salvageDebugInfo() 

and fix a few edge cases that show up in the Swift compiler but
weren't caught by the existing tests. Most notably the old code wasn't
salvaging load operations correctly. The patch also gets rid of the
LoadFromFramePtr argument and replaces it with a more generalized
mechanism.
parent cfcc1110

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

+20 −5
Original line number Diff line number Diff line
@@ -2158,7 +2158,7 @@ static void collectFrameAllocas(Function &F, coro::Shape &Shape, 


void coro::salvageDebugInfo(

    SmallDenseMap<llvm::Value *, llvm::AllocaInst *, 4> &DbgPtrAllocaCache,

    DbgDeclareInst *DDI, bool LoadFromFramePtr) {

    DbgDeclareInst *DDI) {

  Function *F = DDI->getFunction();

  IRBuilder<> Builder(F->getContext());

  auto InsertPt = F->getEntryBlock().getFirstInsertionPt();
@@ -2168,10 +2168,20 @@ void coro::salvageDebugInfo( 
  DIExpression *Expr = DDI->getExpression();

  // Follow the pointer arithmetic all the way to the incoming

  // function argument and convert into a DIExpression.

  bool OutermostLoad = true;

  Value *Storage = DDI->getAddress();

  while (Storage) {

    if (auto *LdInst = dyn_cast<LoadInst>(Storage)) {

      Storage = LdInst->getOperand(0);

      // FIXME: This is a heuristic that works around the fact that

      // LLVM IR debug intrinsics cannot yet distinguish between

      // memory and value locations: Because a dbg.declare(alloca) is

      // implicitly a memory location no DW_OP_deref operation for the

      // last direct load from an alloca is necessary.  This condition

      // effectively drops the *last* DW_OP_deref in the expression.

      if (!OutermostLoad)

        Expr = DIExpression::prepend(Expr, DIExpression::DerefBefore);

      OutermostLoad = false;

    } else if (auto *StInst = dyn_cast<StoreInst>(Storage)) {

      Storage = StInst->getOperand(0);

    } else if (auto *GEPInst = dyn_cast<GetElementPtrInst>(Storage)) {
@@ -2195,11 +2205,16 @@ void coro::salvageDebugInfo( 
      Builder.CreateStore(Storage, Cached);

    }

    Storage = Cached;

  }

  // The FramePtr object adds one extra layer of indirection that

  // needs to be unwrapped.

  if (LoadFromFramePtr)

    // FIXME: LLVM lacks nuanced semantics to differentiate between

    // memory and direct locations at the IR level. The backend will

    // turn a dbg.declare(alloca, ..., DIExpression()) into a memory

    // location. Thus, if there are deref and offset operations in the

    // expression, we need to add a DW_OP_deref at the *start* of the

    // expression to first load the contents of the alloca before

    // adjusting it with the expression.

    if (Expr && Expr->isComplex())

      Expr = DIExpression::prepend(Expr, DIExpression::DerefBefore);

  }

  auto &VMContext = DDI->getFunction()->getContext();

  DDI->setOperand(

      0, MetadataAsValue::get(VMContext, ValueAsMetadata::get(Storage)));

llvm/lib/Transforms/Coroutines/CoroInternal.h

+1 −1
Original line number Diff line number Diff line
@@ -54,7 +54,7 @@ void updateCallGraph(Function &Caller, ArrayRef<Function *> Funcs, 
/// holding a pointer to the coroutine frame.

void salvageDebugInfo(

    SmallDenseMap<llvm::Value *, llvm::AllocaInst *, 4> &DbgPtrAllocaCache,

    DbgDeclareInst *DDI, bool LoadFromCoroFrame = false);

    DbgDeclareInst *DDI);



// Keeps data and helper functions for lowering coroutine intrinsics.

struct LowererBase {

llvm/lib/Transforms/Coroutines/CoroSplit.cpp

+3 −5
Original line number Diff line number Diff line
@@ -639,11 +639,9 @@ void CoroCloner::salvageDebugInfo() { 
    for (auto &I : BB)

      if (auto *DDI = dyn_cast<DbgDeclareInst>(&I))

        Worklist.push_back(DDI);

  for (DbgDeclareInst *DDI : Worklist) {

    // This is a heuristic that detects declares left by CoroFrame.

    bool LoadFromFramePtr = !isa<AllocaInst>(DDI->getAddress());

    coro::salvageDebugInfo(DbgPtrAllocaCache, DDI, LoadFromFramePtr);

  }

  for (DbgDeclareInst *DDI : Worklist)

    coro::salvageDebugInfo(DbgPtrAllocaCache, DDI);



  // Remove all salvaged dbg.declare intrinsics that became

  // either unreachable or stale due to the CoroSplit transformation.

  auto IsUnreachableBlock = [&](BasicBlock *BB) {

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

+11 −1
Original line number Diff line number Diff line
@@ -26,6 +26,9 @@ entry: 
  ], !dbg !17



sw.bb:                                            ; preds = %entry

  %direct = load i32, i32* %x.addr, align 4, !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

  br label %sw.epilog, !dbg !18
@@ -126,15 +129,20 @@ attributes #7 = { noduplicate } 
!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)



; 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,

; 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())

; 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]+]]
@@ -144,6 +152,8 @@ attributes #7 = { noduplicate } 
; 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"