Skip to content

Commit 27aaae0

Browse files
epilkkzhuravl
epilk
authored and
kzhuravl
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 SWDEV-472677 Change-Id: I69e720d87f8090952c6196e0b3c57861d914be09
1 parent 6be8582 commit 27aaae0

16 files changed

+98
-37
lines changed

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

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

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

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

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

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -312,8 +312,12 @@ 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+
std::optional<unsigned> NumLocOps = std::nullopt)
317+
: DIExpr(DIExpr), Indirect(Indirect), IsVariadic(IsVariadic),
318+
NumLocOps(NumLocOps
319+
? *NumLocOps
320+
: (IsVariadic ? DIExpr->getNumLocationOperands() : 1)) {}
317321

318322
/// Extract properties from an existing DBG_VALUE instruction.
319323
DbgValueProperties(const MachineInstr &MI) {
@@ -324,6 +328,7 @@ class DbgValueProperties {
324328
IsVariadic = MI.isDebugValueList();
325329
DIExpr = MI.getDebugExpression();
326330
Indirect = MI.isDebugOffsetImm();
331+
NumLocOps = MI.getNumDebugOperands();
327332
}
328333

329334
bool isJoinable(const DbgValueProperties &Other) const {
@@ -332,21 +337,20 @@ class DbgValueProperties {
332337
}
333338

334339
bool operator==(const DbgValueProperties &Other) const {
335-
return std::tie(DIExpr, Indirect, IsVariadic) ==
336-
std::tie(Other.DIExpr, Other.Indirect, Other.IsVariadic);
340+
return std::tie(DIExpr, Indirect, IsVariadic, NumLocOps) ==
341+
std::tie(Other.DIExpr, Other.Indirect, Other.IsVariadic, NumLocOps);
337342
}
338343

339344
bool operator!=(const DbgValueProperties &Other) const {
340345
return !(*this == Other);
341346
}
342347

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

347350
const DIExpression *DIExpr;
348351
bool Indirect;
349352
bool IsVariadic;
353+
unsigned NumLocOps;
350354
};
351355

352356
/// TODO: Might pack better if we changed this to a Struct of Arrays, since

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

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

140140
bool llvm::debuginfoShouldUseDebugInstrRef(const Triple &T) {
141141
// Enable by default on x86_64, disable if explicitly turned off on cmdline.
142-
if (T.getArch() == llvm::Triple::x86_64 &&
142+
if ((T.getArch() == llvm::Triple::x86_64 ||
143+
T.getArch() == llvm::Triple::amdgcn) &&
143144
ValueTrackingVariableLocations != cl::boolOrDefault::BOU_FALSE)
144145
return true;
145146

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5995,8 +5995,10 @@ bool SelectionDAGBuilder::EmitFuncArgumentDbgValue(
59955995
// the DIExpression.
59965996
if (Indirect)
59975997
NewDIExpr = DIExpression::prepend(FragExpr, DIExpression::DerefBefore);
5998-
SmallVector<uint64_t, 2> Ops({dwarf::DW_OP_LLVM_arg, 0});
5999-
NewDIExpr = DIExpression::prependOpcodes(NewDIExpr, Ops);
5998+
if (NewDIExpr->holdsOldElements()) {
5999+
SmallVector<uint64_t, 2> Ops({dwarf::DW_OP_LLVM_arg, 0});
6000+
NewDIExpr = DIExpression::prependOpcodes(NewDIExpr, Ops);
6001+
}
60006002
return BuildMI(MF, DL, Inst, false, MOs, Variable, NewDIExpr);
60016003
} else {
60026004
// 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
@@ -1507,9 +1507,11 @@ class DIExprVerifier : public DIExprConstVisitor<DIExprVerifier> {
15071507
return true;
15081508
if (Env->Arguments.empty())
15091509
return error("DIOpReferrer requires an argument");
1510-
return expectSameSize(
1511-
ResultType, Env->Arguments[0]->getType(),
1512-
"DIOpReferrer type must be same size in bits as argument");
1510+
const Value *V = Env->Arguments[0];
1511+
return isa<PoisonValue>(V) ||
1512+
expectSameSize(
1513+
ResultType, V->getType(),
1514+
"DIOpReferrer type must be same size in bits as argument");
15131515
}
15141516

15151517
bool visit(DIOp::Arg Op, Type *ResultType, ArrayRef<StackEntry>) {
@@ -1523,7 +1525,9 @@ class DIExprVerifier : public DIExprConstVisitor<DIExprVerifier> {
15231525
// but for now it should be safe to just ignore it.
15241526
// return error("DIOpArg index out of range");
15251527
return true;
1526-
return expectSameSize(ResultType, Env->Arguments[Op.getIndex()]->getType(),
1528+
const Value *V = Env->Arguments[Op.getIndex()];
1529+
return isa<PoisonValue>(V) ||
1530+
expectSameSize(ResultType, V->getType(),
15271531
"DIOpArg type must be same size in bits as argument");
15281532
}
15291533

@@ -1835,6 +1839,9 @@ DIExpression::convertToUndefExpression(const DIExpression *Expr) {
18351839

18361840
const DIExpression *
18371841
DIExpression::convertToVariadicExpression(const DIExpression *Expr) {
1842+
if (Expr->holdsNewElements())
1843+
return Expr;
1844+
18381845
if (any_of(Expr->expr_ops(), [](auto ExprOp) {
18391846
return ExprOp.getOp() == dwarf::DW_OP_LLVM_arg;
18401847
}))
@@ -1848,10 +1855,10 @@ DIExpression::convertToVariadicExpression(const DIExpression *Expr) {
18481855

18491856
std::optional<const DIExpression *>
18501857
DIExpression::convertToNonVariadicExpression(const DIExpression *Expr) {
1851-
if (Expr->holdsNewElements())
1858+
if (!Expr)
18521859
return std::nullopt;
18531860

1854-
if (!Expr)
1861+
if (Expr->holdsNewElements())
18551862
return std::nullopt;
18561863

18571864
if (auto Elts = Expr->getSingleLocationExpressionElements())
@@ -1894,6 +1901,11 @@ bool DIExpression::isEqualExpression(const DIExpression *FirstExpr,
18941901
bool FirstIndirect,
18951902
const DIExpression *SecondExpr,
18961903
bool SecondIndirect) {
1904+
if (FirstExpr->holdsNewElements() != SecondExpr->holdsNewElements())
1905+
return false;
1906+
if (FirstExpr->holdsNewElements())
1907+
return FirstIndirect == SecondIndirect && FirstExpr == SecondExpr;
1908+
18971909
SmallVector<uint64_t> FirstOps;
18981910
DIExpression::canonicalizeExpressionOps(FirstOps, FirstExpr, FirstIndirect);
18991911
SmallVector<uint64_t> SecondOps;
@@ -2045,6 +2057,8 @@ bool DIExpression::hasAllLocationOps(unsigned N) const {
20452057
for (auto ExprOp : expr_ops())
20462058
if (ExprOp.getOp() == dwarf::DW_OP_LLVM_arg)
20472059
SeenOps.insert(ExprOp.getArg(0));
2060+
else if (ExprOp.getOp() == dwarf::DW_OP_LLVM_poisoned)
2061+
return true;
20482062
for (uint64_t Idx = 0; Idx < N; ++Idx)
20492063
if (!SeenOps.contains(Idx))
20502064
return false;

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4540,6 +4540,7 @@ static void copyFlagsToImplicitVCC(MachineInstr &MI,
45404540
MachineInstr *SIInstrInfo::buildShrunkInst(MachineInstr &MI,
45414541
unsigned Op32) const {
45424542
MachineBasicBlock *MBB = MI.getParent();
4543+
MachineFunction *MF = MBB->getParent();
45434544

45444545
const MCInstrDesc &Op32Desc = get(Op32);
45454546
MachineInstrBuilder Inst32 =
@@ -4551,9 +4552,16 @@ MachineInstr *SIInstrInfo::buildShrunkInst(MachineInstr &MI,
45514552

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

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

45594567
int Idx = MI.getNumExplicitDefs();

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

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ class SILoadStoreOptimizer : public MachineFunctionPass {
228228

229229
void copyToDestRegs(CombineInfo &CI, CombineInfo &Paired,
230230
MachineBasicBlock::iterator InsertBefore, int OpName,
231-
Register DestReg) const;
231+
Register DestReg, MachineInstr *NewMI) const;
232232
Register copyFromSrcRegs(CombineInfo &CI, CombineInfo &Paired,
233233
MachineBasicBlock::iterator InsertBefore,
234234
int OpName) const;
@@ -1203,9 +1203,10 @@ SILoadStoreOptimizer::checkAndPrepareMerge(CombineInfo &CI,
12031203
// Paired.
12041204
void SILoadStoreOptimizer::copyToDestRegs(
12051205
CombineInfo &CI, CombineInfo &Paired,
1206-
MachineBasicBlock::iterator InsertBefore, int OpName,
1207-
Register DestReg) const {
1206+
MachineBasicBlock::iterator InsertBefore, int OpName, Register DestReg,
1207+
MachineInstr *NewMI) const {
12081208
MachineBasicBlock *MBB = CI.I->getParent();
1209+
MachineFunction *MF = MBB->getParent();
12091210
DebugLoc DL = CI.I->getDebugLoc();
12101211

12111212
auto [SubRegIdx0, SubRegIdx1] = getSubRegIdxs(CI, Paired);
@@ -1221,6 +1222,17 @@ void SILoadStoreOptimizer::copyToDestRegs(
12211222
BuildMI(*MBB, InsertBefore, DL, CopyDesc)
12221223
.add(*Dest1)
12231224
.addReg(DestReg, RegState::Kill, SubRegIdx1);
1225+
1226+
if (unsigned DINum = CI.I->peekDebugInstrNum()) {
1227+
unsigned NewDINum = NewMI->getDebugInstrNum();
1228+
MF->makeDebugValueSubstitution(std::make_pair(DINum, 0),
1229+
std::make_pair(NewDINum, 0), SubRegIdx0);
1230+
}
1231+
if (unsigned DINum = Paired.I->peekDebugInstrNum()) {
1232+
unsigned NewDINum = NewMI->getDebugInstrNum();
1233+
MF->makeDebugValueSubstitution(std::make_pair(DINum, 0),
1234+
std::make_pair(NewDINum, 0), SubRegIdx1);
1235+
}
12241236
}
12251237

12261238
// Return a register for the source of the merged store after copying the
@@ -1314,7 +1326,8 @@ SILoadStoreOptimizer::mergeRead2Pair(CombineInfo &CI, CombineInfo &Paired,
13141326
.addImm(0) // gds
13151327
.cloneMergedMemRefs({&*CI.I, &*Paired.I});
13161328

1317-
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdst, DestReg);
1329+
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdst, DestReg,
1330+
Read2);
13181331

13191332
CI.I->eraseFromParent();
13201333
Paired.I->eraseFromParent();
@@ -1434,7 +1447,7 @@ SILoadStoreOptimizer::mergeImagePair(CombineInfo &CI, CombineInfo &Paired,
14341447

14351448
MachineInstr *New = MIB.addMemOperand(combineKnownAdjacentMMOs(CI, Paired));
14361449

1437-
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdata, DestReg);
1450+
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdata, DestReg, New);
14381451

14391452
CI.I->eraseFromParent();
14401453
Paired.I->eraseFromParent();
@@ -1466,7 +1479,7 @@ MachineBasicBlock::iterator SILoadStoreOptimizer::mergeSMemLoadImmPair(
14661479
New.addImm(MergedOffset);
14671480
New.addImm(CI.CPol).addMemOperand(combineKnownAdjacentMMOs(CI, Paired));
14681481

1469-
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::sdst, DestReg);
1482+
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::sdst, DestReg, New);
14701483

14711484
CI.I->eraseFromParent();
14721485
Paired.I->eraseFromParent();
@@ -1507,7 +1520,7 @@ MachineBasicBlock::iterator SILoadStoreOptimizer::mergeBufferLoadPair(
15071520
.addImm(0) // swz
15081521
.addMemOperand(combineKnownAdjacentMMOs(CI, Paired));
15091522

1510-
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdata, DestReg);
1523+
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdata, DestReg, New);
15111524

15121525
CI.I->eraseFromParent();
15131526
Paired.I->eraseFromParent();
@@ -1552,7 +1565,7 @@ MachineBasicBlock::iterator SILoadStoreOptimizer::mergeTBufferLoadPair(
15521565
.addImm(0) // swz
15531566
.addMemOperand(combineKnownAdjacentMMOs(CI, Paired));
15541567

1555-
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdata, DestReg);
1568+
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdata, DestReg, New);
15561569

15571570
CI.I->eraseFromParent();
15581571
Paired.I->eraseFromParent();
@@ -1622,7 +1635,7 @@ MachineBasicBlock::iterator SILoadStoreOptimizer::mergeFlatLoadPair(
16221635
.addImm(CI.CPol)
16231636
.addMemOperand(combineKnownAdjacentMMOs(CI, Paired));
16241637

1625-
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdst, DestReg);
1638+
copyToDestRegs(CI, Paired, InsertBefore, AMDGPU::OpName::vdst, DestReg, New);
16261639

16271640
CI.I->eraseFromParent();
16281641
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)