[AsmPrinter][DebugInfo] Create EntryValue mode for DbgVariable

[fdeazeve]

Summary

With D149881, we converted EntryValue MachineFunction table entries into
DbgVariables initialized by a "DbgValue" intrinsic, which can only handle a
single, non-fragment DIExpression. However, it is desirable to handle variables
with multiple fragments and DIExpressions.

To do this, we expand the DbgVariable class to handle the EntryValue case.
This class can already operate under three different "modes" (stack slot,
unchanging location described by a dbg value, changing location described by a
loc list). A fourth case is added as a separate class entirely, but a subsequent
patch should redesign DbgVariable with four subclasses in order to make the
code more readable.

This patch also exposed a bug in the beginEntryValueExpression function, which
was not initializing the LocationFlags properly. Note how the
finalizeEntryValue function resets that flag. We fix this bug here, as testing
this changing in isolation would be tricky.

[fdeazeve]

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

877

FYI I recommend looking at DwarfDebug.h and DwarfDebug.cpp before looking at the other files.

[aprantl]

LGTM, some minor nitpicks inside.

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h

113

///

119

getFragmentOffsetInBits?

126

Why not a DenseSet<std::pair<MCRegister, DIExpression&>> or something that inherits from std::pair, then you don't need to define any sorting/hashing methods.

[fdeazeve]

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h

126

Just to register an offline discussion: this wouldn't quite work as we want to sort based on the fragment offset, not directly on the value of any particular pair member.

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

if (auto *DVal = DV.getValueLoc()) {
     if (!DwarfExpr.addExpression(
             std::move(Cursor),
             [&](unsigned Idx, DIExpressionCursor &Cursor) -> bool {
               return AddEntry(DVal->getLocEntries()[Idx], Cursor);
             }))
       return VariableDie;
 
     // Now attach the location information to the DIE.
     addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
     if (DwarfExpr.TagOffset)
       addUInt(*VariableDie, dwarf::DW_AT_LLVM_tag_offset, dwarf::DW_FORM_data1,
               *DwarfExpr.TagOffset);
 
     return VariableDie;
   }
 
+  if (const std::optional<DbgVariableEntryValue> &EntryValueVar =
+          DV.getEntryValue()) {
+    DIELoc *Loc = new (DIEValueAllocator) DIELoc;
+    DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
+    // Emit each expression as: EntryValue(Register) <other ops> <Fragment>.
+    for (auto [Register, Expr] : EntryValueVar->getEntryValuesInfo()) {
+      DwarfExpr.addFragmentOffset(&Expr);
+      DIExpressionCursor Cursor(Expr.getElements());
+      DwarfExpr.beginEntryValueExpression(Cursor);
+      DwarfExpr.addMachineRegExpression(
+          *Asm->MF->getSubtarget().getRegisterInfo(), Cursor, Register);
+      DwarfExpr.addExpression(std::move(Cursor));
+    }
+    addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
+    return VariableDie;
+  }
+
   // .. else use frame index.
   if (!DV.hasFrameIndexExprs())
     return VariableDie;
 
   std::optional<unsigned> NVPTXAddressSpace;
   DIELoc *Loc = new (DIEValueAllocator) DIELoc;
   DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
   for (const auto &Fragment : DV.getFrameIndexExprs()) {

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h

static bool classof(const DbgEntity *N) {
     case DbgVariableKind:
     case DbgLabelKind:
       return true;
     }
     llvm_unreachable("Invalid DbgEntityKind");
   }
 };
 
+/// Helper class to model a DbgVariable whose location is derived from an
+/// EntryValue.
+/// TODO: split the current implementation of `DbgVariable` into a class per
+/// variant of location that it can represent, and make `DbgVariableEntryValue`
+/// a subclass.
+class DbgVariableEntryValue {
+  struct EntryValueInfo {
+    MCRegister Reg;
+    const DIExpression &Expr;
+
+    // Operator enabling sorting based on fragment offset.
+    bool operator<(const EntryValueInfo &Other) const {
+      return getFragmentOffset() < Other.getFragmentOffset();
+    }
+
+  private:
+    uint64_t getFragmentOffset() const {
+      std::optional<DIExpression::FragmentInfo> Fragment =
+          Expr.getFragmentInfo();
+      return Fragment ? Fragment->OffsetInBits : 0;
+    }
+  };
+
+  std::set<EntryValueInfo> EntryValues;
+
+public:
+  DbgVariableEntryValue(MCRegister Reg, const DIExpression &Expr) {
+    addExpr(Reg, Expr);
+  };
+
+  // Add the pair Reg, Expr to the list of entry values describing the variable.
+  // If multiple expressions are added, it is the callers responsibility to
+  // ensure they are all non-overlapping fragments.
+  void addExpr(MCRegister Reg, const DIExpression &Expr) {
+    std::optional<const DIExpression *> NonVariadicExpr =
+        DIExpression::convertToNonVariadicExpression(&Expr);
+    assert(NonVariadicExpr && *NonVariadicExpr);
+
+    EntryValues.insert({Reg, **NonVariadicExpr});
+  }
+
+  /// Returns the set of EntryValueInfo.
+  const std::set<EntryValueInfo> &getEntryValuesInfo() const {
+    return EntryValues;
+  }
+};
+
 //===----------------------------------------------------------------------===//
 /// This class is used to track local variable information.
 ///
 /// Variables can be created from allocas, in which case they're generated from
 /// the MMI table.  Such variables can have multiple expressions and frame
 /// indices.
 ///
