Skip to content

Commit d58046d

Browse files
epilkepilk
committed
[AMDGPU] Switch LiveDebugValues implementation to instruction referencing
Squashed commit of the following: [Verifier] Relax DIOp type checking for poison values [DebugInfo] Enable instruction referencing for AMDGCN [AMDGPU][DebugInfo] Create DBG_INSTR_REF substitutions when shrinking instructions [AMDGPU][LoadStoreOpt] maintain instruction ref debug info [LiveDebugValues] Support DIOp expressions in InstrRefBasedImpl Change-Id: I69e720d87f8090952c6196e0b3c57861d914be09
1 parent 8e65c42 commit d58046d

17 files changed

+118
-59
lines changed

‎llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -684,7 +684,7 @@ class TransferTracker {
684684
Register Reg = MTracker->LocIdxToLocID[Num.getLoc()];
685685
MachineOperand MO = MachineOperand::CreateReg(Reg, false);
686686
PendingDbgValues.push_back(std::make_pair(
687-
VarID, &*emitMOLoc(MO, Var, {NewExpr, Prop.Indirect, false})));
687+
VarID, &*emitMOLoc(MO, Var, {NewExpr, Prop.Indirect, false, 1})));
688688
return true;
689689
}
690690

@@ -1651,7 +1651,7 @@ bool InstrRefBasedLDV::transferDebugInstrRef(MachineInstr &MI,
16511651
// tracker about it. The rest of this LiveDebugValues implementation acts
16521652
// exactly the same for DBG_INSTR_REFs as DBG_VALUEs (just, the former can
16531653
// refer to values that aren't immediately available).
1654-
DbgValueProperties Properties(Expr, false, true);
1654+
DbgValueProperties Properties(Expr, false, true, MI.getNumDebugOperands());
16551655
if (VTracker)
16561656
VTracker->defVar(MI, Properties, DbgOpIDs);
16571657

@@ -1736,8 +1736,9 @@ bool InstrRefBasedLDV::transferDebugInstrRef(MachineInstr &MI,
17361736
}
17371737
if (IsValidUseBeforeDef) {
17381738
DebugVariableID VID = DVMap.insertDVID(V, MI.getDebugLoc().get());
1739-
TTracker->addUseBeforeDef(VID, {MI.getDebugExpression(), false, true},
1740-
DbgOps, LastUseBeforeDef);
1739+
TTracker->addUseBeforeDef(
1740+
VID, {MI.getDebugExpression(), false, true, MI.getNumDebugOperands()},
1741+
DbgOps, LastUseBeforeDef);
17411742
}
17421743
}
17431744

