Skip to content

Commit a9a7013

Browse files
[mem2reg][debuginfo] Handle op_deref when converting dbg.declare
The conversion of dbg.declare into dbg.values doesn't take into account the DIExpression attached to the intrinsic. In particular, when converting: ``` store %val, ptr %alloca dbg.declare(ptr %alloca, !SomeVar, !DIExpression()) ``` Mem2Reg will try to figure out if `%val` has the size of `!SomeVar`. If it does, then a non-undef dbg.value is inserted: ``` dbg.value(%val, !SomeVar, !DIExpression()) ``` This makes sense: the alloca is _the_ address of the variable. So a store to the alloca is a store to the variable. However, if the expression in the original intrinsic is a `DW_OP_deref`, this logic is not applicable: ``` store ptr %val, ptr %alloca dbg.declare(ptr %alloca, !SomeVar, !DIExpression(DW_OP_deref)) ``` Here, the alloca is *not* the address of the variable. A store to the alloca is *not* a store to the variable. As such, querying whether `%val` has the same size as `!SomeVar` is meaningless. This patch addresses the issue by: 1. Allowing the conversion when the expression is _only_ a `DW_OP_deref` without any other expressions (see code comment). 2. Checking that the expression does not start with a `DW_OP_deref` before applying the logic that checks whether the value being stored and the variable have the same length. Differential Revision: https://reviews.llvm.org/D142160 (cherry picked from commit 055f2f0)
1 parent 36c016a commit a9a7013

File tree

4 files changed

+84
-9
lines changed

4 files changed

+84
-9
lines changed

