[llvm] 055f2f0 - [mem2reg][debuginfo] Handle op_deref when converting dbg.declare
Felipe de Azevedo Piovezan via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 30 08:17:52 PST 2023
Author: Felipe de Azevedo Piovezan
Date: 2023-01-30T11:17:36-05:00
New Revision: 055f2f04e658c7dddd1fc261e5e0c0bd136cc2b5
URL: https://github.com/llvm/llvm-project/commit/055f2f04e658c7dddd1fc261e5e0c0bd136cc2b5
DIFF: https://github.com/llvm/llvm-project/commit/055f2f04e658c7dddd1fc261e5e0c0bd136cc2b5.diff
LOG: [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
Added:
llvm/test/Transforms/Mem2Reg/dbg_declare_to_value_conversions.ll
Modified:
llvm/include/llvm/IR/DebugInfoMetadata.h
llvm/lib/IR/DebugInfoMetadata.cpp
llvm/lib/Transforms/Utils/Local.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index def3ce2b56eab..d83a53f0b4e5b 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -2769,6 +2769,9 @@ class DIExpression : public MDNode {
/// Return whether the first element a DW_OP_deref.
bool startsWithDeref() const;
+ /// Return whether there is exactly one operator and it is a DW_OP_deref;
+ bool isDeref() const;
+
/// Holds the characteristics of one fragment of a larger variable.
struct FragmentInfo {
uint64_t SizeInBits;
diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp
index fb9a7b8822204..ad9540e51932a 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -1285,6 +1285,9 @@ bool DIExpression::isEntryValue() const {
bool DIExpression::startsWithDeref() const {
return getNumElements() > 0 && getElement(0) == dwarf::DW_OP_deref;
}
+bool DIExpression::isDeref() const {
+ return getNumElements() == 1 && startsWithDeref();
+}
DIAssignID *DIAssignID::getImpl(LLVMContext &Context, StorageType Storage,
bool ShouldCreate) {
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 633f0bb8b08b2..b6c6c5a6b52a3 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1524,19 +1524,34 @@ void llvm::ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII,
DebugLoc NewLoc = getDebugValueLoc(DII);
- if (!valueCoversEntireFragment(DV->getType(), DII)) {
- // FIXME: If storing to a part of the variable described by the dbg.declare,
- // then we want to insert a dbg.value for the corresponding fragment.
- LLVM_DEBUG(dbgs() << "Failed to convert dbg.declare to dbg.value: "
- << *DII << '\n');
- // For now, when there is a store to parts of the variable (but we do not
- // know which part) we insert an dbg.value intrinsic to indicate that we
- // know nothing about the variable's content.
- DV = UndefValue::get(DV->getType());
+ // If the alloca describes the variable itself, i.e. the expression in the
+ // dbg.declare doesn't start with a dereference, we can perform the
+ // conversion if the value covers the entire fragment of DII.
+ // If the alloca describes the *address* of DIVar, i.e. DIExpr is
+ // *just* a DW_OP_deref, we use DV as is for the dbg.value.
+ // We conservatively ignore other dereferences, because the following two are
+ // not equivalent:
+ // dbg.declare(alloca, ..., !Expr(deref, plus_uconstant, 2))
+ // dbg.value(DV, ..., !Expr(deref, plus_uconstant, 2))
+ // The former is adding 2 to the address of the variable, whereas the latter
+ // is adding 2 to the value of the variable. As such, we insist on just a
+ // deref expression.
+ bool CanConvert =
+ DIExpr->isDeref() || (!DIExpr->startsWithDeref() &&
+ valueCoversEntireFragment(DV->getType(), DII));
+ if (CanConvert) {
Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, SI);
return;
}
+ // FIXME: If storing to a part of the variable described by the dbg.declare,
+ // then we want to insert a dbg.value for the corresponding fragment.
+ LLVM_DEBUG(dbgs() << "Failed to convert dbg.declare to dbg.value: " << *DII
+ << '\n');
+ // For now, when there is a store to parts of the variable (but we do not
+ // know which part) we insert an dbg.value intrinsic to indicate that we
+ // know nothing about the variable's content.
+ DV = UndefValue::get(DV->getType());
Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, SI);
}
diff --git a/llvm/test/Transforms/Mem2Reg/dbg_declare_to_value_conversions.ll b/llvm/test/Transforms/Mem2Reg/dbg_declare_to_value_conversions.ll
new file mode 100644
index 0000000000000..04d963841acf7
--- /dev/null
+++ b/llvm/test/Transforms/Mem2Reg/dbg_declare_to_value_conversions.ll
@@ -0,0 +1,54 @@
+; RUN: opt < %s -passes='mem2reg' -S | FileCheck %s
+target datalayout = "e-p:64:64"
+
+; An intrinsic without any expressions should always be converted.
+define i64 @foo0(i64 %arg) {
+ %arg.addr = alloca i64
+ store i64 %arg, ptr %arg.addr
+ call void @llvm.dbg.declare(metadata ptr %arg.addr, metadata !26, metadata !DIExpression()), !dbg !40
+ ; CHECK-LABEL: @foo0
+ ; CHECK-SAME: (i64 [[arg:%.*]])
+ ; CHECK-NEXT: dbg.value(metadata i64 [[arg]], {{.*}}, metadata !DIExpression())
+ %val = load i64, ptr %arg.addr
+ ret i64 %val
+}
+
+; An intrinsic with a single deref operator should be converted preserving the deref.
+define i32 @foo1(ptr %arg) {
+ %arg.indirect_addr = alloca ptr
+ store ptr %arg, ptr %arg.indirect_addr
+ call void @llvm.dbg.declare(metadata ptr %arg.indirect_addr, metadata !25, metadata !DIExpression(DW_OP_deref)), !dbg !40
+ ; CHECK-LABEL: @foo1
+ ; CHECK-SAME: (ptr [[arg:%.*]])
+ ; CHECK-NEXT: dbg.value(metadata ptr [[arg]], {{.*}}, metadata !DIExpression(DW_OP_deref))
+ %val = load i32, ptr %arg
+ ret i32 %val
+}
+
+
+; An intrinsic with multiple operators should cause us to conservatively bail.
+define i32 @foo2(ptr %arg) {
+ %arg.indirect_addr = alloca ptr
+ store ptr %arg, ptr %arg.indirect_addr
+ call void @llvm.dbg.declare(metadata ptr %arg.indirect_addr, metadata !25, metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst, 2)), !dbg !40
+ ; CHECK-LABEL: @foo2
+ ; CHECK-NEXT: dbg.value(metadata ptr undef, {{.*}}, metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst, 2))
+ %val = load i32, ptr %arg
+ ret i32 %val
+}
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+!llvm.module.flags = !{!1, !2}
+!llvm.dbg.cu = !{!7}
+
+!1 = !{i32 7, !"Dwarf Version", i32 4}
+!2 = !{i32 2, !"Debug Info Version", i32 3}
+!3 = !DIBasicType(name: "wide type", size: 512)
+!4 = !DIBasicType(name: "ptr sized type", size: 64)
+!7 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !8, producer: "clang", emissionKind: FullDebug)
+!8 = !DIFile(filename: "test.ll", directory: "")
+!10 = distinct !DISubprogram(name: "blah", linkageName: "blah", scope: !8, file: !8, line: 7, unit: !7)
+!25 = !DILocalVariable(name: "blah", arg: 1, scope: !10, file: !8, line: 7, type:!3)
+!26 = !DILocalVariable(name: "ptr sized var", arg: 1, scope: !10, file: !8, line: 7, type:!4)
+!40 = !DILocation(line: 7, column: 35, scope: !10)
More information about the llvm-commits
mailing list