Skip to content

Commit

Permalink
[AsmPrinter][DebugInfo] Create EntryValue mode for DbgVariable
Browse files Browse the repository at this point in the history
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
  • Loading branch information
felipepiovezan committed Aug 23, 2023
1 parent 8841709 commit af6d43e
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 12 deletions.
17 changes: 17 additions & 0 deletions llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
15 changes: 5 additions & 10 deletions llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
66 changes: 65 additions & 1 deletion llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -128,6 +179,8 @@ class DbgVariable : public DbgEntity {
mutable SmallVector<FrameIndexExpr, 1>
FrameIndexExprs; /// Frame index + expression.

std::optional<DbgVariableEntryValue> EntryValue;

public:
/// Construct a DbgVariable.
///
Expand All @@ -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.
Expand All @@ -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);

Expand Down
1 change: 1 addition & 0 deletions llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,7 @@ void DwarfExpression::beginEntryValueExpression(

SavedLocationKind = LocationKind;
LocationKind = Register;
LocationFlags |= EntryValue;
IsEmittingEntryValue = true;
enableTemporaryBuffer();
}
Expand Down
16 changes: 15 additions & 1 deletion llvm/test/DebugInfo/AArch64/dbg-entry-value-swiftasync.mir
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
...
---
Expand All @@ -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
Expand Down

0 comments on commit af6d43e

Please sign in to comment.