Skip to content

Commit

Permalink
[Coroutines] Add missing llvm.dbg.declare's to cover for more allocas
Browse files Browse the repository at this point in the history
Tracking local variables across suspend points is still somewhat incomplete.
Consider this coroutine snippet:

```
resumable foo() {
  int x[10] = {};
  int a = 3;
  co_await std::experimental::suspend_always();
  a++;
  x[0] = 1;
  a += 2;
  x[1] = 2;
  a += 3;
  x[2] = 3;
}
```

Can't manage to print `a` or `x` if they turn out to be allocas during
CoroSplit (which happens if you build this code with `-O0` prior to this
commit):

```
* thread #1, queue = 'com.apple.main-thread', stop reason = step over
    frame #0: 0x0000000100003729 main-noprint`foo() at main-noprint.cpp:43:5
   40     co_await std::experimental::suspend_always();
   41     a++;
   42     x[0] = 1;
-> 43     a += 2;
   44     x[1] = 2;
   45     a += 3;
   46     x[2] = 3;
(lldb) p x
error: <user expression 21>:1:1: use of undeclared identifier 'x'
x
^
```

The generated IR contains a `llvm.dbg.declare` for `x` in it's initialization
basic block. After CoroSplit, the `llvm.dbg.declare` might not dominate all of
`x` uses and we lose debugging quality.

Add `llvm.dbg.value`s to all relevant basic blocks such that if later
transformations break the dominance the reliable debug info is already in
place. For instance, this BB:

```
await.ready:
  ...
  %arrayidx = getelementptr inbounds [10 x i32], [10 x i32]* %x.reload.addr, i64 0, i64 0, !dbg !760
  ...
  %arrayidx19 = getelementptr inbounds [10 x i32], [10 x i32]* %x.reload.addr, i64 0, i64 1, !dbg !763
  ...
  %arrayidx21 = getelementptr inbounds [10 x i32], [10 x i32]* %x.reload.addr, i64 0, i64 2, !dbg !766
```

becomes:

```
await.ready:
  ...
  call void @llvm.dbg.value(metadata [10 x i32]* %x.reload.addr, metadata !751, metadata !DIExpression()), !dbg !753
  ...
  %arrayidx = getelementptr inbounds [10 x i32], [10 x i32]* %x.reload.addr, i64 0, i64 0, !dbg !760
  ...
  %arrayidx19 = getelementptr inbounds [10 x i32], [10 x i32]* %x.reload.addr, i64 0, i64 1, !dbg !763
  ...
  %arrayidx21 = getelementptr inbounds [10 x i32], [10 x i32]* %x.reload.addr, i64 0, i64 2, !dbg !766
