[DebugInfo] Process single-location debug values in variadic form when producing DWARF

Revision c383f4d6550e enabled using variadic-form debug values to represent
single-location, non-stack-value debug values, and a further patch made all
DBG_INSTR_REFs use variadic form. Not all code paths were updated correctly to
handle the new syntax however, with entry values in still expecting an expression
that begins exactly DW_OP_LLVM_entry_value, 1.

A function already exists to select non-variadic-like expressions; this patch
adds an extra function to cheaply simplify such cases to non-variadic form, which
we use prior to any entry-value processing to put DBG_INSTR_REFs and DBG_VALUEs
down the same code path. We also use it for a few DIExpression functions that
check for whether the first element(s) of a DIExpression match a particular
pattern, so that they will return the same result for
DIExpression(DW_OP_LLVM_arg, 0, <ops>) as for DIExpression(<ops>).

Differential Revision: https://reviews.llvm.org/D158185

5 files changed, 168 insertions, 37 deletions

diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index 9beb514..b347664 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -2850,6 +2850,13 @@ public:
   /// DW_OP_LLVM_arg op as its first operand, or if it contains none.
   bool isSingleLocationExpression() const;
 
+  /// Returns a reference to the elements contained in this expression, skipping
+  /// past the leading `DW_OP_LLVM_arg, 0` if one is present.
+  /// Similar to `convertToNonVariadicExpression`, but faster and cheaper - it
+  /// does not check whether the expression is a single-location expression, and
+  /// it returns elements rather than creating a new DIExpression.
+  std::optional<ArrayRef<uint64_t>> getSingleLocationExpressionElements() const;
+
   /// Removes all elements from \p Expr that do not apply to an undef debug
   /// value, which includes every operator that computes the value/location on
   /// the DWARF stack, including any DW_OP_LLVM_arg elements (making the result
@@ -2868,6 +2875,9 @@ public:
   /// single debug operand at the start of the expression, then return that
   /// expression in a non-variadic form by removing DW_OP_LLVM_arg from the
   /// expression if it is present; otherwise returns std::nullopt.
