From a59429a2445d3a4252dc17ac8811707888122ae0 Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Tue, 10 Jun 2025 17:22:43 -0700 Subject: [PATCH 1/9] Dynamic indices --- .../DxilAnnotateWithVirtualRegister.cpp | 129 ++++++++++++------ .../DxilDbgValueToDbgDeclare.cpp | 14 +- .../DxilDebugInstrumentation.cpp | 13 +- ...ValueToDbgDeclare_dynamic_array_index.hlsl | 27 ++++ 4 files changed, 133 insertions(+), 50 deletions(-) create mode 100644 tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_dynamic_array_index.hlsl diff --git a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp index babf5b7953..e07a213fe2 100644 --- a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp +++ b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp @@ -76,19 +76,30 @@ class DxilAnnotateWithVirtualRegister : public llvm::ModulePass { private: void AnnotateValues(llvm::Instruction *pI); - void AnnotateStore(llvm::Instruction *pI); + void AnnotateStore(hlsl::OP* HlslOP, llvm::Instruction *pI); void SplitVectorStores(hlsl::OP *HlslOP, llvm::Instruction *pI); - bool IsAllocaRegisterWrite(llvm::Value *V, llvm::AllocaInst **pAI, + bool IsAllocaRegisterWrite(hlsl::OP* HlslOP, llvm::Value *V, llvm::AllocaInst **pAI, llvm::Value **pIdx); void AnnotateAlloca(llvm::AllocaInst *pAlloca); void AnnotateGeneric(llvm::Instruction *pI); void AssignNewDxilRegister(llvm::Instruction *pI); void AssignNewAllocaRegister(llvm::AllocaInst *pAlloca, std::uint32_t C); - + llvm::Value* AddPossiblyDynamicValues(llvm::IRBuilder<>& Builder, hlsl::OP* HlslOP, llvm::Value* l, llvm::Value* r); + llvm::Value* MultiplyPossiblyDynamicValues(llvm::IRBuilder<>& Builder, hlsl::OP* HlslOP, llvm::Value* l, uint32_t r); + llvm::Value* GetStructOffset(llvm::IRBuilder<>& Builder, hlsl::OP* HlslOP, llvm::GetElementPtrInst* pGEP, + uint32_t& GEPOperandIndex, + llvm::Type* pElementType); hlsl::DxilModule *m_DM; std::uint32_t m_uVReg; std::unique_ptr m_MST; int m_StartInstruction = 0; + struct RememberedAllocaStores + { + llvm::StoreInst* StoreInst; + llvm::Value* Index; + llvm::MDNode* AllocaReg; + }; + std::vector m_RememberedAllocaStores; void Init(llvm::Module &M) { m_DM = &M.GetOrCreateDxilModule(); @@ -129,8 +140,6 @@ bool DxilAnnotateWithVirtualRegister::runOnModule(llvm::Module &M) { m_DM->SetValidatorVersion(1, 4); } - std::uint32_t InstNum = m_StartInstruction; - auto instrumentableFunctions = PIXPassHelpers::GetAllInstrumentableFunctions(*m_DM); @@ -151,17 +160,30 @@ bool DxilAnnotateWithVirtualRegister::runOnModule(llvm::Module &M) { } } - for (auto *F : instrumentableFunctions) { - for (auto &block : F->getBasicBlockList()) { - for (llvm::Instruction &I : block.getInstList()) { - AnnotateStore(&I); + // Process all allocas referenced by dbg.declare intrinsics + for (auto* F : instrumentableFunctions) { + for (auto& block : F->getBasicBlockList()) { + for (auto& I : block) { + if (auto* DbgDeclare = llvm::dyn_cast(&I)) { + // The first operand of DbgDeclare is the address (typically an AllocaInst) + if (auto* AddrVal = llvm::dyn_cast(DbgDeclare->getAddress())) { + AnnotateValues(AddrVal); + } + } + } + } + } + + for (auto* F : instrumentableFunctions) + for (auto& block : F->getBasicBlockList()) { + for (llvm::Instruction& I : block.getInstList()) { + AnnotateStore(m_DM->GetOP(), &I); } - } } for (auto *F : instrumentableFunctions) { - int InstructionRangeStart = InstNum; - int InstructionRangeEnd = InstNum; + int InstructionRangeStart = m_StartInstruction; + int InstructionRangeEnd = m_StartInstruction; for (auto &block : F->getBasicBlockList()) { for (llvm::Instruction &I : block.getInstList()) { // If the instruction is part of the debug value instrumentation added @@ -171,8 +193,8 @@ bool DxilAnnotateWithVirtualRegister::runOnModule(llvm::Module &M) { if (PixAllocaReg::FromInst(Alloca, &unused1, &unused2)) continue; if (!llvm::isa(&I)) { - pix_dxil::PixDxilInstNum::AddMD(M.getContext(), &I, InstNum++); - InstructionRangeEnd = InstNum; + pix_dxil::PixDxilInstNum::AddMD(M.getContext(), &I, m_StartInstruction++); + InstructionRangeEnd = m_StartInstruction; } } } @@ -188,12 +210,17 @@ bool DxilAnnotateWithVirtualRegister::runOnModule(llvm::Module &M) { } } + for (auto const& as : m_RememberedAllocaStores) + { + PixAllocaRegWrite::AddMD(m_DM->GetCtx(), as.StoreInst, as.AllocaReg, as.Index); + } + if (OSOverride != nullptr) { // Print a set of strings of the exemplary form "InstructionCount: // " if (m_DM->GetShaderModel()->GetKind() == hlsl::ShaderModel::Kind::Library) *OSOverride << "\nIsLibrary\n"; - *OSOverride << "\nInstructionCount:" << InstNum << "\n"; + *OSOverride << "\nInstructionCount:" << m_StartInstruction << "\n"; } m_DM = nullptr; @@ -210,7 +237,7 @@ void DxilAnnotateWithVirtualRegister::AnnotateValues(llvm::Instruction *pI) { } } -void DxilAnnotateWithVirtualRegister::AnnotateStore(llvm::Instruction *pI) { +void DxilAnnotateWithVirtualRegister::AnnotateStore(hlsl::OP* HlslOP, llvm::Instruction *pI) { auto *pSt = llvm::dyn_cast(pI); if (pSt == nullptr) { return; @@ -218,7 +245,7 @@ void DxilAnnotateWithVirtualRegister::AnnotateStore(llvm::Instruction *pI) { llvm::AllocaInst *Alloca; llvm::Value *Index; - if (!IsAllocaRegisterWrite(pSt->getPointerOperand(), &Alloca, &Index)) { + if (!IsAllocaRegisterWrite(HlslOP, pSt->getPointerOperand(), &Alloca, &Index)) { return; } @@ -226,15 +253,40 @@ void DxilAnnotateWithVirtualRegister::AnnotateStore(llvm::Instruction *pI) { if (AllocaReg == nullptr) { return; } + m_RememberedAllocaStores.push_back({ pSt, Index, AllocaReg }); +} + +llvm::Value* DxilAnnotateWithVirtualRegister::MultiplyPossiblyDynamicValues(llvm::IRBuilder<> & Builder, hlsl::OP* HlslOP, llvm::Value* l, uint32_t r) +{ + if (r == 1) + return l; + if (auto* lci = llvm::dyn_cast(l)) + return HlslOP->GetU32Const(lci->getLimitedValue() * r); + auto Mul = Builder.CreateMul(l, HlslOP->GetU32Const(r), "Mul_" + l->getName() + "_" + std::to_string(r)); + AnnotateValues(llvm::dyn_cast(Mul)); + return Mul; +} - PixAllocaRegWrite::AddMD(m_DM->GetCtx(), pSt, AllocaReg, Index); +llvm::Value* DxilAnnotateWithVirtualRegister::AddPossiblyDynamicValues(llvm::IRBuilder<>& Builder, hlsl::OP* HlslOP, llvm::Value* l, llvm::Value* r) +{ + auto* rci = llvm::dyn_cast(r); + if (rci && rci->getLimitedValue() == 0) + return l; + auto* lci = llvm::dyn_cast(l); + if (lci && lci->getLimitedValue() == 0) + return r; + if (lci != nullptr && rci != nullptr) + return HlslOP->GetU32Const(lci->getLimitedValue() + rci->getLimitedValue()); + auto * Add = Builder.CreateAdd(l, r, "Add_" + l->getName() + "_" + r->getName()); + AnnotateValues(llvm::dyn_cast(Add)); + return Add; } -static uint32_t GetStructOffset(llvm::GetElementPtrInst *pGEP, +llvm::Value * DxilAnnotateWithVirtualRegister::GetStructOffset(llvm::IRBuilder<>& Builder, hlsl::OP* HlslOP, llvm::GetElementPtrInst *pGEP, uint32_t &GEPOperandIndex, llvm::Type *pElementType) { if (IsInstrumentableFundamentalType(pElementType)) { - return 0; + return HlslOP->GetU32Const(0); } else if (auto *pArray = llvm::dyn_cast(pElementType)) { // 1D-array example: // @@ -248,18 +300,12 @@ static uint32_t GetStructOffset(llvm::GetElementPtrInst *pGEP, // -The zeroth element in the struct (which is the array) // -The zeroth element in that array - auto *pArrayIndex = - llvm::dyn_cast(pGEP->getOperand(GEPOperandIndex++)); - - if (pArrayIndex == nullptr) { - return 0; - } + auto *pArrayIndex = pGEP->getOperand(GEPOperandIndex++); - uint32_t ArrayIndex = pArrayIndex->getLimitedValue(); auto pArrayElementType = pArray->getArrayElementType(); - uint32_t MemberIndex = ArrayIndex * CountStructMembers(pArrayElementType); - return MemberIndex + - GetStructOffset(pGEP, GEPOperandIndex, pArrayElementType); + auto * MemberIndex = MultiplyPossiblyDynamicValues(Builder, HlslOP, pArrayIndex, CountStructMembers(pArrayElementType)); + return AddPossiblyDynamicValues(Builder, HlslOP, MemberIndex, + GetStructOffset(Builder, HlslOP, pGEP, GEPOperandIndex, pArrayElementType)); } else if (auto *pStruct = llvm::dyn_cast(pElementType)) { DXASSERT(GEPOperandIndex < pGEP->getNumOperands(), "Unexpectedly read too many GetElementPtrInst operands"); @@ -268,7 +314,7 @@ static uint32_t GetStructOffset(llvm::GetElementPtrInst *pGEP, llvm::dyn_cast(pGEP->getOperand(GEPOperandIndex++)); if (pMemberIndex == nullptr) { - return 0; + return HlslOP->GetU32Const(0); } uint32_t MemberIndex = pMemberIndex->getLimitedValue(); @@ -278,16 +324,15 @@ static uint32_t GetStructOffset(llvm::GetElementPtrInst *pGEP, MemberOffset += CountStructMembers(pStruct->getElementType(i)); } - return MemberOffset + GetStructOffset(pGEP, GEPOperandIndex, - pStruct->getElementType(MemberIndex)); + return AddPossiblyDynamicValues(Builder, HlslOP, HlslOP->GetU32Const(MemberOffset), GetStructOffset(Builder, HlslOP, pGEP, GEPOperandIndex, + pStruct->getElementType(MemberIndex))); } else { - return 0; + return HlslOP->GetU32Const(0); } } -bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite( +bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite(hlsl::OP* HlslOP, llvm::Value *V, llvm::AllocaInst **pAI, llvm::Value **pIdx) { - llvm::IRBuilder<> B(m_DM->GetCtx()); *pAI = nullptr; *pIdx = nullptr; @@ -364,13 +409,15 @@ bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite( // referenced in the current struct. If that type is an (n-dimensional) // array, then there follow n indices. - auto offset = GetStructOffset(pGEP, GEPOperandIndex, pStructType); + llvm::IRBuilder<> B(pGEP); + + auto offset = GetStructOffset(B, HlslOP, pGEP, GEPOperandIndex, pStructType); - llvm::Value *IndexValue = B.getInt32(offset + precedingMemberCount); + llvm::Value* IndexValue = AddPossiblyDynamicValues(B, HlslOP, offset, HlslOP->GetU32Const(precedingMemberCount)); if (IndexValue != nullptr) { *pAI = Alloca; - *pIdx = IndexValue; + *pIdx = offset; return true; } return false; @@ -383,7 +430,7 @@ bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite( } *pAI = pAlloca; - *pIdx = B.getInt32(0); + *pIdx = HlslOP->GetU32Const(0); return true; } @@ -476,7 +523,7 @@ void DxilAnnotateWithVirtualRegister::SplitVectorStores(hlsl::OP *HlslOP, llvm::AllocaInst *Alloca; llvm::Value *Index; - if (!IsAllocaRegisterWrite(pSt->getPointerOperand(), &Alloca, &Index)) { + if (!IsAllocaRegisterWrite(HlslOP, pSt->getPointerOperand(), &Alloca, &Index)) { return; } diff --git a/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp b/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp index bf25d9f85f..4d8872741f 100644 --- a/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp +++ b/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp @@ -859,8 +859,8 @@ void DxilDbgValueToDbgDeclare::handleDbgValue(llvm::Module &M, VALUE_TO_DECLARE_LOG("... variable was null too"); } - llvm::Value *V = DbgValue->getValue(); - if (V == nullptr) { + llvm::Value *ValueFromDbgInst = DbgValue->getValue(); + if (ValueFromDbgInst == nullptr) { // The metadata contained a null Value, so we ignore it. This // seems to be a dxcompiler bug. VALUE_TO_DECLARE_LOG("...Null value!"); @@ -873,20 +873,20 @@ void DxilDbgValueToDbgDeclare::handleDbgValue(llvm::Module &M, return; } - if (llvm::isa(V->getType())) { + if (llvm::isa(ValueFromDbgInst->getType())) { // Safeguard: If the type is not a pointer type, then this is // dbg.value directly pointing to a memory location instead of // a value. if (!IsDITypePointer(Ty, EmptyMap)) { // We only know how to handle AllocaInsts for now - if (!isa(V)) { + if (!isa(ValueFromDbgInst)) { VALUE_TO_DECLARE_LOG( "... variable had pointer type, but is not an alloca."); return; } IRBuilder<> B(DbgValue->getNextNode()); - V = B.CreateLoad(V); + ValueFromDbgInst = B.CreateLoad(ValueFromDbgInst); } } @@ -931,7 +931,7 @@ void DxilDbgValueToDbgDeclare::handleDbgValue(llvm::Module &M, } const OffsetInBits InitialOffset = PackedOffsetFromVar; - auto *insertPt = llvm::dyn_cast(V); + auto *insertPt = llvm::dyn_cast(ValueFromDbgInst); if (insertPt != nullptr && !llvm::isa(insertPt)) { insertPt = insertPt->getNextNode(); // Drivers may crash if phi nodes aren't always at the top of a block, @@ -950,7 +950,7 @@ void DxilDbgValueToDbgDeclare::handleDbgValue(llvm::Module &M, // Offset}. InitialOffset is the offset from DbgValue's expression // (i.e., the offset from the Variable's start), and Offset is the // Scalar Value's packed offset from DbgValue's value. - for (const ValueAndOffset &VO : SplitValue(V, InitialOffset, B)) { + for (const ValueAndOffset &VO : SplitValue(ValueFromDbgInst, InitialOffset, B)) { OffsetInBits AlignedOffset; if (!Offsets.GetAlignedOffsetFromPackedOffset(VO.m_PackedOffset, diff --git a/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp b/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp index a7d7e72cb4..6cc555d93c 100644 --- a/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp @@ -1356,7 +1356,15 @@ DxilDebugInstrumentation::FindInstrumentableInstructionsInBlock( IndexingToken = "s"; // static indexing, no debug output required } else { IndexingToken = "d"; // dynamic indexing - RegisterOrStaticIndex = std::to_string(IandT->AllocaBase); + int MaxArraySize = 1; + if (auto* Store = dyn_cast(&Inst)) { + if (auto* GEP = dyn_cast(Store->getPointerOperand())) { + if (auto* Alloca = dyn_cast(GEP->getPointerOperand())) { + MaxArraySize = Alloca->getAllocatedType()->getArrayNumElements(); + } + } + } + RegisterOrStaticIndex = std::to_string(IandT->AllocaBase) + "-" + std::to_string(MaxArraySize); DebugOutputForThisInstruction.ValueToWriteToDebugMemory = IandT->AllocaWriteIndex; } @@ -1374,7 +1382,8 @@ DxilDebugInstrumentation::FindInstrumentableInstructionsInBlock( *OSOverride << "," << *RegisterOrStaticIndex; } if (IandT->ConstantAllocaStoreValue) { - *OSOverride << "," << std::to_string(*IandT->ConstantAllocaStoreValue); + uint64_t value = IandT->ConstantAllocaStoreValue.value(); + *OSOverride << "," << std::to_string(value); } *OSOverride << ";"; if (DebugOutputForThisInstruction.ValueToWriteToDebugMemory) diff --git a/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_dynamic_array_index.hlsl b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_dynamic_array_index.hlsl new file mode 100644 index 0000000000..cba891424a --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_dynamic_array_index.hlsl @@ -0,0 +1,27 @@ +// RUN: %dxc -Tcs_6_0 /Od %s | %opt -S -dxil-annotate-with-virtual-regs | %FileCheck %s + +// Check that there is an alloca backing the local array +// CHECK: [[ARRAYNAME:%.*]] = alloca [4 x float] + +// Grab the GEP for the above array's element that we're expecting to store to: +// CHECK: [[ARRAYELEMENTPTR:%.*]] = getelementptr inbounds [4 x float], [4 x float]* [[ARRAYNAME]] + +// Check that the store to the alloca is annotated with pix-alloca-reg-read metadata +// (meaning that the pass accurately noted that the 8.0 is stored to a dynamic array index) +// CHECK: store float 8.000000e+00, float* [[ARRAYELEMENTPTR]] +// CHECK-SAME: !pix-alloca-reg-write + + +RWByteAddressBuffer RawUAV: register(u1); + +[numthreads(1, 1, 1)] +void main() +{ + float local_array[4]; + local_array[RawUAV.Load(0)] = 8; + local_array[RawUAV.Load(1)] = 128; + + RawUAV.Store(64+0,local_array[0]); + RawUAV.Store(64+4,local_array[1]); +} + From 5402b004b04d3580d4b87b7a427fb9b1e81043ce Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Tue, 10 Jun 2025 17:24:35 -0700 Subject: [PATCH 2/9] clang format --- .../DxilAnnotateWithVirtualRegister.cpp | 161 ++++++++++-------- .../DxilDbgValueToDbgDeclare.cpp | 5 +- .../DxilDebugInstrumentation.cpp | 18 +- 3 files changed, 105 insertions(+), 79 deletions(-) diff --git a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp index e07a213fe2..f63e2be36d 100644 --- a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp +++ b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp @@ -76,28 +76,32 @@ class DxilAnnotateWithVirtualRegister : public llvm::ModulePass { private: void AnnotateValues(llvm::Instruction *pI); - void AnnotateStore(hlsl::OP* HlslOP, llvm::Instruction *pI); + void AnnotateStore(hlsl::OP *HlslOP, llvm::Instruction *pI); void SplitVectorStores(hlsl::OP *HlslOP, llvm::Instruction *pI); - bool IsAllocaRegisterWrite(hlsl::OP* HlslOP, llvm::Value *V, llvm::AllocaInst **pAI, - llvm::Value **pIdx); + bool IsAllocaRegisterWrite(hlsl::OP *HlslOP, llvm::Value *V, + llvm::AllocaInst **pAI, llvm::Value **pIdx); void AnnotateAlloca(llvm::AllocaInst *pAlloca); void AnnotateGeneric(llvm::Instruction *pI); void AssignNewDxilRegister(llvm::Instruction *pI); void AssignNewAllocaRegister(llvm::AllocaInst *pAlloca, std::uint32_t C); - llvm::Value* AddPossiblyDynamicValues(llvm::IRBuilder<>& Builder, hlsl::OP* HlslOP, llvm::Value* l, llvm::Value* r); - llvm::Value* MultiplyPossiblyDynamicValues(llvm::IRBuilder<>& Builder, hlsl::OP* HlslOP, llvm::Value* l, uint32_t r); - llvm::Value* GetStructOffset(llvm::IRBuilder<>& Builder, hlsl::OP* HlslOP, llvm::GetElementPtrInst* pGEP, - uint32_t& GEPOperandIndex, - llvm::Type* pElementType); + llvm::Value *AddPossiblyDynamicValues(llvm::IRBuilder<> &Builder, + hlsl::OP *HlslOP, llvm::Value *l, + llvm::Value *r); + llvm::Value *MultiplyPossiblyDynamicValues(llvm::IRBuilder<> &Builder, + hlsl::OP *HlslOP, llvm::Value *l, + uint32_t r); + llvm::Value *GetStructOffset(llvm::IRBuilder<> &Builder, hlsl::OP *HlslOP, + llvm::GetElementPtrInst *pGEP, + uint32_t &GEPOperandIndex, + llvm::Type *pElementType); hlsl::DxilModule *m_DM; std::uint32_t m_uVReg; std::unique_ptr m_MST; int m_StartInstruction = 0; - struct RememberedAllocaStores - { - llvm::StoreInst* StoreInst; - llvm::Value* Index; - llvm::MDNode* AllocaReg; + struct RememberedAllocaStores { + llvm::StoreInst *StoreInst; + llvm::Value *Index; + llvm::MDNode *AllocaReg; }; std::vector m_RememberedAllocaStores; @@ -161,28 +165,30 @@ bool DxilAnnotateWithVirtualRegister::runOnModule(llvm::Module &M) { } // Process all allocas referenced by dbg.declare intrinsics - for (auto* F : instrumentableFunctions) { - for (auto& block : F->getBasicBlockList()) { - for (auto& I : block) { - if (auto* DbgDeclare = llvm::dyn_cast(&I)) { - // The first operand of DbgDeclare is the address (typically an AllocaInst) - if (auto* AddrVal = llvm::dyn_cast(DbgDeclare->getAddress())) { - AnnotateValues(AddrVal); - } - } + for (auto *F : instrumentableFunctions) { + for (auto &block : F->getBasicBlockList()) { + for (auto &I : block) { + if (auto *DbgDeclare = llvm::dyn_cast(&I)) { + // The first operand of DbgDeclare is the address (typically an + // AllocaInst) + if (auto *AddrVal = + llvm::dyn_cast(DbgDeclare->getAddress())) { + AnnotateValues(AddrVal); } + } } + } } - for (auto* F : instrumentableFunctions) - for (auto& block : F->getBasicBlockList()) { - for (llvm::Instruction& I : block.getInstList()) { - AnnotateStore(m_DM->GetOP(), &I); + for (auto *F : instrumentableFunctions) + for (auto &block : F->getBasicBlockList()) { + for (llvm::Instruction &I : block.getInstList()) { + AnnotateStore(m_DM->GetOP(), &I); } - } + } for (auto *F : instrumentableFunctions) { - int InstructionRangeStart = m_StartInstruction; + int InstructionRangeStart = m_StartInstruction; int InstructionRangeEnd = m_StartInstruction; for (auto &block : F->getBasicBlockList()) { for (llvm::Instruction &I : block.getInstList()) { @@ -193,7 +199,8 @@ bool DxilAnnotateWithVirtualRegister::runOnModule(llvm::Module &M) { if (PixAllocaReg::FromInst(Alloca, &unused1, &unused2)) continue; if (!llvm::isa(&I)) { - pix_dxil::PixDxilInstNum::AddMD(M.getContext(), &I, m_StartInstruction++); + pix_dxil::PixDxilInstNum::AddMD(M.getContext(), &I, + m_StartInstruction++); InstructionRangeEnd = m_StartInstruction; } } @@ -210,9 +217,9 @@ bool DxilAnnotateWithVirtualRegister::runOnModule(llvm::Module &M) { } } - for (auto const& as : m_RememberedAllocaStores) - { - PixAllocaRegWrite::AddMD(m_DM->GetCtx(), as.StoreInst, as.AllocaReg, as.Index); + for (auto const &as : m_RememberedAllocaStores) { + PixAllocaRegWrite::AddMD(m_DM->GetCtx(), as.StoreInst, as.AllocaReg, + as.Index); } if (OSOverride != nullptr) { @@ -237,7 +244,8 @@ void DxilAnnotateWithVirtualRegister::AnnotateValues(llvm::Instruction *pI) { } } -void DxilAnnotateWithVirtualRegister::AnnotateStore(hlsl::OP* HlslOP, llvm::Instruction *pI) { +void DxilAnnotateWithVirtualRegister::AnnotateStore(hlsl::OP *HlslOP, + llvm::Instruction *pI) { auto *pSt = llvm::dyn_cast(pI); if (pSt == nullptr) { return; @@ -245,7 +253,8 @@ void DxilAnnotateWithVirtualRegister::AnnotateStore(hlsl::OP* HlslOP, llvm::Inst llvm::AllocaInst *Alloca; llvm::Value *Index; - if (!IsAllocaRegisterWrite(HlslOP, pSt->getPointerOperand(), &Alloca, &Index)) { + if (!IsAllocaRegisterWrite(HlslOP, pSt->getPointerOperand(), &Alloca, + &Index)) { return; } @@ -253,40 +262,43 @@ void DxilAnnotateWithVirtualRegister::AnnotateStore(hlsl::OP* HlslOP, llvm::Inst if (AllocaReg == nullptr) { return; } - m_RememberedAllocaStores.push_back({ pSt, Index, AllocaReg }); + m_RememberedAllocaStores.push_back({pSt, Index, AllocaReg}); } -llvm::Value* DxilAnnotateWithVirtualRegister::MultiplyPossiblyDynamicValues(llvm::IRBuilder<> & Builder, hlsl::OP* HlslOP, llvm::Value* l, uint32_t r) -{ - if (r == 1) - return l; - if (auto* lci = llvm::dyn_cast(l)) - return HlslOP->GetU32Const(lci->getLimitedValue() * r); - auto Mul = Builder.CreateMul(l, HlslOP->GetU32Const(r), "Mul_" + l->getName() + "_" + std::to_string(r)); - AnnotateValues(llvm::dyn_cast(Mul)); - return Mul; +llvm::Value *DxilAnnotateWithVirtualRegister::MultiplyPossiblyDynamicValues( + llvm::IRBuilder<> &Builder, hlsl::OP *HlslOP, llvm::Value *l, uint32_t r) { + if (r == 1) + return l; + if (auto *lci = llvm::dyn_cast(l)) + return HlslOP->GetU32Const(lci->getLimitedValue() * r); + auto Mul = Builder.CreateMul(l, HlslOP->GetU32Const(r), + "Mul_" + l->getName() + "_" + std::to_string(r)); + AnnotateValues(llvm::dyn_cast(Mul)); + return Mul; } -llvm::Value* DxilAnnotateWithVirtualRegister::AddPossiblyDynamicValues(llvm::IRBuilder<>& Builder, hlsl::OP* HlslOP, llvm::Value* l, llvm::Value* r) -{ - auto* rci = llvm::dyn_cast(r); - if (rci && rci->getLimitedValue() == 0) - return l; - auto* lci = llvm::dyn_cast(l); - if (lci && lci->getLimitedValue() == 0) - return r; - if (lci != nullptr && rci != nullptr) - return HlslOP->GetU32Const(lci->getLimitedValue() + rci->getLimitedValue()); - auto * Add = Builder.CreateAdd(l, r, "Add_" + l->getName() + "_" + r->getName()); - AnnotateValues(llvm::dyn_cast(Add)); - return Add; +llvm::Value *DxilAnnotateWithVirtualRegister::AddPossiblyDynamicValues( + llvm::IRBuilder<> &Builder, hlsl::OP *HlslOP, llvm::Value *l, + llvm::Value *r) { + auto *rci = llvm::dyn_cast(r); + if (rci && rci->getLimitedValue() == 0) + return l; + auto *lci = llvm::dyn_cast(l); + if (lci && lci->getLimitedValue() == 0) + return r; + if (lci != nullptr && rci != nullptr) + return HlslOP->GetU32Const(lci->getLimitedValue() + rci->getLimitedValue()); + auto *Add = + Builder.CreateAdd(l, r, "Add_" + l->getName() + "_" + r->getName()); + AnnotateValues(llvm::dyn_cast(Add)); + return Add; } -llvm::Value * DxilAnnotateWithVirtualRegister::GetStructOffset(llvm::IRBuilder<>& Builder, hlsl::OP* HlslOP, llvm::GetElementPtrInst *pGEP, - uint32_t &GEPOperandIndex, - llvm::Type *pElementType) { +llvm::Value *DxilAnnotateWithVirtualRegister::GetStructOffset( + llvm::IRBuilder<> &Builder, hlsl::OP *HlslOP, llvm::GetElementPtrInst *pGEP, + uint32_t &GEPOperandIndex, llvm::Type *pElementType) { if (IsInstrumentableFundamentalType(pElementType)) { - return HlslOP->GetU32Const(0); + return HlslOP->GetU32Const(0); } else if (auto *pArray = llvm::dyn_cast(pElementType)) { // 1D-array example: // @@ -303,9 +315,12 @@ llvm::Value * DxilAnnotateWithVirtualRegister::GetStructOffset(llvm::IRBuilder<> auto *pArrayIndex = pGEP->getOperand(GEPOperandIndex++); auto pArrayElementType = pArray->getArrayElementType(); - auto * MemberIndex = MultiplyPossiblyDynamicValues(Builder, HlslOP, pArrayIndex, CountStructMembers(pArrayElementType)); + auto *MemberIndex = MultiplyPossiblyDynamicValues( + Builder, HlslOP, pArrayIndex, CountStructMembers(pArrayElementType)); return AddPossiblyDynamicValues(Builder, HlslOP, MemberIndex, - GetStructOffset(Builder, HlslOP, pGEP, GEPOperandIndex, pArrayElementType)); + GetStructOffset(Builder, HlslOP, pGEP, + GEPOperandIndex, + pArrayElementType)); } else if (auto *pStruct = llvm::dyn_cast(pElementType)) { DXASSERT(GEPOperandIndex < pGEP->getNumOperands(), "Unexpectedly read too many GetElementPtrInst operands"); @@ -324,15 +339,18 @@ llvm::Value * DxilAnnotateWithVirtualRegister::GetStructOffset(llvm::IRBuilder<> MemberOffset += CountStructMembers(pStruct->getElementType(i)); } - return AddPossiblyDynamicValues(Builder, HlslOP, HlslOP->GetU32Const(MemberOffset), GetStructOffset(Builder, HlslOP, pGEP, GEPOperandIndex, - pStruct->getElementType(MemberIndex))); + return AddPossiblyDynamicValues( + Builder, HlslOP, HlslOP->GetU32Const(MemberOffset), + GetStructOffset(Builder, HlslOP, pGEP, GEPOperandIndex, + pStruct->getElementType(MemberIndex))); } else { return HlslOP->GetU32Const(0); } } -bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite(hlsl::OP* HlslOP, - llvm::Value *V, llvm::AllocaInst **pAI, llvm::Value **pIdx) { +bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite( + hlsl::OP *HlslOP, llvm::Value *V, llvm::AllocaInst **pAI, + llvm::Value **pIdx) { *pAI = nullptr; *pIdx = nullptr; @@ -411,9 +429,11 @@ bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite(hlsl::OP* HlslOP, llvm::IRBuilder<> B(pGEP); - auto offset = GetStructOffset(B, HlslOP, pGEP, GEPOperandIndex, pStructType); + auto offset = + GetStructOffset(B, HlslOP, pGEP, GEPOperandIndex, pStructType); - llvm::Value* IndexValue = AddPossiblyDynamicValues(B, HlslOP, offset, HlslOP->GetU32Const(precedingMemberCount)); + llvm::Value *IndexValue = AddPossiblyDynamicValues( + B, HlslOP, offset, HlslOP->GetU32Const(precedingMemberCount)); if (IndexValue != nullptr) { *pAI = Alloca; @@ -523,7 +543,8 @@ void DxilAnnotateWithVirtualRegister::SplitVectorStores(hlsl::OP *HlslOP, llvm::AllocaInst *Alloca; llvm::Value *Index; - if (!IsAllocaRegisterWrite(HlslOP, pSt->getPointerOperand(), &Alloca, &Index)) { + if (!IsAllocaRegisterWrite(HlslOP, pSt->getPointerOperand(), &Alloca, + &Index)) { return; } diff --git a/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp b/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp index 4d8872741f..9ddbe876b5 100644 --- a/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp +++ b/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp @@ -36,7 +36,7 @@ using namespace PIXPassHelpers; using namespace llvm; -//#define VALUE_TO_DECLARE_LOGGING +// #define VALUE_TO_DECLARE_LOGGING #ifdef VALUE_TO_DECLARE_LOGGING #ifndef PIX_DEBUG_DUMP_HELPER @@ -950,7 +950,8 @@ void DxilDbgValueToDbgDeclare::handleDbgValue(llvm::Module &M, // Offset}. InitialOffset is the offset from DbgValue's expression // (i.e., the offset from the Variable's start), and Offset is the // Scalar Value's packed offset from DbgValue's value. - for (const ValueAndOffset &VO : SplitValue(ValueFromDbgInst, InitialOffset, B)) { + for (const ValueAndOffset &VO : + SplitValue(ValueFromDbgInst, InitialOffset, B)) { OffsetInBits AlignedOffset; if (!Offsets.GetAlignedOffsetFromPackedOffset(VO.m_PackedOffset, diff --git a/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp b/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp index 6cc555d93c..4dd43b07cc 100644 --- a/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp @@ -1357,14 +1357,18 @@ DxilDebugInstrumentation::FindInstrumentableInstructionsInBlock( } else { IndexingToken = "d"; // dynamic indexing int MaxArraySize = 1; - if (auto* Store = dyn_cast(&Inst)) { - if (auto* GEP = dyn_cast(Store->getPointerOperand())) { - if (auto* Alloca = dyn_cast(GEP->getPointerOperand())) { - MaxArraySize = Alloca->getAllocatedType()->getArrayNumElements(); - } + if (auto *Store = dyn_cast(&Inst)) { + if (auto *GEP = + dyn_cast(Store->getPointerOperand())) { + if (auto *Alloca = + dyn_cast(GEP->getPointerOperand())) { + MaxArraySize = + Alloca->getAllocatedType()->getArrayNumElements(); } + } } - RegisterOrStaticIndex = std::to_string(IandT->AllocaBase) + "-" + std::to_string(MaxArraySize); + RegisterOrStaticIndex = std::to_string(IandT->AllocaBase) + "-" + + std::to_string(MaxArraySize); DebugOutputForThisInstruction.ValueToWriteToDebugMemory = IandT->AllocaWriteIndex; } @@ -1382,7 +1386,7 @@ DxilDebugInstrumentation::FindInstrumentableInstructionsInBlock( *OSOverride << "," << *RegisterOrStaticIndex; } if (IandT->ConstantAllocaStoreValue) { - uint64_t value = IandT->ConstantAllocaStoreValue.value(); + uint64_t value = IandT->ConstantAllocaStoreValue.value(); *OSOverride << "," << std::to_string(value); } *OSOverride << ";"; From 4e96f8e9b8826fc2c256a64e96c7be6140b4d66a Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Wed, 11 Jun 2025 12:31:04 -0700 Subject: [PATCH 3/9] Test fixes --- .../DxilAnnotateWithVirtualRegister.cpp | 6 ++++-- lib/DxilPIXPasses/DxilPIXVirtualRegisters.cpp | 12 ++++++++---- tools/clang/unittests/HLSL/PixTest.cpp | 3 +-- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp index f63e2be36d..2c7211c34a 100644 --- a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp +++ b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp @@ -530,8 +530,10 @@ void DxilAnnotateWithVirtualRegister::AssignNewDxilRegister( void DxilAnnotateWithVirtualRegister::AssignNewAllocaRegister( llvm::AllocaInst *pAlloca, std::uint32_t C) { - PixAllocaReg::AddMD(m_DM->GetCtx(), pAlloca, m_uVReg, C); - m_uVReg += C; + if (!PixAllocaReg::FromInst(pAlloca, nullptr, nullptr)) { + PixAllocaReg::AddMD(m_DM->GetCtx(), pAlloca, m_uVReg, C); + m_uVReg += C; + } } void DxilAnnotateWithVirtualRegister::SplitVectorStores(hlsl::OP *HlslOP, diff --git a/lib/DxilPIXPasses/DxilPIXVirtualRegisters.cpp b/lib/DxilPIXPasses/DxilPIXVirtualRegisters.cpp index f68e2082bc..a60f6a77a7 100644 --- a/lib/DxilPIXPasses/DxilPIXVirtualRegisters.cpp +++ b/lib/DxilPIXPasses/DxilPIXVirtualRegisters.cpp @@ -124,8 +124,10 @@ static bool ParsePixAllocaReg(llvm::MDNode *MD, std::uint32_t *RegNum, return false; } - *RegNum = mdRegNum->getLimitedValue(); - *Count = mdCount->getLimitedValue(); + if (RegNum != nullptr) + *RegNum = mdRegNum->getLimitedValue(); + if (Count != nullptr) + *Count = mdCount->getLimitedValue(); return true; } @@ -144,8 +146,10 @@ void pix_dxil::PixAllocaReg::AddMD(llvm::LLVMContext &Ctx, bool pix_dxil::PixAllocaReg::FromInst(llvm::AllocaInst const *pAlloca, std::uint32_t *pRegBase, std::uint32_t *pRegSize) { - *pRegBase = 0; - *pRegSize = 0; + if (pRegBase != nullptr) + *pRegBase = 0; + if (pRegSize != nullptr) + *pRegSize = 0; auto *mdNodes = pAlloca->getMetadata(MDName); if (mdNodes == nullptr) { diff --git a/tools/clang/unittests/HLSL/PixTest.cpp b/tools/clang/unittests/HLSL/PixTest.cpp index af7801c7bf..6141769fbe 100644 --- a/tools/clang/unittests/HLSL/PixTest.cpp +++ b/tools/clang/unittests/HLSL/PixTest.cpp @@ -1218,9 +1218,8 @@ PixTest::TestableResults PixTest::TestStructAnnotationCase( ReplaceDxilBlobPart(pBlob->GetBufferPointer(), pBlob->GetBufferSize(), pAnnotated, &pAnnotatedContainer); -#if 0 // handy for debugging +#if 1 // handy for debugging auto disTextW = Disassemble(pAnnotatedContainer); - WEX::Logging::Log::Comment(disTextW.c_str()); #endif ModuleAndHangersOn moduleEtc(pAnnotatedContainer); From a5e1e2423d5e88a203b490e4b01d72eff902b3b4 Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Wed, 11 Jun 2025 16:25:36 -0700 Subject: [PATCH 4/9] wrong index! --- lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp index 2c7211c34a..2b3e0fd345 100644 --- a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp +++ b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp @@ -437,7 +437,7 @@ bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite( if (IndexValue != nullptr) { *pAI = Alloca; - *pIdx = offset; + *pIdx = IndexValue; return true; } return false; From 155fb7352851af55a200754dea7574761189fce5 Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Wed, 11 Jun 2025 17:04:38 -0700 Subject: [PATCH 5/9] turn off debug disasm --- tools/clang/unittests/HLSL/PixTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/clang/unittests/HLSL/PixTest.cpp b/tools/clang/unittests/HLSL/PixTest.cpp index 6141769fbe..c032e9e872 100644 --- a/tools/clang/unittests/HLSL/PixTest.cpp +++ b/tools/clang/unittests/HLSL/PixTest.cpp @@ -1218,7 +1218,7 @@ PixTest::TestableResults PixTest::TestStructAnnotationCase( ReplaceDxilBlobPart(pBlob->GetBufferPointer(), pBlob->GetBufferSize(), pAnnotated, &pAnnotatedContainer); -#if 1 // handy for debugging +#if 0 // handy for debugging auto disTextW = Disassemble(pAnnotatedContainer); #endif From a13046f114d3c3592deebe92098a1316703a2257 Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Thu, 12 Jun 2025 09:24:41 -0700 Subject: [PATCH 6/9] Redo some checks, add new test for reporting dynamic-indexed array size --- .../DxilAnnotateWithVirtualRegister.cpp | 80 +++++++++---------- .../pix/Debug_dynamic_array_index.hlsl | 19 +++++ 2 files changed, 57 insertions(+), 42 deletions(-) create mode 100644 tools/clang/test/HLSLFileCheck/pix/Debug_dynamic_array_index.hlsl diff --git a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp index 2b3e0fd345..5cca9ff8df 100644 --- a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp +++ b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp @@ -84,14 +84,9 @@ class DxilAnnotateWithVirtualRegister : public llvm::ModulePass { void AnnotateGeneric(llvm::Instruction *pI); void AssignNewDxilRegister(llvm::Instruction *pI); void AssignNewAllocaRegister(llvm::AllocaInst *pAlloca, std::uint32_t C); - llvm::Value *AddPossiblyDynamicValues(llvm::IRBuilder<> &Builder, - hlsl::OP *HlslOP, llvm::Value *l, - llvm::Value *r); - llvm::Value *MultiplyPossiblyDynamicValues(llvm::IRBuilder<> &Builder, - hlsl::OP *HlslOP, llvm::Value *l, - uint32_t r); - llvm::Value *GetStructOffset(llvm::IRBuilder<> &Builder, hlsl::OP *HlslOP, - llvm::GetElementPtrInst *pGEP, + llvm::Value *AddConstIntValues(llvm::Value *l, llvm::Value *r); + llvm::Value *MultiplyConstIntValue(llvm::Value *l, uint32_t r); + llvm::Value *GetStructOffset(llvm::GetElementPtrInst *pGEP, uint32_t &GEPOperandIndex, llvm::Type *pElementType); hlsl::DxilModule *m_DM; @@ -265,40 +260,44 @@ void DxilAnnotateWithVirtualRegister::AnnotateStore(hlsl::OP *HlslOP, m_RememberedAllocaStores.push_back({pSt, Index, AllocaReg}); } -llvm::Value *DxilAnnotateWithVirtualRegister::MultiplyPossiblyDynamicValues( - llvm::IRBuilder<> &Builder, hlsl::OP *HlslOP, llvm::Value *l, uint32_t r) { +llvm::Value * +DxilAnnotateWithVirtualRegister::MultiplyConstIntValue(llvm::Value *l, + uint32_t r) { if (r == 1) return l; if (auto *lci = llvm::dyn_cast(l)) - return HlslOP->GetU32Const(lci->getLimitedValue() * r); - auto Mul = Builder.CreateMul(l, HlslOP->GetU32Const(r), - "Mul_" + l->getName() + "_" + std::to_string(r)); - AnnotateValues(llvm::dyn_cast(Mul)); - return Mul; + return m_DM->GetOP()->GetU32Const(lci->getLimitedValue() * r); + // Should never get here, but if we do, return the left as a reasonable + // default: + return l; } -llvm::Value *DxilAnnotateWithVirtualRegister::AddPossiblyDynamicValues( - llvm::IRBuilder<> &Builder, hlsl::OP *HlslOP, llvm::Value *l, - llvm::Value *r) { +llvm::Value * +DxilAnnotateWithVirtualRegister::AddConstIntValues(llvm::Value *l, + llvm::Value *r) { auto *rci = llvm::dyn_cast(r); if (rci && rci->getLimitedValue() == 0) return l; auto *lci = llvm::dyn_cast(l); if (lci && lci->getLimitedValue() == 0) return r; + // Both an assert and a check, in case of unexpected circumstances. + DXASSERT(lci != nullptr && rci != nullptr, + "Both sides of add should be constant ints"); if (lci != nullptr && rci != nullptr) - return HlslOP->GetU32Const(lci->getLimitedValue() + rci->getLimitedValue()); - auto *Add = - Builder.CreateAdd(l, r, "Add_" + l->getName() + "_" + r->getName()); - AnnotateValues(llvm::dyn_cast(Add)); - return Add; + return m_DM->GetOP()->GetU32Const(lci->getLimitedValue() + + rci->getLimitedValue()); + // In an emergency, return the left argument. It'll be closest to + // the desired value. + return l; } -llvm::Value *DxilAnnotateWithVirtualRegister::GetStructOffset( - llvm::IRBuilder<> &Builder, hlsl::OP *HlslOP, llvm::GetElementPtrInst *pGEP, - uint32_t &GEPOperandIndex, llvm::Type *pElementType) { +llvm::Value * +DxilAnnotateWithVirtualRegister::GetStructOffset(llvm::GetElementPtrInst *pGEP, + uint32_t &GEPOperandIndex, + llvm::Type *pElementType) { if (IsInstrumentableFundamentalType(pElementType)) { - return HlslOP->GetU32Const(0); + return m_DM->GetOP()->GetU32Const(0); } else if (auto *pArray = llvm::dyn_cast(pElementType)) { // 1D-array example: // @@ -315,12 +314,10 @@ llvm::Value *DxilAnnotateWithVirtualRegister::GetStructOffset( auto *pArrayIndex = pGEP->getOperand(GEPOperandIndex++); auto pArrayElementType = pArray->getArrayElementType(); - auto *MemberIndex = MultiplyPossiblyDynamicValues( - Builder, HlslOP, pArrayIndex, CountStructMembers(pArrayElementType)); - return AddPossiblyDynamicValues(Builder, HlslOP, MemberIndex, - GetStructOffset(Builder, HlslOP, pGEP, - GEPOperandIndex, - pArrayElementType)); + auto *MemberIndex = MultiplyConstIntValue( + pArrayIndex, CountStructMembers(pArrayElementType)); + return AddConstIntValues( + MemberIndex, GetStructOffset(pGEP, GEPOperandIndex, pArrayElementType)); } else if (auto *pStruct = llvm::dyn_cast(pElementType)) { DXASSERT(GEPOperandIndex < pGEP->getNumOperands(), "Unexpectedly read too many GetElementPtrInst operands"); @@ -329,7 +326,7 @@ llvm::Value *DxilAnnotateWithVirtualRegister::GetStructOffset( llvm::dyn_cast(pGEP->getOperand(GEPOperandIndex++)); if (pMemberIndex == nullptr) { - return HlslOP->GetU32Const(0); + return m_DM->GetOP()->GetU32Const(0); } uint32_t MemberIndex = pMemberIndex->getLimitedValue(); @@ -339,12 +336,12 @@ llvm::Value *DxilAnnotateWithVirtualRegister::GetStructOffset( MemberOffset += CountStructMembers(pStruct->getElementType(i)); } - return AddPossiblyDynamicValues( - Builder, HlslOP, HlslOP->GetU32Const(MemberOffset), - GetStructOffset(Builder, HlslOP, pGEP, GEPOperandIndex, + return AddConstIntValues( + m_DM->GetOP()->GetU32Const(MemberOffset), + GetStructOffset(pGEP, GEPOperandIndex, pStruct->getElementType(MemberIndex))); } else { - return HlslOP->GetU32Const(0); + return m_DM->GetOP()->GetU32Const(0); } } @@ -429,11 +426,10 @@ bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite( llvm::IRBuilder<> B(pGEP); - auto offset = - GetStructOffset(B, HlslOP, pGEP, GEPOperandIndex, pStructType); + auto offset = GetStructOffset(pGEP, GEPOperandIndex, pStructType); - llvm::Value *IndexValue = AddPossiblyDynamicValues( - B, HlslOP, offset, HlslOP->GetU32Const(precedingMemberCount)); + llvm::Value *IndexValue = + AddConstIntValues(offset, HlslOP->GetU32Const(precedingMemberCount)); if (IndexValue != nullptr) { *pAI = Alloca; diff --git a/tools/clang/test/HLSLFileCheck/pix/Debug_dynamic_array_index.hlsl b/tools/clang/test/HLSLFileCheck/pix/Debug_dynamic_array_index.hlsl new file mode 100644 index 0000000000..9ab5bce95a --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/Debug_dynamic_array_index.hlsl @@ -0,0 +1,19 @@ +// RUN: %dxc -Tcs_6_0 /Od %s | %opt -S -dxil-annotate-with-virtual-regs -hlsl-dxil-debug-instrumentation,UAVSize=128,upstreamSVPositionRow=2 -hlsl-dxilemit | %FileCheck %s + +// Check that there is a block precis that correctly returns that the array is a 4-value float array +// CHECK: Block#0 +// CHECK-SAME: d,0-4 + +RWByteAddressBuffer RawUAV: register(u1); + +[numthreads(1, 1, 1)] +void main() +{ + float local_array[4]; + local_array[RawUAV.Load(0)] = 8; + local_array[RawUAV.Load(1)] = 128; + + RawUAV.Store(64+0,local_array[0]); + RawUAV.Store(64+4,local_array[1]); +} + From 60f8ec41f3e422a9e41fb7496c39c2447dc844b7 Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Tue, 17 Jun 2025 09:39:40 -0700 Subject: [PATCH 7/9] reduce arguments --- .../DxilAnnotateWithVirtualRegister.cpp | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp index 5cca9ff8df..fd8a1d04b6 100644 --- a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp +++ b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp @@ -77,8 +77,8 @@ class DxilAnnotateWithVirtualRegister : public llvm::ModulePass { private: void AnnotateValues(llvm::Instruction *pI); void AnnotateStore(hlsl::OP *HlslOP, llvm::Instruction *pI); - void SplitVectorStores(hlsl::OP *HlslOP, llvm::Instruction *pI); - bool IsAllocaRegisterWrite(hlsl::OP *HlslOP, llvm::Value *V, + void SplitVectorStores(llvm::Instruction *pI); + bool IsAllocaRegisterWrite(llvm::Value *V, llvm::AllocaInst **pAI, llvm::Value **pIdx); void AnnotateAlloca(llvm::AllocaInst *pAlloca); void AnnotateGeneric(llvm::Instruction *pI); @@ -146,7 +146,7 @@ bool DxilAnnotateWithVirtualRegister::runOnModule(llvm::Module &M) { for (auto &block : F->getBasicBlockList()) { for (auto it = block.begin(); it != block.end();) { llvm::Instruction *I = &*(it++); - SplitVectorStores(m_DM->GetOP(), I); + SplitVectorStores(I); } } } @@ -248,8 +248,7 @@ void DxilAnnotateWithVirtualRegister::AnnotateStore(hlsl::OP *HlslOP, llvm::AllocaInst *Alloca; llvm::Value *Index; - if (!IsAllocaRegisterWrite(HlslOP, pSt->getPointerOperand(), &Alloca, - &Index)) { + if (!IsAllocaRegisterWrite(pSt->getPointerOperand(), &Alloca, &Index)) { return; } @@ -346,7 +345,7 @@ DxilAnnotateWithVirtualRegister::GetStructOffset(llvm::GetElementPtrInst *pGEP, } bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite( - hlsl::OP *HlslOP, llvm::Value *V, llvm::AllocaInst **pAI, + llvm::Value *V, llvm::AllocaInst **pAI, llvm::Value **pIdx) { *pAI = nullptr; @@ -429,7 +428,7 @@ bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite( auto offset = GetStructOffset(pGEP, GEPOperandIndex, pStructType); llvm::Value *IndexValue = - AddConstIntValues(offset, HlslOP->GetU32Const(precedingMemberCount)); + AddConstIntValues(offset, m_DM->GetOP()->GetU32Const(precedingMemberCount)); if (IndexValue != nullptr) { *pAI = Alloca; @@ -446,7 +445,7 @@ bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite( } *pAI = pAlloca; - *pIdx = HlslOP->GetU32Const(0); + *pIdx = m_DM->GetOP()->GetU32Const(0); return true; } @@ -532,8 +531,7 @@ void DxilAnnotateWithVirtualRegister::AssignNewAllocaRegister( } } -void DxilAnnotateWithVirtualRegister::SplitVectorStores(hlsl::OP *HlslOP, - llvm::Instruction *pI) { +void DxilAnnotateWithVirtualRegister::SplitVectorStores(llvm::Instruction *pI) { auto *pSt = llvm::dyn_cast(pI); if (pSt == nullptr) { return; @@ -541,8 +539,7 @@ void DxilAnnotateWithVirtualRegister::SplitVectorStores(hlsl::OP *HlslOP, llvm::AllocaInst *Alloca; llvm::Value *Index; - if (!IsAllocaRegisterWrite(HlslOP, pSt->getPointerOperand(), &Alloca, - &Index)) { + if (!IsAllocaRegisterWrite(pSt->getPointerOperand(), &Alloca, &Index)) { return; } From 17f991b1330cb1437dde1d58650ceb9913ae3833 Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Tue, 17 Jun 2025 09:44:23 -0700 Subject: [PATCH 8/9] clangformat --- lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp index fd8a1d04b6..a5be71f5b2 100644 --- a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp +++ b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp @@ -78,8 +78,8 @@ class DxilAnnotateWithVirtualRegister : public llvm::ModulePass { void AnnotateValues(llvm::Instruction *pI); void AnnotateStore(hlsl::OP *HlslOP, llvm::Instruction *pI); void SplitVectorStores(llvm::Instruction *pI); - bool IsAllocaRegisterWrite(llvm::Value *V, - llvm::AllocaInst **pAI, llvm::Value **pIdx); + bool IsAllocaRegisterWrite(llvm::Value *V, llvm::AllocaInst **pAI, + llvm::Value **pIdx); void AnnotateAlloca(llvm::AllocaInst *pAlloca); void AnnotateGeneric(llvm::Instruction *pI); void AssignNewDxilRegister(llvm::Instruction *pI); @@ -345,8 +345,7 @@ DxilAnnotateWithVirtualRegister::GetStructOffset(llvm::GetElementPtrInst *pGEP, } bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite( - llvm::Value *V, llvm::AllocaInst **pAI, - llvm::Value **pIdx) { + llvm::Value *V, llvm::AllocaInst **pAI, llvm::Value **pIdx) { *pAI = nullptr; *pIdx = nullptr; @@ -427,8 +426,8 @@ bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite( auto offset = GetStructOffset(pGEP, GEPOperandIndex, pStructType); - llvm::Value *IndexValue = - AddConstIntValues(offset, m_DM->GetOP()->GetU32Const(precedingMemberCount)); + llvm::Value *IndexValue = AddConstIntValues( + offset, m_DM->GetOP()->GetU32Const(precedingMemberCount)); if (IndexValue != nullptr) { *pAI = Alloca; From 872f3e2d0acbfb9724a85fd8f43d42b32c9618dd Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Tue, 17 Jun 2025 15:12:38 -0700 Subject: [PATCH 9/9] delete builder --- lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp index a5be71f5b2..88f696b7fa 100644 --- a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp +++ b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp @@ -422,8 +422,6 @@ bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite( // referenced in the current struct. If that type is an (n-dimensional) // array, then there follow n indices. - llvm::IRBuilder<> B(pGEP); - auto offset = GetStructOffset(pGEP, GEPOperandIndex, pStructType); llvm::Value *IndexValue = AddConstIntValues(