diff --git a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp index babf5b7953..88f696b7fa 100644 --- a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp +++ b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp @@ -76,19 +76,29 @@ class DxilAnnotateWithVirtualRegister : public llvm::ModulePass { private: void AnnotateValues(llvm::Instruction *pI); - void AnnotateStore(llvm::Instruction *pI); - void SplitVectorStores(hlsl::OP *HlslOP, 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); 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 *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; 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 +139,6 @@ bool DxilAnnotateWithVirtualRegister::runOnModule(llvm::Module &M) { m_DM->SetValidatorVersion(1, 4); } - std::uint32_t InstNum = m_StartInstruction; - auto instrumentableFunctions = PIXPassHelpers::GetAllInstrumentableFunctions(*m_DM); @@ -138,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); } } } @@ -151,17 +159,32 @@ bool DxilAnnotateWithVirtualRegister::runOnModule(llvm::Module &M) { } } + // Process all allocas referenced by dbg.declare intrinsics for (auto *F : instrumentableFunctions) { for (auto &block : F->getBasicBlockList()) { - for (llvm::Instruction &I : block.getInstList()) { - AnnotateStore(&I); + 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 +194,9 @@ 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 +212,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 +239,8 @@ 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; @@ -226,15 +256,47 @@ void DxilAnnotateWithVirtualRegister::AnnotateStore(llvm::Instruction *pI) { if (AllocaReg == nullptr) { return; } + m_RememberedAllocaStores.push_back({pSt, Index, AllocaReg}); +} + +llvm::Value * +DxilAnnotateWithVirtualRegister::MultiplyConstIntValue(llvm::Value *l, + uint32_t r) { + if (r == 1) + return l; + if (auto *lci = llvm::dyn_cast(l)) + 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; +} - PixAllocaRegWrite::AddMD(m_DM->GetCtx(), pSt, AllocaReg, Index); +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 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; } -static uint32_t GetStructOffset(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 0; + return m_DM->GetOP()->GetU32Const(0); } else if (auto *pArray = llvm::dyn_cast(pElementType)) { // 1D-array example: // @@ -248,18 +310,13 @@ 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 = 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"); @@ -268,7 +325,7 @@ static uint32_t GetStructOffset(llvm::GetElementPtrInst *pGEP, llvm::dyn_cast(pGEP->getOperand(GEPOperandIndex++)); if (pMemberIndex == nullptr) { - return 0; + return m_DM->GetOP()->GetU32Const(0); } uint32_t MemberIndex = pMemberIndex->getLimitedValue(); @@ -278,16 +335,17 @@ static uint32_t GetStructOffset(llvm::GetElementPtrInst *pGEP, MemberOffset += CountStructMembers(pStruct->getElementType(i)); } - return MemberOffset + GetStructOffset(pGEP, GEPOperandIndex, - pStruct->getElementType(MemberIndex)); + return AddConstIntValues( + m_DM->GetOP()->GetU32Const(MemberOffset), + GetStructOffset(pGEP, GEPOperandIndex, + pStruct->getElementType(MemberIndex))); } else { - return 0; + return m_DM->GetOP()->GetU32Const(0); } } bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite( llvm::Value *V, llvm::AllocaInst **pAI, llvm::Value **pIdx) { - llvm::IRBuilder<> B(m_DM->GetCtx()); *pAI = nullptr; *pIdx = nullptr; @@ -366,7 +424,8 @@ bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite( auto offset = GetStructOffset(pGEP, GEPOperandIndex, pStructType); - llvm::Value *IndexValue = B.getInt32(offset + precedingMemberCount); + llvm::Value *IndexValue = AddConstIntValues( + offset, m_DM->GetOP()->GetU32Const(precedingMemberCount)); if (IndexValue != nullptr) { *pAI = Alloca; @@ -383,7 +442,7 @@ bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite( } *pAI = pAlloca; - *pIdx = B.getInt32(0); + *pIdx = m_DM->GetOP()->GetU32Const(0); return true; } @@ -463,12 +522,13 @@ 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, - llvm::Instruction *pI) { +void DxilAnnotateWithVirtualRegister::SplitVectorStores(llvm::Instruction *pI) { auto *pSt = llvm::dyn_cast(pI); if (pSt == nullptr) { return; diff --git a/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp b/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp index bf25d9f85f..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 @@ -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,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(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..4dd43b07cc 100644 --- a/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp @@ -1356,7 +1356,19 @@ 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 +1386,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/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/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]); +} + 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]); +} + diff --git a/tools/clang/unittests/HLSL/PixTest.cpp b/tools/clang/unittests/HLSL/PixTest.cpp index af7801c7bf..c032e9e872 100644 --- a/tools/clang/unittests/HLSL/PixTest.cpp +++ b/tools/clang/unittests/HLSL/PixTest.cpp @@ -1220,7 +1220,6 @@ PixTest::TestableResults PixTest::TestStructAnnotationCase( #if 0 // handy for debugging auto disTextW = Disassemble(pAnnotatedContainer); - WEX::Logging::Log::Comment(disTextW.c_str()); #endif ModuleAndHangersOn moduleEtc(pAnnotatedContainer);