+  /// See also `getSingleLocationExpressionElements` above, which skips
+  /// checking `isSingleLocationExpression` and returns a list of elements
+  /// rather than a DIExpression.
   static std::optional<const DIExpression *>
   convertToNonVariadicExpression(const DIExpression *Expr);
 
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 1cb65a8..05fb219c 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -234,7 +234,15 @@ const DIType *DbgVariable::getType() const {
 /// Get .debug_loc entry for the instruction range starting at MI.
 static DbgValueLoc getDebugLocValue(const MachineInstr *MI) {
   const DIExpression *Expr = MI->getDebugExpression();
-  const bool IsVariadic = MI->isDebugValueList();
+  auto SingleLocExprOpt = DIExpression::convertToNonVariadicExpression(Expr);
+  const bool IsVariadic = !SingleLocExprOpt;
+  // If we have a variadic debug value instruction that is equivalent to a
+  // non-variadic instruction, then convert it to non-variadic form here.
+  if (!IsVariadic && !MI->isNonListDebugValue()) {
+    assert(MI->getNumDebugOperands() == 1 &&
+           "Mismatched DIExpression and debug operands for debug instruction.");
+    Expr = *SingleLocExprOpt;
+  }
   assert(MI->getNumOperands() >= 3);
   SmallVector<DbgValueLocEntry, 4> DbgValueLocEntries;
   for (const MachineOperand &Op : MI->debug_operands()) {
diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp
index 4933b603..f7f3612 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -1345,13 +1345,23 @@ DIExpression *DIExpression::getImpl(LLVMContext &Context,
   DEFINE_GETIMPL_STORE_NO_OPS(DIExpression, (Elements));
 }
 bool DIExpression::isEntryValue() const {
-  return getNumElements() > 0 && getElement(0) == dwarf::DW_OP_LLVM_entry_value;
+  if (auto singleLocElts = getSingleLocationExpressionElements()) {
+    return singleLocElts->size() > 0 &&
+           (*singleLocElts)[0] == dwarf::DW_OP_LLVM_entry_value;
+  }
+  return false;
 }
 bool DIExpression::startsWithDeref() const {
-  return getNumElements() > 0 && getElement(0) == dwarf::DW_OP_deref;
+  if (auto singleLocElts = getSingleLocationExpressionElements())
+    return singleLocElts->size() > 0 &&
+           (*singleLocElts)[0] == dwarf::DW_OP_deref;
+  return false;
 }
 bool DIExpression::isDeref() const {
-  return getNumElements() == 1 && startsWithDeref();
+  if (auto singleLocElts = getSingleLocationExpressionElements())
+    return singleLocElts->size() == 1 &&
+           (*singleLocElts)[0] == dwarf::DW_OP_deref;
+  return false;
 }
 
 DIAssignID *DIAssignID::getImpl(LLVMContext &Context, StorageType Storage,
@@ -1528,14 +1538,34 @@ bool DIExpression::isSingleLocationExpression() const {
 
   auto ExprOpBegin = expr_ops().begin();
   auto ExprOpEnd = expr_ops().end();
-  if (ExprOpBegin->getOp() == dwarf::DW_OP_LLVM_arg)
+  if (ExprOpBegin->getOp() == dwarf::DW_OP_LLVM_arg) {
+    if (ExprOpBegin->getArg(0) != 0)
+      return false;
     ++ExprOpBegin;
+  }
 
   return !std::any_of(ExprOpBegin, ExprOpEnd, [](auto Op) {
     return Op.getOp() == dwarf::DW_OP_LLVM_arg;
   });
 }
 
+std::optional<ArrayRef<uint64_t>>
+DIExpression::getSingleLocationExpressionElements() const {
+  // Check for `isValid` covered by `isSingleLocationExpression`.
+  if (!isSingleLocationExpression())
+    return std::nullopt;
+
+  // An empty expression is already non-variadic.
+  if (!getNumElements())
+    return ArrayRef<uint64_t>();
+
+  // If Expr does not have a leading DW_OP_LLVM_arg then we don't need to do
+  // anything.
+  if (getElements()[0] == dwarf::DW_OP_LLVM_arg)
+    return getElements().drop_front(2);
+  return getElements();
+}
+
 const DIExpression *
 DIExpression::convertToUndefExpression(const DIExpression *Expr) {
   SmallVector<uint64_t, 3> UndefOps;
@@ -1561,23 +1591,13 @@ DIExpression::convertToVariadicExpression(const DIExpression *Expr) {
 
 std::optional<const DIExpression *>
 DIExpression::convertToNonVariadicExpression(const DIExpression *Expr) {
-  // Check for `isValid` covered by `isSingleLocationExpression`.
-  if (!Expr->isSingleLocationExpression())
+  if (!Expr)
     return std::nullopt;
 
-  // An empty expression is already non-variadic.
-  if (!Expr->getNumElements())
-    return Expr;
+  if (auto Elts = Expr->getSingleLocationExpressionElements())
+    return DIExpression::get(Expr->getContext(), *Elts);
 
-  auto ElementsBegin = Expr->elements_begin();
-  // If Expr does not have a leading DW_OP_LLVM_arg then we don't need to do
-  // anything.
-  if (*ElementsBegin != dwarf::DW_OP_LLVM_arg)
-    return Expr;
-
-  SmallVector<uint64_t> NonVariadicOps(
-      make_range(ElementsBegin + 2, Expr->elements_end()));
-  return DIExpression::get(Expr->getContext(), NonVariadicOps);
+  return std::nullopt;
 }
 
 void DIExpression::canonicalizeExpressionOps(SmallVectorImpl<uint64_t> &Ops,
@@ -1648,23 +1668,29 @@ void DIExpression::appendOffset(SmallVectorImpl<uint64_t> &Ops,
 }
 
 bool DIExpression::extractIfOffset(int64_t &Offset) const {
-  if (getNumElements() == 0) {
+  auto SingleLocEltsOpt = getSingleLocationExpressionElements();
+  if (!SingleLocEltsOpt)
+    return false;
+  auto SingleLocElts = *SingleLocEltsOpt;
+
+  if (SingleLocElts.size() == 0) {
     Offset = 0;
     return true;
   }
 
-  if (getNumElements() == 2 && Elements[0] == dwarf::DW_OP_plus_uconst) {
-    Offset = Elements[1];
+  if (SingleLocElts.size() == 2 &&
+      SingleLocElts[0] == dwarf::DW_OP_plus_uconst) {
+    Offset = SingleLocElts[1];
     return true;
   }
 
-  if (getNumElements() == 3 && Elements[0] == dwarf::DW_OP_constu) {
-    if (Elements[2] == dwarf::DW_OP_plus) {
-      Offset = Elements[1];
+  if (SingleLocElts.size() == 3 && SingleLocElts[0] == dwarf::DW_OP_constu) {
+    if (SingleLocElts[2] == dwarf::DW_OP_plus) {
+      Offset = SingleLocElts[1];
       return true;
     }
-    if (Elements[2] == dwarf::DW_OP_minus) {
-      Offset = -Elements[1];
+    if (SingleLocElts[2] == dwarf::DW_OP_minus) {
+      Offset = -SingleLocElts[1];
       return true;
     }
   }
@@ -1687,18 +1713,23 @@ const DIExpression *DIExpression::extractAddressClass(const DIExpression *Expr,
                                                       unsigned &AddrClass) {
   // FIXME: This seems fragile. Nothing that verifies that these elements
   // actually map to ops and not operands.
+  auto SingleLocEltsOpt = Expr->getSingleLocationExpressionElements();
+  if (!SingleLocEltsOpt)
+    return nullptr;
+  auto SingleLocElts = *SingleLocEltsOpt;
+
   const unsigned PatternSize = 4;
-  if (Expr->Elements.size() >= PatternSize &&
-      Expr->Elements[PatternSize - 4] == dwarf::DW_OP_constu &&
-      Expr->Elements[PatternSize - 2] == dwarf::DW_OP_swap &&
-      Expr->Elements[PatternSize - 1] == dwarf::DW_OP_xderef) {
-    AddrClass = Expr->Elements[PatternSize - 3];
+  if (SingleLocElts.size() >= PatternSize &&
+      SingleLocElts[PatternSize - 4] == dwarf::DW_OP_constu &&
+      SingleLocElts[PatternSize - 2] == dwarf::DW_OP_swap &&
+      SingleLocElts[PatternSize - 1] == dwarf::DW_OP_xderef) {
+    AddrClass = SingleLocElts[PatternSize - 3];
 
-    if (Expr->Elements.size() == PatternSize)
+    if (SingleLocElts.size() == PatternSize)
       return nullptr;
-    return DIExpression::get(Expr->getContext(),
-                             ArrayRef(&*Expr->Elements.begin(),
-                                      Expr->Elements.size() - PatternSize));
+    return DIExpression::get(
+        Expr->getContext(),
+        ArrayRef(&*SingleLocElts.begin(), SingleLocElts.size() - PatternSize));
   }
   return Expr;
 }
diff --git a/llvm/test/DebugInfo/X86/debug-value-list-entry-value.mir b/llvm/test/DebugInfo/X86/debug-value-list-entry-value.mir
new file mode 100644
index 0000000..06a72c6
--- /dev/null
+++ b/llvm/test/DebugInfo/X86/debug-value-list-entry-value.mir
@@ -0,0 +1,82 @@
+# RUN: llc %s --start-after=livedebugvalues --filetype=obj -o - \
+# RUN:     | llvm-dwarfdump - --name=test-var -o - | FileCheck %s
+
+# Test that when an entry value expression appears in a DBG_VALUE_LIST, we are
+# able to produce a valid entry value location in DWARF.
+
+# CHECK: DW_OP_entry_value(DW_OP_reg14 R14), DW_OP_plus_uconst 0x10, DW_OP_plus_uconst 0x10, DW_OP_deref
+
+--- |
+  source_filename = "test.ll"
+  target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+  target triple = "x86_64-apple-macosx12.1.0"
+
+  declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+  define swifttailcc void @blah(ptr swiftasync %0) !dbg !15 {
+    %use = getelementptr i8, ptr %0, i64 9
+    call void @llvm.dbg.value(metadata ptr %0, metadata !18, metadata !DIExpression(DW_OP_LLVM_entry_value, 1, DW_OP_plus_uconst, 16, DW_OP_plus_uconst, 16, DW_OP_deref, DW_OP_deref)), !dbg !22
+    %use1 = load i32, ptr null, align 4, !dbg !27
+    %use2 = sext i32 %use1 to i64
+    %use3 = getelementptr i8, ptr null, i64 %use2
+    store ptr %use3, ptr %0, align 8
+    ret void
+  }
+
+  !llvm.module.flags = !{!0}
+  !llvm.dbg.cu = !{!1}
+
+  !0 = !{i32 2, !"Debug Info Version", i32 3}
+  !1 = distinct !DICompileUnit(language: DW_LANG_Swift, file: !2, producer: "blah", isOptimized: true, flags: "blah", runtimeVersion: 5, emissionKind: FullDebug, globals: !3, imports: !9, sysroot: "blah", sdk: "blah")
+  !2 = !DIFile(filename: "blah", directory: "blah")
+  !3 = !{!4, !10}
+  !4 = !DIGlobalVariableExpression(var: !5, expr: !DIExpression())
+  !5 = distinct !DIGlobalVariable(name: "blah", linkageName: "blah", scope: !6, file: !2, line: 49, type: !7, isLocal: true, isDefinition: true)
+  !6 = !DIModule(scope: null, name: "blah", includePath: "blah")
+  !7 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !8)
+  !8 = !DICompositeType(tag: DW_TAG_structure_type, name: "blah", scope: !6, file: !2, size: 64, elements: !9, runtimeLang: DW_LANG_Swift, identifier: "blah")
+  !9 = !{}
+  !10 = !DIGlobalVariableExpression(var: !11, expr: !DIExpression())
+  !11 = distinct !DIGlobalVariable(name: "blah", linkageName: "blah", scope: !6, file: !2, line: 44, type: !12, isLocal: false, isDefinition: true)
+  !12 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !13)
+  !13 = !DICompositeType(tag: DW_TAG_structure_type, name: "blah", scope: !14, file: !2, size: 64, elements: !9, runtimeLang: DW_LANG_Swift, identifier: "blah")
+  !14 = !DIModule(scope: null, name: "blah", configMacros: "blah", includePath: "blah")
+  !15 = distinct !DISubprogram(name: "blah", linkageName: "blah", scope: !16, file: !2, line: 115, type: !17, scopeLine: 117, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !1, retainedNodes: !9, thrownTypes: !9)
+  !16 = !DICompositeType(tag: DW_TAG_structure_type, name: "blah", scope: !6, file: !2, elements: !9, runtimeLang: DW_LANG_Swift, identifier: "blah")
+  !17 = !DISubroutineType(types: !9)
+  !18 = !DILocalVariable(name: "test-var", arg: 1, scope: !19, file: !2, line: 95, type: !21, flags: DIFlagArtificial)
+  !19 = distinct !DISubprogram(name: "blah", linkageName: "blah", scope: !16, file: !2, line: 95, type: !20, scopeLine: 95, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !1, retainedNodes: !9)
+  !20 = distinct !DISubroutineType(types: !9)
+  !21 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !16)
+  !22 = !DILocation(line: 95, column: 9, scope: !19, inlinedAt: !23)
+  !23 = distinct !DILocation(line: 0, scope: !24, inlinedAt: !25)
+  !24 = distinct !DISubprogram(name: "blah", linkageName: "blah", scope: !16, file: !2, type: !20, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !1)
+  !25 = distinct !DILocation(line: 121, column: 36, scope: !26)
+  !26 = distinct !DILexicalBlock(scope: !15, file: !2, line: 116, column: 7)
+  !27 = !DILocation(line: 0, scope: !28, inlinedAt: !23)
+  !28 = !DILexicalBlockFile(scope: !19, file: !2, discriminator: 0)
+
+...
+---
+name:            blah
+alignment:       16
+tracksRegLiveness: true
+debugInstrRef:   true
+tracksDebugUserValues: true
+registers:       []
+liveins:
+  - { reg: '$r14', virtual-reg: '' }
+frameInfo:
+  maxAlignment:    1
+body:             |
+  bb.0 (%ir-block.1):
+    liveins: $r14
+
+    DBG_PHI $r14, 1
+    DBG_INSTR_REF !18, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_entry_value, 1, DW_OP_plus_uconst, 16, DW_OP_plus_uconst, 16, DW_OP_deref, DW_OP_deref), dbg-instr-ref(1, 0), debug-location !22
+    DBG_VALUE_LIST !18, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_entry_value, 1, DW_OP_plus_uconst, 16, DW_OP_plus_uconst, 16, DW_OP_deref, DW_OP_deref), $r14, debug-location !DILocation(line: 0, scope: !19, inlinedAt: !23)
+    renamable $rax = MOVSX64rm32 $noreg, 1, $noreg, 0, $noreg, debug-location !27 :: (load (s32) from `ptr null`)
+    MOV64mr killed renamable $r14, 1, $noreg, 0, $noreg, killed renamable $rax :: (store (s64) into %ir.0)
+    RETI64 8
+
+...
diff --git a/llvm/test/DebugInfo/X86/pr52584.ll b/llvm/test/DebugInfo/X86/pr52584.ll
index 5ceb0d82..a08a40a7 100644
--- a/llvm/test/DebugInfo/X86/pr52584.ll
+++ b/llvm/test/DebugInfo/X86/pr52584.ll
@@ -9,7 +9,7 @@
 ; CHECK:      DW_TAG
 define dso_local void @test() !dbg !4 {
 entry:
-  call void @llvm.dbg.value(metadata !DIArgList(i128 0), metadata !7, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_stack_value, DW_OP_LLVM_fragment, 0, 127)), !dbg !12
+  call void @llvm.dbg.value(metadata !DIArgList(i128 0), metadata !7, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 0, DW_OP_plus, DW_OP_stack_value, DW_OP_LLVM_fragment, 0, 127)), !dbg !12
   ret void, !dbg !12
 }