[llvm] dc14542 - [Coroutines] Add missing llvm.dbg.declare's to cover for more allocas
Bruno Cardoso Lopes via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 10 12:36:16 PST 2020
Author: Bruno Cardoso Lopes
Date: 2020-11-10T12:36:07-08:00
New Revision: dc14542a71f8f46f592a42bee6a85f563b501890
URL: https://github.com/llvm/llvm-project/commit/dc14542a71f8f46f592a42bee6a85f563b501890
DIFF: https://github.com/llvm/llvm-project/commit/dc14542a71f8f46f592a42bee6a85f563b501890.diff
LOG: [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
Added:
Modified:
llvm/lib/Transforms/Coroutines/CoroFrame.cpp
llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index af07d28fc7c5..5265977ea5a0 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -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) {
diff --git a/llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll b/llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll
index 889816f61faf..00a77cb790dc 100644
--- a/llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll
+++ b/llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll
@@ -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
@@ -28,28 +32,39 @@
; 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() {
@@ -57,6 +72,7 @@ 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
@@ -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()
@@ -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
@@ -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}
@@ -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)
\ No newline at end of file
More information about the llvm-commits
mailing list