@@ -3176,7 +3177,7 @@ void InstrRefBasedLDV::buildVLocValueMap(
31763177

31773178
// Initialize all values to start as NoVals. This signifies "it's live
31783179
// through, but we don't know what it is".
3179-
DbgValueProperties EmptyProperties(EmptyExpr, false, false);
3180+
DbgValueProperties EmptyProperties(EmptyExpr, false, false, 1);
31803181
for (unsigned int I = 0; I < NumBlocks; ++I) {
31813182
DbgValue EmptyDbgValue(I, EmptyProperties, DbgValue::NoVal);
31823183
LiveIns.push_back(EmptyDbgValue);

‎llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -312,8 +312,10 @@ class SpillLocationNo {
312312
/// the value, and Boolean of whether or not it's indirect.
313313
class DbgValueProperties {
314314
public:
315-
DbgValueProperties(const DIExpression *DIExpr, bool Indirect, bool IsVariadic)
316-
: DIExpr(DIExpr), Indirect(Indirect), IsVariadic(IsVariadic) {}
315+
DbgValueProperties(const DIExpression *DIExpr, bool Indirect, bool IsVariadic,
316+
unsigned NumLocOps)
317+
: DIExpr(DIExpr), Indirect(Indirect), IsVariadic(IsVariadic),
318+
NumLocOps(NumLocOps) {}
317319

318320
/// Extract properties from an existing DBG_VALUE instruction.
319321
DbgValueProperties(const MachineInstr &MI) {
@@ -324,6 +326,7 @@ class DbgValueProperties {
324326
IsVariadic = MI.isDebugValueList();
325327
DIExpr = MI.getDebugExpression();
326328
Indirect = MI.isDebugOffsetImm();
329+
NumLocOps = MI.getNumDebugOperands();
327330
}
328331

329332
bool isJoinable(const DbgValueProperties &Other) const {
@@ -332,21 +335,20 @@ class DbgValueProperties {
332335
}
333336

334337
bool operator==(const DbgValueProperties &Other) const {
335-
return std::tie(DIExpr, Indirect, IsVariadic) ==
336-
std::tie(Other.DIExpr, Other.Indirect, Other.IsVariadic);
338+
return std::tie(DIExpr, Indirect, IsVariadic, NumLocOps) ==
339+
std::tie(Other.DIExpr, Other.Indirect, Other.IsVariadic, NumLocOps);
337340
}
338341

339342
bool operator!=(const DbgValueProperties &Other) const {
340343
return !(*this == Other);
341344
}
342345

343-
unsigned getLocationOpCount() const {
344-
return IsVariadic ? DIExpr->getNumLocationOperands() : 1;
345-
}
346+
unsigned getLocationOpCount() const { return NumLocOps; }
346347

347348
const DIExpression *DIExpr;
348349
bool Indirect;
349350
bool IsVariadic;
351+
unsigned NumLocOps;
350352
};
351353

352354
/// TODO: Might pack better if we changed this to a Struct of Arrays, since
@@ -1057,7 +1059,7 @@ class VLocTracker {
10571059
VLocTracker(DebugVariableMap &DVMap, const OverlapMap &O,
10581060
const DIExpression *EmptyExpr)
10591061
: DVMap(DVMap), OverlappingFragments(O),
1060-
EmptyProperties(EmptyExpr, false, false) {}
1062+
EmptyProperties(EmptyExpr, false, false, 1) {}
10611063

10621064
void defVar(const MachineInstr &MI, const DbgValueProperties &Properties,
10631065
const SmallVectorImpl<DbgOpID> &DebugOps) {

‎llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,8 @@ bool LiveDebugValues::runOnMachineFunction(MachineFunction &MF) {
131131

132132
bool llvm::debuginfoShouldUseDebugInstrRef(const Triple &T) {
133133
// Enable by default on x86_64, disable if explicitly turned off on cmdline.
134-
if (T.getArch() == llvm::Triple::x86_64 &&
134+
if ((T.getArch() == llvm::Triple::x86_64 ||
135+
T.getArch() == llvm::Triple::amdgcn) &&
135136
ValueTrackingVariableLocations != cl::boolOrDefault::BOU_FALSE)
136137
return true;
137138

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6042,8 +6042,10 @@ bool SelectionDAGBuilder::EmitFuncArgumentDbgValue(
60426042
// the DIExpression.
60436043
if (Indirect)
60446044
NewDIExpr = DIExpression::prepend(FragExpr, DIExpression::DerefBefore);
6045-
SmallVector<uint64_t, 2> Ops({dwarf::DW_OP_LLVM_arg, 0});
6046-
NewDIExpr = DIExpression::prependOpcodes(NewDIExpr, Ops);
6045+
if (NewDIExpr->holdsOldElements()) {
6046+
SmallVector<uint64_t, 2> Ops({dwarf::DW_OP_LLVM_arg, 0});
6047+
NewDIExpr = DIExpression::prependOpcodes(NewDIExpr, Ops);
6048+
}
60476049
return BuildMI(MF, DL, Inst, false, MOs, Variable, NewDIExpr);
60486050
} else {
60496051
// Create a completely standard DBG_VALUE.

‎llvm/lib/IR/DebugInfoMetadata.cpp‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1511,17 +1511,21 @@ class DIExprVerifier : public DIExprConstVisitor<DIExprVerifier> {
15111511
return true;
15121512
if (Env->Arguments.empty())
15131513
return error("DIOpReferrer requires an argument");
1514-
return expectSameSize(
1515-
ResultType, Env->Arguments[0]->getType(),
1516-
"DIOpReferrer type must be same size in bits as argument");
1514+
const Value *V = Env->Arguments[0];
1515+
return isa<PoisonValue>(V) ||
1516+
expectSameSize(
1517+
ResultType, V->getType(),
1518+
"DIOpReferrer type must be same size in bits as argument");
15171519
}
15181520

15191521
bool visit(DIOp::Arg Op, Type *ResultType, ArrayRef<StackEntry>) {
15201522
if (!Env)
15211523
return true;
15221524
if (Op.getIndex() >= Env->Arguments.size())
15231525
return error("DIOpArg index out of range");
1524-
return expectSameSize(ResultType, Env->Arguments[Op.getIndex()]->getType(),
1526+
const Value *V = Env->Arguments[Op.getIndex()];
1527+
return isa<PoisonValue>(V) ||
1528+
expectSameSize(ResultType, V->getType(),
15251529
"DIOpArg type must be same size in bits as argument");
15261530
}
15271531

@@ -1833,6 +1837,9 @@ DIExpression::convertToUndefExpression(const DIExpression *Expr) {
18331837

18341838
const DIExpression *
18351839
DIExpression::convertToVariadicExpression(const DIExpression *Expr) {
1840+
if (Expr->holdsNewElements())
1841+
return Expr;
1842+
18361843
if (any_of(Expr->expr_ops(), [](auto ExprOp) {
18371844
return ExprOp.getOp() == dwarf::DW_OP_LLVM_arg;
18381845
}))
@@ -1846,10 +1853,10 @@ DIExpression::convertToVariadicExpression(const DIExpression *Expr) {
18461853

18471854
std::optional<const DIExpression *>
18481855
DIExpression::convertToNonVariadicExpression(const DIExpression *Expr) {
1849-
if (Expr->holdsNewElements())
1856+
if (!Expr)
18501857
return std::nullopt;
18511858

1852-
if (!Expr)
1859+
if (Expr->holdsNewElements())
18531860
return std::nullopt;
18541861

18551862
if (auto Elts = Expr->getSingleLocationExpressionElements())
@@ -1892,6 +1899,11 @@ bool DIExpression::isEqualExpression(const DIExpression *FirstExpr,
18921899
bool FirstIndirect,
18931900
const DIExpression *SecondExpr,
18941901
bool SecondIndirect) {
1902+
if (FirstExpr->holdsNewElements() != SecondExpr->holdsNewElements())
1903+
return false;
1904+
if (FirstExpr->holdsNewElements())
1905+
return FirstIndirect == SecondIndirect && FirstExpr == SecondExpr;
1906+
18951907
SmallVector<uint64_t> FirstOps;
18961908
DIExpression::canonicalizeExpressionOps(FirstOps, FirstExpr, FirstIndirect);
18971909
SmallVector<uint64_t> SecondOps;
@@ -2043,6 +2055,8 @@ bool DIExpression::hasAllLocationOps(unsigned N) const {
20432055
for (auto ExprOp : expr_ops())
20442056
if (ExprOp.getOp() == dwarf::DW_OP_LLVM_arg)
20452057
SeenOps.insert(ExprOp.getArg(0));
2058+
else if (ExprOp.getOp() == dwarf::DW_OP_LLVM_poisoned)
2059+
return true;
20462060
for (uint64_t Idx = 0; Idx < N; ++Idx)
20472061
if (!SeenOps.contains(Idx))
20482062
return false;

‎llvm/lib/Target/AMDGPU/SIInstrInfo.cpp‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4544,6 +4544,7 @@ static void copyFlagsToImplicitVCC(MachineInstr &MI,
45444544
MachineInstr *SIInstrInfo::buildShrunkInst(MachineInstr &MI,
45454545
unsigned Op32) const {
45464546
MachineBasicBlock *MBB = MI.getParent();
4547+
MachineFunction *MF = MBB->getParent();
45474548

45484549
const MCInstrDesc &Op32Desc = get(Op32);
45494550
MachineInstrBuilder Inst32 =
@@ -4555,9 +4556,16 @@ MachineInstr *SIInstrInfo::buildShrunkInst(MachineInstr &MI,
45554556

45564557
// We assume the defs of the shrunk opcode are in the same order, and the
45574558
// shrunk opcode loses the last def (SGPR def, in the VOP3->VOPC case).
4558-
for (int I = 0, E = Op32Desc.getNumDefs(); I != E; ++I)
4559+
for (int I = 0, E = Op32Desc.getNumDefs(); I != E; ++I) {
45594560
Inst32.add(MI.getOperand(I));
45604561

4562+
// If this def is used by a DBG_INSTR_REF, create a substitution for the new
4563+
// instruction.
4564+
if (unsigned DINum = MI.peekDebugInstrNum())
4565+
MF->makeDebugValueSubstitution({DINum, I},
4566+
{Inst32->getDebugInstrNum(), I});
4567+
}
4568+
45614569
const MachineOperand *Src2 = getNamedOperand(MI, AMDGPU::OpName::src2);
45624570

45634571
int Idx = MI.getNumExplicitDefs();

‎llvm/lib/Target/AMDGPU/SILoadStoreOptimizer.cpp‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ class SILoadStoreOptimizer {
229229

230230
void copyToDestRegs(CombineInfo &CI, CombineInfo &Paired,
231231
MachineBasicBlock::iterator InsertBefore, int OpName,
232-
Register DestReg) const;
232+
Register DestReg, MachineInstr *NewMI) const;
233233
Register copyFromSrcRegs(CombineInfo &CI, CombineInfo &Paired,
234234
MachineBasicBlock::iterator InsertBefore,
235235
int OpName) const;
@@ -1256,9 +1256,10 @@ SILoadStoreOptimizer::checkAndPrepareMerge(CombineInfo &CI,
12561256
// Paired.
12571257
void SILoadStoreOptimizer::copyToDestRegs(
12581258
CombineInfo &CI, CombineInfo &Paired,
1259-
MachineBasicBlock::iterator InsertBefore, int OpName,
1260-
Register DestReg) const {
1259+
MachineBasicBlock::iterator InsertBefore, int OpName, Register DestReg,
1260+
MachineInstr *NewMI) const {
12611261
MachineBasicBlock *MBB = CI.I->getParent();
1262+
MachineFunction *MF = MBB->getParent();
12621263
DebugLoc DL = CI.I->getDebugLoc();
12631264

12641265
auto [SubRegIdx0, SubRegIdx1] = getSubRegIdxs(CI, Paired);
@@ -1280,6 +1281,17 @@ void SILoadStoreOptimizer::copyToDestRegs(
12801281
BuildMI(*MBB, InsertBefore, DL, CopyDesc)
12811282
.add(*Dest1)
12821283
.addReg(DestReg, RegState::Kill, SubRegIdx1);
1284+
1285+
if (unsigned DINum = CI.I->peekDebugInstrNum()) {
1286+
unsigned NewDINum = NewMI->getDebugInstrNum();
1287+
MF->makeDebugValueSubstitution(std::make_pair(DINum, 0),
1288+
std::make_pair(NewDINum, 0), SubRegIdx0);
1289+
}
1290+
if (unsigned DINum = Paired.I->peekDebugInstrNum()) {
1291+
unsigned NewDINum = NewMI->getDebugInstrNum();
1292+
MF->makeDebugValueSubstitution(std::make_pair(DINum, 0),
1293+
std::make_pair(NewDINum, 0), SubRegIdx1);
1294+
}
12831295
}
12841296

12851297
// Return a register for the source of the merged store after copying the
@@ -1373,7 +1385,8 @@ SILoadStoreOptimizer::mergeRead2Pair(CombineInfo &CI, CombineInfo &Paired,
13731385
.addImm(0) // gds
13741386
.cloneMergedMemRefs({&*CI.I, &*Paired.I});
13751387

1376-
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdst, DestReg);
1388+
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdst, DestReg,
1389+
Read2);
13771390

13781391
CI.I->eraseFromParent();
13791392
Paired.I->eraseFromParent();
@@ -1493,7 +1506,7 @@ SILoadStoreOptimizer::mergeImagePair(CombineInfo &CI, CombineInfo &Paired,
14931506

14941507
MachineInstr *New = MIB.addMemOperand(combineKnownAdjacentMMOs(CI, Paired));
14951508

1496-
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdata, DestReg);
1509+
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdata, DestReg, New);
14971510

14981511
CI.I->eraseFromParent();
14991512
Paired.I->eraseFromParent();
@@ -1525,7 +1538,7 @@ MachineBasicBlock::iterator SILoadStoreOptimizer::mergeSMemLoadImmPair(
15251538
New.addImm(MergedOffset);
15261539
New.addImm(CI.CPol).addMemOperand(combineKnownAdjacentMMOs(CI, Paired));
15271540

1528-
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::sdst, DestReg);
1541+
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::sdst, DestReg, New);
15291542

15301543
CI.I->eraseFromParent();
15311544
Paired.I->eraseFromParent();
@@ -1566,7 +1579,7 @@ MachineBasicBlock::iterator SILoadStoreOptimizer::mergeBufferLoadPair(
15661579
.addImm(0) // swz
15671580
.addMemOperand(combineKnownAdjacentMMOs(CI, Paired));
15681581

1569-
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdata, DestReg);
1582+
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdata, DestReg, New);
15701583

15711584
CI.I->eraseFromParent();
15721585
Paired.I->eraseFromParent();
@@ -1611,7 +1624,7 @@ MachineBasicBlock::iterator SILoadStoreOptimizer::mergeTBufferLoadPair(
16111624
.addImm(0) // swz
16121625
.addMemOperand(combineKnownAdjacentMMOs(CI, Paired));
16131626

1614-
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdata, DestReg);
1627+
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdata, DestReg, New);
16151628

16161629
CI.I->eraseFromParent();
16171630
Paired.I->eraseFromParent();
@@ -1681,7 +1694,7 @@ MachineBasicBlock::iterator SILoadStoreOptimizer::mergeFlatLoadPair(
16811694
.addImm(CI.CPol)
16821695
.addMemOperand(combineKnownAdjacentMMOs(CI, Paired));
16831696

1684-
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdst, DestReg);
1697+
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdst, DestReg, New);
16851698

16861699
CI.I->eraseFromParent();
16871700
Paired.I->eraseFromParent();

‎llvm/test/CodeGen/AMDGPU/debug-value.ll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
; RUN: llc -mtriple=amdgcn-amd-amdhsa -verify-machineinstrs -amdgpu-codegenprepare-break-large-phis=0 < %s | FileCheck %s
1+
; RUN: llc -mtriple=amdgcn-amd-amdhsa -verify-machineinstrs -experimental-debug-variable-locations=false -amdgpu-codegenprepare-break-large-phis=0 < %s | FileCheck %s
22

33
%struct.wombat = type { [4 x i32], [4 x i32], [4 x i32] }
44

‎llvm/test/CodeGen/AMDGPU/debug-value2.ll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
; RUN: llc -mtriple=amdgcn-amd-amdhsa -verify-machineinstrs < %s | FileCheck %s
1+
; RUN: llc -mtriple=amdgcn-amd-amdhsa -verify-machineinstrs -experimental-debug-variable-locations=false < %s | FileCheck %s
22

33
%struct.ShapeData = type { <4 x float>, <4 x float>, <4 x float>, <4 x float>, <4 x float>, i32, i32, i64, <4 x float>, i32, i8, i8, i16, i32, i32 }
44

‎llvm/test/CodeGen/AMDGPU/dwarf-multi-register-use-crash.ll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
2-
; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx908 < %s | FileCheck %s
2+
; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx908 -experimental-debug-variable-locations=false < %s | FileCheck %s
33

44
; Don't crash.
55

0 commit comments

Comments
 (0)