llvm/include/llvm/IR/DebugInfoMetadata.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2786,6 +2786,9 @@ class DIExpression : public MDNode {
27862786
/// Return whether the first element a DW_OP_deref.
27872787
bool startsWithDeref() const;
27882788

2789+
/// Return whether there is exactly one operator and it is a DW_OP_deref;
2790+
bool isDeref() const;
2791+
27892792
/// Holds the characteristics of one fragment of a larger variable.
27902793
struct FragmentInfo {
27912794
uint64_t SizeInBits;

llvm/lib/IR/DebugInfoMetadata.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,6 +1245,9 @@ bool DIExpression::isEntryValue() const {
12451245
bool DIExpression::startsWithDeref() const {
12461246
return getNumElements() > 0 && getElement(0) == dwarf::DW_OP_deref;
12471247
}
1248+
bool DIExpression::isDeref() const {
1249+
return getNumElements() == 1 && startsWithDeref();
1250+
}
12481251

12491252
unsigned DIExpression::ExprOperand::getSize() const {
12501253
uint64_t Op = getOp();

llvm/lib/Transforms/Utils/Local.cpp

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1520,19 +1520,34 @@ void llvm::ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII,
15201520

15211521
DebugLoc NewLoc = getDebugValueLoc(DII, SI);
15221522

1523-
if (!valueCoversEntireFragment(DV->getType(), DII)) {
1524-
// FIXME: If storing to a part of the variable described by the dbg.declare,
1525-
// then we want to insert a dbg.value for the corresponding fragment.
1526-
LLVM_DEBUG(dbgs() << "Failed to convert dbg.declare to dbg.value: "
1527-
<< *DII << '\n');
1528-
// For now, when there is a store to parts of the variable (but we do not
1529-
// know which part) we insert an dbg.value intrinsic to indicate that we
1530-
// know nothing about the variable's content.
1531-
DV = UndefValue::get(DV->getType());
1523+
// If the alloca describes the variable itself, i.e. the expression in the
1524+
// dbg.declare doesn't start with a dereference, we can perform the
1525+
// conversion if the value covers the entire fragment of DII.
1526+
// If the alloca describes the *address* of DIVar, i.e. DIExpr is
1527+
// *just* a DW_OP_deref, we use DV as is for the dbg.value.
1528+
// We conservatively ignore other dereferences, because the following two are
1529+
// not equivalent:
1530+
// dbg.declare(alloca, ..., !Expr(deref, plus_uconstant, 2))
1531+
// dbg.value(DV, ..., !Expr(deref, plus_uconstant, 2))
1532+
// The former is adding 2 to the address of the variable, whereas the latter
1533+
// is adding 2 to the value of the variable. As such, we insist on just a
1534+
// deref expression.
1535+
bool CanConvert =
1536+
DIExpr->isDeref() || (!DIExpr->startsWithDeref() &&
1537+
valueCoversEntireFragment(DV->getType(), DII));
1538+
if (CanConvert) {
15321539
Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, SI);
15331540
return;
15341541
}
15351542

1543+
// FIXME: If storing to a part of the variable described by the dbg.declare,
1544+
// then we want to insert a dbg.value for the corresponding fragment.
1545+
LLVM_DEBUG(dbgs() << "Failed to convert dbg.declare to dbg.value: " << *DII
1546+
<< '\n');
1547+
// For now, when there is a store to parts of the variable (but we do not
1548+
// know which part) we insert an dbg.value intrinsic to indicate that we
1549+
// know nothing about the variable's content.
1550+
DV = UndefValue::get(DV->getType());
15361551
Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, SI);
15371552
}
15381553

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
; RUN: opt < %s -passes='mem2reg' -S | FileCheck %s
2+
target datalayout = "e-p:64:64"
3+
4+
; An intrinsic without any expressions should always be converted.
5+
define i64 @foo0(i64 %arg) {
6+
%arg.addr = alloca i64
7+
store i64 %arg, ptr %arg.addr
8+
call void @llvm.dbg.declare(metadata ptr %arg.addr, metadata !26, metadata !DIExpression()), !dbg !40
9+
; CHECK-LABEL: @foo0
10+
; CHECK-SAME: (i64 [[arg:%.*]])
11+
; CHECK-NEXT: dbg.value(metadata i64 [[arg]], {{.*}}, metadata !DIExpression())
12+
%val = load i64, ptr %arg.addr
13+
ret i64 %val
14+
}
15+
16+
; An intrinsic with a single deref operator should be converted preserving the deref.
17+
define i32 @foo1(ptr %arg) {
18+
%arg.indirect_addr = alloca ptr
19+
store ptr %arg, ptr %arg.indirect_addr
20+
call void @llvm.dbg.declare(metadata ptr %arg.indirect_addr, metadata !25, metadata !DIExpression(DW_OP_deref)), !dbg !40
21+
; CHECK-LABEL: @foo1
22+
; CHECK-SAME: (ptr [[arg:%.*]])
23+
; CHECK-NEXT: dbg.value(metadata ptr [[arg]], {{.*}}, metadata !DIExpression(DW_OP_deref))
24+
%val = load i32, ptr %arg
25+
ret i32 %val
26+
}
27+
28+
29+
; An intrinsic with multiple operators should cause us to conservatively bail.
30+
define i32 @foo2(ptr %arg) {
31+
%arg.indirect_addr = alloca ptr
32+
store ptr %arg, ptr %arg.indirect_addr
33+
call void @llvm.dbg.declare(metadata ptr %arg.indirect_addr, metadata !25, metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst, 2)), !dbg !40
34+
; CHECK-LABEL: @foo2
35+
; CHECK-NEXT: dbg.value(metadata ptr undef, {{.*}}, metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst, 2))
36+
%val = load i32, ptr %arg
37+
ret i32 %val
38+
}
39+
40+
declare void @llvm.dbg.declare(metadata, metadata, metadata)
41+
42+
!llvm.module.flags = !{!1, !2}
43+
!llvm.dbg.cu = !{!7}
44+
45+
!1 = !{i32 7, !"Dwarf Version", i32 4}
46+
!2 = !{i32 2, !"Debug Info Version", i32 3}
47+
!3 = !DIBasicType(name: "wide type", size: 512)
48+
!4 = !DIBasicType(name: "ptr sized type", size: 64)
49+
!7 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !8, producer: "clang", emissionKind: FullDebug)
50+
!8 = !DIFile(filename: "test.ll", directory: "")
51+
!10 = distinct !DISubprogram(name: "blah", linkageName: "blah", scope: !8, file: !8, line: 7, unit: !7)
52+
!25 = !DILocalVariable(name: "blah", arg: 1, scope: !10, file: !8, line: 7, type:!3)
53+
!26 = !DILocalVariable(name: "ptr sized var", arg: 1, scope: !10, file: !8, line: 7, type:!4)
54+
!40 = !DILocation(line: 7, column: 35, scope: !10)

0 commit comments

Comments
 (0)