[llvm] 62140d9 - Better document the limitations of coro::salvageDebugInfo()
Adrian Prantl via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 28 09:53:29 PST 2021
Author: Adrian Prantl
Date: 2021-01-28T09:53:19-08:00
New Revision: 62140d943cc200f66fc3274bbef5f9934e01c324
URL: https://github.com/llvm/llvm-project/commit/62140d943cc200f66fc3274bbef5f9934e01c324
DIFF: https://github.com/llvm/llvm-project/commit/62140d943cc200f66fc3274bbef5f9934e01c324.diff
LOG: 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.
Added:
Modified:
llvm/lib/Transforms/Coroutines/CoroFrame.cpp
llvm/lib/Transforms/Coroutines/CoroInternal.h
llvm/lib/Transforms/Coroutines/CoroSplit.cpp
llvm/test/Transforms/Coroutines/coro-debug.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index e53e7605b254..7538b1bcc347 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -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;
+ // FIXME: LLVM lacks nuanced semantics to
diff erentiate 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);
}
- // The FramePtr object adds one extra layer of indirection that
- // needs to be unwrapped.
- if (LoadFromFramePtr)
- Expr = DIExpression::prepend(Expr, DIExpression::DerefBefore);
auto &VMContext = DDI->getFunction()->getContext();
DDI->setOperand(
0, MetadataAsValue::get(VMContext, ValueAsMetadata::get(Storage)));
diff --git a/llvm/lib/Transforms/Coroutines/CoroInternal.h b/llvm/lib/Transforms/Coroutines/CoroInternal.h
index 6c0e52f24542..cba32222290e 100644
--- a/llvm/lib/Transforms/Coroutines/CoroInternal.h
+++ b/llvm/lib/Transforms/Coroutines/CoroInternal.h
@@ -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 {
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index 27259e40ad48..e5876d612b6c 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -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) {
diff --git a/llvm/test/Transforms/Coroutines/coro-debug.ll b/llvm/test/Transforms/Coroutines/coro-debug.ll
index cde1ddcbffe8..4ac55e9ee2a6 100644
--- a/llvm/test/Transforms/Coroutines/coro-debug.ll
+++ b/llvm/test/Transforms/Coroutines/coro-debug.ll
@@ -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"
More information about the llvm-commits
mailing list