Skip to content

Commit 06f7c4e

Browse files
felipepiovezanfelipepiovezan
authored
Merge pull request #7311 from felipepiovezan/felipe/safe_to_cherrypick
[rebranch] Cherry-pick debug info entry value fixes
2 parents 3782f0c + e28dac0 commit 06f7c4e

File tree

16 files changed

+237
-54
lines changed

16 files changed

+237
-54
lines changed

‎llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -888,6 +888,23 @@ DIE *DwarfCompileUnit::constructVariableDIEImpl(const DbgVariable &DV,
888888
return VariableDie;
889889
}
890890

891+
if (const std::optional<DbgVariableEntryValue> &EntryValueVar =
892+
DV.getEntryValue()) {
893+
DIELoc *Loc = new (DIEValueAllocator) DIELoc;
894+
DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
895+
// Emit each expression as: EntryValue(Register) <other ops> <Fragment>.
896+
for (auto [Register, Expr] : EntryValueVar->getEntryValuesInfo()) {
897+
DwarfExpr.addFragmentOffset(&Expr);
898+
DIExpressionCursor Cursor(Expr.getElements());
899+
DwarfExpr.beginEntryValueExpression(Cursor);
900+
DwarfExpr.addMachineRegExpression(
901+
*Asm->MF->getSubtarget().getRegisterInfo(), Cursor, Register);
902+
DwarfExpr.addExpression(std::move(Cursor));
903+
}
904+
addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
905+
return VariableDie;
906+
}
907+
891908
// .. else use frame index.
892909
if (!DV.hasFrameIndexExprs())
893910
return VariableDie;

‎llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1572,21 +1572,16 @@ void DwarfDebug::collectVariableInfoFromMFTable(
15721572
cast<DILocalVariable>(Var.first), Var.second);
15731573
if (VI.inStackSlot())
15741574
RegVar->initializeMMI(VI.Expr, VI.getStackSlot());
1575-
else {
1576-
MachineLocation MLoc(VI.getEntryValueRegister(), /*IsIndirect*/ true);
1577-
auto LocEntry = DbgValueLocEntry(MLoc);
1578-
RegVar->initializeDbgValue(DbgValueLoc(VI.Expr, LocEntry));
1579-
}
1575+
else
1576+
RegVar->initializeEntryValue(VI.getEntryValueRegister(), *VI.Expr);
15801577
LLVM_DEBUG(dbgs() << "Created DbgVariable for " << VI.Var->getName()
15811578
<< "\n");
15821579

15831580
if (DbgVariable *DbgVar = MFVars.lookup(Var)) {
1584-
if (DbgVar->getValueLoc())
1585-
LLVM_DEBUG(dbgs() << "Dropping repeated entry value debug info for "
1586-
"variable "
1587-
<< VI.Var->getName() << "\n");
1588-
else
1581+
if (DbgVar->hasFrameIndexExprs())
15891582
DbgVar->addMMIEntry(*RegVar);
1583+
else
1584+
DbgVar->getEntryValue()->addExpr(VI.getEntryValueRegister(), *VI.Expr);
15901585
} else if (InfoHolder.addScopeVariable(Scope, RegVar.get())) {
15911586
MFVars.insert({Var, RegVar.get()});
15921587
ConcreteEntities.push_back(std::move(RegVar));

‎llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h

Lines changed: 70 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,64 @@ class DbgEntity {
100100
}
101101
};
102102

