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

Fixes issues raised on: https://reviews.llvm.org/D133926

The patch above 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>)`.

Reviewed By: fdeazeve, Orlando

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

Author: felipepiovezan

llvm/include/llvm/IR/DebugInfoMetadata.h

@@ -2900,6 +2900,13 @@ class DIExpression : public MDNode {
   /// 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.
+  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
@@ -2918,6 +2925,9 @@ class DIExpression : public MDNode {
   /// 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);
 

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

@@ -795,7 +795,7 @@ DIE *DwarfCompileUnit::constructVariableDIEImpl(const DbgVariable &DV,
       if (Entry->isLocation()) {
         addVariableAddress(DV, *VariableDie, Entry->getLoc());
       } else if (Entry->isInt()) {
-        auto *Expr = DV.getSingleExpression();
+        auto *Expr = DVal->getExpression();
         if (Expr && Expr->getNumElements()) {
           DIELoc *Loc = new (DIEValueAllocator) DIELoc;
           DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
@@ -829,7 +829,7 @@ DIE *DwarfCompileUnit::constructVariableDIEImpl(const DbgVariable &DV,
           return Entry.isLocation() && !Entry.getLoc().getReg();
         }))
       return VariableDie;
-    const DIExpression *Expr = DV.getSingleExpression();
+    const DIExpression *Expr = DVal->getExpression();
     assert(Expr && "Variadic Debug Value must have an Expression.");
     DIELoc *Loc = new (DIEValueAllocator) DIELoc;
     DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

@@ -240,7 +240,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();
+  const bool IsVariadic = !Expr->isSingleLocationExpression();
+  // 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 = DIExpression::get(Expr->getContext(),
+                             Expr->getSingleLocationExpressionElements());
+  }
   assert(MI->getNumOperands() >= 3);
   SmallVector<DbgValueLocEntry, 4> DbgValueLocEntries;
   for (const MachineOperand &Op : MI->debug_operands()) {
@@ -272,9 +280,12 @@ void DbgVariable::initializeDbgValue(const MachineInstr *DbgValue) {
          "Wrong inlined-at");
 
   ValueLoc = std::make_unique<DbgValueLoc>(getDebugLocValue(DbgValue));
-  if (auto *E = DbgValue->getDebugExpression())
-    if (E->getNumElements())
-      FrameIndexExprs.push_back({0, E});
+  // Use the debug value's expression as a FrameIndexExpr iff it is suitable,
+  // which requires it to be non-variadic.
+  if (auto E = DIExpression::convertToNonVariadicExpression(
+          DbgValue->getDebugExpression()))
+    if ((*E)->getNumElements())
+      FrameIndexExprs.push_back({0, *E});
 }
 
 ArrayRef<DbgVariable::FrameIndexExpr> DbgVariable::getFrameIndexExprs() const {

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h

@@ -209,9 +209,12 @@ class DbgVariable : public DbgEntity {
     assert(!Value.getExpression()->isFragment() && "Fragments not supported.");
 
     ValueLoc = std::make_unique<DbgValueLoc>(Value);
-    if (auto *E = ValueLoc->getExpression())
-      if (E->getNumElements())
-        FrameIndexExprs.push_back({0, E});
+    // Use the DbgValueLoc's expression as a FrameIndexExpr iff it is suitable,
+    // which requires it to be non-variadic.
+    if (auto E = DIExpression::convertToNonVariadicExpression(
+            ValueLoc->getExpression()))
+      if ((*E)->getNumElements())
+        FrameIndexExprs.push_back({0, *E});
   }
 
   void initializeEntryValue(MCRegister Reg, const DIExpression &Expr) {

llvm/lib/IR/DebugInfoMetadata.cpp

@@ -1343,13 +1343,17 @@ 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;
+  auto singleLocElts = getSingleLocationExpressionElements();
+  return singleLocElts.size() > 0 &&
+         singleLocElts[0] == dwarf::DW_OP_LLVM_entry_value;
 }
 bool DIExpression::startsWithDeref() const {
-  return getNumElements() > 0 && getElement(0) == dwarf::DW_OP_deref;
+  auto singleLocElts = getSingleLocationExpressionElements();
+  return singleLocElts.size() > 0 && singleLocElts[0] == dwarf::DW_OP_deref;
 }
 bool DIExpression::isDeref() const {
-  return getNumElements() == 1 && startsWithDeref();
+  auto singleLocElts = getSingleLocationExpressionElements();
+  return singleLocElts.size() == 1 && singleLocElts[0] == dwarf::DW_OP_deref;
 }
 
 DIAssignID *DIAssignID::getImpl(LLVMContext &Context, StorageType Storage,
@@ -1526,14 +1530,24 @@ 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;
   });
 }
 
+ArrayRef<uint64_t> DIExpression::getSingleLocationExpressionElements() const {
+  if (getNumElements() < 2 || Elements[0] != dwarf::DW_OP_LLVM_arg ||
+      Elements[1] != 0)
+    return Elements;
+  return getElements().drop_front(2);
+}
+
 const DIExpression *
 DIExpression::convertToUndefExpression(const DIExpression *Expr) {
   SmallVector<uint64_t, 3> UndefOps;
@@ -1559,6 +1573,9 @@ DIExpression::convertToVariadicExpression(const DIExpression *Expr) {
 
 std::optional<const DIExpression *>
 DIExpression::convertToNonVariadicExpression(const DIExpression *Expr) {
+  if (!Expr)
+    return std::nullopt;
+
   // Check for `isValid` covered by `isSingleLocationExpression`.
   if (!Expr->isSingleLocationExpression())
     return std::nullopt;
@@ -1573,9 +1590,8 @@ DIExpression::convertToNonVariadicExpression(const DIExpression *Expr) {
   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 DIExpression::get(Expr->getContext(),
+                           Expr->getElements().drop_front(2));
 }
 
 void DIExpression::canonicalizeExpressionOps(SmallVectorImpl<uint64_t> &Ops,
@@ -1646,23 +1662,25 @@ void DIExpression::appendOffset(SmallVectorImpl<uint64_t> &Ops,
 }
 
 bool DIExpression::extractIfOffset(int64_t &Offset) const {
-  if (getNumElements() == 0) {
+  auto SingleLocElts = getSingleLocationExpressionElements();
+  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;
     }
   }
@@ -1685,18 +1703,19 @@ 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 SingleLocElts = Expr->getSingleLocationExpressionElements();
   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;
 }

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
+
+...

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
 }