[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

diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index def3ce2..d83a53f 100644 (file)
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -2769,6 +2769,9 @@ public:
   /// 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 fb9a7b8..ad9540e 100644 (file)
--- 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 633f0bb..b6c6c5a 100644 (file)
--- 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 (file)
index 0000000..04d9638
--- /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)