+/// Variables can be created from the entry value of registers, in which case
+/// they're generated from the MMI table. Such variables can have either a
+/// single expression or multiple *fragment* expressions.
+///
 /// Variables can be created from \c DBG_VALUE instructions.  Those whose
 /// location changes over time use \a DebugLocListIndex, while those with a
 /// single location use \a ValueLoc and (optionally) a single entry of \a Expr.
 ///
 /// Variables that have been optimized out use none of these fields.
 class DbgVariable : public DbgEntity {
   /// Index of the entry list in DebugLocs.
   unsigned DebugLocListIndex = ~0u;
   /// DW_OP_LLVM_tag_offset value from DebugLocs.
   std::optional<uint8_t> DebugLocListTagOffset;
 
   /// Single value location description.
   std::unique_ptr<DbgValueLoc> ValueLoc = nullptr;
 
   struct FrameIndexExpr {
     int FI;
     const DIExpression *Expr;
   };
   mutable SmallVector<FrameIndexExpr, 1>
       FrameIndexExprs; /// Frame index + expression.
 
+  std::optional<DbgVariableEntryValue> EntryValue;
+
 public:
   /// Construct a DbgVariable.
   ///
   /// Creates a variable without any DW_AT_location.  Call \a initializeMMI()
   /// for MMI entries, or \a initializeDbgValue() for DBG_VALUE instructions.
   DbgVariable(const DILocalVariable *V, const DILocation *IA)
       : DbgEntity(V, IA, DbgVariableKind) {}
 
   bool isInitialized() const {
-    return !FrameIndexExprs.empty() || ValueLoc;
+    return !FrameIndexExprs.empty() || ValueLoc || EntryValue;
   }
 
   /// Initialize from the MMI table.
   void initializeMMI(const DIExpression *E, int FI) {
     assert(!isInitialized() && "Already initialized?");
 
     assert((!E || E->isValid()) && "Expected valid expression");
     assert(FI != std::numeric_limits<int>::max() && "Expected valid index");
 
     FrameIndexExprs.push_back({FI, E});
   }
 
   // Initialize variable's location.
   void initializeDbgValue(DbgValueLoc Value) {
     assert(!isInitialized() && "Already initialized?");
     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});
   }
 
+  void initializeEntryValue(MCRegister Reg, const DIExpression &Expr) {
+    assert(!isInitialized() && "Already initialized?");
+    EntryValue = DbgVariableEntryValue(Reg, Expr);
+  }
+
+  const std::optional<DbgVariableEntryValue> &getEntryValue() const {
+    return EntryValue;
+  }
+
+  std::optional<DbgVariableEntryValue> &getEntryValue() { return EntryValue; }
+
   /// Initialize from a DBG_VALUE instruction.
   void initializeDbgValue(const MachineInstr *DbgValue);
 
   // Accessors.
   const DILocalVariable *getVariable() const {
     return cast<DILocalVariable>(getEntity());
   }
 

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

if (!Scope) {
       continue;
     }
 
     ensureAbstractEntityIsCreatedIfScoped(TheCU, Var.first, Scope->getScopeNode());
     auto RegVar = std::make_unique<DbgVariable>(
                     cast<DILocalVariable>(Var.first), Var.second);
     if (VI.inStackSlot())
       RegVar->initializeMMI(VI.Expr, VI.getStackSlot());
-    else {
-      MachineLocation MLoc(VI.getEntryValueRegister(), /*IsIndirect*/ false);
-      auto LocEntry = DbgValueLocEntry(MLoc);
-      RegVar->initializeDbgValue(DbgValueLoc(VI.Expr, LocEntry));
-    }
+    else
+      RegVar->initializeEntryValue(VI.getEntryValueRegister(), *VI.Expr);
     LLVM_DEBUG(dbgs() << "Created DbgVariable for " << VI.Var->getName()
                       << "\n");
 
     if (DbgVariable *DbgVar = MFVars.lookup(Var)) {
-      if (DbgVar->getValueLoc())
-        LLVM_DEBUG(dbgs() << "Dropping repeated entry value debug info for "
-                             "variable "
-                          << VI.Var->getName() << "\n");
-      else
+      if (DbgVar->hasFrameIndexExprs())
         DbgVar->addMMIEntry(*RegVar);
+      else
+        DbgVar->getEntryValue()->addExpr(VI.getEntryValueRegister(), *VI.Expr);
     } else if (InfoHolder.addScopeVariable(Scope, RegVar.get())) {
       MFVars.insert({Var, RegVar.get()});
       ConcreteEntities.push_back(std::move(RegVar));
     }
   }
 }
 
 /// Determine whether a *singular* DBG_VALUE is valid for the entirety of its

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

