Skip to content

Commit

Permalink
Tighten heuristic for coroutine debug info workaround.
Browse files Browse the repository at this point in the history
The OutermostLoad condition is supposed to strip the outermost
DW_OP_deref operation because dbg.declares are implicitly
indirect. This patch makes sure the heuristic is only applied to
dbg.declare intrinsics and only if the outermost instruction is a
load.

This was found while qualifying the latest Swift compiler rebranch.

rdar://82037764
  • Loading branch information
adrian-prantl committed Sep 1, 2021
1 parent af1ca43 commit 12de296
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 3 deletions.
6 changes: 3 additions & 3 deletions llvm/lib/Transforms/Coroutines/CoroFrame.cpp
Expand Up @@ -2511,7 +2511,7 @@ void coro::salvageDebugInfo(
DIExpression *Expr = DVI->getExpression();
// Follow the pointer arithmetic all the way to the incoming
// function argument and convert into a DIExpression.
bool OutermostLoad = true;
bool SkipOutermostLoad = !isa<DbgValueInst>(DVI);
Value *Storage = DVI->getVariableLocationOp(0);
Value *OriginalStorage = Storage;
while (auto *Inst = dyn_cast_or_null<Instruction>(Storage)) {
Expand All @@ -2523,9 +2523,8 @@ void coro::salvageDebugInfo(
// 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)
if (!SkipOutermostLoad)
Expr = DIExpression::prepend(Expr, DIExpression::DerefBefore);
OutermostLoad = false;
} else if (auto *StInst = dyn_cast<StoreInst>(Inst)) {
Storage = StInst->getOperand(0);
} else {
Expand All @@ -2542,6 +2541,7 @@ void coro::salvageDebugInfo(
Storage = Op;
Expr = DIExpression::appendOpsToArg(Expr, Ops, 0, /*StackValue*/ false);
}
SkipOutermostLoad = false;
}
if (!Storage)
return;
Expand Down
7 changes: 7 additions & 0 deletions llvm/test/Transforms/Coroutines/coro-debug.ll
Expand Up @@ -35,6 +35,7 @@ sw.bb: ; preds = %entry
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 i32* %late_local, metadata !29, metadata !13), !dbg !16
call void @llvm.dbg.value(metadata i32 %direct, metadata !30, metadata !13), !dbg !14
; don't crash when encountering nonsensical debug info, verfifier doesn't yet reject these
call void @llvm.dbg.declare(metadata i8* null, metadata !28, metadata !13), !dbg !16
call void @llvm.dbg.declare(metadata !{}, metadata !28, metadata !13), !dbg !16
Expand Down Expand Up @@ -68,6 +69,9 @@ coro_Suspend: ; preds = %coro_Cleanup, %sw.d
ret i8* %8, !dbg !24
}

; Function Attrs: nounwind readnone speculatable
declare void @llvm.dbg.value(metadata, metadata, metadata) #1

; Function Attrs: nounwind readnone speculatable
declare void @llvm.dbg.declare(metadata, metadata, metadata) #1

Expand Down Expand Up @@ -146,6 +150,7 @@ attributes #7 = { noduplicate }
!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)
!30 = !DILocalVariable(name: "direct_value", 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]+]]
Expand All @@ -160,6 +165,7 @@ attributes #7 = { noduplicate }
; 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: call void @llvm.dbg.value(metadata %f.Frame** {{.*}}, metadata ![[RESUME_DIRECT_VALUE:[0-9]+]], metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst, {{[0-9]+}}, DW_OP_deref))
; 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]+]]

Expand All @@ -170,6 +176,7 @@ attributes #7 = { noduplicate }
; 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: ![[RESUME_DIRECT_VALUE]] = !DILocalVariable(name: "direct_value", scope: ![[RESUME]]

; CHECK: ![[DESTROY]] = distinct !DISubprogram(name: "f", linkageName: "flink"

Expand Down

0 comments on commit 12de296

Please sign in to comment.