```

Differential Revision: https://reviews.llvm.org/D90772
  • Loading branch information
bcardosolopes committed Nov 10, 2020
1 parent 2ef4791 commit dc14542
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 7 deletions.
46 changes: 39 additions & 7 deletions llvm/lib/Transforms/Coroutines/CoroFrame.cpp
Expand Up @@ -1191,19 +1191,51 @@ static Instruction *insertSpills(const FrameDataInfo &FrameData,
continue;
auto *G = GetFramePointer(Alloca);
G->setName(Alloca->getName() + Twine(".reload.addr"));

SmallPtrSet<BasicBlock *, 4> SeenDbgBBs;
TinyPtrVector<DbgDeclareInst *> DIs = FindDbgDeclareUses(Alloca);
if (!DIs.empty())
DIBuilder(*Alloca->getModule(),
/*AllowUnresolved*/ false)
.insertDeclare(G, DIs.front()->getVariable(),
DIs.front()->getExpression(),
DIs.front()->getDebugLoc(), DIs.front());
DIBuilder DIB(*Alloca->getModule(), /*AllowUnresolved*/ false);
Instruction *FirstDbgDecl = nullptr;

if (!DIs.empty()) {
FirstDbgDecl = DIB.insertDeclare(G, DIs.front()->getVariable(),
DIs.front()->getExpression(),
DIs.front()->getDebugLoc(), DIs.front());
SeenDbgBBs.insert(DIs.front()->getParent());
}
for (auto *DI : FindDbgDeclareUses(Alloca))
DI->eraseFromParent();
replaceDbgUsesWithUndef(Alloca);

for (Instruction *I : UsersToUpdate)
for (Instruction *I : UsersToUpdate) {
I->replaceUsesOfWith(Alloca, G);

// After cloning, transformations might not guarantee that all uses
// of this alloca are dominated by the already existing dbg.declare's,
// compromising the debug quality. Instead of writing another
// transformation to patch each clone, go ahead and early populate
// basic blocks that use such allocas with more debug info.
if (SeenDbgBBs.count(I->getParent()))
continue;

// If there isn't a prior dbg.declare for this alloca, it probably
// means the state hasn't changed prior to one of the relevant suspend
// point for this frame access.
if (!FirstDbgDecl)
continue;

// These instructions are all dominated by the alloca, insert the
// dbg.value in the beginning of the BB to enhance debugging
// experience and allow values to be inspected as early as possible.
// Prefer dbg.value over dbg.declare since it better sets expectations
// that control flow can be later changed by other passes.
auto *DI = cast<DbgDeclareInst>(FirstDbgDecl);
BasicBlock *CurrentBlock = I->getParent();
DIB.insertDbgValueIntrinsic(G, DI->getVariable(), DI->getExpression(),
DI->getDebugLoc(),
&*CurrentBlock->getFirstInsertionPt());
SeenDbgBBs.insert(CurrentBlock);
}
}
Builder.SetInsertPoint(FramePtr->getNextNode());
for (const auto &A : FrameData.Allocas) {
Expand Down
34 changes: 34 additions & 0 deletions llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll
Expand Up @@ -7,15 +7,19 @@
; void foo() {
; int i = 0;
; ++i;
; int x = {};
; print(i); // Prints '1'
;
; co_await suspend_always();
;
; int j = 0;
; x[0] = 1;
; x[1] = 2;
; ++i;
; print(i); // Prints '2'
; ++j;
; print(j); // Prints '1'
; print(x); // Print '1'
; }
;
; The CHECKs verify that dbg.declare intrinsics are created for the coroutine
Expand All @@ -28,35 +32,47 @@
; CHECK: entry:
; CHECK: %j = alloca i32, align 4
; CHECK: [[IGEP:%.+]] = getelementptr inbounds %f.Frame, %f.Frame* %FramePtr, i32 0, i32 4
; CHECK: [[XGEP:%.+]] = getelementptr inbounds %f.Frame, %f.Frame* %FramePtr, i32 0, i32 6
; CHECK: init.ready:
; CHECK: call void @llvm.dbg.declare(metadata i32* [[IGEP]], metadata ![[IVAR:[0-9]+]], metadata !DIExpression()), !dbg ![[IDBGLOC:[0-9]+]]
; CHECK: call void @llvm.dbg.declare(metadata [10 x i32]* [[XGEP]], metadata ![[XVAR:[0-9]+]], metadata !DIExpression()), !dbg ![[IDBGLOC]]
; CHECK: await.ready:
; CHECK: call void @llvm.dbg.value(metadata [10 x i32]* [[XGEP]], metadata ![[XVAR]], metadata !DIExpression()), !dbg ![[IDBGLOC]]
; CHECK: call void @llvm.dbg.value(metadata i32* [[IGEP]], metadata ![[IVAR]], metadata !DIExpression()), !dbg ![[IDBGLOC]]
; CHECK: call void @llvm.dbg.declare(metadata i32* %j, metadata ![[JVAR:[0-9]+]], metadata !DIExpression()), !dbg ![[JDBGLOC:[0-9]+]]
;
; CHECK-LABEL: define internal fastcc void @f.resume({{.*}}) {
; CHECK: entry.resume:
; CHECK: %j = alloca i32, align 4
; CHECK: [[IGEP_RESUME:%.+]] = getelementptr inbounds %f.Frame, %f.Frame* %FramePtr, i32 0, i32 4
; CHECK: [[XGEP_RESUME:%.+]] = getelementptr inbounds %f.Frame, %f.Frame* %FramePtr, i32 0, i32 6
; CHECK: init.ready:
; CHECK: call void @llvm.dbg.declare(metadata i32* [[IGEP_RESUME]], metadata ![[IVAR_RESUME:[0-9]+]], metadata !DIExpression()), !dbg ![[IDBGLOC_RESUME:[0-9]+]]
; CHECK: call void @llvm.dbg.declare(metadata [10 x i32]* [[XGEP_RESUME]], metadata ![[XVAR_RESUME:[0-9]+]], metadata !DIExpression()), !dbg ![[IDBGLOC_RESUME]]
; CHECK: await.ready:
; CHECK: call void @llvm.dbg.value(metadata [10 x i32]* [[XGEP_RESUME]], metadata ![[XVAR_RESUME]], metadata !DIExpression()), !dbg ![[IDBGLOC_RESUME]]
; CHECK: call void @llvm.dbg.value(metadata i32* [[IGEP_RESUME]], metadata ![[IVAR_RESUME]], metadata !DIExpression()), !dbg ![[IDBGLOC_RESUME]]
; CHECK: call void @llvm.dbg.declare(metadata i32* %j, metadata ![[JVAR_RESUME:[0-9]+]], metadata !DIExpression()), !dbg ![[JDBGLOC_RESUME:[0-9]+]]
;
; CHECK: ![[IVAR]] = !DILocalVariable(name: "i"
; CHECK: ![[SCOPE:[0-9]+]] = distinct !DILexicalBlock(scope: !8, file: !1, line: 23, column: 12)
; CHECK: ![[IDBGLOC]] = !DILocation(line: 24, column: 7, scope: ![[SCOPE]])
; CHECK: ![[XVAR]] = !DILocalVariable(name: "x"
; CHECK: ![[JVAR]] = !DILocalVariable(name: "j"
; CHECK: ![[JDBGLOC]] = !DILocation(line: 32, column: 7, scope: ![[SCOPE]])

; CHECK: ![[IVAR_RESUME]] = !DILocalVariable(name: "i"
; CHECK: ![[RESUME_SCOPE:[0-9]+]] = distinct !DILexicalBlock(scope: !8, file: !1, line: 23, column: 12)
; CHECK: ![[IDBGLOC_RESUME]] = !DILocation(line: 24, column: 7, scope: ![[RESUME_SCOPE]])
; CHECK: ![[XVAR_RESUME]] = !DILocalVariable(name: "x"
; CHECK: ![[JVAR_RESUME]] = !DILocalVariable(name: "j"
; CHECK: ![[JDBGLOC_RESUME]] = !DILocation(line: 32, column: 7, scope: ![[RESUME_SCOPE]])
define void @f() {
entry:
%__promise = alloca i8, align 8
%i = alloca i32, align 4
%j = alloca i32, align 4
%x = alloca [10 x i32], align 16
%id = call token @llvm.coro.id(i32 16, i8* %__promise, i8* null, i8* null)
%alloc = call i1 @llvm.coro.alloc(token %id)
br i1 %alloc, label %coro.alloc, label %coro.init
Expand Down Expand Up @@ -91,6 +107,9 @@ init.ready: ; preds = %init.suspend, %coro
%i.init.ready.load = load i32, i32* %i, align 4
%i.init.ready.inc = add nsw i32 %i.init.ready.load, 1
store i32 %i.init.ready.inc, i32* %i, align 4
call void @llvm.dbg.declare(metadata [10 x i32]* %x, metadata !14, metadata !DIExpression()), !dbg !11
%memset = bitcast [10 x i32]* %x to i8*, !dbg !11
call void @llvm.memset.p0i8.i64(i8* align 16 %memset, i8 0, i64 40, i1 false), !dbg !11
%i.init.ready.reload = load i32, i32* %i, align 4
call void @print(i32 %i.init.ready.reload)
%ready.again = call zeroext i1 @await_ready()
Expand All @@ -113,6 +132,10 @@ await.ready: ; preds = %await.suspend, %ini
call void @await_resume()
call void @llvm.dbg.declare(metadata i32* %j, metadata !12, metadata !DIExpression()), !dbg !13
store i32 0, i32* %j, align 4
%arrayidx0 = getelementptr inbounds [10 x i32], [10 x i32]* %x, i64 0, i64 0, !dbg !18
store i32 1, i32* %arrayidx0, align 16, !dbg !19
%arrayidx1 = getelementptr inbounds [10 x i32], [10 x i32]* %x, i64 0, i64 1, !dbg !20
store i32 2, i32* %arrayidx1, align 4, !dbg !21
%i.await.ready.load = load i32, i32* %i, align 4
%i.await.ready.inc = add nsw i32 %i.await.ready.load, 1
store i32 %i.await.ready.inc, i32* %i, align 4
Expand Down Expand Up @@ -195,6 +218,8 @@ declare i8* @from_address(i8*)
declare void @return_void()
declare void @final_suspend()

declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg)

!llvm.dbg.cu = !{!0}
!llvm.linker.options = !{}
!llvm.module.flags = !{!3, !4}
Expand All @@ -214,3 +239,12 @@ declare void @final_suspend()
!11 = !DILocation(line: 24, column: 7, scope: !7)
!12 = !DILocalVariable(name: "j", scope: !7, file: !1, line: 32, type: !10)
!13 = !DILocation(line: 32, column: 7, scope: !7)
!14 = !DILocalVariable(name: "x", scope: !22, file: !1, line: 34, type: !15)
!15 = !DICompositeType(tag: DW_TAG_array_type, baseType: !10, size: 320, elements: !16)
!16 = !{!17}
!17 = !DISubrange(count: 10)
!18 = !DILocation(line: 42, column: 3, scope: !7)
!19 = !DILocation(line: 42, column: 8, scope: !7)
!20 = !DILocation(line: 43, column: 3, scope: !7)
!21 = !DILocation(line: 43, column: 8, scope: !7)
!22 = distinct !DILexicalBlock(scope: !8, file: !1, line: 23, column: 12)

0 comments on commit dc14542

Please sign in to comment.