void DwarfExpression::beginEntryValueExpression(
   (void)Op;
   assert(Op && Op->getOp() == dwarf::DW_OP_LLVM_entry_value);
   assert(!IsEmittingEntryValue && "Already emitting entry value?");
   assert(Op->getArg(0) == 1 &&
          "Can currently only emit entry values covering a single operation");
 
   SavedLocationKind = LocationKind;
   LocationKind = Register;
+  LocationFlags |= EntryValue;
   IsEmittingEntryValue = true;
   enableTemporaryBuffer();
 }
 
 void DwarfExpression::finalizeEntryValue() {
   assert(IsEmittingEntryValue && "Entry value not open?");
   disableTemporaryBuffer();
 

llvm/test/DebugInfo/AArch64/dbg-entry-value-swiftasync.mir

 # RUN: llc -start-after=machineverifier -O0 %s -filetype=obj -o - | llvm-dwarfdump - | FileCheck %s
 
 # CHECK:      DW_TAG_variable
 # CHECK-NEXT:   DW_AT_location        (DW_OP_GNU_entry_value(DW_OP_reg22 W22))
 # CHECK-NEXT:   DW_AT_name    ("a")
+# CHECK:      DW_TAG_variable
+# CHECK-NEXT:   DW_AT_location
+# CHECK-SAME:     DW_OP_GNU_entry_value(DW_OP_reg22 W22), DW_OP_piece 0x8,
+# CHECK-SAME:     DW_OP_GNU_entry_value(DW_OP_reg22 W22), DW_OP_plus_uconst 0x2a, DW_OP_piece 0x8)
+# CHECK-NEXT:   DW_AT_name    ("fragmented_var")
 
 
 --- |
   target triple = "aarch64--"
   define void @foo(ptr %unused_arg, ptr swiftasync %async_arg) !dbg !4 {
     call void @llvm.dbg.declare(metadata ptr %async_arg, metadata !10, metadata !DIExpression(DW_OP_LLVM_entry_value, 1)), !dbg !12
+    ; A two fragment variable.
+    ; Fragments intentionally out of order to ensure the code can handle this.
+    call void @llvm.dbg.declare(metadata ptr %async_arg, metadata !10, metadata !DIExpression(DW_OP_LLVM_entry_value, 1, DW_OP_plus_uconst, 42, DW_OP_LLVM_fragment, 64, 64)), !dbg !12
+    call void @llvm.dbg.declare(metadata ptr %async_arg, metadata !11, metadata !DIExpression(DW_OP_LLVM_entry_value, 1, DW_OP_LLVM_fragment, 0, 64)), !dbg !12
     ret void, !dbg !12
   }
   declare void @llvm.dbg.declare(metadata, metadata, metadata)
   declare void @consume(ptr)
   !llvm.dbg.cu = !{!0}
   !llvm.module.flags = !{!2, !3}
   !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)
   !1 = !DIFile(filename: "x.c", directory: "/")
   !2 = !{i32 2, !"Dwarf Version", i32 4}
   !3 = !{i32 2, !"Debug Info Version", i32 3}
   !4 = distinct !DISubprogram(name: "f", scope: !1, file: !1, line: 1, type: !5, scopeLine: 1, spFlags: DISPFlagDefinition, unit: !0)
   !5 = !DISubroutineType(types: !6)
   !6 = !{null, !7, !7, !7}
-  !7 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64)
+  !7 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 128)
   !9 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
   !10 = !DILocalVariable(name: "a", scope: !4, file: !1, line: 1, type: !7)
+  !11 = !DILocalVariable(name: "fragmented_var", scope: !4, file: !1, line: 1, type: !7)
   !12 = !DILocation(line: 1, column: 37, scope: !4)
 ...
 ---
 name:            foo
 liveins:
   - { reg: '$x0', virtual-reg: '' }
   - { reg: '$x22', virtual-reg: '' }
 stack:
   - { id: 0, name: '', type: spill-slot, offset: -16, size: 8, alignment: 16,
       stack-id: default, callee-saved-register: '$lr', callee-saved-restored: true }
 entry_values:
   - { entry-value-register: '$x22', debug-info-variable: '!10', debug-info-expression: '!DIExpression(DW_OP_LLVM_entry_value, 1, DW_OP_deref)',
       debug-info-location: '!12' }
+  - { entry-value-register: '$x22', debug-info-variable: '!11', debug-info-expression: '!DIExpression(DW_OP_LLVM_entry_value, 1, DW_OP_plus_uconst, 42, DW_OP_deref, DW_OP_LLVM_fragment, 64, 64)',
+      debug-info-location: '!12' }
+  - { entry-value-register: '$x22', debug-info-variable: '!11', debug-info-expression: '!DIExpression(DW_OP_LLVM_entry_value, 1,  DW_OP_deref, DW_OP_LLVM_fragment, 0, 64)',
+      debug-info-location: '!12' }
 body:             |
   bb.0 (%ir-block.0):
     liveins: $x0, $x22, $lr
     RET undef $lr, debug-location !12
 ...