Skip to content

Commit 8a77b0c

Browse files
jeffnnjeffnn
authored
PIX shader debugger: Support dynamic indices for local arrays (microsoft#7536)
The root of the problem being addressed here is this line from the previous version of DxilAnnotateWithVirtualRegister.cpp at (old) line 251 in function GetStructOffset: ``` auto *pArrayIndex = llvm::dyn_cast<llvm::ConstantInt>(pGEP->getOperand(GEPOperandIndex++)); ``` When an array is dynamically indexed, this dyn_cast of course returns nullptr, and this function returns a zero, which eventually caused the values of all dynamically-indexed array elements in PIX's shader debugger to be reported as the value of the zeroth element in the array. The next issue was that stores to an alloca-backed dynamic array weren't being properly recognized as significant events from PIX debugger's point of view. PIX adds its own "fake" alloca stores to help tie its debug output with the debug info that ends up in the PDB, so it's easy enough to co-opt that machinery to cover stores to "real" allocas, i.e. function-local array storage. To do so, the "AnnotateStore" function needs some of the metadata (i.e. PIX instruction number) that is added during runOnModule here. This necessitated rearranging runOnModule and putting stores into a vector that we then iterate over at the end of runOnModule. Now that indices aren't collapsed into just the zeroth, PIX needs to know how much storage to allocate for the full array, which is the motivation for the change in DxilDebugInstrumentation.cpp to return some metadata that PIX can parse. DxilDbgValueToDbgDeclare.cpp's changes are just a variable rename to aid readability. The rearrangement of runOnModule can induce some allocas to be visited more than once, so there are changes in DxilPIXVirtualRegisters.cpp to make sure we don't overwrite an existing alloca ordinal with a new one (which would confuse previously-established references to that alloca). file-check tests have been added to validate that -the stores to local arrays are being noticed properly. -the debug pass correctly outputs the metadata that informs PIX about alloca sizes The majority of these changes really needs end-to-end testing in PIX, where I can gather real debug output as generated by the GPU in response to the instrumentation, then match those results up with PDB data and finally show HLSL variable contents in the shader debugger, so there are some tests waiting on the PIX side for when this change makes its way there.
1 parent f94396d commit 8a77b0c

7 files changed

Lines changed: 178 additions & 55 deletions

File tree

‎lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp‎

Lines changed: 100 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -76,19 +76,29 @@ class DxilAnnotateWithVirtualRegister : public llvm::ModulePass {
7676

7777
private:
7878
void AnnotateValues(llvm::Instruction *pI);
79-
void AnnotateStore(llvm::Instruction *pI);
80-
void SplitVectorStores(hlsl::OP *HlslOP, llvm::Instruction *pI);
79+
void AnnotateStore(hlsl::OP *HlslOP, llvm::Instruction *pI);
80+
void SplitVectorStores(llvm::Instruction *pI);
8181
bool IsAllocaRegisterWrite(llvm::Value *V, llvm::AllocaInst **pAI,
8282
llvm::Value **pIdx);
8383
void AnnotateAlloca(llvm::AllocaInst *pAlloca);
8484
void AnnotateGeneric(llvm::Instruction *pI);
8585
void AssignNewDxilRegister(llvm::Instruction *pI);
8686
void AssignNewAllocaRegister(llvm::AllocaInst *pAlloca, std::uint32_t C);
87-
87+
llvm::Value *AddConstIntValues(llvm::Value *l, llvm::Value *r);
88+
llvm::Value *MultiplyConstIntValue(llvm::Value *l, uint32_t r);
89+
llvm::Value *GetStructOffset(llvm::GetElementPtrInst *pGEP,
90+
uint32_t &GEPOperandIndex,
91+
llvm::Type *pElementType);
8892
hlsl::DxilModule *m_DM;
8993
std::uint32_t m_uVReg;
9094
std::unique_ptr<llvm::ModuleSlotTracker> m_MST;
9195
int m_StartInstruction = 0;
96+
struct RememberedAllocaStores {
97+
llvm::StoreInst *StoreInst;
98+
llvm::Value *Index;
99+
llvm::MDNode *AllocaReg;
100+
};
101+
std::vector<RememberedAllocaStores> m_RememberedAllocaStores;
92102

93103
void Init(llvm::Module &M) {
94104
m_DM = &M.GetOrCreateDxilModule();
@@ -129,16 +139,14 @@ bool DxilAnnotateWithVirtualRegister::runOnModule(llvm::Module &M) {
129139
m_DM->SetValidatorVersion(1, 4);
130140
}
131141

132-
std::uint32_t InstNum = m_StartInstruction;
133-
134142
auto instrumentableFunctions =
135143
PIXPassHelpers::GetAllInstrumentableFunctions(*m_DM);
136144

137145
for (auto *F : instrumentableFunctions) {
138146
for (auto &block : F->getBasicBlockList()) {
139147
for (auto it = block.begin(); it != block.end();) {
140148
llvm::Instruction *I = &*(it++);
141-
SplitVectorStores(m_DM->GetOP(), I);
149+
SplitVectorStores(I);
142150
}
143151
}
144152
}
@@ -151,17 +159,32 @@ bool DxilAnnotateWithVirtualRegister::runOnModule(llvm::Module &M) {
151159
}
152160
}
153161

162+
// Process all allocas referenced by dbg.declare intrinsics
154163
for (auto *F : instrumentableFunctions) {
155164
for (auto &block : F->getBasicBlockList()) {
156-
for (llvm::Instruction &I : block.getInstList()) {
157-
AnnotateStore(&I);
165+
for (auto &I : block) {
166+
if (auto *DbgDeclare = llvm::dyn_cast<llvm::DbgDeclareInst>(&I)) {
167+
// The first operand of DbgDeclare is the address (typically an
168+
// AllocaInst)
169+
if (auto *AddrVal =
170+
llvm::dyn_cast<llvm::Instruction>(DbgDeclare->getAddress())) {
171+
AnnotateValues(AddrVal);
172+
}
173+
}
158174
}
159175
}
160176
}
161177

178+
for (auto *F : instrumentableFunctions)
179+
for (auto &block : F->getBasicBlockList()) {
180+
for (llvm::Instruction &I : block.getInstList()) {
181+
AnnotateStore(m_DM->GetOP(), &I);
182+
}
183+
}
184+
162185
for (auto *F : instrumentableFunctions) {
163-
int InstructionRangeStart = InstNum;
164-
int InstructionRangeEnd = InstNum;
186+
int InstructionRangeStart = m_StartInstruction;
187+
int InstructionRangeEnd = m_StartInstruction;
165188
for (auto &block : F->getBasicBlockList()) {
166189
for (llvm::Instruction &I : block.getInstList()) {
167190
// If the instruction is part of the debug value instrumentation added
@@ -171,8 +194,9 @@ bool DxilAnnotateWithVirtualRegister::runOnModule(llvm::Module &M) {
171194
if (PixAllocaReg::FromInst(Alloca, &unused1, &unused2))
172195
continue;
173196
if (!llvm::isa<llvm::DbgDeclareInst>(&I)) {
174-
pix_dxil::PixDxilInstNum::AddMD(M.getContext(), &I, InstNum++);
175-
InstructionRangeEnd = InstNum;
197+
pix_dxil::PixDxilInstNum::AddMD(M.getContext(), &I,
198+
m_StartInstruction++);
199+
InstructionRangeEnd = m_StartInstruction;
176200
}
177201
}
178202
}
@@ -188,12 +212,17 @@ bool DxilAnnotateWithVirtualRegister::runOnModule(llvm::Module &M) {
188212
}
189213
}
190214

215+
for (auto const &as : m_RememberedAllocaStores) {
216+
PixAllocaRegWrite::AddMD(m_DM->GetCtx(), as.StoreInst, as.AllocaReg,
217+
as.Index);
218+
}
219+
191220
if (OSOverride != nullptr) {
192221
// Print a set of strings of the exemplary form "InstructionCount: <n>
193222
// <fnName>"
194223
if (m_DM->GetShaderModel()->GetKind() == hlsl::ShaderModel::Kind::Library)
195224
*OSOverride << "\nIsLibrary\n";
196-
*OSOverride << "\nInstructionCount:" << InstNum << "\n";
225+
*OSOverride << "\nInstructionCount:" << m_StartInstruction << "\n";
197226
}
198227

199228
m_DM = nullptr;
@@ -210,7 +239,8 @@ void DxilAnnotateWithVirtualRegister::AnnotateValues(llvm::Instruction *pI) {
210239
}
211240
}
212241

213-
void DxilAnnotateWithVirtualRegister::AnnotateStore(llvm::Instruction *pI) {
242+
void DxilAnnotateWithVirtualRegister::AnnotateStore(hlsl::OP *HlslOP,
243+
llvm::Instruction *pI) {
214244
auto *pSt = llvm::dyn_cast<llvm::StoreInst>(pI);
215245
if (pSt == nullptr) {
216246
return;
@@ -226,15 +256,47 @@ void DxilAnnotateWithVirtualRegister::AnnotateStore(llvm::Instruction *pI) {
226256
if (AllocaReg == nullptr) {
227257
return;
228258
}
259+
m_RememberedAllocaStores.push_back({pSt, Index, AllocaReg});
260+
}
261+
262+
llvm::Value *
263+
DxilAnnotateWithVirtualRegister::MultiplyConstIntValue(llvm::Value *l,
264+
uint32_t r) {
265+
if (r == 1)
266+
return l;
267+
if (auto *lci = llvm::dyn_cast<llvm::ConstantInt>(l))
268+
return m_DM->GetOP()->GetU32Const(lci->getLimitedValue() * r);
269+
// Should never get here, but if we do, return the left as a reasonable
270+
// default:
271+
return l;
272+
}
229273

230-
PixAllocaRegWrite::AddMD(m_DM->GetCtx(), pSt, AllocaReg, Index);
274+
llvm::Value *
275+
DxilAnnotateWithVirtualRegister::AddConstIntValues(llvm::Value *l,
276+
llvm::Value *r) {
277+
auto *rci = llvm::dyn_cast<llvm::ConstantInt>(r);
278+
if (rci && rci->getLimitedValue() == 0)
279+
return l;
280+
auto *lci = llvm::dyn_cast<llvm::ConstantInt>(l);
281+
if (lci && lci->getLimitedValue() == 0)
282+
return r;
283+
// Both an assert and a check, in case of unexpected circumstances.
284+
DXASSERT(lci != nullptr && rci != nullptr,
285+
"Both sides of add should be constant ints");
286+
if (lci != nullptr && rci != nullptr)
287+
return m_DM->GetOP()->GetU32Const(lci->getLimitedValue() +
288+
rci->getLimitedValue());
289+
// In an emergency, return the left argument. It'll be closest to
290+
// the desired value.
291+
return l;
231292
}
232293

233-
static uint32_t GetStructOffset(llvm::GetElementPtrInst *pGEP,
234-
uint32_t &GEPOperandIndex,
235-
llvm::Type *pElementType) {
294+
llvm::Value *
295+
DxilAnnotateWithVirtualRegister::GetStructOffset(llvm::GetElementPtrInst *pGEP,
296+
uint32_t &GEPOperandIndex,
297+
llvm::Type *pElementType) {
236298
if (IsInstrumentableFundamentalType(pElementType)) {
237-
return 0;
299+
return m_DM->GetOP()->GetU32Const(0);
238300
} else if (auto *pArray = llvm::dyn_cast<llvm::ArrayType>(pElementType)) {
239301
// 1D-array example:
240302
//
@@ -248,18 +310,13 @@ static uint32_t GetStructOffset(llvm::GetElementPtrInst *pGEP,
248310
// -The zeroth element in the struct (which is the array)
249311
// -The zeroth element in that array
250312

251-
auto *pArrayIndex =
252-
llvm::dyn_cast<llvm::ConstantInt>(pGEP->getOperand(GEPOperandIndex++));
253-
254-
if (pArrayIndex == nullptr) {
255-
return 0;
256-
}
313+
auto *pArrayIndex = pGEP->getOperand(GEPOperandIndex++);
257314

258-
uint32_t ArrayIndex = pArrayIndex->getLimitedValue();
259315
auto pArrayElementType = pArray->getArrayElementType();
260-
uint32_t MemberIndex = ArrayIndex * CountStructMembers(pArrayElementType);
261-
return MemberIndex +
262-
GetStructOffset(pGEP, GEPOperandIndex, pArrayElementType);
316+
auto *MemberIndex = MultiplyConstIntValue(
317+
pArrayIndex, CountStructMembers(pArrayElementType));
318+
return AddConstIntValues(
319+
MemberIndex, GetStructOffset(pGEP, GEPOperandIndex, pArrayElementType));
263320
} else if (auto *pStruct = llvm::dyn_cast<llvm::StructType>(pElementType)) {
264321
DXASSERT(GEPOperandIndex < pGEP->getNumOperands(),
265322
"Unexpectedly read too many GetElementPtrInst operands");
@@ -268,7 +325,7 @@ static uint32_t GetStructOffset(llvm::GetElementPtrInst *pGEP,
268325
llvm::dyn_cast<llvm::ConstantInt>(pGEP->getOperand(GEPOperandIndex++));
269326

270327
if (pMemberIndex == nullptr) {
271-
return 0;
328+
return m_DM->GetOP()->GetU32Const(0);
272329
}
273330

274331
uint32_t MemberIndex = pMemberIndex->getLimitedValue();
@@ -278,16 +335,17 @@ static uint32_t GetStructOffset(llvm::GetElementPtrInst *pGEP,
278335
MemberOffset += CountStructMembers(pStruct->getElementType(i));
279336
}
280337

281-
return MemberOffset + GetStructOffset(pGEP, GEPOperandIndex,
282-
pStruct->getElementType(MemberIndex));
338+
return AddConstIntValues(
339+
m_DM->GetOP()->GetU32Const(MemberOffset),
340+
GetStructOffset(pGEP, GEPOperandIndex,
341+
pStruct->getElementType(MemberIndex)));
283342
} else {
284-
return 0;
343+
return m_DM->GetOP()->GetU32Const(0);
285344
}
286345
}
287346

288347
bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite(
289348
llvm::Value *V, llvm::AllocaInst **pAI, llvm::Value **pIdx) {
290-
llvm::IRBuilder<> B(m_DM->GetCtx());
291349

292350
*pAI = nullptr;
293351
*pIdx = nullptr;
@@ -366,7 +424,8 @@ bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite(
366424

367425
auto offset = GetStructOffset(pGEP, GEPOperandIndex, pStructType);
368426

369-
llvm::Value *IndexValue = B.getInt32(offset + precedingMemberCount);
427+
llvm::Value *IndexValue = AddConstIntValues(
428+
offset, m_DM->GetOP()->GetU32Const(precedingMemberCount));
370429

371430
if (IndexValue != nullptr) {
372431
*pAI = Alloca;
@@ -383,7 +442,7 @@ bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite(
383442
}
384443

385444
*pAI = pAlloca;
386-
*pIdx = B.getInt32(0);
445+
*pIdx = m_DM->GetOP()->GetU32Const(0);
387446
return true;
388447
}
389448

@@ -463,12 +522,13 @@ void DxilAnnotateWithVirtualRegister::AssignNewDxilRegister(
463522

464523
void DxilAnnotateWithVirtualRegister::AssignNewAllocaRegister(
465524
llvm::AllocaInst *pAlloca, std::uint32_t C) {
466-
PixAllocaReg::AddMD(m_DM->GetCtx(), pAlloca, m_uVReg, C);
467-
m_uVReg += C;
525+
if (!PixAllocaReg::FromInst(pAlloca, nullptr, nullptr)) {
526+
PixAllocaReg::AddMD(m_DM->GetCtx(), pAlloca, m_uVReg, C);
527+
m_uVReg += C;
528+
}
468529
}
469530

470-
void DxilAnnotateWithVirtualRegister::SplitVectorStores(hlsl::OP *HlslOP,
471-
llvm::Instruction *pI) {
531+
void DxilAnnotateWithVirtualRegister::SplitVectorStores(llvm::Instruction *pI) {
472532
auto *pSt = llvm::dyn_cast<llvm::StoreInst>(pI);
473533
if (pSt == nullptr) {
474534
return;

‎lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ using namespace PIXPassHelpers;
3636

3737
using namespace llvm;
3838

39-
//#define VALUE_TO_DECLARE_LOGGING
39+
// #define VALUE_TO_DECLARE_LOGGING
4040

4141
#ifdef VALUE_TO_DECLARE_LOGGING
4242
#ifndef PIX_DEBUG_DUMP_HELPER
@@ -859,8 +859,8 @@ void DxilDbgValueToDbgDeclare::handleDbgValue(llvm::Module &M,
859859
VALUE_TO_DECLARE_LOG("... variable was null too");
860860
}
861861

862-
llvm::Value *V = DbgValue->getValue();
863-
if (V == nullptr) {
862+
llvm::Value *ValueFromDbgInst = DbgValue->getValue();
863+
if (ValueFromDbgInst == nullptr) {
864864
// The metadata contained a null Value, so we ignore it. This
865865
// seems to be a dxcompiler bug.
866866
VALUE_TO_DECLARE_LOG("...Null value!");
@@ -873,20 +873,20 @@ void DxilDbgValueToDbgDeclare::handleDbgValue(llvm::Module &M,
873873
return;
874874
}
875875

876-
if (llvm::isa<llvm::PointerType>(V->getType())) {
876+
if (llvm::isa<llvm::PointerType>(ValueFromDbgInst->getType())) {
877877
// Safeguard: If the type is not a pointer type, then this is
878878
// dbg.value directly pointing to a memory location instead of
879879
// a value.
880880
if (!IsDITypePointer(Ty, EmptyMap)) {
881881
// We only know how to handle AllocaInsts for now
882-
if (!isa<AllocaInst>(V)) {
882+
if (!isa<AllocaInst>(ValueFromDbgInst)) {
883883
VALUE_TO_DECLARE_LOG(
884884
"... variable had pointer type, but is not an alloca.");
885885
return;
886886
}
887887

888888
IRBuilder<> B(DbgValue->getNextNode());
889-
V = B.CreateLoad(V);
889+
ValueFromDbgInst = B.CreateLoad(ValueFromDbgInst);
890890
}
891891
}
892892

@@ -931,7 +931,7 @@ void DxilDbgValueToDbgDeclare::handleDbgValue(llvm::Module &M,
931931
}
932932

933933
const OffsetInBits InitialOffset = PackedOffsetFromVar;
934-
auto *insertPt = llvm::dyn_cast<llvm::Instruction>(V);
934+
auto *insertPt = llvm::dyn_cast<llvm::Instruction>(ValueFromDbgInst);
935935
if (insertPt != nullptr && !llvm::isa<TerminatorInst>(insertPt)) {
936936
insertPt = insertPt->getNextNode();
937937
// Drivers may crash if phi nodes aren't always at the top of a block,
@@ -950,7 +950,8 @@ void DxilDbgValueToDbgDeclare::handleDbgValue(llvm::Module &M,
950950
// Offset}. InitialOffset is the offset from DbgValue's expression
951951
// (i.e., the offset from the Variable's start), and Offset is the
952952
// Scalar Value's packed offset from DbgValue's value.
953-
for (const ValueAndOffset &VO : SplitValue(V, InitialOffset, B)) {
953+
for (const ValueAndOffset &VO :
954+
SplitValue(ValueFromDbgInst, InitialOffset, B)) {
954955

955956
OffsetInBits AlignedOffset;
956957
if (!Offsets.GetAlignedOffsetFromPackedOffset(VO.m_PackedOffset,

‎lib/DxilPIXPasses/DxilDebugInstrumentation.cpp‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1356,7 +1356,19 @@ DxilDebugInstrumentation::FindInstrumentableInstructionsInBlock(
13561356
IndexingToken = "s"; // static indexing, no debug output required
13571357
} else {
13581358
IndexingToken = "d"; // dynamic indexing
1359-
RegisterOrStaticIndex = std::to_string(IandT->AllocaBase);
1359+
int MaxArraySize = 1;
1360+
if (auto *Store = dyn_cast<StoreInst>(&Inst)) {
1361+
if (auto *GEP =
1362+
dyn_cast<GetElementPtrInst>(Store->getPointerOperand())) {
1363+
if (auto *Alloca =
1364+
dyn_cast<AllocaInst>(GEP->getPointerOperand())) {
1365+
MaxArraySize =
1366+
Alloca->getAllocatedType()->getArrayNumElements();
1367+
}
1368+
}
1369+
}
1370+
RegisterOrStaticIndex = std::to_string(IandT->AllocaBase) + "-" +
1371+
std::to_string(MaxArraySize);
13601372
DebugOutputForThisInstruction.ValueToWriteToDebugMemory =
13611373
IandT->AllocaWriteIndex;
13621374
}
@@ -1374,7 +1386,8 @@ DxilDebugInstrumentation::FindInstrumentableInstructionsInBlock(
13741386
*OSOverride << "," << *RegisterOrStaticIndex;
13751387
}
13761388
if (IandT->ConstantAllocaStoreValue) {
1377-
*OSOverride << "," << std::to_string(*IandT->ConstantAllocaStoreValue);
1389+
uint64_t value = IandT->ConstantAllocaStoreValue.value();
1390+
*OSOverride << "," << std::to_string(value);
13781391
}
13791392
*OSOverride << ";";
13801393
if (DebugOutputForThisInstruction.ValueToWriteToDebugMemory)

‎lib/DxilPIXPasses/DxilPIXVirtualRegisters.cpp‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,10 @@ static bool ParsePixAllocaReg(llvm::MDNode *MD, std::uint32_t *RegNum,
124124
return false;
125125
}
126126

127-
*RegNum = mdRegNum->getLimitedValue();
128-
*Count = mdCount->getLimitedValue();
127+
if (RegNum != nullptr)
128+
*RegNum = mdRegNum->getLimitedValue();
129+
if (Count != nullptr)
130+
*Count = mdCount->getLimitedValue();
129131
return true;
130132
}
131133

@@ -144,8 +146,10 @@ void pix_dxil::PixAllocaReg::AddMD(llvm::LLVMContext &Ctx,
144146
bool pix_dxil::PixAllocaReg::FromInst(llvm::AllocaInst const *pAlloca,
145147
std::uint32_t *pRegBase,
146148
std::uint32_t *pRegSize) {
147-
*pRegBase = 0;
148-
*pRegSize = 0;
149+
if (pRegBase != nullptr)
150+
*pRegBase = 0;
151+
if (pRegSize != nullptr)
152+
*pRegSize = 0;
149153

150154
auto *mdNodes = pAlloca->getMetadata(MDName);
151155
if (mdNodes == nullptr) {

0 commit comments

Comments
 (0)