103+
/// Helper class to model a DbgVariable whose location is derived from an
104+
/// EntryValue.
105+
/// TODO: split the current implementation of `DbgVariable` into a class per
106+
/// variant of location that it can represent, and make `DbgVariableEntryValue`
107+
/// a subclass.
108+
class DbgVariableEntryValue {
109+
struct EntryValueInfo {
110+
MCRegister Reg;
111+
const DIExpression &Expr;
112+
113+
/// Operator enabling sorting based on fragment offset.
114+
bool operator<(const EntryValueInfo &Other) const {
115+
return getFragmentOffsetInBits() < Other.getFragmentOffsetInBits();
116+
}
117+
118+
private:
119+
uint64_t getFragmentOffsetInBits() const {
120+
std::optional<DIExpression::FragmentInfo> Fragment =
121+
Expr.getFragmentInfo();
122+
return Fragment ? Fragment->OffsetInBits : 0;
123+
}
124+
};
125+
126+
std::set<EntryValueInfo> EntryValues;
127+
128+
public:
129+
DbgVariableEntryValue(MCRegister Reg, const DIExpression &Expr) {
130+
addExpr(Reg, Expr);
131+
};
132+
133+
// Add the pair Reg, Expr to the list of entry values describing the variable.
134+
// If multiple expressions are added, it is the callers responsibility to
135+
// ensure they are all non-overlapping fragments.
136+
void addExpr(MCRegister Reg, const DIExpression &Expr) {
137+
std::optional<const DIExpression *> NonVariadicExpr =
138+
DIExpression::convertToNonVariadicExpression(&Expr);
139+
assert(NonVariadicExpr && *NonVariadicExpr);
140+
141+
EntryValues.insert({Reg, **NonVariadicExpr});
142+
}
143+
144+
/// Returns the set of EntryValueInfo.
145+
const std::set<EntryValueInfo> &getEntryValuesInfo() const {
146+
return EntryValues;
147+
}
148+
};
149+
103150
//===----------------------------------------------------------------------===//
104151
/// This class is used to track local variable information.
105152
///
106153
/// Variables can be created from allocas, in which case they're generated from
107154
/// the MMI table. Such variables can have multiple expressions and frame
108155
/// indices.
109156
///
157+
/// Variables can be created from the entry value of registers, in which case
158+
/// they're generated from the MMI table. Such variables can have either a
159+
/// single expression or multiple *fragment* expressions.
160+
///
110161
/// Variables can be created from \c DBG_VALUE instructions. Those whose
111162
/// location changes over time use \a DebugLocListIndex, while those with a
112163
/// single location use \a ValueLoc and (optionally) a single entry of \a Expr.
@@ -128,6 +179,8 @@ class DbgVariable : public DbgEntity {
128179
mutable SmallVector<FrameIndexExpr, 1>
129180
FrameIndexExprs; /// Frame index + expression.
130181

182+
std::optional<DbgVariableEntryValue> EntryValue;
183+
131184
public:
132185
/// Construct a DbgVariable.
133186
///
@@ -136,10 +189,13 @@ class DbgVariable : public DbgEntity {
136189
DbgVariable(const DILocalVariable *V, const DILocation *IA)
137190
: DbgEntity(V, IA, DbgVariableKind) {}
138191

192+
bool isInitialized() const {
193+
return !FrameIndexExprs.empty() || ValueLoc || EntryValue;
194+
}
195+
139196
/// Initialize from the MMI table.
140197
void initializeMMI(const DIExpression *E, int FI) {
141-
assert(FrameIndexExprs.empty() && "Already initialized?");
142-
assert(!ValueLoc.get() && "Already initialized?");
198+
assert(!isInitialized() && "Already initialized?");
143199

144200
assert((!E || E->isValid()) && "Expected valid expression");
145201
assert(FI != std::numeric_limits<int>::max() && "Expected valid index");
@@ -149,8 +205,7 @@ class DbgVariable : public DbgEntity {
149205

150206
// Initialize variable's location.
151207
void initializeDbgValue(DbgValueLoc Value) {
152-
assert(FrameIndexExprs.empty() && "Already initialized?");
153-
assert(!ValueLoc && "Already initialized?");
208+
assert(!isInitialized() && "Already initialized?");
154209
assert(!Value.getExpression()->isFragment() && "Fragments not supported.");
155210

156211
ValueLoc = std::make_unique<DbgValueLoc>(Value);
@@ -159,6 +214,17 @@ class DbgVariable : public DbgEntity {
159214
FrameIndexExprs.push_back({0, E});
160215
}
161216

217+
void initializeEntryValue(MCRegister Reg, const DIExpression &Expr) {
218+
assert(!isInitialized() && "Already initialized?");
219+
EntryValue = DbgVariableEntryValue(Reg, Expr);
220+
}
221+
222+
const std::optional<DbgVariableEntryValue> &getEntryValue() const {
223+
return EntryValue;
224+
}
225+
226+
std::optional<DbgVariableEntryValue> &getEntryValue() { return EntryValue; }
227+
162228
/// Initialize from a DBG_VALUE instruction.
163229
void initializeDbgValue(const MachineInstr *DbgValue);
164230

‎llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,7 @@ void DwarfExpression::beginEntryValueExpression(
414414

415415
SavedLocationKind = LocationKind;
416416
LocationKind = Register;
417+
LocationFlags |= EntryValue;
417418
IsEmittingEntryValue = true;
418419
enableTemporaryBuffer();
419420
}

‎llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1939,6 +1939,8 @@ bool IRTranslator::translateIfEntryValueArgument(
19391939
if (!PhysReg)
19401940
return false;
19411941

1942+
// Append an op deref to account for the fact that this is a dbg_declare.
1943+
Expr = DIExpression::append(Expr, dwarf::DW_OP_deref);
19421944
MF->setVariableDbgInfo(DebugInst.getVariable(), Expr, *PhysReg,
19431945
DebugInst.getDebugLoc());
19441946
return true;

‎llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5845,26 +5845,6 @@ bool SelectionDAGBuilder::EmitFuncArgumentDbgValue(
58455845
if (!Op)
58465846
return false;
58475847

5848-
// If the expression refers to the entry value of an Argument, use the
5849-
// corresponding livein physical register. As per the Verifier, this is only
5850-
// allowed for swiftasync Arguments.
5851-
if (Op->isReg() && Expr->isEntryValue()) {
5852-
assert(Arg->hasAttribute(Attribute::AttrKind::SwiftAsync));
5853-
auto OpReg = Op->getReg();
5854-
for (auto [PhysReg, VirtReg] : FuncInfo.RegInfo->liveins())
5855-
if (OpReg == VirtReg || OpReg == PhysReg) {
5856-
SDDbgValue *SDV = DAG.getVRegDbgValue(
5857-
Variable, Expr, PhysReg,
5858-
Kind != FuncArgumentDbgValueKind::Value /*is indirect*/, DL,
5859-
SDNodeOrder);
5860-
DAG.AddDbgValue(SDV, false /*treat as dbg.declare byval parameter*/);
5861-
return true;
5862-
}
5863-
LLVM_DEBUG(dbgs() << "Dropping dbg.value: expression is entry_value but "
5864-
"couldn't find a physical register\n");
5865-
return true;
5866-
}
5867-
58685848
assert(Variable->isValidLocationForIntrinsic(DL) &&
58695849
"Expected inlined-at fields to agree");
58705850
MachineInstr *NewMI = nullptr;
@@ -5953,6 +5933,41 @@ static const CallBase *FindPreallocatedCall(const Value *PreallocatedSetup) {
59535933
llvm_unreachable("expected corresponding call to preallocated setup/arg");
59545934
}
59555935

5936+
/// If DI is a debug value with an EntryValue expression, lower it using the
5937+
/// corresponding physical register of the associated Argument value
5938+
/// (guaranteed to exist by the verifier).
5939+
bool SelectionDAGBuilder::visitEntryValueDbgValue(const DbgValueInst &DI) {
5940+
DILocalVariable *Variable = DI.getVariable();
5941+
DIExpression *Expr = DI.getExpression();
5942+
if (!Expr->isEntryValue() || !hasSingleElement(DI.getValues()))
5943+
return false;
5944+
5945+
// These properties are guaranteed by the verifier.
5946+
Argument *Arg = cast<Argument>(DI.getValue(0));
5947+
assert(Arg->hasAttribute(Attribute::AttrKind::SwiftAsync));
5948+
5949+
auto ArgIt = FuncInfo.ValueMap.find(Arg);
5950+
if (ArgIt == FuncInfo.ValueMap.end()) {
5951+
LLVM_DEBUG(
5952+
dbgs() << "Dropping dbg.value: expression is entry_value but "
5953+
"couldn't find an associated register for the Argument\n");
5954+
return true;
5955+
}
5956+
Register ArgVReg = ArgIt->getSecond();
5957+
5958+
for (auto [PhysReg, VirtReg] : FuncInfo.RegInfo->liveins())
5959+
if (ArgVReg == VirtReg || ArgVReg == PhysReg) {
5960+
SDDbgValue *SDV =
5961+
DAG.getVRegDbgValue(Variable, Expr, PhysReg, false /*IsIndidrect*/,
5962+
DI.getDebugLoc(), SDNodeOrder);
5963+
DAG.AddDbgValue(SDV, false /*treat as dbg.declare byval parameter*/);
5964+
return true;
5965+
}
5966+
LLVM_DEBUG(dbgs() << "Dropping dbg.value: expression is entry_value but "
5967+
"couldn't find a physical register\n");
5968+
return true;
5969+
}
5970+
59565971
/// Lower the call to the specified intrinsic function.
59575972
void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I,
59585973
unsigned Intrinsic) {
@@ -6284,6 +6299,9 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I,
62846299
DIExpression *Expression = DI.getExpression();
62856300
dropDanglingDebugInfo(Variable, Expression);
62866301

6302+
if (visitEntryValueDbgValue(DI))
6303+
return;
6304+
62876305
if (DI.isKillLocation()) {
62886306
handleKillDebugValue(Variable, Expression, DI.getDebugLoc(), SDNodeOrder);
62896307
return;

‎llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,8 @@ class SelectionDAGBuilder {
630630

631631
void visitInlineAsm(const CallBase &Call,
632632
const BasicBlock *EHPadBB = nullptr);
633+
634+
bool visitEntryValueDbgValue(const DbgValueInst &I);
633635
void visitIntrinsicCall(const CallInst &I, unsigned Intrinsic);
634636
void visitTargetIntrinsic(const CallInst &I, unsigned Intrinsic);
635637
void visitConstrainedFPIntrinsic(const ConstrainedFPIntrinsic &FPI);

‎llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1357,6 +1357,8 @@ static bool processIfEntryValueDbgDeclare(FunctionLoweringInfo &FuncInfo,
13571357
// Find the corresponding livein physical register to this argument.
13581358
for (auto [PhysReg, VirtReg] : FuncInfo.RegInfo->liveins())
13591359
if (VirtReg == ArgVReg) {
1360+
// Append an op deref to account for the fact that this is a dbg_declare.
1361+
Expr = DIExpression::append(Expr, dwarf::DW_OP_deref);
13601362
FuncInfo.MF->setVariableDbgInfo(Var, Expr, PhysReg, DbgLoc);
13611363
LLVM_DEBUG(dbgs() << "processDbgDeclare: setVariableDbgInfo Var=" << *Var
13621364
<< ", Expr=" << *Expr << ", MCRegister=" << PhysReg

‎llvm/lib/Transforms/Coroutines/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,5 @@ add_llvm_component_library(LLVMCoroutines
2121
Scalar
2222
Support
2323
TransformUtils
24+
TargetParser
2425
)

‎llvm/lib/Transforms/Coroutines/CoroFrame.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1879,7 +1879,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
18791879
// This dbg.declare is for the main function entry point. It
18801880
// will be deleted in all coro-split functions.
18811881
coro::salvageDebugInfo(ArgToAllocaMap, DDI, Shape.OptimizeFrame,
1882-
true /*IsEntryPoint*/);
1882+
false /*UseEntryValue*/);
18831883
}
18841884
}
18851885

@@ -2819,7 +2819,7 @@ static void collectFrameAlloca(AllocaInst *AI, coro::Shape &Shape,
28192819

28202820
void coro::salvageDebugInfo(
28212821
SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
2822-
DbgVariableIntrinsic *DVI, bool OptimizeFrame, bool IsEntryPoint) {
2822+
DbgVariableIntrinsic *DVI, bool OptimizeFrame, bool UseEntryValue) {
28232823
Function *F = DVI->getFunction();
28242824
IRBuilder<> Builder(F->getContext());
28252825
auto InsertPt = F->getEntryBlock().getFirstInsertionPt();
@@ -2878,7 +2878,7 @@ void coro::salvageDebugInfo(
28782878
// For the EntryPoint funclet, don't use EntryValues. This funclet can be
28792879
// inlined, which would remove the guarantee that this intrinsic targets an
28802880
// Argument.
2881-
if (IsSwiftAsyncArg && !IsEntryPoint && !Expr->isEntryValue())
2881+
if (IsSwiftAsyncArg && UseEntryValue && !Expr->isEntryValue())
28822882
Expr = DIExpression::prepend(Expr, DIExpression::EntryValue);
28832883

28842884
// If the coroutine frame is an Argument, store it in an alloca to improve

‎llvm/lib/Transforms/Coroutines/CoroSplit.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -701,9 +701,13 @@ void CoroCloner::salvageDebugInfo() {
701701
SmallVector<DbgVariableIntrinsic *, 8> Worklist =
702702
collectDbgVariableIntrinsics(*NewF);
703703
SmallDenseMap<Argument *, AllocaInst *, 4> ArgToAllocaMap;
704+
705+
// Only 64-bit ABIs have a register we can refer to with the entry value.
706+
bool UseEntryValue =
707+
llvm::Triple(OrigF.getParent()->getTargetTriple()).isArch64Bit();
704708
for (DbgVariableIntrinsic *DVI : Worklist)
705709
coro::salvageDebugInfo(ArgToAllocaMap, DVI, Shape.OptimizeFrame,
706-
false /*IsEntryPoint*/);
710+
UseEntryValue);
707711

708712
// Remove all salvaged dbg.declare intrinsics that became
709713
// either unreachable or stale due to the CoroSplit transformation.
@@ -1989,7 +1993,7 @@ splitCoroutine(Function &F, SmallVectorImpl<Function *> &Clones,
19891993
SmallDenseMap<Argument *, AllocaInst *, 4> ArgToAllocaMap;
19901994
for (auto *DDI : collectDbgVariableIntrinsics(F))
19911995
coro::salvageDebugInfo(ArgToAllocaMap, DDI, Shape.OptimizeFrame,
1992-
true /*IsEntryPoint*/);
1996+
false /*UseEntryValue*/);
19931997

19941998
return Shape;
19951999
}

‎llvm/test/CodeGen/AArch64/dbg-declare-swift-async.ll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@
33
; RUN: llc -O0 -fast-isel=false -global-isel=false -stop-after=finalize-isel %s -o - | FileCheck %s
44

55
; CHECK: void @foo
6-
; CHECK-NEXT: dbg.declare(metadata {{.*}}, metadata ![[VAR:.*]], metadata ![[EXPR:.*]]), !dbg ![[LOC:.*]]
6+
; CHECK-NEXT: dbg.declare(metadata {{.*}}, metadata ![[VAR:.*]], metadata !DIExpression([[EXPR:.*]])), !dbg ![[LOC:.*]]
77
; CHECK: entry_values:
8-
; CHECK-NEXT: entry-value-register: '$x22', debug-info-variable: '![[VAR]]', debug-info-expression: '![[EXPR]]',
8+
; CHECK-NEXT: entry-value-register: '$x22', debug-info-variable: '![[VAR]]', debug-info-expression: '!DIExpression([[EXPR]], DW_OP_deref)',
99
; CHECK-NEXT: debug-info-location: '![[LOC]]
10-
; CHECK-NEXT: entry-value-register: '$x22', debug-info-variable: '![[VAR]]', debug-info-expression: '![[EXPR]]'
10+
; CHECK-NEXT: entry-value-register: '$x22', debug-info-variable: '![[VAR]]', debug-info-expression: '!DIExpression([[EXPR]], DW_OP_deref)'
1111
; CHECK-NEXT: debug-info-location: '![[LOC]]
1212

1313
; CHECK-NOT: DBG_VALUE

0 commit comments

Comments
 (0)