[AsmPrinter][DebugInfo] Create EntryValue mode for DbgVariable

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.

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

Author: felipepiovezan

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

@@ -882,6 +882,23 @@ DIE *DwarfCompileUnit::constructVariableDIEImpl(const DbgVariable &DV,
     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;

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

@@ -1564,21 +1564,16 @@ void DwarfDebug::collectVariableInfoFromMFTable(
                     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));

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h

@@ -100,13 +100,64 @@ class DbgEntity {
   }
 };
 
+/// 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 getFragmentOffsetInBits() < Other.getFragmentOffsetInBits();
+    }
+
+  private:
+    uint64_t getFragmentOffsetInBits() 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.
@@ -128,6 +179,8 @@ class DbgVariable : public DbgEntity {
   mutable SmallVector<FrameIndexExpr, 1>
       FrameIndexExprs; /// Frame index + expression.
 
+  std::optional<DbgVariableEntryValue> EntryValue;
+
 public:
   /// Construct a DbgVariable.
   ///
@@ -137,7 +190,7 @@ class DbgVariable : public DbgEntity {
       : DbgEntity(V, IA, DbgVariableKind) {}
 
   bool isInitialized() const {
-    return !FrameIndexExprs.empty() || ValueLoc;
+    return !FrameIndexExprs.empty() || ValueLoc || EntryValue;
   }
 
   /// Initialize from the MMI table.
@@ -161,6 +214,17 @@ class DbgVariable : public DbgEntity {
         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);
 

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

@@ -414,6 +414,7 @@ void DwarfExpression::beginEntryValueExpression(
 
   SavedLocationKind = LocationKind;
   LocationKind = Register;
+  LocationFlags |= EntryValue;
   IsEmittingEntryValue = true;
   enableTemporaryBuffer();
 }

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

@@ -3,12 +3,21 @@
 # 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)
@@ -22,9 +31,10 @@
   !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)
 ...
 ---
@@ -38,6 +48,10 @@ stack:
 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