Skip to content

Commit dc14542

Browse files
bcardosolopesbcardosolopes
committed
[Coroutines] Add missing llvm.dbg.declare's to cover for more allocas
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
1 parent 2ef4791 commit dc14542

File tree

2 files changed

+73
-7
lines changed

2 files changed

+73
-7
lines changed

‎llvm/lib/Transforms/Coroutines/CoroFrame.cpp

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1191,19 +1191,51 @@ static Instruction *insertSpills(const FrameDataInfo &FrameData,
11911191
continue;
11921192
auto *G = GetFramePointer(Alloca);
11931193
G->setName(Alloca->getName() + Twine(".reload.addr"));
1194+
1195+
SmallPtrSet<BasicBlock *, 4> SeenDbgBBs;
11941196
TinyPtrVector<DbgDeclareInst *> DIs = FindDbgDeclareUses(Alloca);
1195-
if (!DIs.empty())
1196-
DIBuilder(*Alloca->getModule(),
1197-
/*AllowUnresolved*/ false)
1198-
.insertDeclare(G, DIs.front()->getVariable(),
1199-
DIs.front()->getExpression(),
1200-
DIs.front()->getDebugLoc(), DIs.front());
1197+
DIBuilder DIB(*Alloca->getModule(), /*AllowUnresolved*/ false);
1198+
Instruction *FirstDbgDecl = nullptr;
1199+
1200+
if (!DIs.empty()) {
1201+
FirstDbgDecl = DIB.insertDeclare(G, DIs.front()->getVariable(),
1202+
DIs.front()->getExpression(),
1203+
DIs.front()->getDebugLoc(), DIs.front());
1204+
SeenDbgBBs.insert(DIs.front()->getParent());
1205+
}
12011206
for (auto *DI : FindDbgDeclareUses(Alloca))
12021207
DI->eraseFromParent();
12031208
replaceDbgUsesWithUndef(Alloca);
12041209

1205-
for (Instruction *I : UsersToUpdate)
1210+
for (Instruction *I : UsersToUpdate) {
12061211
I->replaceUsesOfWith(Alloca, G);
1212+
1213+
// After cloning, transformations might not guarantee that all uses
1214+
// of this alloca are dominated by the already existing dbg.declare's,
1215+
// compromising the debug quality. Instead of writing another
1216+
// transformation to patch each clone, go ahead and early populate
1217+
// basic blocks that use such allocas with more debug info.
1218+
if (SeenDbgBBs.count(I->getParent()))
1219+
continue;
1220+
1221+
// If there isn't a prior dbg.declare for this alloca, it probably
1222+
// means the state hasn't changed prior to one of the relevant suspend
1223+
// point for this frame access.
1224+
if (!FirstDbgDecl)
1225+
continue;
1226+
1227+
// These instructions are all dominated by the alloca, insert the
1228+
// dbg.value in the beginning of the BB to enhance debugging
1229+
// experience and allow values to be inspected as early as possible.
1230+
// Prefer dbg.value over dbg.declare since it better sets expectations
1231+
// that control flow can be later changed by other passes.
1232+
auto *DI = cast<DbgDeclareInst>(FirstDbgDecl);
1233+
BasicBlock *CurrentBlock = I->getParent();
1234+
DIB.insertDbgValueIntrinsic(G, DI->getVariable(), DI->getExpression(),
1235+
DI->getDebugLoc(),
1236+
&*CurrentBlock->getFirstInsertionPt());
1237+
SeenDbgBBs.insert(CurrentBlock);
1238+
}
12071239
}
12081240
Builder.SetInsertPoint(FramePtr->getNextNode());
12091241
for (const auto &A : FrameData.Allocas) {

‎llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,19 @@
77
; void foo() {
88
; int i = 0;
99
; ++i;
10+
; int x = {};
1011
; print(i); // Prints '1'
1112
;
1213
; co_await suspend_always();
1314
;
1415
; int j = 0;
16+
; x[0] = 1;
17+
; x[1] = 2;
1518
; ++i;
1619
; print(i); // Prints '2'
1720
; ++j;
1821
; print(j); // Prints '1'
22+
; print(x); // Print '1'
1923
; }
2024
;
2125
; The CHECKs verify that dbg.declare intrinsics are created for the coroutine
@@ -28,35 +32,47 @@
2832
; CHECK: entry:
2933
; CHECK: %j = alloca i32, align 4
3034
; CHECK: [[IGEP:%.+]] = getelementptr inbounds %f.Frame, %f.Frame* %FramePtr, i32 0, i32 4
35+
; CHECK: [[XGEP:%.+]] = getelementptr inbounds %f.Frame, %f.Frame* %FramePtr, i32 0, i32 6
3136
; CHECK: init.ready:
3237
; CHECK: call void @llvm.dbg.declare(metadata i32* [[IGEP]], metadata ![[IVAR:[0-9]+]], metadata !DIExpression()), !dbg ![[IDBGLOC:[0-9]+]]
38+
; CHECK: call void @llvm.dbg.declare(metadata [10 x i32]* [[XGEP]], metadata ![[XVAR:[0-9]+]], metadata !DIExpression()), !dbg ![[IDBGLOC]]
3339
; CHECK: await.ready:
40+
; CHECK: call void @llvm.dbg.value(metadata [10 x i32]* [[XGEP]], metadata ![[XVAR]], metadata !DIExpression()), !dbg ![[IDBGLOC]]
41+
; CHECK: call void @llvm.dbg.value(metadata i32* [[IGEP]], metadata ![[IVAR]], metadata !DIExpression()), !dbg ![[IDBGLOC]]
3442
; CHECK: call void @llvm.dbg.declare(metadata i32* %j, metadata ![[JVAR:[0-9]+]], metadata !DIExpression()), !dbg ![[JDBGLOC:[0-9]+]]
3543
;
3644
; CHECK-LABEL: define internal fastcc void @f.resume({{.*}}) {
3745
; CHECK: entry.resume:
3846
; CHECK: %j = alloca i32, align 4
3947
; CHECK: [[IGEP_RESUME:%.+]] = getelementptr inbounds %f.Frame, %f.Frame* %FramePtr, i32 0, i32 4
48+
; CHECK: [[XGEP_RESUME:%.+]] = getelementptr inbounds %f.Frame, %f.Frame* %FramePtr, i32 0, i32 6
4049
; CHECK: init.ready:
4150
; CHECK: call void @llvm.dbg.declare(metadata i32* [[IGEP_RESUME]], metadata ![[IVAR_RESUME:[0-9]+]], metadata !DIExpression()), !dbg ![[IDBGLOC_RESUME:[0-9]+]]
51+
; CHECK: call void @llvm.dbg.declare(metadata [10 x i32]* [[XGEP_RESUME]], metadata ![[XVAR_RESUME:[0-9]+]], metadata !DIExpression()), !dbg ![[IDBGLOC_RESUME]]
4252
; CHECK: await.ready:
53+
; CHECK: call void @llvm.dbg.value(metadata [10 x i32]* [[XGEP_RESUME]], metadata ![[XVAR_RESUME]], metadata !DIExpression()), !dbg ![[IDBGLOC_RESUME]]
54+
; CHECK: call void @llvm.dbg.value(metadata i32* [[IGEP_RESUME]], metadata ![[IVAR_RESUME]], metadata !DIExpression()), !dbg ![[IDBGLOC_RESUME]]
4355
; CHECK: call void @llvm.dbg.declare(metadata i32* %j, metadata ![[JVAR_RESUME:[0-9]+]], metadata !DIExpression()), !dbg ![[JDBGLOC_RESUME:[0-9]+]]
4456
;
4557
; CHECK: ![[IVAR]] = !DILocalVariable(name: "i"
4658
; CHECK: ![[SCOPE:[0-9]+]] = distinct !DILexicalBlock(scope: !8, file: !1, line: 23, column: 12)
4759
; CHECK: ![[IDBGLOC]] = !DILocation(line: 24, column: 7, scope: ![[SCOPE]])
60+
; CHECK: ![[XVAR]] = !DILocalVariable(name: "x"
4861
; CHECK: ![[JVAR]] = !DILocalVariable(name: "j"
4962
; CHECK: ![[JDBGLOC]] = !DILocation(line: 32, column: 7, scope: ![[SCOPE]])
63+
5064
; CHECK: ![[IVAR_RESUME]] = !DILocalVariable(name: "i"
5165
; CHECK: ![[RESUME_SCOPE:[0-9]+]] = distinct !DILexicalBlock(scope: !8, file: !1, line: 23, column: 12)
5266
; CHECK: ![[IDBGLOC_RESUME]] = !DILocation(line: 24, column: 7, scope: ![[RESUME_SCOPE]])
67+
; CHECK: ![[XVAR_RESUME]] = !DILocalVariable(name: "x"
5368
; CHECK: ![[JVAR_RESUME]] = !DILocalVariable(name: "j"
5469
; CHECK: ![[JDBGLOC_RESUME]] = !DILocation(line: 32, column: 7, scope: ![[RESUME_SCOPE]])
5570
define void @f() {
5671
entry:
5772
%__promise = alloca i8, align 8
5873
%i = alloca i32, align 4
5974
%j = alloca i32, align 4
75+
%x = alloca [10 x i32], align 16
6076
%id = call token @llvm.coro.id(i32 16, i8* %__promise, i8* null, i8* null)
6177
%alloc = call i1 @llvm.coro.alloc(token %id)
6278
br i1 %alloc, label %coro.alloc, label %coro.init
@@ -91,6 +107,9 @@ init.ready: ; preds = %init.suspend, %coro
91107
%i.init.ready.load = load i32, i32* %i, align 4
92108
%i.init.ready.inc = add nsw i32 %i.init.ready.load, 1
93109
store i32 %i.init.ready.inc, i32* %i, align 4
110+
call void @llvm.dbg.declare(metadata [10 x i32]* %x, metadata !14, metadata !DIExpression()), !dbg !11
111+
%memset = bitcast [10 x i32]* %x to i8*, !dbg !11
112+
call void @llvm.memset.p0i8.i64(i8* align 16 %memset, i8 0, i64 40, i1 false), !dbg !11
94113
%i.init.ready.reload = load i32, i32* %i, align 4
95114
call void @print(i32 %i.init.ready.reload)
96115
%ready.again = call zeroext i1 @await_ready()
@@ -113,6 +132,10 @@ await.ready: ; preds = %await.suspend, %ini
113132
call void @await_resume()
114133
call void @llvm.dbg.declare(metadata i32* %j, metadata !12, metadata !DIExpression()), !dbg !13
115134
store i32 0, i32* %j, align 4
135+
%arrayidx0 = getelementptr inbounds [10 x i32], [10 x i32]* %x, i64 0, i64 0, !dbg !18
136+
store i32 1, i32* %arrayidx0, align 16, !dbg !19
137+
%arrayidx1 = getelementptr inbounds [10 x i32], [10 x i32]* %x, i64 0, i64 1, !dbg !20
138+
store i32 2, i32* %arrayidx1, align 4, !dbg !21
116139
%i.await.ready.load = load i32, i32* %i, align 4
117140
%i.await.ready.inc = add nsw i32 %i.await.ready.load, 1
118141
store i32 %i.await.ready.inc, i32* %i, align 4
@@ -195,6 +218,8 @@ declare i8* @from_address(i8*)
195218
declare void @return_void()
196219
declare void @final_suspend()
197220

221+
declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg)
222+
198223
!llvm.dbg.cu = !{!0}
199224
!llvm.linker.options = !{}
200225
!llvm.module.flags = !{!3, !4}
@@ -214,3 +239,12 @@ declare void @final_suspend()
214239
!11 = !DILocation(line: 24, column: 7, scope: !7)
215240
!12 = !DILocalVariable(name: "j", scope: !7, file: !1, line: 32, type: !10)
216241
!13 = !DILocation(line: 32, column: 7, scope: !7)
242+
!14 = !DILocalVariable(name: "x", scope: !22, file: !1, line: 34, type: !15)
243+
!15 = !DICompositeType(tag: DW_TAG_array_type, baseType: !10, size: 320, elements: !16)
244+
!16 = !{!17}
245+
!17 = !DISubrange(count: 10)
246+
!18 = !DILocation(line: 42, column: 3, scope: !7)
247+
!19 = !DILocation(line: 42, column: 8, scope: !7)
248+
!20 = !DILocation(line: 43, column: 3, scope: !7)
249+
!21 = !DILocation(line: 43, column: 8, scope: !7)
250+
!22 = distinct !DILexicalBlock(scope: !8, file: !1, line: 23, column: 12)

0 commit comments

Comments
 (0)