diff --git a/lib/DxilPIXPasses/DxilAddPixelHitInstrumentation.cpp b/lib/DxilPIXPasses/DxilAddPixelHitInstrumentation.cpp index 23bc224826..a1f6affdee 100644 --- a/lib/DxilPIXPasses/DxilAddPixelHitInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilAddPixelHitInstrumentation.cpp @@ -23,6 +23,11 @@ #include "PixPassHelpers.h" +#include "dxc/Support/Global.h" +#ifdef _WIN32 +#include +#endif + using namespace llvm; using namespace hlsl; @@ -41,7 +46,9 @@ class DxilAddPixelHitInstrumentation : public ModulePass { } void applyOptions(PassOptions O) override; bool runOnModule(Module &M) override; - unsigned m_upstreamSVPositionRow; + unsigned m_upstreamSVPositionRow = PIXPassHelpers::kUnknownSVPositionRow; + PIXPassHelpers::SVPositionRowAuthority m_svPositionRowAuthority = + PIXPassHelpers::SVPositionRowAuthority::Hint; }; void DxilAddPixelHitInstrumentation::applyOptions(PassOptions O) { @@ -49,8 +56,50 @@ void DxilAddPixelHitInstrumentation::applyOptions(PassOptions O) { GetPassOptionBool(O, "add-pixel-cost", &AddPixelCost, false); GetPassOptionInt(O, "rt-width", &RTWidth, 0); GetPassOptionInt(O, "num-pixels", &NumPixels, 0); - GetPassOptionUnsigned(O, "upstream-sv-position-row", &m_upstreamSVPositionRow, - 0); + + // RTWidth and NumPixels size the counter UAV and convert SV_Position to a + // byte offset into it. Reject a width or pixel count this pass cannot + // represent -- zero, negative, or large enough that the pixel-cost half's + // high water mark (NumPixels * 2 * 4 bytes) does not fit in 32 bits -- + // instead of emitting a shader whose offset arithmetic silently wraps. + if (RTWidth <= 0 || NumPixels <= 0 || + static_cast(NumPixels) * 2 * 4 > UINT32_MAX) { + throw ::hlsl::Exception( + E_FAIL, "PIX: the pixel-hit instrumentation was given a render " + "target width or pixel count it cannot represent."); + } + + // This option always sets a hint, never a required row: treating an + // unverified row as required could evict a real interpolant based on a + // guess. + // + // GetPassOptionUnsigned leaves the value untouched when the option is + // present but unparseable, so seed the member before the call rather than + // rely on the default argument. + // + // "upstream-sv-position-row" is the pre-rename spelling: old PIX versions + // predate this rename and still send it, so it is kept as an accepted + // alias indefinitely rather than only for a deprecation window. New + // callers should prefer "preferred-sv-position-row"; if both are + // supplied, the preferred spelling wins. + m_upstreamSVPositionRow = PIXPassHelpers::kUnknownSVPositionRow; + if (!GetPassOptionUnsigned(O, "preferred-sv-position-row", + &m_upstreamSVPositionRow, + PIXPassHelpers::kUnknownSVPositionRow)) { + GetPassOptionUnsigned(O, "upstream-sv-position-row", + &m_upstreamSVPositionRow, + PIXPassHelpers::kUnknownSVPositionRow); + } + m_svPositionRowAuthority = PIXPassHelpers::SVPositionRowAuthority::Hint; + + unsigned RequiredRow = PIXPassHelpers::kUnknownSVPositionRow; + GetPassOptionUnsigned(O, "required-sv-position-row", &RequiredRow, + PIXPassHelpers::kUnknownSVPositionRow); + if (RequiredRow != PIXPassHelpers::kUnknownSVPositionRow) { + m_upstreamSVPositionRow = RequiredRow; + m_svPositionRowAuthority = + PIXPassHelpers::SVPositionRowAuthority::Authoritative; + } } bool DxilAddPixelHitInstrumentation::runOnModule(Module &M) { @@ -66,13 +115,11 @@ bool DxilAddPixelHitInstrumentation::runOnModule(Module &M) { DM.m_ShaderFlags.SetForceEarlyDepthStencil(true); } - auto SV_Position_ID = - PIXPassHelpers::FindOrAddSV_Position(DM, m_upstreamSVPositionRow); + auto SV_Position_ID = PIXPassHelpers::FindOrAddSV_Position( + DM, m_upstreamSVPositionRow, m_svPositionRowAuthority); auto EntryPointFunction = PIXPassHelpers::GetEntryFunction(DM); - auto &EntryBlock = EntryPointFunction->getEntryBlock(); - CallInst *HandleForUAV; { IRBuilder<> Builder(dxilutil::FirstNonAllocaInsertionPt( @@ -83,18 +130,32 @@ bool DxilAddPixelHitInstrumentation::runOnModule(Module &M) { DM.ReEmitDxilResources(); } - // todo: is it a reasonable assumption that there will be a "Ret" in the entry - // block, and that these are the only points from which the shader can exit - // (except for a pixel-kill?) - auto &Instructions = EntryBlock.getInstList(); - auto It = Instructions.begin(); - while (It != Instructions.end()) { - auto ThisInstruction = It++; + // Every point where the shader completes must bump the counter. A + // straight-line shader keeps its Ret in the entry block, but a shader with + // a loop or branch ends the entry block early, so every basic block is + // scanned for a Ret. + llvm::SmallVector ReturnInstructions; + bool FunctionHasWork = false; + for (auto &ThisBlock : EntryPointFunction->getBasicBlockList()) { + for (auto &ThisInstruction : ThisBlock) { + LlvmInst_Ret Ret(&ThisInstruction); + if (Ret) { + ReturnInstructions.push_back(&ThisInstruction); + } else if (!llvm::isa(&ThisInstruction)) { + FunctionHasWork = true; + } + } + } + + bool Modified = false; + + for (auto ThisInstruction : ReturnInstructions) { LlvmInst_Ret Ret(ThisInstruction); if (Ret) { - // Check that there is at least one instruction preceding the Ret (no need - // to instrument it if there isn't) - if (ThisInstruction->getPrevNode() != nullptr) { + // A function that contains nothing but terminators has no pixel work + // worth counting. + if (FunctionHasWork) { + Modified = true; // Start adding instructions right before the Ret: IRBuilder<> Builder(ThisInstruction); @@ -110,7 +171,13 @@ bool DxilAddPixelHitInstrumentation::runOnModule(Module &M) { Constant *One32Arg = HlslOP->GetU32Const(1); Constant *One8Arg = HlslOP->GetI8Const(1); UndefValue *UndefArg = UndefValue::get(Type::getInt32Ty(Ctx)); - Constant *NumPixelsByteOffsetArg = HlslOP->GetU32Const(NumPixels * 4); + // Compute as uint32_t, not NumPixels' own int: applyOptions + // guarantees NumPixels * 2 * 4 fits in 32 bits only for unsigned + // arithmetic. The signed multiply would overflow int32 for a + // NumPixels this pass accepts, which is undefined behavior on the + // host, not just a wrapped value in the shader. + Constant *NumPixelsByteOffsetArg = + HlslOP->GetU32Const(static_cast(NumPixels) * 4u); // Step 1: Convert SV_POSITION to UINT Value *XAsInt; @@ -141,12 +208,31 @@ bool DxilAddPixelHitInstrumentation::runOnModule(Module &M) { // Step 2: Calculate pixel index Value *Index; { - Constant *RTWidthArg = HlslOP->GetI32Const(RTWidth); + Constant *RTWidthArg = + HlslOP->GetU32Const(static_cast(RTWidth)); auto YOffset = Builder.CreateMul(YAsInt, RTWidthArg, "YOffset"); auto Elementoffset = Builder.CreateAdd(XAsInt, YOffset, "ElementOffset"); - Index = Builder.CreateMul(Elementoffset, HlslOP->GetU32Const(4), - "ByteIndex"); + + // The viewport can be offset from the render target's origin, or + // smaller than the counter buffer PIX sized for it, so + // SV_Position's X and Y can land ElementOffset past the last valid + // element. Clamp the element count before scaling to a byte + // offset: applyOptions guarantees (NumPixels-1)*4 fits in uint32, + // so the clamped multiply cannot wrap. Clamping after scaling + // would let an oversized ElementOffset overflow the multiply + // first. + Function *UMinOpFunc = + HlslOP->GetOpFunc(OP::OpCode::UMin, Type::getInt32Ty(Ctx)); + Constant *UMinOpcode = + HlslOP->GetU32Const((unsigned)OP::OpCode::UMin); + Constant *LastElementArg = + HlslOP->GetU32Const(static_cast(NumPixels) - 1); + auto ClampedElementOffset = Builder.CreateCall( + UMinOpFunc, {UMinOpcode, Elementoffset, LastElementArg}, + "ClampedElementOffset"); + Index = Builder.CreateMul(ClampedElementOffset, + HlslOP->GetU32Const(4), "ByteIndex"); } // Insert the UAV increment instruction: @@ -188,7 +274,8 @@ bool DxilAddPixelHitInstrumentation::runOnModule(Module &M) { Type::getInt32Ty(Ctx)); Constant *LoadWeightOpcode = HlslOP->GetU32Const((unsigned)DXIL::OpCode::BufferLoad); - Constant *OffsetIntoUAV = HlslOP->GetU32Const(NumPixels * 2 * 4); + Constant *OffsetIntoUAV = + HlslOP->GetU32Const(static_cast(NumPixels) * 2u * 4u); auto WeightStruct = Builder.CreateCall( LoadWeight, { @@ -202,7 +289,9 @@ bool DxilAddPixelHitInstrumentation::runOnModule(Module &M) { WeightStruct, static_cast(0LL), "Weight"); } - // Step 2: Update write position ("Index") to second half of the UAV + // Step 2: Update write position ("Index") to second half of the UAV. + // Index is already clamped to the first half, so this can only land + // in the second half without a clamp of its own. auto OffsetIndex = Builder.CreateAdd(Index, NumPixelsByteOffsetArg, "OffsetByteIndex"); @@ -225,8 +314,6 @@ bool DxilAddPixelHitInstrumentation::runOnModule(Module &M) { } } - bool Modified = false; - return Modified; } diff --git a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp index 88f696b7fa..196da98c87 100644 --- a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp +++ b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp @@ -354,51 +354,70 @@ bool DxilAnnotateWithVirtualRegister::IsAllocaRegisterWrite( uint32_t precedingMemberCount = 0; auto *Alloca = llvm::dyn_cast(pGEP->getPointerOperand()); if (Alloca == nullptr) { - // In the case of vector types (floatN, matrixNxM), the pointer operand - // will actually point to another element pointer instruction. But this - // isn't a recursive thing- we only need to check these two levels. - if (auto *pPointerGEP = llvm::dyn_cast( - pGEP->getPointerOperand())) { - Alloca = - llvm::dyn_cast(pPointerGEP->getPointerOperand()); - if (Alloca == nullptr) { + // The pointer operand can itself be a GEP whenever the value being + // written is nested in more than one aggregate: a vector member of a + // struct (floatN, matrixNxM), or a struct member of a struct. Walk + // the whole chain of ancestor GEPs, outermost first, and require it + // to bottom out at an alloca. + llvm::SmallVector AncestorGEPs; + llvm::Value *PointerOperand = pGEP->getPointerOperand(); + while (auto *pAncestorGEP = + llvm::dyn_cast(PointerOperand)) { + AncestorGEPs.push_back(pAncestorGEP); + PointerOperand = pAncestorGEP->getPointerOperand(); + } + + Alloca = llvm::dyn_cast(PointerOperand); + if (Alloca == nullptr) { + return false; + } + + // Each level contributes the flattened count of whatever members precede + // the one it selects, so the offsets accumulate down the chain. + for (auto *pAncestorGEP : AncestorGEPs) { + auto *pStructType = llvm::dyn_cast( + pAncestorGEP->getPointerOperandType()->getPointerElementType()); + if (pStructType == nullptr) { + continue; + } + if (pAncestorGEP->getNumOperands() < 3) { + continue; + } + auto *pStructMember = + llvm::dyn_cast(pAncestorGEP->getOperand(2)); + if (pStructMember == nullptr) { + // A dynamically selected member has no constant offset; guessing + // zero would attribute the write to the wrong register. return false; } - // And of course the member we're after might not be at the beginning of - // any containing struct: - if (auto *pStructType = llvm::dyn_cast( - pPointerGEP->getPointerOperandType() - ->getPointerElementType())) { - auto *pStructMember = - llvm::dyn_cast(pPointerGEP->getOperand(2)); - uint64_t memberIndex = pStructMember->getLimitedValue(); - for (uint64_t i = 0; i < memberIndex; ++i) { - precedingMemberCount += - CountStructMembers(pStructType->getStructElementType(i)); - } + uint64_t memberIndex = pStructMember->getLimitedValue(); + if (memberIndex > pStructType->getStructNumElements()) { + return false; + } + for (uint64_t i = 0; i < memberIndex; ++i) { + precedingMemberCount += + CountStructMembers(pStructType->getStructElementType(i)); } + } - // And the source pointer may be a vector (floatn) type, - // and if so, that's another offset to consider. - llvm::Type *DestType = pGEP->getPointerOperand()->getType(); - // We expect this to be a pointer type (it's a GEP after all): - if (DestType->isPointerTy()) { - llvm::Type *PointedType = DestType->getPointerElementType(); - // Being careful to check num operands too in order to avoid false - // positives: - if (PointedType->isVectorTy() && pGEP->getNumOperands() == 3) { - // Fetch the second deref (in operand 2). - // (the first derefs the pointer to the "floatn", - // and the second denotes the index into the floatn.) - llvm::Value *vectorIndex = pGEP->getOperand(2); - if (auto *constIntIIndex = - llvm::cast(vectorIndex)) { - precedingMemberCount += constIntIIndex->getLimitedValue(); - } + // And the source pointer may be a vector (floatn) type, + // and if so, that's another offset to consider. + llvm::Type *DestType = pGEP->getPointerOperand()->getType(); + // We expect this to be a pointer type (it's a GEP after all): + if (DestType->isPointerTy()) { + llvm::Type *PointedType = DestType->getPointerElementType(); + // Being careful to check num operands too in order to avoid false + // positives: + if (PointedType->isVectorTy() && pGEP->getNumOperands() == 3) { + // Fetch the second deref (in operand 2). + // (the first derefs the pointer to the "floatn", + // and the second denotes the index into the floatn.) + llvm::Value *vectorIndex = pGEP->getOperand(2); + if (auto *constIntIIndex = + llvm::dyn_cast(vectorIndex)) { + precedingMemberCount += constIntIIndex->getLimitedValue(); } } - } else { - return false; } } diff --git a/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp b/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp index 9ddbe876b5..15a7e6c666 100644 --- a/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp +++ b/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp @@ -145,6 +145,22 @@ class OffsetManager { } } + // Move the aligned offset forward without adding padding to the packed + // offset. Overlapping debug information cannot move storage mappings + // backward. + void AdvanceAlignedOffsetTo(OffsetInBits AlignedOffset) { + if (AlignedOffset > m_CurrentAlignedOffset) { + VALUE_TO_DECLARE_LOG("Advancing aligned offset from %d to %d", + m_CurrentAlignedOffset, AlignedOffset); + m_CurrentAlignedOffset = AlignedOffset; + } else if (AlignedOffset < m_CurrentAlignedOffset) { + VALUE_TO_DECLARE_LOG("Refusing to move aligned offset back from %d to %d", + m_CurrentAlignedOffset, AlignedOffset); + // Keep existing mappings monotonic when debug information overlaps. + return; + } + } + // Add is used to "add" an aggregate element (struct field, array element) // at the current aligned/packed offsets, bumping them by Ty's size. Offsets Add(llvm::DIBasicType *Ty, unsigned sizeOverride) { @@ -443,63 +459,61 @@ DescendTypeAndFindEmbeddedArrayElements(llvm::StringRef VariableName, } else if (auto *CompositeTy = llvm::dyn_cast(Ty)) { switch (CompositeTy->getTag()) { case llvm::dwarf::DW_TAG_array_type: { + // DXC flattens a multi-dimensional array to a single one-dimensional + // array in the module. The true array extent is the product of the + // dimensions. + uint64_t TotalElementCount = 1; + bool FoundSubrange = false; for (auto Element : CompositeTy->getElements()) { - // First element for an array is DISubrange - if (auto Subrange = llvm::dyn_cast(Element)) { - auto ElementTy = CompositeTy->getBaseType().resolve(EmptyMap); - if (auto *BasicTy = llvm::dyn_cast(ElementTy)) { - bool CorrectLowerOffset = AccumulatedMemberOffset == OffsetToSeek; - bool CorrectUpperOffset = - AccumulatedMemberOffset + - Subrange->getCount() * BasicTy->getSizeInBits() == - OffsetToSeek + SizeToSeek; - if (BasicTy != nullptr && CorrectLowerOffset && - CorrectUpperOffset) { - std::vector storage; - for (int64_t i = 0; i < Subrange->getCount(); ++i) { - auto ElementOffset = - AccumulatedMemberOffset + i * BasicTy->getSizeInBits(); - GlobalEmbeddedArrayElementStorage element; - element.Name = VariableName.str() + "." + std::to_string(i); - element.Offset = static_cast(ElementOffset); - element.Size = - static_cast(BasicTy->getSizeInBits()); - storage.push_back(std::move(element)); - } - return storage; - } - } + if (auto *Subrange = llvm::dyn_cast(Element)) { + TotalElementCount *= Subrange->getCount(); + FoundSubrange = true; + } + } + + if (!FoundSubrange) { + break; + } - // If we didn't succeed and return above, then we need to process each - // element in the array + auto ElementTy = CompositeTy->getBaseType().resolve(EmptyMap); + if (ElementTy == nullptr) { + break; + } + + if (auto *BasicTy = llvm::dyn_cast(ElementTy)) { + const bool CorrectLowerOffset = AccumulatedMemberOffset == OffsetToSeek; + const bool CorrectUpperOffset = + AccumulatedMemberOffset + + TotalElementCount * BasicTy->getSizeInBits() == + OffsetToSeek + SizeToSeek; + if (CorrectLowerOffset && CorrectUpperOffset) { std::vector storage; - for (int64_t i = 0; i < Subrange->getCount(); ++i) { - auto elementStorage = DescendTypeAndFindEmbeddedArrayElements( - VariableName, - AccumulatedMemberOffset + ElementTy->getSizeInBits() * i, - ElementTy, OffsetToSeek, SizeToSeek); - std::move(elementStorage.begin(), elementStorage.end(), - std::back_inserter(storage)); - } - if (!storage.empty()) { - return storage; + for (uint64_t i = 0; i < TotalElementCount; ++i) { + auto ElementOffset = + AccumulatedMemberOffset + i * BasicTy->getSizeInBits(); + GlobalEmbeddedArrayElementStorage element; + element.Name = VariableName.str() + "." + std::to_string(i); + element.Offset = static_cast(ElementOffset); + element.Size = static_cast(BasicTy->getSizeInBits()); + storage.push_back(std::move(element)); } + return storage; } } - for (auto Element : CompositeTy->getElements()) { - // First element for an array is DISubrange - if (auto Subrange = llvm::dyn_cast(Element)) { - auto ElementType = CompositeTy->getBaseType().resolve(EmptyMap); - for (int64_t i = 0; i < Subrange->getCount(); ++i) { - auto storage = DescendTypeAndFindEmbeddedArrayElements( - VariableName, - AccumulatedMemberOffset + ElementType->getSizeInBits() * i, - ElementType, OffsetToSeek, SizeToSeek); - if (!storage.empty()) { - return storage; - } - } - } + + // The array's elements are themselves aggregates, so descend into each of + // them in turn looking for the sought offset. + std::vector storage; + for (uint64_t i = 0; i < TotalElementCount; ++i) { + auto elementStorage = DescendTypeAndFindEmbeddedArrayElements( + VariableName, + AccumulatedMemberOffset + ElementTy->getSizeInBits() * i, ElementTy, + OffsetToSeek, SizeToSeek); + std::move(elementStorage.begin(), elementStorage.end(), + std::back_inserter(storage)); + } + if (!storage.empty()) { + return storage; } } break; case llvm::dwarf::DW_TAG_structure_type: @@ -554,11 +568,18 @@ GlobalStorageMap GatherGlobalEmbeddedArrayStorage(llvm::Module &M) { if (auto *DIGVDerivedType = llvm::dyn_cast(DIGVType)) { if (DIGVDerivedType->getTag() == llvm::dwarf::DW_TAG_member) { - // This type is embedded within the containing DIGSV type + // This type is embedded within the containing DIGSV type. + // A flattened multi-dimensional array member renames the module + // global but not the debug variable, so only the linkage name + // still identifies it. + llvm::StringRef GlobalName = DIGV->getLinkageName(); + if (GlobalName.empty()) { + GlobalName = DIGV->getName(); + } const llvm::DITypeIdentifierMap EmptyMap; auto *Ty = HLSLStruct->getType().resolve(EmptyMap); auto Storage = DescendTypeAndFindEmbeddedArrayElements( - DIGV->getName(), 0, Ty, DIGVDerivedType->getOffsetInBits(), + GlobalName, 0, Ty, DIGVDerivedType->getOffsetInBits(), DIGVDerivedType->getSizeInBits()); auto &ArrayStorage = ret[HLSLStruct].ArrayElementStorage; std::move(Storage.begin(), Storage.end(), @@ -617,7 +638,8 @@ bool DxilDbgValueToDbgDeclare::runOnModule(llvm::Module &M) { // lists. for (auto &instruction : instructions) { if (auto *Store = llvm::dyn_cast(instruction)) { - Changed = + // Preserve changes reported by every processed store. + Changed |= handleStoreIfDestIsGlobal(M, GlobalEmbeddedArrayStorage, Store); } } @@ -847,6 +869,33 @@ static bool IsDITypePointer(DIType *DTy, return false; } +static bool HasPointerBackedCompositeCopy(llvm::DbgValueInst *DbgValue, + llvm::DIType *Ty) { + const llvm::DITypeIdentifierMap EmptyMap; + llvm::DIType *UnaliasedTy = DITypePeelTypeAlias(Ty); + if (!llvm::isa(DbgValue->getValue()) || + !llvm::isa(UnaliasedTy)) { + return false; + } + + for (llvm::BasicBlock &Block : *DbgValue->getParent()->getParent()) { + for (llvm::Instruction &Instruction : Block) { + auto *OtherDbgValue = llvm::dyn_cast(&Instruction); + if (OtherDbgValue == nullptr || OtherDbgValue == DbgValue || + !llvm::isa(OtherDbgValue->getValue())) { + continue; + } + + llvm::DIType *OtherTy = + OtherDbgValue->getVariable()->getType().resolve(EmptyMap); + if (OtherTy != nullptr && DITypePeelTypeAlias(OtherTy) == UnaliasedTy) { + return true; + } + } + } + return false; +} + void DxilDbgValueToDbgDeclare::handleDbgValue(llvm::Module &M, llvm::DbgValueInst *DbgValue) { VALUE_TO_DECLARE_LOG("DbgValue named %s", DbgValue->getName().str().c_str()); @@ -908,6 +957,11 @@ void DxilDbgValueToDbgDeclare::handleDbgValue(llvm::Module &M, } } + if (HasPointerBackedCompositeCopy(DbgValue, Ty)) { + VALUE_TO_DECLARE_LOG("Using pointer-backed composite storage"); + return; + } + auto &Register = m_Registers[Variable]; if (Register == nullptr) { Register.reset(new VariableRegisters( @@ -932,6 +986,10 @@ void DxilDbgValueToDbgDeclare::handleDbgValue(llvm::Module &M, const OffsetInBits InitialOffset = PackedOffsetFromVar; auto *insertPt = llvm::dyn_cast(ValueFromDbgInst); + if (insertPt == nullptr) { + // Constants and arguments are available at the dbg.value location. + insertPt = DbgValue; + } if (insertPt != nullptr && !llvm::isa(insertPt)) { insertPt = insertPt->getNextNode(); // Drivers may crash if phi nodes aren't always at the top of a block, @@ -965,11 +1023,28 @@ void DxilDbgValueToDbgDeclare::handleDbgValue(llvm::Module &M, continue; } - if (AllocaInst->getAllocatedType()->getArrayElementType() == - VO.m_V->getType()) { - auto *GEP = B.CreateGEP(AllocaInst, {Zero, Zero}); - B.CreateStore(VO.m_V, GEP); + llvm::Type *ShadowElementType = + AllocaInst->getAllocatedType()->getArrayElementType(); + llvm::Value *ValueToStore = VO.m_V; + if (ShadowElementType != ValueToStore->getType()) { + // Emit a bitcast to match the shadow alloca element type when the + // shader reinterprets the bits without converting them. + const llvm::DataLayout &DataLayout = M.getDataLayout(); + const bool SameWidth = + !ShadowElementType->isAggregateType() && + !ValueToStore->getType()->isAggregateType() && + !ShadowElementType->isPointerTy() && + !ValueToStore->getType()->isPointerTy() && + DataLayout.getTypeSizeInBits(ShadowElementType) == + DataLayout.getTypeSizeInBits(ValueToStore->getType()); + if (!SameWidth) { + continue; + } + ValueToStore = B.CreateBitCast(ValueToStore, ShadowElementType); } + + auto *GEP = B.CreateGEP(AllocaInst, {Zero, Zero}); + B.CreateStore(ValueToStore, GEP); } } } @@ -1158,7 +1233,13 @@ void VariableRegisters::PopulateAllocaMap(llvm::DIType *Ty) { case llvm::dwarf::DW_TAG_enumeration_type: { auto *baseType = CompositeTy->getBaseType().resolve(EmptyMap); if (baseType != nullptr) { + const OffsetInBits EnumerationStart = + m_Offsets.GetCurrentAlignedOffset(); PopulateAllocaMap(baseType); + // Advance to the enumeration's own declared size. + m_Offsets.AdvanceAlignedOffsetTo( + EnumerationStart + + static_cast(CompositeTy->getSizeInBits())); } else { m_Offsets.AlignToAndAddUnhandledType(CompositeTy); } @@ -1228,9 +1309,10 @@ void VariableRegisters::PopulateAllocaMap_BasicType(llvm::DIBasicType *Ty, auto *Storage = GetMetadataAsValue(llvm::ValueAsMetadata::get(Alloca)); auto *Variable = GetMetadataAsValue(m_Variable); - auto *Expression = GetMetadataAsValue( - GetDIExpression(Ty, sizeOverride == 0 ? offsets.Aligned : offsets.Packed, - GetVariableSizeInbits(m_Variable), sizeOverride)); + // Describe the aligned offset in the bit_piece so it agrees with the + // debug-info field's declared offset. + auto *Expression = GetMetadataAsValue(GetDIExpression( + Ty, offsets.Aligned, GetVariableSizeInbits(m_Variable), sizeOverride)); auto *DbgDeclare = m_B.CreateCall(m_DbgDeclareFn, {Storage, Variable, Expression}); DbgDeclare->setDebugLoc(m_dbgLoc); @@ -1261,7 +1343,6 @@ void VariableRegisters::PopulateAllocaMap_ArrayType(llvm::DICompositeType *Ty) { } const SizeInBits ArraySizeInBits = Ty->getSizeInBits(); - (void)ArraySizeInBits; const llvm::DITypeIdentifierMap EmptyMap; llvm::DIType *ElementTy = Ty->getBaseType().resolve(EmptyMap); @@ -1274,18 +1355,25 @@ void VariableRegisters::PopulateAllocaMap_ArrayType(llvm::DICompositeType *Ty) { // in bits. m_Offsets.AlignTo(ElementTy); + const OffsetInBits ArrayStart = m_Offsets.GetCurrentAlignedOffset(); + for (unsigned i = 0; i < NumElements; ++i) { // This is only needed if ElementTy's size is not a multiple of // its natural alignment. m_Offsets.AlignTo(ElementTy); PopulateAllocaMap(ElementTy); } + + // The elements only account for the bits they occupy, which stops short + // of the array's real end for a padded element type. Advance to the end. + m_Offsets.AdvanceAlignedOffsetTo(ArrayStart + ArraySizeInBits); } void VariableRegisters::PopulateAllocaMap_StructType( llvm::DICompositeType *Ty) { VALUE_TO_DECLARE_LOG("Struct type : %s, size %d", Ty->getName().str().c_str(), Ty->getSizeInBits()); + const SizeInBits StructSizeInBits = Ty->getSizeInBits(); std::map SortedMembers; if (!SortMembers(Ty, &SortedMembers)) { m_Offsets.AlignToAndAddUnhandledType(Ty); @@ -1294,7 +1382,6 @@ void VariableRegisters::PopulateAllocaMap_StructType( m_Offsets.AlignTo(Ty); const OffsetInBits StructStart = m_Offsets.GetCurrentAlignedOffset(); - (void)StructStart; const llvm::DITypeIdentifierMap EmptyMap; for (auto OffsetAndMember : SortedMembers) { @@ -1310,6 +1397,10 @@ void VariableRegisters::PopulateAllocaMap_StructType( // than the type in which it resides). If we were to take // the base type, then the information about the member's // size would be lost + // + // The AlignTo above is a no-op for a bitfield, so snap to the declared + // offset here to ensure it aligns with the debug info. + m_Offsets.AdvanceAlignedOffsetTo(StructStart + OffsetAndMember.first); PopulateAllocaMap(OffsetAndMember.second); } else { if (OffsetAndMember.second->getAlignInBits() == @@ -1326,6 +1417,11 @@ void VariableRegisters::PopulateAllocaMap_StructType( } } } + + // The members between them only account for the bits they occupy, which stops + // short of the struct's real end whenever the struct's alignment requires + // tail padding. Advance to the struct's full size. + m_Offsets.AdvanceAlignedOffsetTo(StructStart + StructSizeInBits); } // HLSL Change: remove unused function diff --git a/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp b/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp index a40acfe860..26f562f180 100644 --- a/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp @@ -235,6 +235,7 @@ struct InstructionAndType { DebugShaderModifierRecordType Type; std::uint32_t RegisterNumber; std::uint32_t AllocaBase; + std::uint32_t AllocaRegisterSize = 0; Value *AllocaWriteIndex = nullptr; std::optional ConstantAllocaStoreValue; }; @@ -283,7 +284,9 @@ class DxilDebugInstrumentation : public ModulePass { unsigned m_LastInstruction = static_cast(-1); uint64_t m_UAVSize = 1024 * 1024; - unsigned m_upstreamSVPositionRow; + unsigned m_upstreamSVPositionRow = PIXPassHelpers::kUnknownSVPositionRow; + PIXPassHelpers::SVPositionRowAuthority m_svPositionRowAuthority = + PIXPassHelpers::SVPositionRowAuthority::Hint; struct PerFunctionValues { CallInst *UAVHandle = nullptr; @@ -382,14 +385,38 @@ void DxilDebugInstrumentation::applyOptions(PassOptions O) { GetPassOptionUnsigned(O, "parameter1", &m_Parameters.Parameters[1], 0); GetPassOptionUnsigned(O, "parameter2", &m_Parameters.Parameters[2], 0); GetPassOptionUInt64(O, "UAVSize", &m_UAVSize, 1024 * 1024); + // The legacy option always sets a hint, never an authoritative row, + // matching DxilAddPixelHitInstrumentation: treating an unverified row as + // authoritative could evict a real interpolant based on a guess. + // + // GetPassOptionUnsigned leaves the value untouched when the option is + // present but unparseable, so seed the member before the call rather than + // rely on the default argument. + m_upstreamSVPositionRow = PIXPassHelpers::kUnknownSVPositionRow; GetPassOptionUnsigned(O, "upstreamSVPositionRow", &m_upstreamSVPositionRow, - 0); + PIXPassHelpers::kUnknownSVPositionRow); + m_svPositionRowAuthority = PIXPassHelpers::SVPositionRowAuthority::Hint; + + unsigned AuthoritativeRow = PIXPassHelpers::kUnknownSVPositionRow; + GetPassOptionUnsigned(O, "authoritativeSVPositionRow", &AuthoritativeRow, + PIXPassHelpers::kUnknownSVPositionRow); + if (AuthoritativeRow != PIXPassHelpers::kUnknownSVPositionRow) { + m_upstreamSVPositionRow = AuthoritativeRow; + m_svPositionRowAuthority = + PIXPassHelpers::SVPositionRowAuthority::Authoritative; + } } uint32_t DxilDebugInstrumentation::UAVDumpingGroundOffset() { return static_cast(m_UAVSize / 2); } +// Returned in place of a signature element ID when the element the selection +// prolog wanted could not be made available. See +// FindOrAddVSInSignatureElementForInstanceOrVertexID. +static constexpr unsigned kUnavailableSignatureElementID = + static_cast(-1); + unsigned int GetNextEmptyRow( std::vector> const &Elements) { unsigned int Row = 0; @@ -417,12 +444,27 @@ unsigned FindOrAddVSInSignatureElementForInstanceOrVertexID( }); if (ExistingElement == InputElements.end()) { + unsigned Row = GetNextEmptyRow(InputElements); + + // A signature holds at most kMaxSignatureTotalVectors registers. Each + // vertex-shader input gets its own register (PackingKind::InputAssembler), + // so a dense enough signature has no room for this element. Appending it + // anyway would write a register past the end of the signature, which the + // validator rejects; PIX does not re-validate what it patches, so the + // driver would see the failure instead. Report the element as + // unavailable and let the caller select an invocation with whatever + // identity remains. + if (Row >= hlsl::DXIL::kMaxSignatureTotalVectors) { + return kUnavailableSignatureElementID; + } + auto AddedElement = llvm::make_unique(DXIL::SigPointKind::VSIn); - unsigned Row = GetNextEmptyRow(InputElements); + // A vertex shader input is not interpolated, so the interpolation mode has + // to be Undefined; the validator rejects anything else on a VSIn element. AddedElement->Initialize( hlsl::Semantic::Get(semanticKind)->GetName(), hlsl::CompType::getU32(), - hlsl::DXIL::InterpolationMode::Constant, 1, 1, Row, 0); + hlsl::DXIL::InterpolationMode::Undefined, 1, 1, Row, 0); AddedElement->AppendSemanticIndex(0); AddedElement->SetKind(semanticKind); AddedElement->SetUsageMask(1); @@ -453,12 +495,34 @@ DxilDebugInstrumentation::addRequiredSystemValues(BuilderContext &BC, break; case DXIL::ShaderKind::Vertex: { hlsl::DxilSignature &InputSignature = BC.DM.GetInputSignature(); + size_t const ElementCountBeforeInjection = + InputSignature.GetElements().size(); SVIndices.VertexShader.VertexId = FindOrAddVSInSignatureElementForInstanceOrVertexID( InputSignature, hlsl::DXIL::SemanticKind::VertexID); SVIndices.VertexShader.InstanceId = FindOrAddVSInSignatureElementForInstanceOrVertexID( InputSignature, hlsl::DXIL::SemanticKind::InstanceID); + // Adding an input-signature element invalidates any ViewID dependency + // table in the module. + if (InputSignature.GetElements().size() != ElementCountBeforeInjection) { + PIXPassHelpers::ClearViewIdState(BC.DM); + } + // VertexID is asked for first on purpose: when the signature has room for + // only one more element, the vertex index is the more discriminating of + // the two, because a draw always has vertices and only sometimes has more + // than one instance. + char const *Selection = "None"; + if (SVIndices.VertexShader.VertexId != kUnavailableSignatureElementID) { + Selection = + SVIndices.VertexShader.InstanceId != kUnavailableSignatureElementID + ? "VertexIdAndInstanceId" + : "VertexIdOnly"; + } else if (SVIndices.VertexShader.InstanceId != + kUnavailableSignatureElementID) { + Selection = "InstanceIdOnly"; + } + *OSOverride << "VertexShaderSelection:" << Selection << "\n"; } break; case DXIL::ShaderKind::Geometry: case DXIL::ShaderKind::Hull: @@ -467,8 +531,8 @@ DxilDebugInstrumentation::addRequiredSystemValues(BuilderContext &BC, // in the input signature break; case DXIL::ShaderKind::Pixel: { - SVIndices.PixelShader.Position = - PIXPassHelpers::FindOrAddSV_Position(BC.DM, m_upstreamSVPositionRow); + SVIndices.PixelShader.Position = PIXPassHelpers::FindOrAddSV_Position( + BC.DM, m_upstreamSVPositionRow, m_svPositionRowAuthority); } break; default: assert(false); // guaranteed by runOnModule @@ -558,33 +622,63 @@ DxilDebugInstrumentation::addVertexShaderProlog(BuilderContext &BC, BC.HlslOP->GetOpFunc(DXIL::OpCode::LoadInput, Type::getInt32Ty(BC.Ctx)); Constant *LoadInputOpcode = BC.HlslOP->GetU32Const((unsigned)DXIL::OpCode::LoadInput); - Constant *SV_Vert_ID = - BC.HlslOP->GetU32Const(SVIndices.VertexShader.VertexId); - auto VertId = - BC.Builder.CreateCall(LoadInputOpFunc, - {LoadInputOpcode, SV_Vert_ID, Zero32Arg /*row*/, - Zero8Arg /*column*/, UndefArg}, - "VertId"); - - Constant *SV_Instance_ID = - BC.HlslOP->GetU32Const(SVIndices.VertexShader.InstanceId); - auto InstanceId = - BC.Builder.CreateCall(LoadInputOpFunc, - {LoadInputOpcode, SV_Instance_ID, Zero32Arg /*row*/, - Zero8Arg /*column*/, UndefArg}, - "InstanceId"); + + // A full input signature can leave this shader without one of these system + // values. One surviving value still narrows the selection to a smaller + // set, which is an acceptable approximation; if neither value is + // available, there is nothing left to narrow with, handled below. + auto LoadSystemValue = [&](unsigned ElementID, char const *Name) -> Value * { + if (ElementID == kUnavailableSignatureElementID) { + return nullptr; + } + return BC.Builder.CreateCall( + LoadInputOpFunc, + {LoadInputOpcode, BC.HlslOP->GetU32Const(ElementID), Zero32Arg /*row*/, + Zero8Arg /*column*/, UndefArg}, + Name); + }; + + Value *VertId = LoadSystemValue(SVIndices.VertexShader.VertexId, "VertId"); + Value *InstanceId = + LoadSystemValue(SVIndices.VertexShader.InstanceId, "InstanceId"); // Compare to expected vertex ID and instance ID - auto CompareToVert = BC.Builder.CreateICmpEQ( - VertId, BC.HlslOP->GetU32Const(m_Parameters.VertexShader.VertexId), - "CompareToVertId"); - auto CompareToInstance = BC.Builder.CreateICmpEQ( - InstanceId, BC.HlslOP->GetU32Const(m_Parameters.VertexShader.InstanceId), - "CompareToInstanceId"); - auto CompareBoth = - BC.Builder.CreateAnd(CompareToVert, CompareToInstance, "CompareBoth"); + Value *CompareToVert = + VertId == nullptr + ? nullptr + : BC.Builder.CreateICmpEQ( + VertId, + BC.HlslOP->GetU32Const(m_Parameters.VertexShader.VertexId), + "CompareToVertId"); + Value *CompareToInstance = + InstanceId == nullptr + ? nullptr + : BC.Builder.CreateICmpEQ( + InstanceId, + BC.HlslOP->GetU32Const(m_Parameters.VertexShader.InstanceId), + "CompareToInstanceId"); + + if (CompareToVert != nullptr && CompareToInstance != nullptr) { + return BC.Builder.CreateAnd(CompareToVert, CompareToInstance, + "CompareBoth"); + } + if (CompareToVert != nullptr) { + return CompareToVert; + } + if (CompareToInstance != nullptr) { + return CompareToInstance; + } - return CompareBoth; + // Neither identity is available, so select none: presenting an arbitrary + // vertex's trace as the one the user asked for would be misleading, and + // this lets PIX report the shader as undebuggable instead. + // VertexShaderSelection:None already records this case. + // + // GetOpFunc materializes the loadInput declaration before either system + // value's availability is known. With neither call emitted, the + // declaration is unused, which the validator rejects. + PIXPassHelpers::eraseIfUnused(BC.DM, LoadInputOpFunc); + return BC.HlslOP->GetI1Const(0); } Value *DxilDebugInstrumentation::addHullhaderProlog(BuilderContext &BC) { @@ -993,11 +1087,10 @@ std::optional DxilDebugInstrumentation::addStoreStepDebugEntry(BuilderContext *BC, StoreInst *Inst) { std::uint32_t ValueOrdinalBase; - std::uint32_t UnusedValueOrdinalSize; + std::uint32_t ValueOrdinalSize; llvm::Value *ValueOrdinalIndex; - if (!pix_dxil::PixAllocaRegWrite::FromInst(Inst, &ValueOrdinalBase, - &UnusedValueOrdinalSize, - &ValueOrdinalIndex)) { + if (!pix_dxil::PixAllocaRegWrite::FromInst( + Inst, &ValueOrdinalBase, &ValueOrdinalSize, &ValueOrdinalIndex)) { return std::nullopt; } @@ -1019,6 +1112,7 @@ DxilDebugInstrumentation::addStoreStepDebugEntry(BuilderContext *BC, ret.Type = *Type; ret.RegisterNumber = RegNum; ret.AllocaBase = ValueOrdinalBase; + ret.AllocaRegisterSize = ValueOrdinalSize; ret.AllocaWriteIndex = ValueOrdinalIndex; return ret; } @@ -1029,6 +1123,7 @@ DxilDebugInstrumentation::addStoreStepDebugEntry(BuilderContext *BC, ret.InstructionOrdinal = InstNum; ret.Type = *Type; ret.AllocaBase = ValueOrdinalBase; + ret.AllocaRegisterSize = ValueOrdinalSize; ret.AllocaWriteIndex = ValueOrdinalIndex; switch (ValueAsConst->getType()->getTypeID()) { @@ -1356,21 +1451,18 @@ DxilDebugInstrumentation::FindInstrumentableInstructionsInBlock( IndexingToken = "s"; // static indexing, no debug output required } 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(); - } - } - } + // The register span for a dynamic write comes from the + // !pix-alloca-reg-write metadata the annotation pass attached to + // this instruction, so it always matches the virtual-register + // numbering the annotation pass assigned. + uint32_t MaxArraySize = std::max(1u, IandT->AllocaRegisterSize); RegisterOrStaticIndex = std::to_string(IandT->AllocaBase) + "-" + std::to_string(MaxArraySize); DebugOutputForThisInstruction.ValueToWriteToDebugMemory = IandT->AllocaWriteIndex; + // Dynamic alloca records store the i32 index as a four-byte payload. + DebugOutputForThisInstruction.ValueType = + DebugShaderModifierRecordTypeDXILStepUint32; } } else { IndexingToken = "a"; // meaning an SSA assignment diff --git a/lib/DxilPIXPasses/DxilNonUniformResourceIndexInstrumentation.cpp b/lib/DxilPIXPasses/DxilNonUniformResourceIndexInstrumentation.cpp index 65be5e44b5..ea35002614 100644 --- a/lib/DxilPIXPasses/DxilNonUniformResourceIndexInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilNonUniformResourceIndexInstrumentation.cpp @@ -160,6 +160,8 @@ bool DxilNonUniformResourceIndexInstrumentation::runOnModule(Module &M) { modified |= PIXPassHelpers::eraseIfUnused(DM, AtomicOpFunc); if (modified) { + // Recompute shader flags after inserting WaveActiveAllEqual so the + // declared flags match the module. DM.CollectShaderFlagsForModule(); DM.ReEmitDxilResources(); diff --git a/lib/DxilPIXPasses/DxilOutputColorBecomesConstant.cpp b/lib/DxilPIXPasses/DxilOutputColorBecomesConstant.cpp index 7ecb712ea2..036858e55e 100644 --- a/lib/DxilPIXPasses/DxilOutputColorBecomesConstant.cpp +++ b/lib/DxilPIXPasses/DxilOutputColorBecomesConstant.cpp @@ -12,6 +12,7 @@ #include "dxc/DXIL/DxilModule.h" #include "dxc/DXIL/DxilOperations.h" +#include "dxc/DXIL/DxilTypeSystem.h" #include "dxc/DxilPIXPasses/DxilPIXPasses.h" #include "dxc/HLSL/DxilGenerationPass.h" #include "dxc/HLSL/DxilSpanAllocator.h" @@ -108,52 +109,78 @@ bool DxilOutputColorBecomesConstant::runOnModule(Module &M) { const hlsl::DxilSignature &OutputSignature = DM.GetOutputSignature(); - Function *FloatOutputFunction = - HlslOP->GetOpFunc(DXIL::OpCode::StoreOutput, Type::getFloatTy(Ctx)); - Function *IntOutputFunction = - HlslOP->GetOpFunc(DXIL::OpCode::StoreOutput, Type::getInt32Ty(Ctx)); + // dx.op.storeOutput has four legal overloads: f16, f32, i16 and i32. + // A min16float or min16int SV_Target lowers through the f16 or i16 form, + // as does a native half or int16_t target under -enable-16bit-types. + const std::array OverloadTypes{ + Type::getHalfTy(Ctx), Type::getFloatTy(Ctx), Type::getInt16Ty(Ctx), + Type::getInt32Ty(Ctx)}; - bool hasFloatOutputs = false; - bool hasIntOutputs = false; + std::array OutputFunctions{}; + size_t ActiveOverload = OverloadTypes.size(); - visitOutputInstructionCallers( - FloatOutputFunction, OutputSignature, HlslOP, - [&hasFloatOutputs](CallInst *) { hasFloatOutputs = true; }); + for (size_t OverloadIndex = 0; OverloadIndex < OverloadTypes.size(); + ++OverloadIndex) { + OutputFunctions[OverloadIndex] = HlslOP->GetOpFunc( + DXIL::OpCode::StoreOutput, OverloadTypes[OverloadIndex]); - visitOutputInstructionCallers( - IntOutputFunction, OutputSignature, HlslOP, - [&hasIntOutputs](CallInst *) { hasIntOutputs = true; }); + bool HasTargetZeroStores = false; + visitOutputInstructionCallers( + OutputFunctions[OverloadIndex], OutputSignature, HlslOP, + [&HasTargetZeroStores](CallInst *) { HasTargetZeroStores = true; }); + + if (HasTargetZeroStores) { + // visitOutputInstructionCallers filters on SemanticKind::Target with + // GetSemanticStartIndex() == 0, so at most one overload writes + // SV_Target0. + DXASSERT(ActiveOverload == OverloadTypes.size(), + "Only one storeOutput overload can write SV_Target0"); + ActiveOverload = OverloadIndex; + } + } - if (!hasFloatOutputs && !hasIntOutputs) { - bool Modified = PIXPassHelpers::eraseIfUnused(DM, FloatOutputFunction); - Modified |= PIXPassHelpers::eraseIfUnused(DM, IntOutputFunction); - return Modified; + // GetOpFunc materialises each overload declaration on demand. Any + // overload with no callers must be erased before the pass returns; the + // validator rejects a module carrying an unused dx.op declaration. + struct EraseUnusedOutputFunctionsOnExit { + hlsl::DxilModule &DM; + std::array &OutputFunctions; + ~EraseUnusedOutputFunctionsOnExit() { + for (Function *OutputFunction : OutputFunctions) { + PIXPassHelpers::eraseIfUnused(DM, OutputFunction); + } + } + } EraseUnusedOutputFunctions{DM, OutputFunctions}; + + if (ActiveOverload == OverloadTypes.size()) { + return false; } - // Otherwise, we assume the shader outputs only one or the other (because the - // 0th RTV can't have a mixed type) - DXASSERT(!hasFloatOutputs || !hasIntOutputs, - "Only one or the other type of output: float or int"); + // Replacement values must match the store's own overload type. + llvm::Type *const OutputValueType = OverloadTypes[ActiveOverload]; + const bool IsFloatOutput = OutputValueType->isFloatingPointTy(); std::array ReplacementColors; switch (Mode) { case FromLiteralConstant: { - if (hasFloatOutputs) { - ReplacementColors[0] = HlslOP->GetFloatConst(Red); - ReplacementColors[1] = HlslOP->GetFloatConst(Green); - ReplacementColors[2] = HlslOP->GetFloatConst(Blue); - ReplacementColors[3] = HlslOP->GetFloatConst(Alpha); - } - if (hasIntOutputs) { - ReplacementColors[0] = HlslOP->GetI32Const(static_cast(Red)); - ReplacementColors[1] = HlslOP->GetI32Const(static_cast(Green)); - ReplacementColors[2] = HlslOP->GetI32Const(static_cast(Blue)); - ReplacementColors[3] = HlslOP->GetI32Const(static_cast(Alpha)); + const std::array Channels{Red, Green, Blue, Alpha}; + for (size_t ChannelIndex = 0; ChannelIndex < Channels.size(); + ++ChannelIndex) { + ReplacementColors[ChannelIndex] = + IsFloatOutput + ? ConstantFP::get(OutputValueType, Channels[ChannelIndex]) + : ConstantInt::get(OutputValueType, + static_cast(static_cast( + Channels[ChannelIndex])), + /*isSigned*/ true); } } break; case FromConstantBuffer: { + // A float4 constant buffer row is 16 bytes wide. + constexpr unsigned int ConstantColorCBufferSizeInBytes = 4 * sizeof(float); + // Setup a constant buffer with a single float4 in it: SmallVector Elements{ Type::getFloatTy(Ctx), Type::getFloatTy(Ctx), Type::getFloatTy(Ctx), @@ -162,13 +189,31 @@ bool DxilOutputColorBecomesConstant::runOnModule(Module &M) { llvm::StructType::create(Elements, "PIX_ConstantColorCB_Type"); std::unique_ptr pCBuf = llvm::make_unique(); pCBuf->SetGlobalName("PIX_ConstantColorCBName"); - pCBuf->SetGlobalSymbol(UndefValue::get(CBStructTy)); + // The global symbol and HLSL type must be pointers to the struct so + // ValidateCBuffer can reach the annotation. + pCBuf->SetGlobalSymbol(UndefValue::get(CBStructTy->getPointerTo())); + pCBuf->SetHLSLType(CBStructTy->getPointerTo()); pCBuf->SetID(static_cast(DM.GetCBuffers().size())); pCBuf->SetSpaceID( (unsigned int)-2); // This is the reserved-for-tools register space pCBuf->SetLowerBound(0); pCBuf->SetRangeSize(1); - pCBuf->SetSize(4); + pCBuf->SetSize(ConstantColorCBufferSizeInBytes); + + auto *StructAnnotation = DM.GetTypeSystem().GetStructAnnotation(CBStructTy); + if (StructAnnotation == nullptr) { + StructAnnotation = DM.GetTypeSystem().AddStructAnnotation(CBStructTy); + StructAnnotation->SetCBufferSize(ConstantColorCBufferSizeInBytes); + static const char *const ComponentNames[] = {"r", "g", "b", "a"}; + for (unsigned int ComponentIndex = 0; ComponentIndex < 4; + ++ComponentIndex) { + auto &FieldAnnotation = + StructAnnotation->GetFieldAnnotation(ComponentIndex); + FieldAnnotation.SetCBufferOffset(ComponentIndex * sizeof(float)); + FieldAnnotation.SetCompType(hlsl::DXIL::ComponentType::F32); + FieldAnnotation.SetFieldName(ComponentNames[ComponentIndex]); + } + } Instruction *entryPointInstruction = &*(PIXPassHelpers::GetEntryFunction(DM)->begin()->begin()); @@ -188,9 +233,12 @@ bool DxilOutputColorBecomesConstant::runOnModule(Module &M) { #define PIX_CONSTANT_VALUE "PIX_Constant_Color_Value" // Insert the Buffer load instruction: - Function *CBLoad = HlslOP->GetOpFunc( - OP::OpCode::CBufferLoadLegacy, - hasFloatOutputs ? Type::getFloatTy(Ctx) : Type::getInt32Ty(Ctx)); + // The tools constant buffer is always four 32-bit components; PIX + // uploads that layout. + llvm::Type *const CBufferComponentType = + IsFloatOutput ? Type::getFloatTy(Ctx) : Type::getInt32Ty(Ctx); + Function *CBLoad = + HlslOP->GetOpFunc(OP::OpCode::CBufferLoadLegacy, CBufferComponentType); Constant *OpArg = HlslOP->GetU32Const((unsigned)OP::OpCode::CBufferLoadLegacy); Value *ResourceHandle = callCreateHandle; @@ -207,54 +255,47 @@ bool DxilOutputColorBecomesConstant::runOnModule(Module &M) { Builder.CreateExtractValue(loadLegacy, 2, PIX_CONSTANT_VALUE "2"); ReplacementColors[3] = Builder.CreateExtractValue(loadLegacy, 3, PIX_CONSTANT_VALUE "3"); + + // Narrow the loaded components to a 16-bit output overload. + if (OutputValueType != CBufferComponentType) { + static const char *const NarrowedNames[] = { + PIX_CONSTANT_VALUE "Narrowed0", PIX_CONSTANT_VALUE "Narrowed1", + PIX_CONSTANT_VALUE "Narrowed2", PIX_CONSTANT_VALUE "Narrowed3"}; + for (size_t ChannelIndex = 0; ChannelIndex < ReplacementColors.size(); + ++ChannelIndex) { + ReplacementColors[ChannelIndex] = + IsFloatOutput + ? Builder.CreateFPTrunc(ReplacementColors[ChannelIndex], + OutputValueType, + NarrowedNames[ChannelIndex]) + : Builder.CreateTrunc(ReplacementColors[ChannelIndex], + OutputValueType, + NarrowedNames[ChannelIndex]); + } + } } break; default: assert(false); - return 0; + return false; } bool Modified = false; - // The StoreOutput function can store either a float or an integer, depending - // on the intended output render-target resource view. - if (hasFloatOutputs) { - visitOutputInstructionCallers( - FloatOutputFunction, OutputSignature, HlslOP, - [&ReplacementColors, &Modified](CallInst *CallInstruction) { - Modified = true; - // The output column is the channel (red, green, blue or alpha) within - // the output pixel - Value *OutputColumnOperand = CallInstruction->getOperand( - hlsl::DXIL::OperandIndex::kStoreOutputColOpIdx); - ConstantInt *OutputColumnConstant = - cast(OutputColumnOperand); - APInt OutputColumn = OutputColumnConstant->getValue(); - CallInstruction->setOperand( - hlsl::DXIL::OperandIndex::kStoreOutputValOpIdx, - ReplacementColors[*OutputColumn.getRawData()]); - }); - } - - if (hasIntOutputs) { - visitOutputInstructionCallers( - IntOutputFunction, OutputSignature, HlslOP, - [&ReplacementColors, &Modified](CallInst *CallInstruction) { - Modified = true; - // The output column is the channel (red, green, blue or alpha) within - // the output pixel - Value *OutputColumnOperand = CallInstruction->getOperand( - hlsl::DXIL::OperandIndex::kStoreOutputColOpIdx); - ConstantInt *OutputColumnConstant = - cast(OutputColumnOperand); - APInt OutputColumn = OutputColumnConstant->getValue(); - CallInstruction->setOperand( - hlsl::DXIL::OperandIndex::kStoreOutputValOpIdx, - ReplacementColors[*OutputColumn.getRawData()]); - }); - } - - Modified |= PIXPassHelpers::eraseIfUnused(DM, FloatOutputFunction); - Modified |= PIXPassHelpers::eraseIfUnused(DM, IntOutputFunction); + visitOutputInstructionCallers( + OutputFunctions[ActiveOverload], OutputSignature, HlslOP, + [&ReplacementColors, &Modified](CallInst *CallInstruction) { + Modified = true; + // The output column is the channel (red, green, blue or alpha) within + // the output pixel + Value *OutputColumnOperand = CallInstruction->getOperand( + hlsl::DXIL::OperandIndex::kStoreOutputColOpIdx); + ConstantInt *OutputColumnConstant = + cast(OutputColumnOperand); + APInt OutputColumn = OutputColumnConstant->getValue(); + CallInstruction->setOperand( + hlsl::DXIL::OperandIndex::kStoreOutputValOpIdx, + ReplacementColors[*OutputColumn.getRawData()]); + }); return Modified; } diff --git a/lib/DxilPIXPasses/DxilReduceMSAAToSingleSample.cpp b/lib/DxilPIXPasses/DxilReduceMSAAToSingleSample.cpp index 01f06605a5..9cb685c647 100644 --- a/lib/DxilPIXPasses/DxilReduceMSAAToSingleSample.cpp +++ b/lib/DxilPIXPasses/DxilReduceMSAAToSingleSample.cpp @@ -13,6 +13,7 @@ #include "dxc/DXIL/DxilInstructions.h" #include "dxc/DXIL/DxilModule.h" +#include "dxc/DXIL/DxilResourceProperties.h" #include "dxc/DxilPIXPasses/DxilPIXPasses.h" #include "dxc/HLSL/DxilGenerationPass.h" @@ -34,50 +35,72 @@ class DxilReduceMSAAToSingleSample : public ModulePass { bool runOnModule(Module &M) override; }; -bool DxilReduceMSAAToSingleSample::runOnModule(Module &M) { - DxilModule &DM = M.GetOrCreateDxilModule(); +static bool IsMultisampledSRVHandle(Value *TextureHandle, DxilModule &DM) { + auto *TextureHandleInst = dyn_cast(TextureHandle); + if (!TextureHandleInst) + return false; + + if (OP::IsDxilOpFuncCallInst(TextureHandleInst, OP::OpCode::CreateHandle)) { + DxilInst_CreateHandle CreateHandle(TextureHandleInst); + if (!isa(CreateHandle.get_rangeId())) + return false; + + if (static_cast( + CreateHandle.get_resourceClass_val()) != DXIL::ResourceClass::SRV) + return false; + + unsigned RangeId = + cast(CreateHandle.get_rangeId())->getLimitedValue(); + auto Resource = DM.GetSRV(RangeId); + return Resource.GetKind() == DXIL::ResourceKind::Texture2DMS || + Resource.GetKind() == DXIL::ResourceKind::Texture2DMSArray; + } - LLVMContext &Ctx = M.getContext(); - OP *HlslOP = DM.GetOP(); + // SM 6.6 handles carry the resource kind in the annotateHandle + // properties operand. + if (OP::IsDxilOpFuncCallInst(TextureHandleInst, OP::OpCode::AnnotateHandle)) { + DxilInst_AnnotateHandle AnnotateHandle(TextureHandleInst); + DxilResourceProperties ResourceProperties = + resource_helper::loadPropsFromAnnotateHandle(AnnotateHandle, + *DM.GetShaderModel()); + return ResourceProperties.getResourceClass() == DXIL::ResourceClass::SRV && + (ResourceProperties.getResourceKind() == + DXIL::ResourceKind::Texture2DMS || + ResourceProperties.getResourceKind() == + DXIL::ResourceKind::Texture2DMSArray); + } - // FP16 type doesn't have its own identity, and is covered by float type... + return false; +} - auto TextureLoadOverloads = std::vector{ - Type::getFloatTy(Ctx), Type::getInt16Ty(Ctx), Type::getInt32Ty(Ctx)}; +bool DxilReduceMSAAToSingleSample::runOnModule(Module &M) { + DxilModule &DM = M.GetOrCreateDxilModule(); + OP *HlslOP = DM.GetOP(); bool Modified = false; - for (const auto &Overload : TextureLoadOverloads) { + // Iterate every materialised TextureLoad overload; the 16-bit form + // lowers Texture2DMS.Load. + for (const auto &TextureLoadOverload : + HlslOP->GetOpFuncList(DXIL::OpCode::TextureLoad)) { + Function *TexLoadFunction = TextureLoadOverload.second; + if (!TexLoadFunction) + continue; - Function *TexLoadFunction = - HlslOP->GetOpFunc(DXIL::OpCode::TextureLoad, Overload); - auto TexLoadFunctionUses = TexLoadFunction->uses(); - - for (auto FI = TexLoadFunctionUses.begin(); - FI != TexLoadFunctionUses.end();) { + for (auto FI = TexLoadFunction->use_begin(); + FI != TexLoadFunction->use_end();) { auto &FunctionUse = *FI++; - auto FunctionUser = FunctionUse.getUser(); - auto instruction = cast(FunctionUser); - DxilInst_TextureLoad LoadInstruction(instruction); - auto TextureHandle = LoadInstruction.get_srv(); - auto TextureHandleInst = cast(TextureHandle); - DxilInst_CreateHandle createHandle(TextureHandleInst); - // Dynamic rangeId is not supported - if (isa(createHandle.get_rangeId())) { - unsigned rangeId = - cast(createHandle.get_rangeId())->getLimitedValue(); - if (static_cast( - createHandle.get_resourceClass_val()) == - DXIL::ResourceClass::SRV) { - auto Resource = DM.GetSRV(rangeId); - if (Resource.GetKind() == DXIL::ResourceKind::Texture2DMS || - Resource.GetKind() == DXIL::ResourceKind::Texture2DMSArray) { - // "2" is the mip-level/sample-index operand index: - // https://github.com/Microsoft/DirectXShaderCompiler/blob/master/docs/DXIL.rst#textureload - instruction->setOperand(2, HlslOP->GetI32Const(0)); - Modified = true; - } - } + auto *InstructionUser = dyn_cast(FunctionUse.getUser()); + if (!InstructionUser) + continue; + + DxilInst_TextureLoad LoadInstruction(InstructionUser); + if (!LoadInstruction) + continue; + + if (IsMultisampledSRVHandle(LoadInstruction.get_srv(), DM)) { + LoadInstruction.set_mipLevelOrSampleCount(HlslOP->GetI32Const(0)); + Modified = true; } } } diff --git a/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp b/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp index ed1d1b26cc..c48533f85d 100644 --- a/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp +++ b/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp @@ -28,6 +28,8 @@ #include "llvm/Transforms/Utils/Local.h" #include +#include +#include #include "PixPassHelpers.h" @@ -174,8 +176,9 @@ struct RSRegisterIdentifier { unsigned Index; bool operator<(const RSRegisterIdentifier &o) const { - return static_cast(Type) < static_cast(o.Type) && - Space < o.Space && Index < o.Index; + return static_cast(Type) < static_cast(o.Type) || + (Type == o.Type && + (Space < o.Space || (Space == o.Space && Index < o.Index))); } }; @@ -428,7 +431,8 @@ bool DxilShaderAccessTracking::EmitResourceAccess(DxilModule &DM, if (isa(res.index) && res.indexDynamicOffset == nullptr) { unsigned index = cast(res.index)->getLimitedValue(); - if (index > slot->second.numSlots) { + // Index is 0-based, so numSlots is the first out-of-range value. + if (index >= slot->second.numSlots) { // out-of-range accesses are written to slot zero: slotIndex = HlslOP->GetU32Const(0); } else { @@ -544,11 +548,16 @@ bool DxilShaderAccessTracking::EmitResourceAccess(DxilModule &DM, Builder.CreateMul(ZeroIfOutOfBounds, EncodedFlags); uint32_t InstructionNumber = 0; (void)pix_dxil::PixDxilInstNum::FromInst(instruction, &InstructionNumber); - auto const *shaderModel = DM.GetShaderModel(); - auto shaderKind = shaderModel->GetKind(); - uint32_t EncodedInstructionNumber = InstructionNumber | - InstructionOrdinalndicator | - EncodeShaderModel(shaderKind); + auto *EncodedShaderKindConstant = cast( + m_FunctionToEncodedAccess.at(Builder.GetInsertBlock()->getParent()) + .at(ResourceAccessStyle::None)); + uint32_t EncodedShaderKind = EncodedShaderKindConstant->getLimitedValue(); + // The ordinal occupies the low 24 bits. Mask it so it does not overlap + // the indicator or the shader kind. + constexpr uint32_t InstructionOrdinalMask = 0x00FF'FFFF; + uint32_t EncodedInstructionNumber = + (InstructionNumber & InstructionOrdinalMask) | + InstructionOrdinalndicator | EncodedShaderKind; auto *MultipliedOutOfBoundsValue = Builder.CreateMul( OneIfOutOfBounds, HlslOP->GetU32Const(EncodedInstructionNumber)); auto *CombinedFlagOrInstructionValue = @@ -659,6 +668,17 @@ DxilResourceAndClass DxilShaderAccessTracking::DetermineAccessForHandleForLib( } } } + if (ret.registerType == RegisterType::Invalid) { + auto const &Samplers = DM.GetSamplers(); + for (auto &Sampler : Samplers) { + if (global == Sampler->GetGlobalSymbol()) { + binding = + hlsl::resource_helper::loadBindingFromResourceBase(Sampler.get()); + ret.registerType = RegisterType::Sampler; + break; + } + } + } if (ret.registerType != RegisterType::Invalid) { ret.accessStyle = AccessStyle::FromRootSig; ret.RegisterID = binding.rangeLowerBound; @@ -735,7 +755,8 @@ DxilShaderAccessTracking::GetResourceFromHandle(Value *resHandle, ret.index = createHandle.get_index(); ret.registerType = registerType; ret.accessStyle = AccessStyle::FromRootSig; - ret.RegisterID = resource->GetID(); + // RegisterID is the binding lower bound, not the resource-list ID. + ret.RegisterID = resource->GetLowerBound(); ret.RegisterSpace = resource->GetSpaceID(); } } @@ -757,6 +778,7 @@ DxilShaderAccessTracking::GetResourceFromHandle(Value *resHandle, ret.index = createHandleFromBinding.get_index(); ret.registerType = RegisterTypeFromResourceClass( static_cast(binding.resourceClass)); + ret.RegisterID = binding.rangeLowerBound; ret.RegisterSpace = binding.spaceID; } else if (hlsl::OP::IsDxilOpFuncCallInst( handleCreation, hlsl::OP::OpCode::CreateHandleFromHeap)) { @@ -795,6 +817,69 @@ DxilShaderAccessTracking::GetResourceFromHandle(Value *resHandle, return ret; } +// Map each function to the shader kind of the entry point that reaches it. +// A library helper has no DxilFunctionProps, so the module kind is Library, +// which PIX cannot attribute to a pipeline stage. If more than one entry +// kind reaches the same helper, keep the module kind. Entry points keep +// their own kind. +static std::map +ResolveShaderKindByReachingEntryPoint(DxilModule &DM) { + std::map functionToShaderKind; + + const DXIL::ShaderKind ambiguousShaderKind = DM.GetShaderModel()->GetKind(); + auto entryPoints = DM.GetExportedFunctions(); + + for (llvm::Function *entryPoint : entryPoints) { + if (entryPoint == nullptr || entryPoint->isDeclaration()) { + continue; + } + + const DXIL::ShaderKind entryPointShaderKind = + PIXPassHelpers::GetFunctionShaderKind(DM, entryPoint); + + std::vector pending{entryPoint}; + std::set visited; + while (!pending.empty()) { + llvm::Function *reached = pending.back(); + pending.pop_back(); + if (!visited.insert(reached).second) { + continue; + } + + auto emplaced = + functionToShaderKind.emplace(reached, entryPointShaderKind); + if (!emplaced.second && emplaced.first->second != entryPointShaderKind) { + emplaced.first->second = ambiguousShaderKind; + } + + for (llvm::BasicBlock &block : reached->getBasicBlockList()) { + for (llvm::Instruction &instruction : block.getInstList()) { + auto *call = llvm::dyn_cast(&instruction); + if (call == nullptr) { + continue; + } + llvm::Function *callee = call->getCalledFunction(); + if (callee == nullptr || callee->isDeclaration() || + callee->isIntrinsic() || hlsl::OP::IsDxilOpFunc(callee)) { + continue; + } + pending.push_back(callee); + } + } + } + } + + for (llvm::Function *entryPoint : entryPoints) { + if (entryPoint == nullptr || entryPoint->isDeclaration()) { + continue; + } + functionToShaderKind[entryPoint] = + PIXPassHelpers::GetFunctionShaderKind(DM, entryPoint); + } + + return functionToShaderKind; +} + bool DxilShaderAccessTracking::runOnModule(Module &M) { // This pass adds instrumentation for shader access to resources @@ -825,6 +910,8 @@ bool DxilShaderAccessTracking::runOnModule(Module &M) { auto instrumentableFunctions = PIXPassHelpers::GetAllInstrumentableFunctions(DM); + auto functionToShaderKind = ResolveShaderKindByReachingEntryPoint(DM); + if (DM.m_ShaderFlags.GetForceEarlyDepthStencil()) { if (OSOverride != nullptr) { formatted_raw_ostream FOS(*OSOverride); @@ -836,17 +923,11 @@ bool DxilShaderAccessTracking::runOnModule(Module &M) { PIXPassHelpers::CreateGlobalUAVResource(DM, 0u, "PIX_ShaderAccessUAV"); for (auto *F : instrumentableFunctions) { - DXIL::ShaderKind shaderKind = DXIL::ShaderKind::Invalid; - if (!DM.HasDxilFunctionProps(F)) { - auto ShaderModel = DM.GetShaderModel(); - shaderKind = ShaderModel->GetKind(); - if (shaderKind == DXIL::ShaderKind::Library) { - continue; - } - } else { - hlsl::DxilFunctionProps const &props = DM.GetDxilFunctionProps(F); - shaderKind = props.shaderKind; - } + auto reachedFrom = functionToShaderKind.find(F); + DXIL::ShaderKind shaderKind = + reachedFrom != functionToShaderKind.end() + ? reachedFrom->second + : PIXPassHelpers::GetFunctionShaderKind(DM, F); IRBuilder<> Builder(F->getEntryBlock().getFirstInsertionPt()); @@ -898,6 +979,15 @@ bool DxilShaderAccessTracking::runOnModule(Module &M) { // Special cases switch (opCode) { + case DXIL::OpCode::AnnotateHandle: + // annotateHandle attaches type information. It is not a resource + // access. GetResourceFromHandle still walks through it when a + // later access uses the annotated handle. + continue; + case DXIL::OpCode::BarrierByMemoryHandle: + // A barrier orders accesses to a resource. It is not itself an + // access. + continue; case DXIL::OpCode::GetDimensions: // readWrite = ShaderAccessFlags::DescriptorRead; // TODO: Support // GetDimensions @@ -935,9 +1025,13 @@ bool DxilShaderAccessTracking::runOnModule(Module &M) { } for (unsigned iParam : handleParams) { + auto uavHandle = + m_FunctionToUAVHandle.find(CallerParent->getParent()); + if (uavHandle == m_FunctionToUAVHandle.end()) + continue; + // Don't instrument the accesses to the UAV that we just added - if (Call->getArgOperand(iParam) == - m_FunctionToUAVHandle[CallerParent->getParent()]) + if (Call->getArgOperand(iParam) == uavHandle->second) continue; auto res = GetResourceFromHandle(Call->getArgOperand(iParam), DM); if (res.accessStyle == AccessStyle::None) { diff --git a/lib/DxilPIXPasses/LLVMBuild.txt b/lib/DxilPIXPasses/LLVMBuild.txt index 20428b278b..a41faeec6f 100644 --- a/lib/DxilPIXPasses/LLVMBuild.txt +++ b/lib/DxilPIXPasses/LLVMBuild.txt @@ -13,4 +13,4 @@ type = Library name = DxilPIXPasses parent = Libraries -required_libraries = BitReader Core DxcSupport TransformUtils Support +required_libraries = BitReader Core DxcSupport HLSL TransformUtils Support diff --git a/lib/DxilPIXPasses/PixPassHelpers.cpp b/lib/DxilPIXPasses/PixPassHelpers.cpp index 472cf77e79..9aa14c7846 100644 --- a/lib/DxilPIXPasses/PixPassHelpers.cpp +++ b/lib/DxilPIXPasses/PixPassHelpers.cpp @@ -9,11 +9,13 @@ #include "dxc/DXIL/DxilFunctionProps.h" #include "dxc/DXIL/DxilInstructions.h" +#include "dxc/DXIL/DxilMetadataHelper.h" #include "dxc/DXIL/DxilModule.h" #include "dxc/DXIL/DxilOperations.h" #include "dxc/DXIL/DxilResourceBinding.h" #include "dxc/DXIL/DxilResourceProperties.h" #include "dxc/DxilRootSignature/DxilRootSignature.h" +#include "dxc/HLSL/DxilPackSignatureElement.h" #include "dxc/HLSL/DxilSpanAllocator.h" #include "llvm/IR/IRBuilder.h" @@ -401,6 +403,18 @@ bool eraseIfUnused(hlsl::DxilModule &DM, llvm::Function *OpFunction) { return false; } +// A stale ViewID dependency table describes registers that do not match the +// module's current signature sizes. Clearing it removes both the module's +// cached copy and its IR metadata, so a downstream pass can recompute the +// table for the current signature. +void ClearViewIdState(hlsl::DxilModule &DM) { + DM.GetSerializedViewIdState().clear(); + if (auto *ViewIdStateMD = DM.GetModule()->getNamedMetadata( + hlsl::DxilMDHelper::kDxilViewIdStateMDName)) { + DM.GetModule()->eraseNamedMetadata(ViewIdStateMD); + } +} + // Set up a UAV with structure of a single int llvm::CallInst *CreateUAVOnceForModule(hlsl::DxilModule &DM, llvm::IRBuilder<> &Builder, @@ -513,8 +527,168 @@ void ReplaceAllUsesOfInstructionWithNewValueAndDeleteInstruction( delete Instr; } +// An authoritative row is mandatory: D3D12 matches signature elements +// between stages by register, so SV_Position on any other row fails +// pipeline creation with a linkage error. That row can already hold one of +// this shader's own elements, since pixel-shader-only system values +// (SV_IsFrontFace, SV_SampleIndex, SV_PrimitiveID without a geometry shader) +// pack after the interpolated attributes. +// +// Whatever occupies that row is safe to move: the upstream stage writes +// SV_Position there, so no upstream element shares that register, so +// nothing occupying it in this shader is linkage-bound. +// +// A hint carries no such guarantee and never displaces anything: SV_Position +// goes on a free row instead. +// +// Moving an element is metadata-only: dx.op.loadInput addresses elements by +// signature element ID and its row operand is relative to the element, so no +// instruction refers to the absolute row. +static std::vector FindElementsOccupyingSignatureRow( + std::vector> const &Elements, + unsigned int Row) { + std::vector Occupants; + for (auto const &Element : Elements) { + if (!Element->IsAllocated()) + continue; + unsigned int FirstRow = static_cast(Element->GetStartRow()); + if (Row >= FirstRow && Row < FirstRow + Element->GetRows()) + Occupants.push_back(Element.get()); + } + return Occupants; +} + +// Mirrors how the validator checks a pre-allocated element against the +// allocator: kInsufficientFreeComponents from the row check only says the row +// is partly used, which is exactly what packing two scalars into one register +// looks like, so the column check is what decides. +static bool ElementFitsAtLocation(hlsl::DxilSignatureAllocator &Allocator, + hlsl::DxilPackElement const &Element, + unsigned int Row, unsigned int Column) { + hlsl::DxilSignatureAllocator::ConflictType Conflict = + Allocator.DetectRowConflict(&Element, Row); + if (Conflict != hlsl::DxilSignatureAllocator::kNoConflict && + Conflict != hlsl::DxilSignatureAllocator::kInsufficientFreeComponents) { + return false; + } + return Allocator.DetectColConflict(&Element, Row, Column) == + hlsl::DxilSignatureAllocator::kNoConflict; +} + +// Gives Added_SV_Position a home -- TargetRow when the caller has one, +// otherwise wherever it fits -- and repacks whatever that displaces, using the +// same allocator the front end packs signatures with. +// +// A displaced element takes the first row that fits it, reusing gaps instead +// of appending past the end of the signature. DxilSignatureAllocator models +// rows, component columns, interpolation-mode and data-width compatibility, +// and the 32-register signature limit together, so no placement can exceed +// that limit. +// +// Returns false with every element left exactly where it was when the +// signature has no room, rather than emit an out-of-range register. +static bool PlaceSVPositionAndRepackDisplacedElements( + hlsl::DxilSignature &Signature, DxilSignatureElement &Added_SV_Position, + unsigned int TargetRow) { + auto const &Elements = Signature.GetElements(); + bool const UseMinPrecision = Signature.UseMinPrecision(); + + std::vector Displaced; + if (TargetRow != kUnknownSVPositionRow) + Displaced = FindElementsOccupyingSignatureRow(Elements, TargetRow); + + // The allocator takes raw pointers to these adapters and holds them across + // calls, so both vectors are sized up front and never grow afterwards. + std::vector Retained; + Retained.reserve(Elements.size()); + std::vector ToRepack; + ToRepack.reserve(Displaced.size()); + + for (auto const &Element : Elements) { + DxilSignatureElement *SignatureElement = Element.get(); + bool const Displacing = std::find(Displaced.begin(), Displaced.end(), + SignatureElement) != Displaced.end(); + // Elements the packer never places -- SV_Coverage and similar, whose + // interpretation is NotPacked -- use no register, and + // DxilSignatureAllocator asserts if handed one. A well-formed signature + // never marks such an element as allocated, so one occupying the target + // row means the signature is malformed. + if (!hlsl::DxilSignature::ShouldBeAllocated( + SignatureElement->GetInterpretation()) || + !SignatureElement->IsAllocated()) { + if (Displacing) + return false; + continue; + } + if (Displacing) { + ToRepack.emplace_back(SignatureElement, UseMinPrecision); + } else { + Retained.emplace_back(SignatureElement, UseMinPrecision); + } + } + + hlsl::DxilSignatureAllocator Allocator(hlsl::DXIL::kMaxSignatureTotalVectors, + UseMinPrecision); + + // Everything that is staying put keeps the register the front end gave it: + // those elements are paired with the upstream stage by row, so repacking them + // would break exactly the linkage this function exists to preserve. + for (hlsl::DxilPackElement &Element : Retained) { + unsigned int Row = Element.GetStartRow(); + unsigned int Column = Element.GetStartCol(); + if (!ElementFitsAtLocation(Allocator, Element, Row, Column)) { + // The signature handed to this pass already overlaps itself, so there + // is no consistent register layout to add to. Refuse rather than add + // another element on top of it. + return false; + } + Allocator.PlaceElement(&Element, Row, Column); + } + + hlsl::DxilPackElement PositionElement(&Added_SV_Position, UseMinPrecision); + if (TargetRow == kUnknownSVPositionRow) { + if (Allocator.PackNext(&PositionElement, 0, + hlsl::DXIL::kMaxSignatureTotalVectors) == 0) { + return false; + } + } else { + // SV_Position is four components wide, so it always starts at column 0 and + // owns the whole register once the occupants have been evicted. + if (!ElementFitsAtLocation(Allocator, PositionElement, TargetRow, 0)) { + return false; + } + Allocator.PlaceElement(&PositionElement, TargetRow, 0); + PositionElement.SetLocation(TargetRow, 0); + } + + // The target row must be reserved before displaced elements can be + // repacked around it. Their old locations are saved so a partial repack + // that runs out of registers can be undone. + std::vector> OriginalLocations; + OriginalLocations.reserve(ToRepack.size()); + for (hlsl::DxilPackElement &Element : ToRepack) { + OriginalLocations.emplace_back(Element.Get()->GetStartRow(), + Element.Get()->GetStartCol()); + } + + for (size_t Index = 0; Index < ToRepack.size(); ++Index) { + ToRepack[Index].ClearLocation(); + if (Allocator.PackNext(&ToRepack[Index], 0, + hlsl::DXIL::kMaxSignatureTotalVectors) == 0) { + for (size_t Undo = 0; Undo <= Index; ++Undo) { + ToRepack[Undo].Get()->SetStartRow(OriginalLocations[Undo].first); + ToRepack[Undo].Get()->SetStartCol(OriginalLocations[Undo].second); + } + return false; + } + } + + return true; +} + unsigned int FindOrAddSV_Position(hlsl::DxilModule &DM, - unsigned UpStreamSVPosRow) { + unsigned UpStreamSVPosRow, + SVPositionRowAuthority RowAuthority) { hlsl::DxilSignature &InputSignature = DM.GetInputSignature(); auto &InputElements = InputSignature.GetElements(); @@ -527,25 +701,92 @@ unsigned int FindOrAddSV_Position(hlsl::DxilModule &DM, // SV_Position, if present, has to have full mask, so we needn't worry // about the shader having selected components that don't include x or y. - // If not present, we add it. - if (Existing_SV_Position == InputElements.end()) { - unsigned int StartColumn = 0; - unsigned int RowCount = 1; - unsigned int ColumnCount = 4; - auto Added_SV_Position = - llvm::make_unique(DXIL::SigPointKind::PSIn); - Added_SV_Position->Initialize("Position", hlsl::CompType::getF32(), - hlsl::DXIL::InterpolationMode::Linear, - RowCount, ColumnCount, UpStreamSVPosRow, - StartColumn); - Added_SV_Position->AppendSemanticIndex(0); - Added_SV_Position->SetKind(hlsl::DXIL::SemanticKind::Position); - // AppendElement sets the element's ID by default - auto index = InputSignature.AppendElement(std::move(Added_SV_Position)); - return InputElements[index]->GetID(); - } else { + if (Existing_SV_Position != InputElements.end()) return Existing_SV_Position->get()->GetID(); + + constexpr unsigned int RowCount = 1; + constexpr unsigned int ColumnCount = 4; + + llvm::Function *EntryFunction = GetEntryFunction(DM); + hlsl::DXIL::ShaderKind ShaderKind = + EntryFunction != nullptr ? GetFunctionShaderKind(DM, EntryFunction) + : DM.GetShaderModel()->GetKind(); + + // Evicting an occupant is sound only for a pixel shader's input signature: + // the reasoning that the upstream stage writes SV_Position at this + // register, and so nothing else, assumes one flat register space. A mesh + // shader has two -- per-vertex and per-primitive, each numbered from zero + // and packed by different rules -- so the row says nothing about what else + // may be bound there. Mesh-to-pixel pipelines do not need the relocation + // anyway, since that pairing is matched by semantic name, not register. + // + // This only rules out the shader being instrumented here. Whether the + // upstream stage was a mesh shader is not visible from this module; the + // caller that read the upstream signature decides that by declining to + // claim the row is authoritative. + unsigned int TargetRow = kUnknownSVPositionRow; + if (UpStreamSVPosRow < hlsl::DXIL::kMaxSignatureTotalVectors) { + bool const RowIsOccupied = + !FindElementsOccupyingSignatureRow(InputElements, UpStreamSVPosRow) + .empty(); + bool const MayDisplaceOccupants = + RowAuthority == SVPositionRowAuthority::Authoritative && + ShaderKind == hlsl::DXIL::ShaderKind::Pixel; + if (!RowIsOccupied || MayDisplaceOccupants) + TargetRow = UpStreamSVPosRow; + } + + auto Added_SV_Position = + llvm::make_unique(DXIL::SigPointKind::PSIn); + // LinearNoperspective is the interpolation mode the front end gives a + // pixel shader that declares SV_Position itself, so an instrumented + // shader must match it: a driver honoring a different mode would hand the + // instrumentation perspective-divided coordinates, and PIX would silently + // attribute hits to the wrong pixel. + Added_SV_Position->Initialize( + "Position", hlsl::CompType::getF32(), + hlsl::DXIL::InterpolationMode::LinearNoperspective, RowCount, + ColumnCount); + Added_SV_Position->AppendSemanticIndex(0); + Added_SV_Position->SetKind(hlsl::DXIL::SemanticKind::Position); + + if (!PlaceSVPositionAndRepackDisplacedElements( + InputSignature, *Added_SV_Position, TargetRow)) { + // An authoritative row promises which register the upstream stage + // writes SV_Position to. Placing it elsewhere would read pixel position + // from a register nothing writes and misattribute PIX's results, so fail + // instead and let the caller drop the feature for this draw. + // + // A hint carries no such promise, so the free-row fallback is still + // usable. Emitting a register past the end of the signature is never an + // option: that is invalid DXIL, and PIX does not validate what it + // patches, so the module would reach the driver unchecked. + bool const RowWasPromised = + RowAuthority == SVPositionRowAuthority::Authoritative && + TargetRow != kUnknownSVPositionRow; + if (RowWasPromised) { + throw ::hlsl::Exception( + E_FAIL, "PIX: the shader's input signature cannot accommodate the " + "SV_Position element at the register the upstream stage " + "writes it to."); + } + if (TargetRow == kUnknownSVPositionRow || + !PlaceSVPositionAndRepackDisplacedElements( + InputSignature, *Added_SV_Position, kUnknownSVPositionRow)) { + throw ::hlsl::Exception( + E_FAIL, "PIX: the shader's input signature has no room for the " + "SV_Position element the instrumentation needs to read."); + } } + + // Adding an input-signature element invalidates any ViewID dependency + // table in the module: the table's size matches the previous element + // count. + ClearViewIdState(DM); + + // AppendElement sets the element's ID by default + auto index = InputSignature.AppendElement(std::move(Added_SV_Position)); + return InputElements[index]->GetID(); } bool ForEachDynamicallyIndexedResource( diff --git a/lib/DxilPIXPasses/PixPassHelpers.h b/lib/DxilPIXPasses/PixPassHelpers.h index 7dc13e1b7e..32dd0c1bf5 100644 --- a/lib/DxilPIXPasses/PixPassHelpers.h +++ b/lib/DxilPIXPasses/PixPassHelpers.h @@ -9,6 +9,7 @@ #pragma once +#include #include #include @@ -49,6 +50,10 @@ llvm::CallInst *CreateHandleForResource(hlsl::DxilModule &DM, const char *name); llvm::Function *GetEntryFunction(hlsl::DxilModule &DM); bool eraseIfUnused(hlsl::DxilModule &DM, llvm::Function *OpFunction); +// A stale ViewID dependency table describes registers that do not match the +// module's current signature sizes. Call after appending a signature +// element. +void ClearViewIdState(hlsl::DxilModule &DM); std::vector GetAllInstrumentableFunctions(hlsl::DxilModule &DM); hlsl::DXIL::ShaderKind GetFunctionShaderKind(hlsl::DxilModule &DM, @@ -81,8 +86,27 @@ ExpandedStruct ExpandStructType(llvm::LLVMContext &Ctx, llvm::Type *OriginalPayloadStructType); void ReplaceAllUsesOfInstructionWithNewValueAndDeleteInstruction( llvm::Instruction *Instr, llvm::Value *newValue, llvm::Type *newType); -unsigned int FindOrAddSV_Position(hlsl::DxilModule &DM, - unsigned UpStreamSVPosRow); +// Passed as UpStreamSVPosRow when the caller cannot determine which row the +// previous stage uses for SV_Position. See FindOrAddSV_Position. +constexpr unsigned kUnknownSVPositionRow = UINT_MAX; + +// States how much the caller of FindOrAddSV_Position knows about +// UpStreamSVPosRow. The row value alone cannot distinguish the two states, so +// the caller states its confidence explicitly. +enum class SVPositionRowAuthority { + // The row may not be genuine. SV_Position is placed there only if the row + // is free; nothing already in the signature moves. + Hint, + // The row is the register the previous stage writes SV_Position to. + // SV_Position lands there, and any occupant is repacked elsewhere. + Authoritative, +}; + +// Hint is the default: it cannot make an existing signature worse, because +// nothing already present is moved. +unsigned int FindOrAddSV_Position( + hlsl::DxilModule &DM, unsigned UpStreamSVPosRow, + SVPositionRowAuthority RowAuthority = SVPositionRowAuthority::Hint); bool ForEachDynamicallyIndexedResource( hlsl::DxilModule &DM, const std::function diff --git a/lib/HLSL/DxcOptimizer.cpp b/lib/HLSL/DxcOptimizer.cpp index 772cfcc3db..a035c874cf 100644 --- a/lib/HLSL/DxcOptimizer.cpp +++ b/lib/HLSL/DxcOptimizer.cpp @@ -43,6 +43,7 @@ #include #include // should change this for string_table +#include #include #include "llvm/PassPrinters/PassPrinters.h" @@ -518,14 +519,16 @@ HRESULT STDMETHODCALLTYPE DxcOptimizer::RunOptimizer( DXASSERT(PassInf->getNormalCtor(), "else pass with no default .ctor was added"); - Pass *pass = PassInf->getNormalCtor()(); + // Own the pass until the pass manager takes it, so it isn't leaked if + // applyOptions throws. + std::unique_ptr pass(PassInf->getNormalCtor()()); pass->setOSOverride(&outStream); pass->applyOptions(options); options.clear(); - pPassManager->add(pass); + const PassKind Kind = pass->getPassKind(); + pPassManager->add(pass.release()); if (AnalyzeOnly) { const bool Quiet = false; - PassKind Kind = pass->getPassKind(); switch (Kind) { case PT_BasicBlock: pPassManager->add( diff --git a/tools/clang/test/HLSLFileCheck/pix/AccessTrackingBarrierIsNotAnAccess.hlsl b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingBarrierIsNotAnAccess.hlsl new file mode 100644 index 0000000000..73da834b3e --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingBarrierIsNotAnAccess.hlsl @@ -0,0 +1,27 @@ +// RUN: %dxc -T cs_6_8 -E main -Od %s | %opt -S -hlsl-dxil-pix-shader-access-instrumentation,config=U0:0:2i0;.0;256;512. | %FileCheck %s + +// Barrier() on a resource handle orders accesses to that resource. It is +// not itself an access. +// +// The config puts the UAVs of space 0 at slot 0 onwards, so g_out is slot +// 0 and g_rw is slot 1. A slot is three dwords, so g_out's write dword is +// at byte 4 and g_rw's write dword is at byte 16. + +// g_rw is only barriered, never accessed, so nothing is recorded against +// it. +// CHECK-NOT: bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 16, + +// The store to g_out is a genuine write and is recorded. +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 4, + +// CHECK-NOT: bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 16, + +RWByteAddressBuffer g_out : register(u0); +RWTexture2D g_rw : register(u1); + +[numthreads(1, 1, 1)] +void main(uint index : SV_GroupIndex) +{ + Barrier(g_rw, DEVICE_SCOPE); + g_out.Store(0, 1); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/AccessTrackingLibAnnotateHandleIsNotAnAccess.hlsl b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingLibAnnotateHandleIsNotAnAccess.hlsl new file mode 100644 index 0000000000..d81de35182 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingLibAnnotateHandleIsNotAnAccess.hlsl @@ -0,0 +1,30 @@ +// RUN: %dxc -T lib_6_6 -Od %s | %opt -S -hlsl-dxil-pix-shader-access-instrumentation,config=S0:1:1i0;U0:2:1i0;.0;0;0. | %FileCheck %s + +// annotateHandle attaches type information to a handle. It is not a +// memory operation. g_untouched is only passed to GetDimensions, which +// this pass skips, so the annotation is that resource's only handle use. +// Nothing is recorded against it. +// +// The config puts the SRV of space 0 at slot 1 and the UAV of space 0 at +// slot 2. A slot is three dwords, so g_untouched's read dword is at byte +// 12 and its write dword at byte 16. g_output's write dword is at byte 28. + +// CHECK-NOT: bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 12, +// CHECK-NOT: bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 16, + +// The store to g_output is a genuine access and is recorded: +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 28, + +// CHECK-NOT: bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 12, +// CHECK-NOT: bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 16, + +Texture2D g_untouched : register(t0); +RWByteAddressBuffer g_output : register(u0); + +[shader("raygeneration")] +void RayGen() +{ + uint width, height; + g_untouched.GetDimensions(width, height); + g_output.Store(0, width + height); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/AccessTrackingLibAnnotatedHandleReadStillRecorded.hlsl b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingLibAnnotatedHandleReadStillRecorded.hlsl new file mode 100644 index 0000000000..b14588c0a4 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingLibAnnotatedHandleReadStillRecorded.hlsl @@ -0,0 +1,33 @@ +// RUN: %dxc -T lib_6_6 -Od %s | %opt -S -hlsl-dxil-pix-shader-access-instrumentation,config=S0:1:1i0;U0:2:1i0;.256;512;1024. | %FileCheck %s + +// annotateHandle is not an access, but a genuine access that uses an +// annotated handle still records the resource class from that annotation. +// +// Offsets with this config (SRV space 0 at slot 1, UAV space 0 at slot 2, +// three dwords per slot, descriptor-heap records at byte 256): +// g_input read slot 1, read dword -> 12 +// g_input write slot 1, write dword -> 16 (must not appear) +// g_output write slot 2, write dword -> 28 +// heapTexture read descriptor 3 -> 292 +// +// A descriptor-heap record encodes shader kind in its top four bits and +// ResourceAccessStyle in the next four. RayGeneration is 7 and SRVRead is +// 5, so 0x75000000 == 1962934272. + +// CHECK-NOT: bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 16, +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 12, +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 292, i32 undef, i32 1962934272, +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 28, +// CHECK-NOT: bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 16, + +ByteAddressBuffer g_input : register(t0); +RWByteAddressBuffer g_output : register(u0); + +[shader("raygeneration")] +void RayGen() +{ + Texture2D heapTexture = ResourceDescriptorHeap[3]; + uint value = g_input.Load(0); + value += asuint(heapTexture.Load(int3(0, 0, 0)).x); + g_output.Store(0, value); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/AccessTrackingLibHelperShaderKind.hlsl b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingLibHelperShaderKind.hlsl new file mode 100644 index 0000000000..a998d42218 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingLibHelperShaderKind.hlsl @@ -0,0 +1,31 @@ +// RUN: %dxc -T lib_6_6 -Od %s | %opt -S -hlsl-dxil-pix-shader-access-instrumentation,config=.256;512;1024. | %FileCheck %s + +// A descriptor-heap access record carries the shader kind of the entry +// point that reached the access. HeapHelper is [noinline] so the access +// stays in the helper. The descriptor index is a parameter so the helper +// is not folded away. +// +// The kind occupies bits 31:28. An out-of-bounds record sets the +// instruction-ordinal indicator (bit 27). RayGeneration is 7 and UAVWrite +// is 3, so the in-bounds flags are 0x73000000 == 1929379840 and the +// out-of-bounds value is 0x78000000 == 2013265920. Under the module kind, +// Library (6), those values would be 0x63000000 and 0x68000000. + +// CHECK: define void {{.*}}HeapHelper +// CHECK-NOT: 1660944384 +// CHECK: mul i32 {{.*}}, 1929379840 +// CHECK-NOT: 1744830464 +// CHECK: mul i32 {{.*}}, 2013265920 + +[noinline] +export void HeapHelper(uint descriptorIndex) +{ + RWByteAddressBuffer heapBuffer = ResourceDescriptorHeap[descriptorIndex]; + heapBuffer.Store(0, 1); +} + +[shader("raygeneration")] +void RayGen() +{ + HeapHelper(1); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_16bit_bitfields.hlsl b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_16bit_bitfields.hlsl new file mode 100644 index 0000000000..f60df66212 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_16bit_bitfields.hlsl @@ -0,0 +1,27 @@ +// RUN: %dxc -Tcs_6_6 -enable-16bit-types -Emain /Od /Zi %s | %opt -S -dxil-dbg-value-to-dbg-declare | %FileCheck %s + +// Verify that 16-bit and 32-bit bitfields use their declared storage units. + +RWByteAddressBuffer RawUAV : register(u0); + +struct HalfBitfield +{ + uint16_t Small : 5; + uint32_t Wide : 20; +}; + +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 0, 5) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 32, 20) + +// CHECK: store i16 %{{[^,]+}}, i16* +// CHECK: store i32 %{{[^,]+}}, i32* + +[numthreads(1, 1, 1)] +void main() +{ + HalfBitfield bitfield; + bitfield.Small = (uint16_t)RawUAV.Load(2 * 4); + bitfield.Wide = RawUAV.Load(3 * 4); + + RawUAV.Store(0, bitfield.Small + bitfield.Wide); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_16bit_tail_padding.hlsl b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_16bit_tail_padding.hlsl new file mode 100644 index 0000000000..f0a5f5291d --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_16bit_tail_padding.hlsl @@ -0,0 +1,37 @@ +// RUN: %dxc -Tcs_6_6 -enable-16bit-types -Emain /Od /Zi %s | %opt -S -dxil-dbg-value-to-dbg-declare | %FileCheck %s + +// Verify a 16-bit member after a tail-padded 64-bit struct. + +RWByteAddressBuffer RawUAV : register(u0); + +struct HalfTail +{ + float Wide; + float16_t Small; +}; + +struct Holder +{ + HalfTail Padded; + float16_t Trailing; +}; + +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 0, 32) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 32, 16) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 64, 16) + +// CHECK: store float +// CHECK: store half +// CHECK: store half + +[numthreads(1, 1, 1)] +void main() +{ + Holder holder; + holder.Padded.Wide = (float)RawUAV.Load(2 * 4); + holder.Padded.Small = (float16_t)RawUAV.Load(3 * 4); + holder.Trailing = (float16_t)RawUAV.Load(4 * 4); + + RawUAV.Store(0, holder.Padded.Wide + (float)holder.Padded.Small + + (float)holder.Trailing); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_aggregate_tail_padding.hlsl b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_aggregate_tail_padding.hlsl new file mode 100644 index 0000000000..4180e594f9 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_aggregate_tail_padding.hlsl @@ -0,0 +1,36 @@ +// RUN: %dxc -Tcs_6_6 -Emain /Od /Zi %s | %opt -S -dxil-dbg-value-to-dbg-declare | %FileCheck %s + +// Verify a member after a 128-bit struct with 32 bits of tail padding. + +RWByteAddressBuffer RawUAV : register(u0); + +struct TailPadded +{ + double Wide; + float Narrow; +}; + +struct Holder +{ + TailPadded Padded; + float Trailing; +}; + +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 0, 64) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 64, 32) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 128, 32) + +// CHECK: store double +// CHECK: store float +// CHECK: store float + +[numthreads(1, 1, 1)] +void main() +{ + Holder holder; + holder.Padded.Wide = (double)RawUAV.Load(2 * 4); + holder.Padded.Narrow = (float)RawUAV.Load(3 * 4); + holder.Trailing = (float)RawUAV.Load(7 * 4); + + RawUAV.Store(0, (float)holder.Padded.Wide + holder.Padded.Narrow + holder.Trailing); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_array_of_padded_structs.hlsl b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_array_of_padded_structs.hlsl new file mode 100644 index 0000000000..001a1353cc --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_array_of_padded_structs.hlsl @@ -0,0 +1,44 @@ +// RUN: %dxc -Tcs_6_6 -Emain /Od /Zi %s | %opt -S -dxil-dbg-value-to-dbg-declare | %FileCheck %s + +// Verify an array of 128-bit padded structs followed by another member. + +RWByteAddressBuffer RawUAV : register(u0); + +struct TailPadded +{ + double Wide; + float Narrow; +}; + +struct Holder +{ + TailPadded Elements[2]; + float Trailing; +}; + +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 0, 64) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 64, 32) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 128, 64) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 192, 32) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 256, 32) + +// CHECK: store double +// CHECK: store float +// CHECK: store double +// CHECK: store float +// CHECK: store float + +[numthreads(1, 1, 1)] +void main() +{ + Holder holder; + holder.Elements[0].Wide = (double)RawUAV.Load(2 * 4); + holder.Elements[0].Narrow = (float)RawUAV.Load(3 * 4); + holder.Elements[1].Wide = (double)RawUAV.Load(4 * 4); + holder.Elements[1].Narrow = (float)RawUAV.Load(5 * 4); + holder.Trailing = (float)RawUAV.Load(7 * 4); + + RawUAV.Store(0, (float)holder.Elements[0].Wide + holder.Elements[0].Narrow + + (float)holder.Elements[1].Wide + holder.Elements[1].Narrow + + holder.Trailing); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_constant_local.hlsl b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_constant_local.hlsl new file mode 100644 index 0000000000..38d82b5b92 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_constant_local.hlsl @@ -0,0 +1,18 @@ +// RUN: %dxc -T cs_6_0 -Od -Zi %s | %opt -S -dxil-dbg-value-to-dbg-declare | %FileCheck %s + +RWByteAddressBuffer RawUAV : register(u0); + +[numthreads(1, 1, 1)] +void main() +{ + float bias = 0.5; + bool hitFlag = true; + RawUAV.Store(0, asuint(bias + (hitFlag ? 2.0 : 3.0))); +} + +// CHECK: %[[hitFlag:.*]] = alloca [1 x i32] +// CHECK: %[[bias:.*]] = alloca [1 x float] +// CHECK: %[[bias_gep:.*]] = getelementptr [1 x float], [1 x float]* %[[bias]], i32 0, i32 0 +// CHECK: store float 5.000000e-01, float* %[[bias_gep]] +// CHECK: %[[hitFlag_gep:.*]] = getelementptr [1 x i32], [1 x i32]* %[[hitFlag]], i32 0, i32 0 +// CHECK: store i32 1, i32* %[[hitFlag_gep]] diff --git a/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_front_and_tail_padding.hlsl b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_front_and_tail_padding.hlsl new file mode 100644 index 0000000000..fc5a6e216b --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_front_and_tail_padding.hlsl @@ -0,0 +1,50 @@ +// RUN: %dxc -Tcs_6_6 -Emain /Od /Zi %s | %opt -S -dxil-dbg-value-to-dbg-declare | %FileCheck %s + +// Verify declared offsets for front-padded and tail-padded structs. + +RWByteAddressBuffer RawUAV : register(u0); + +struct FrontPadded +{ + float Narrow; + double Wide; +}; + +struct TailPadded +{ + double Wide; + float Narrow; +}; + +struct Holder +{ + FrontPadded Front; + TailPadded Tail; + float Trailing; +}; + +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 0, 32) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 64, 64) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 128, 64) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 192, 32) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 256, 32) + +// CHECK: store float +// CHECK: store double +// CHECK: store double +// CHECK: store float +// CHECK: store float + +[numthreads(1, 1, 1)] +void main() +{ + Holder holder; + holder.Front.Narrow = (float)RawUAV.Load(2 * 4); + holder.Front.Wide = (double)RawUAV.Load(3 * 4); + holder.Tail.Wide = (double)RawUAV.Load(4 * 4); + holder.Tail.Narrow = (float)RawUAV.Load(5 * 4); + holder.Trailing = (float)RawUAV.Load(6 * 4); + + RawUAV.Store(0, holder.Front.Narrow + (float)holder.Front.Wide + + (float)holder.Tail.Wide + holder.Tail.Narrow + holder.Trailing); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_matrix_after_padding.hlsl b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_matrix_after_padding.hlsl new file mode 100644 index 0000000000..d678350e3d --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_matrix_after_padding.hlsl @@ -0,0 +1,37 @@ +// RUN: %dxc -Tcs_6_6 -Emain /Od /Zi %s | %opt -S -dxil-dbg-value-to-dbg-declare | %FileCheck %s + +// Verify matrix offsets after a 128-bit tail-padded struct. + +RWByteAddressBuffer RawUAV : register(u0); + +struct TailPadded +{ + double Wide; + float Narrow; +}; + +struct Holder +{ + TailPadded Padded; + float2x2 Mat; +}; + +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 0, 64) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 64, 32) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 128, 32) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 160, 32) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 192, 32) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 224, 32) + +[numthreads(1, 1, 1)] +void main() +{ + Holder holder; + holder.Padded.Wide = (double)RawUAV.Load(2 * 4); + holder.Padded.Narrow = (float)RawUAV.Load(3 * 4); + holder.Mat = float2x2(RawUAV.Load(4 * 4), RawUAV.Load(5 * 4), + RawUAV.Load(6 * 4), RawUAV.Load(7 * 4)); + + RawUAV.Store(0, (float)holder.Padded.Wide + holder.Padded.Narrow + + holder.Mat._11 + holder.Mat._22); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_mixed_width_bitfields.hlsl b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_mixed_width_bitfields.hlsl new file mode 100644 index 0000000000..1a72d36ef4 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_mixed_width_bitfields.hlsl @@ -0,0 +1,31 @@ +// RUN: %dxc -Tcs_6_6 -Emain /Od /Zi %s | %opt -S -dxil-dbg-value-to-dbg-declare | %FileCheck %s + +// Verify that mixed-width bitfields use their declared storage units. + +RWByteAddressBuffer RawUAV : register(u0); + +struct MixedWidthBitfield +{ + uint32_t Leading : 5; + uint64_t Middle : 59; + uint32_t Trailing : 5; +}; + +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 0, 5) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 64, 59) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 128, 5) + +// CHECK: store i32 %{{[^,]+}}, i32* +// CHECK: store i64 %{{[^,]+}}, i64* +// CHECK: store i32 %{{[^,]+}}, i32* + +[numthreads(1, 1, 1)] +void main() +{ + MixedWidthBitfield bitfield; + bitfield.Leading = RawUAV.Load(9 * 4); + bitfield.Middle = RawUAV.Load(21 * 4); + bitfield.Trailing = RawUAV.Load(13 * 4); + + RawUAV.Store(0, (uint)(bitfield.Leading + bitfield.Middle + bitfield.Trailing)); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_multidim_array.hlsl b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_multidim_array.hlsl new file mode 100644 index 0000000000..e4eb97405a --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_multidim_array.hlsl @@ -0,0 +1,19 @@ +// RUN: %dxc -T cs_6_0 -Od -Zi %s | %opt -S -dxil-dbg-value-to-dbg-declare | %FileCheck %s + +static float MyArray[2][2] = { + { 1.0, 2.0 }, + { 3.0, 4.0 } +}; + +RWByteAddressBuffer RawUAV : register(u0); + +[numthreads(1, 1, 1)] +void main(uint3 tid : SV_DispatchThreadID) +{ + RawUAV.Store(0, asuint(MyArray[tid.x][tid.y])); +} + +// Verify stores for every flattened element of the multidimensional array. +// CHECK: store float 2.000000e+00, float* +// CHECK: store float 3.000000e+00, float* +// CHECK: store float 4.000000e+00, float* diff --git a/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_nested_aggregate_padding.hlsl b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_nested_aggregate_padding.hlsl new file mode 100644 index 0000000000..fa9ed43a92 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_nested_aggregate_padding.hlsl @@ -0,0 +1,46 @@ +// RUN: %dxc -Tcs_6_6 -Emain /Od /Zi %s | %opt -S -dxil-dbg-value-to-dbg-declare | %FileCheck %s + +// Verify declared offsets across nested tail-padded aggregates. + +RWByteAddressBuffer RawUAV : register(u0); + +struct Leaf +{ + double Wide; + float Narrow; +}; + +struct Middle +{ + Leaf Nested; + float AfterNested; +}; + +struct Root +{ + Middle Inner; + float Trailing; +}; + +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 0, 64) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 64, 32) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 128, 32) +// CHECK: dbg.declare{{.*}}!DIExpression(DW_OP_bit_piece, 192, 32) + +// CHECK: store double +// CHECK: store float +// CHECK: store float +// CHECK: store float + +[numthreads(1, 1, 1)] +void main() +{ + Root root; + root.Inner.Nested.Wide = (double)RawUAV.Load(2 * 4); + root.Inner.Nested.Narrow = (float)RawUAV.Load(3 * 4); + root.Inner.AfterNested = (float)RawUAV.Load(4 * 4); + root.Trailing = (float)RawUAV.Load(5 * 4); + + RawUAV.Store(0, (float)root.Inner.Nested.Wide + root.Inner.Nested.Narrow + + root.Inner.AfterNested + root.Trailing); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugAuthoritativeSVPositionRow.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugAuthoritativeSVPositionRow.hlsl new file mode 100644 index 0000000000..7b81887f3d --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DebugAuthoritativeSVPositionRow.hlsl @@ -0,0 +1,44 @@ +// RUN: %dxc -Emain -Tps_6_0 -Od %s | %opt -S -hlsl-dxil-debug-instrumentation,authoritativeSVPositionRow=0,UAVSize=65536 -hlsl-dxilemit | %FileCheck %s -check-prefixes=AUTHORITATIVE +// RUN: %dxc -Emain -Tps_6_0 -Od %s | %opt -S -hlsl-dxil-debug-instrumentation,upstreamSVPositionRow=0,UAVSize=65536 -hlsl-dxilemit | %FileCheck %s -check-prefixes=HINT + +// The debugger needs SV_Position to identify a pixel, and injects one when the +// shader does not declare it. Which register it lands on matters: the upstream +// stage writes position to a particular register, and reading it from any other +// gives the debugger coordinates nothing wrote. +// +// PIX cannot always read the upstream signature. Some PIX builds send row 0 +// both when the previous stage genuinely uses row 0 and when the row is +// unknown. The two cannot be distinguished by value, so they are +// distinguished by option name, and this pass has to honour that distinction +// the same way DxilAddPixelHitInstrumentation does. +// +// The shader below packs TEXCOORD0 and TEXCOORD1 into register 0, so the two +// spellings have visibly different correct answers. + +// Authoritative: the caller vouches for register 0, so SV_Position must land +// there and the TEXCOORDs are repacked out of the way. Checked with -DAG +// because the signature elements are emitted in element order, which puts the +// displaced TEXCOORDs ahead of the injected SV_Position. +// Row Col +// | | +// AUTHORITATIVE-DAG: !{i32 3, !"SV_Position", i8 9, i8 3, {{.*}}, i32 0, i8 0, null} +// AUTHORITATIVE-DAG: !{i32 0, !"TEXCOORD", i8 9, i8 0, {{.*}}, i32 2, i8 0, +// AUTHORITATIVE-DAG: !{i32 1, !"TEXCOORD", i8 9, i8 0, {{.*}}, i32 2, i8 2, + +// Hint: the row may have been fabricated, so nothing already in the signature +// is moved. The TEXCOORDs keep register 0 and SV_Position goes elsewhere. +// HINT-DAG: !{i32 0, !"TEXCOORD", i8 9, i8 0, {{.*}}, i32 0, i8 0, +// HINT-DAG: !{i32 1, !"TEXCOORD", i8 9, i8 0, {{.*}}, i32 0, i8 2, +// HINT-NOT: !{i32 3, !"SV_Position", i8 9, i8 3, {{.*}}, i32 0, i8 0, null} + +struct PSInput +{ + float2 firstUV : TEXCOORD0; + float2 secondUV : TEXCOORD1; + float4 color : COLOR0; +}; + +float4 main(PSInput input) : SV_Target +{ + return input.color + float4(input.firstUV, input.secondUV); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugBasic.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugBasic.hlsl index b211e22959..b019b46ca3 100644 --- a/tools/clang/test/HLSLFileCheck/pix/DebugBasic.hlsl +++ b/tools/clang/test/HLSLFileCheck/pix/DebugBasic.hlsl @@ -27,7 +27,7 @@ // See DxilMDHelper::EmitSignatureElement for the meaning of these entries: // ID TypeF32 SemKin Sem-Idx-Vec interp Rows Cols Row Col // | | | | | | | | | -// CHECK: !{i32 0, !"SV_Position", i8 9, i8 3, ![[SEMIDXVEC:[0-9]*]], i8 2, i32 1, i8 4, i32 2, i8 0, null} +// CHECK: !{i32 0, !"SV_Position", i8 9, i8 3, ![[SEMIDXVEC:[0-9]*]], i8 4, i32 1, i8 4, i32 2, i8 0, null} // CHECK: ![[SEMIDXVEC]] = !{i32 0} [RootSignature("")] diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugDenseVertexShaderInput.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugDenseVertexShaderInput.hlsl new file mode 100644 index 0000000000..86f1fc9db9 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DebugDenseVertexShaderInput.hlsl @@ -0,0 +1,43 @@ +// RUN: %dxc -Emain -Tvs_6_0 %s | %opt -S -hlsl-dxil-debug-instrumentation,parameter0=1,parameter1=2 -hlsl-dxilemit | %FileCheck %s + +// The debugger identifies a vertex shader invocation by (SV_VertexID, SV_InstanceID), +// injecting whichever of the two the shader did not already declare. A vertex shader +// input signature is allocated by the input assembler, which gives every element its +// own register, and D3D allows only 32 of them. The shader below uses 31 already, so +// there is room for exactly one injected system value. +// +// The pass injects as many as fit, most discriminating first, and reports which ones +// it got so PIX knows the selection is by vertex only. Selection is then exact for a +// single-instance draw and degrades to "first matching instance" otherwise, which is +// strictly better than refusing to debug the shader. + +// CHECK: VertexShaderSelection:VertexIdOnly + +// The injected SV_VertexID is element 1 (the 31-row ATTR array is a single element). +// CHECK: %VertId = call i32 @dx.op.loadInput.i32(i32 4, i32 1, i32 0, i8 0, i32 undef) +// Nothing may be loaded in between: there is no instance id to compare against. +// CHECK-NEXT: %CompareToVertId = icmp eq i32 %VertId, 1 +// CHECK-NEXT: br i1 %CompareToVertId, label %PIXInterestingBlock, label %PIXNonInterestingBlock + +// SV_VertexID must occupy the one free register, 31, and be the last thing injected. +// See DxilMDHelper::EmitSignatureElement for the meaning of these entries: +// ID TypeU32 SemKin Sem-Idx interp Rows Cols Row Col +// | | | | | | | | | +// CHECK: = !{i32 1, !"SV_VertexID", i8 5, i8 1, ![[VIDID:[0-9]*]], i8 0, i32 1, i8 1, i32 31, i8 0, + +// CHECK-NOT: !"SV_InstanceID" + +struct DenseVertexShaderInput +{ + float4 attributes[31] : ATTR; +}; + +float4 main(DenseVertexShaderInput input) : SV_Position +{ + float4 result = 0; + [unroll] for (uint index = 0; index < 31; ++index) + { + result += input.attributes[index]; + } + return result; +} diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugEmitCorrectViewIdStatePS.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugEmitCorrectViewIdStatePS.hlsl index 51eb86a8f9..41b90dd5a9 100644 --- a/tools/clang/test/HLSLFileCheck/pix/DebugEmitCorrectViewIdStatePS.hlsl +++ b/tools/clang/test/HLSLFileCheck/pix/DebugEmitCorrectViewIdStatePS.hlsl @@ -2,9 +2,16 @@ // CHECK: !dx.viewIdState = !{![[VIEWIDDATA:[0-9]*]]} -// The debug instrumentation will have added SV_Position to the input signature for this PS. -// If view id state is correct, then this entry should have expanded to 6 i32s (previously it would have been 4) -// CHECK: ![[VIEWIDDATA]] = !{[6 x i32] +// The debug instrumentation adds SV_Position to the input signature for this +// PS on a row of its own -- overlapping TEXCOORD's row produces a module the +// validator rejects -- so the signature spans two rows and view id state +// describes eight input components. +// +// The first two entries are the input and output component counts. The remaining +// eight are the output mask for each input component: TEXCOORD.x and .y are +// components 0 and 1, and "input.Tex.xyxy" makes them drive outputs {0,2} and +// {1,3} respectively. SV_Position's four components drive nothing. +// CHECK: ![[VIEWIDDATA]] = !{[10 x i32] [i32 8, i32 4, i32 5, i32 10, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0]} struct VS_OUTPUT { diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugInstrumentation_dynamic_index_span.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugInstrumentation_dynamic_index_span.hlsl new file mode 100644 index 0000000000..65c20c4f7e --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DebugInstrumentation_dynamic_index_span.hlsl @@ -0,0 +1,24 @@ +// RUN: %dxc -T cs_6_0 -Od -Zi %s | %opt -S -dxil-annotate-with-virtual-regs -hlsl-dxil-debug-instrumentation,UAVSize=1048576 | %FileCheck %s -check-prefix=SIZE +// RUN: %dxc -T cs_6_0 -Od -Zi %s | %opt -S -dxil-annotate-with-virtual-regs -hlsl-dxil-debug-instrumentation,UAVSize=1048576 | %FileCheck %s -check-prefix=NO-OLD + +RWByteAddressBuffer RawUAV : register(u0); + +[numthreads(1, 1, 1)] void main() { + double local_array[4]; + uint index = RawUAV.Load(0); + double value = asdouble(RawUAV.Load(4), RawUAV.Load(8)); + if (index < 4) { + local_array[index] = value; + } + RawUAV.Store(12, asuint((float)local_array[0])); +} + +// The store block reserves a 12-byte header and a 4-byte index payload. +// SIZE-LABEL: define void @main() +// SIZE: getelementptr inbounds [4 x double], [4 x double]* %{{[a-zA-Z0-9._]+}}, i32 0, i32 %{{[a-zA-Z0-9._]+}} +// SIZE: call i32 @dx.op.atomicBinOp.i32(i32 78, %dx.types.Handle {{.*}}, i32 0, i32 {{.*}}, i32 undef, i32 undef, i32 16) +// SIZE-LABEL: declare double @dx.op.makeDouble.f64 + +// NO-OLD-LABEL: define void @main() +// NO-OLD-NOT: i32 undef, i32 undef, i32 20) +// NO-OLD-LABEL: declare double @dx.op.makeDouble.f64 diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugVSParameters.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugVSParameters.hlsl index 4f629ee0d6..307a037284 100644 --- a/tools/clang/test/HLSLFileCheck/pix/DebugVSParameters.hlsl +++ b/tools/clang/test/HLSLFileCheck/pix/DebugVSParameters.hlsl @@ -13,12 +13,14 @@ // Check that the correct metadata was emitted for vertex id and instance id. // They should have 1 row, 1 column each. Vertex ID first at row 0, then instnce at row 1. // (With each row have the same value as the corresponding ID) +// A vertex shader input is not interpolated, so the interpolation mode has to be +// Undefined (0); the validator rejects anything else. // See DxilMDHelper::EmitSignatureElement for the meaning of these entries: // ID TypeU32 SemKin Sem-Idx interp Rows Cols Row Col // | | | | | | | | | -// CHECK: = !{i32 0, !"SV_VertexID", i8 5, i8 1, ![[VIDID:[0-9]*]], i8 1, i32 1, i8 1, i32 0, i8 0, +// CHECK: = !{i32 0, !"SV_VertexID", i8 5, i8 1, ![[VIDID:[0-9]*]], i8 0, i32 1, i8 1, i32 0, i8 0, // | | | | | | | | | -// CHECK: = !{i32 1, !"SV_InstanceID", i8 5, i8 2, ![[IID:[0-9]*]], i8 1, i32 1, i8 1, i32 1, i8 0, +// CHECK: = !{i32 1, !"SV_InstanceID", i8 5, i8 2, ![[IID:[0-9]*]], i8 0, i32 1, i8 1, i32 1, i8 0, [RootSignature("")] float4 main() : SV_Position{ return float4(0,0,0,0); diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugVertexShaderInputSignatureFull.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugVertexShaderInputSignatureFull.hlsl new file mode 100644 index 0000000000..5c14d19a63 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DebugVertexShaderInputSignatureFull.hlsl @@ -0,0 +1,47 @@ +// RUN: %dxc -Emain -Tvs_6_0 %s | %opt -S -hlsl-dxil-debug-instrumentation,parameter0=1,parameter1=2 -hlsl-dxilemit | %FileCheck %s + +// The companion to DebugDenseVertexShaderInput.hlsl, which uses 31 of the 32 +// input registers and so has room for one injected system value. This shader +// uses all 32, so neither SV_VertexID nor SV_InstanceID fits and the debugger +// has no identity at all to select an invocation by. +// +// Two things are checked below. +// +// First, the loadInput declaration must not survive unused: it is materialised +// before the pass knows whether either system value is available, and with +// neither call emitted it would be left behind as an unused external function, +// which the validator rejects with "External function 'dx.op.loadInput.i32' is +// unused". +// +// Second, the fallback selects no invocation rather than every invocation. +// Selecting every invocation would hand PIX an arbitrary vertex's trace to +// present as the one the user asked for. Selecting none is the honest answer: +// PIX reports the shader as undebuggable rather than debugging the wrong +// vertex. + +// CHECK: VertexShaderSelection:None + +// Neither identity is available, so no invocation is selected. "br i1 true" +// here would mean every vertex writes debug records. +// CHECK: br i1 false, label %PIXInterestingBlock, label %PIXNonInterestingBlock + +// The integer loadInput overload must not survive as an unused declaration. +// The shader's own attributes are float, so any .i32 loadInput at all - call +// or declare - means the orphan is back. Checked after the branch above so +// this scans the declaration block at the end of the module. +// CHECK-NOT: loadInput.i32 + +struct DenseVertexShaderInput +{ + float4 attributes[32] : ATTR; +}; + +float4 main(DenseVertexShaderInput input) : SV_Position +{ + float4 result = 0; + [unroll] for (uint index = 0; index < 32; ++index) + { + result += input.attributes[index]; + } + return result; +} diff --git a/tools/clang/test/HLSLFileCheck/pix/GeometryShaderMultiStreamSignatureIsNotRelocated.hlsl b/tools/clang/test/HLSLFileCheck/pix/GeometryShaderMultiStreamSignatureIsNotRelocated.hlsl new file mode 100644 index 0000000000..500c1f471c --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/GeometryShaderMultiStreamSignatureIsNotRelocated.hlsl @@ -0,0 +1,60 @@ +// RUN: %dxc -Emain -Tgs_6_0 %s | %opt -S -hlsl-dxil-debug-instrumentation,UAVSize=128,parameter0=1,parameter1=2,upstreamSVPositionRow=0 | %FileCheck %s + +// A geometry shader can write several output streams, each with its own +// register space, and only one of them is rasterized. This shader deliberately +// puts SV_Position on register 0 of stream 0 and register 1 of stream 1, so +// "the row SV_Position is on" is ambiguous unless the stream is part of the +// question. +// +// Whoever reads this signature to decide where a downstream pixel shader should +// expect SV_Position has to filter on the rasterized stream; picking the first +// SV_Position in the list gets register 0 here, which is the wrong answer +// whenever stream 1 is the rasterized one. The instrumentation itself never +// relocates a geometry shader signature, and this test pins that down: all four +// elements have to come out exactly as the front end packed them, so that a +// stream-blind row query cannot be papered over by a relocation. +// +// As with MeshShaderSignatureIsNotRelocated.hlsl, this pins the end-to-end +// behaviour rather than the ShaderKind guard in FindOrAddSV_Position: the +// debug-instrumentation pass never reaches that helper for a geometry shader, +// and the checks below are on the output signature while the relocation only +// touches the input one. Removing the guard would leave this test green. + +struct FirstStreamOut +{ + float4 position : SV_Position; + float2 uv : TEXCOORD0; +}; + +struct SecondStreamOut +{ + float2 uv : TEXCOORD0; + float4 position : SV_Position; +}; + +[maxvertexcount(3)] +void main(triangle float4 input[3] : SV_Position, + inout PointStream firstStream, + inout PointStream secondStream) +{ + FirstStreamOut first = (FirstStreamOut)0; + first.position = input[0]; + first.uv = float2(1, 2); + firstStream.Append(first); + + SecondStreamOut second = (SecondStreamOut)0; + second.position = input[1]; + second.uv = float2(3, 4); + secondStream.Append(second); +} + +// The pass really did run, so the signature checks below are not vacuous. +// CHECK: call i32 @dx.op.primitiveID.i32(i32 108) + +// Stream 0: SV_Position at register 0, TEXCOORD0 at register 1. +// CHECK-DAG: !{i32 0, !"SV_Position", i8 9, i8 3, !{{[0-9]+}}, i8 4, i32 1, i8 4, i32 0, i8 0, {{.*}}} +// CHECK-DAG: !{i32 1, !"TEXCOORD", i8 9, i8 0, !{{[0-9]+}}, i8 2, i32 1, i8 2, i32 1, i8 0, {{.*}}} + +// Stream 1: the same two semantics, at the opposite registers. +// CHECK-DAG: !{i32 2, !"TEXCOORD", i8 9, i8 0, !{{[0-9]+}}, i8 2, i32 1, i8 2, i32 0, i8 0, {{.*}}} +// CHECK-DAG: !{i32 3, !"SV_Position", i8 9, i8 3, !{{[0-9]+}}, i8 4, i32 1, i8 4, i32 1, i8 0, {{.*}}} diff --git a/tools/clang/test/HLSLFileCheck/pix/MeshShaderSignatureIsNotRelocated.hlsl b/tools/clang/test/HLSLFileCheck/pix/MeshShaderSignatureIsNotRelocated.hlsl new file mode 100644 index 0000000000..7831d66cbc --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/MeshShaderSignatureIsNotRelocated.hlsl @@ -0,0 +1,64 @@ +// RUN: %dxc -Emain -Tms_6_5 %s | %opt -S -hlsl-dxil-debug-instrumentation,UAVSize=128,parameter0=10,parameter1=20,parameter2=30,upstreamSVPositionRow=0 | %FileCheck %s + +// A mesh shader is the one upstream stage whose signature the SV_Position +// relocation cannot reason about. The relocation rests on the claim that if the +// previous stage writes SV_Position at register N then it writes nothing else +// there, so anything the pixel shader has at register N is unpaired and safe to +// move. A mesh shader has two output signatures, per-vertex and per-primitive, +// each numbered from zero and packed by its own rules -- as this shader shows, +// with SV_Position at per-vertex register 0 and a per-primitive attribute also +// at register 0 -- so "register N" does not identify one thing to reason about. +// +// The instrumentation therefore leaves mesh shader signatures alone even when +// it is handed a row, and this test pins that down: the three signature +// elements have to come out exactly as the front end packed them. +// +// Note on what this does and does not cover. There are two independent reasons +// a mesh shader is unaffected: the ShaderKind guard in FindOrAddSV_Position, and +// the fact that DxilDebugInstrumentation only calls it for pixel shaders at all. +// The second alone is enough to make this test pass, and the checks below are on +// the *output* signature whereas the relocation only ever touches the *input* +// one, so deleting the ShaderKind guard would not turn this test red. It +// documents the front-end packing the guard's rationale rests on (two register +// spaces, both numbered from zero) and pins the end-to-end behaviour; it is not +// a unit test of the guard itself. + +struct VertexOut +{ + float4 position : SV_Position; + float2 uv : TEXCOORD0; +}; + +struct PrimitiveOut +{ + uint layer : TEXCOORD1; +}; + +[outputtopology("triangle")] +[numthreads(3, 1, 1)] +void main(uint threadIndex : SV_GroupIndex, + out vertices VertexOut vertices[3], + out primitives PrimitiveOut primitives[1], + out indices uint3 indices[1]) +{ + SetMeshOutputCounts(3, 1); + vertices[threadIndex].position = float4(threadIndex, 0, 0, 1); + vertices[threadIndex].uv = float2(threadIndex, 1); + if (threadIndex == 0) + { + primitives[0].layer = 7; + indices[0] = uint3(0, 1, 2); + } +} + +// The pass really did run, so the signature checks below are not vacuous. +// CHECK: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle +// CHECK: %ThreadIdX = call i32 @dx.op.threadId.i32(i32 93, i32 0) + +// Per-vertex outputs: SV_Position at register 0, TEXCOORD0 at register 1. +// CHECK-DAG: !{i32 0, !"SV_Position", i8 9, i8 3, !{{[0-9]+}}, i8 4, i32 1, i8 4, i32 0, i8 0, {{.*}}} +// CHECK-DAG: !{i32 1, !"TEXCOORD", i8 9, i8 0, !{{[0-9]+}}, i8 2, i32 1, i8 2, i32 1, i8 0, {{.*}}} + +// Per-primitive output: a different register space, whose register 0 has +// nothing to do with the per-vertex register 0 above. +// CHECK-DAG: !{i32 0, !"TEXCOORD", i8 5, i8 0, !{{[0-9]+}}, i8 1, i32 1, i8 1, i32 0, i8 0, {{.*}}} diff --git a/tools/clang/test/HLSLFileCheck/pix/constantcolorhalf.hlsl b/tools/clang/test/HLSLFileCheck/pix/constantcolorhalf.hlsl new file mode 100644 index 0000000000..e7129392ca --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/constantcolorhalf.hlsl @@ -0,0 +1,19 @@ +// RUN: %dxc -enable-16bit-types -Emain -Tps_6_2 %s | %opt -S -hlsl-dxil-constantColor,constant-red=0.5,constant-green=0.25,constant-blue=0.125,constant-alpha=1 | %FileCheck %s + +// A native half SV_Target lowers to dx.op.storeOutput.f16. + +// The override values are 0.5, 0.25, 0.125 and 1.0 as half: +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 0, half 0xH3800) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 1, half 0xH3400) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 2, half 0xH3000) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 3, half 0xH3C00) + +// Unused storeOutput overloads must not remain as external declarations. +// CHECK-NOT: declare void @dx.op.storeOutput.f32 +// CHECK-NOT: declare void @dx.op.storeOutput.i16 +// CHECK-NOT: declare void @dx.op.storeOutput.i32 + +[RootSignature("")] +half4 main() : SV_Target { + return half4(0, 0, 0, 0); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/constantcolorhalfFromCB.hlsl b/tools/clang/test/HLSLFileCheck/pix/constantcolorhalfFromCB.hlsl new file mode 100644 index 0000000000..5638a098ee --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/constantcolorhalfFromCB.hlsl @@ -0,0 +1,36 @@ +// RUN: %dxc -enable-16bit-types -Emain -Tps_6_2 %s | %opt -S -hlsl-dxil-constantColor,mod-mode=1 | %FileCheck %s + +// From-constant-buffer mode against a native half SV_Target0. The tools +// constant buffer is four 32-bit components; loaded values narrow to half. + +// CB return type is f32: +// CHECK: %dx.types.CBufRet.f32 = type { float, float, float, float } + +// Create handle: +// CHECK: %PIX_Constant_Color_CB_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 2, i32 0, i32 0, i1 false) + +// Load the row: +// CHECK: %PIX_Constant_Color_Value = call %dx.types.CBufRet.f32 @dx.op.cbufferLoadLegacy.f32(i32 59, %dx.types.Handle %PIX_Constant_Color_CB_Handle, i32 0) + +// Extract components: +// CHECK: %PIX_Constant_Color_Value0 = extractvalue %dx.types.CBufRet.f32 %PIX_Constant_Color_Value, 0 +// CHECK: %PIX_Constant_Color_Value1 = extractvalue %dx.types.CBufRet.f32 %PIX_Constant_Color_Value, 1 +// CHECK: %PIX_Constant_Color_Value2 = extractvalue %dx.types.CBufRet.f32 %PIX_Constant_Color_Value, 2 +// CHECK: %PIX_Constant_Color_Value3 = extractvalue %dx.types.CBufRet.f32 %PIX_Constant_Color_Value, 3 + +// Narrow to half: +// CHECK: %PIX_Constant_Color_ValueNarrowed0 = fptrunc float %PIX_Constant_Color_Value0 to half +// CHECK: %PIX_Constant_Color_ValueNarrowed1 = fptrunc float %PIX_Constant_Color_Value1 to half +// CHECK: %PIX_Constant_Color_ValueNarrowed2 = fptrunc float %PIX_Constant_Color_Value2 to half +// CHECK: %PIX_Constant_Color_ValueNarrowed3 = fptrunc float %PIX_Constant_Color_Value3 to half + +// Store SV_Target0: +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 0, half %PIX_Constant_Color_ValueNarrowed0) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 1, half %PIX_Constant_Color_ValueNarrowed1) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 2, half %PIX_Constant_Color_ValueNarrowed2) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 3, half %PIX_Constant_Color_ValueNarrowed3) + +[RootSignature("")] +half4 main() : SV_Target { + return half4(0, 0, 0, 0); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/constantcolorhalfMRT.hlsl b/tools/clang/test/HLSLFileCheck/pix/constantcolorhalfMRT.hlsl new file mode 100644 index 0000000000..5d4462c463 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/constantcolorhalfMRT.hlsl @@ -0,0 +1,29 @@ +// RUN: %dxc -enable-16bit-types -Emain -Tps_6_2 %s | %opt -S -hlsl-dxil-constantColor | %FileCheck %s + +// MRT: RTV0 is half4, RTV1 is float4. The override applies to SV_Target0 +// only. Default constant colour is 1.0 (0xH3C00 as half). + +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 0, half 0xH3C00) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 1, half 0xH3C00) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 2, half 0xH3C00) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 3, half 0xH3C00) + +// RTV1 stays unchanged: +// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 1, i32 0, i8 0, float 0.000000e+00) +// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 1, i32 0, i8 1, float 0.000000e+00) +// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 1, i32 0, i8 2, float 0.000000e+00) +// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 1, i32 0, i8 3, float 0.000000e+00) + +struct RTOut +{ + half4 h : SV_Target; + float4 c : SV_Target1; +}; + +[RootSignature("")] +RTOut main() { + RTOut rtOut; + rtOut.h = half4(0, 0, 0, 0); + rtOut.c = float4(0.f, 0.f, 0.f, 0.f); + return rtOut; +} diff --git a/tools/clang/test/HLSLFileCheck/pix/constantcolorhalfMRTOnRTV1.hlsl b/tools/clang/test/HLSLFileCheck/pix/constantcolorhalfMRTOnRTV1.hlsl new file mode 100644 index 0000000000..f8e625001e --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/constantcolorhalfMRTOnRTV1.hlsl @@ -0,0 +1,33 @@ +// RUN: %dxc -enable-16bit-types -Emain -Tps_6_2 %s | %opt -S -hlsl-dxil-constantColor | %FileCheck %s + +// MRT: RTV0 is float4, RTV1 is half4. The override applies to SV_Target0 +// only; RTV1 stays unchanged. + +// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 0, i32 0, i8 0, float 1.000000e+00) +// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 0, i32 0, i8 1, float 1.000000e+00) +// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 0, i32 0, i8 2, float 1.000000e+00) +// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 0, i32 0, i8 3, float 1.000000e+00) + +// RTV1 stays 0xH0000 (half 0.0): +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 1, i32 0, i8 0, half 0xH0000) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 1, i32 0, i8 1, half 0xH0000) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 1, i32 0, i8 2, half 0xH0000) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 1, i32 0, i8 3, half 0xH0000) + +// Unused integer overloads must not remain as external declarations. +// CHECK-NOT: declare void @dx.op.storeOutput.i16 +// CHECK-NOT: declare void @dx.op.storeOutput.i32 + +struct RTOut +{ + float4 c : SV_Target; + half4 h : SV_Target1; +}; + +[RootSignature("")] +RTOut main() { + RTOut rtOut; + rtOut.c = float4(0.f, 0.f, 0.f, 0.f); + rtOut.h = half4(0, 0, 0, 0); + return rtOut; +} diff --git a/tools/clang/test/HLSLFileCheck/pix/constantcolorint16.hlsl b/tools/clang/test/HLSLFileCheck/pix/constantcolorint16.hlsl new file mode 100644 index 0000000000..835fc3f013 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/constantcolorint16.hlsl @@ -0,0 +1,18 @@ +// RUN: %dxc -enable-16bit-types -Emain -Tps_6_2 %s | %opt -S -hlsl-dxil-constantColor,constant-red=8,constant-green=7,constant-blue=6,constant-alpha=5 | %FileCheck %s + +// A native uint16_t SV_Target lowers to dx.op.storeOutput.i16. + +// CHECK: call void @dx.op.storeOutput.i16(i32 5, i32 0, i32 0, i8 0, i16 8) +// CHECK: call void @dx.op.storeOutput.i16(i32 5, i32 0, i32 0, i8 1, i16 7) +// CHECK: call void @dx.op.storeOutput.i16(i32 5, i32 0, i32 0, i8 2, i16 6) +// CHECK: call void @dx.op.storeOutput.i16(i32 5, i32 0, i32 0, i8 3, i16 5) + +// Unused storeOutput overloads must not remain as external declarations. +// CHECK-NOT: declare void @dx.op.storeOutput.f16 +// CHECK-NOT: declare void @dx.op.storeOutput.f32 +// CHECK-NOT: declare void @dx.op.storeOutput.i32 + +[RootSignature("")] +uint16_t4 main() : SV_Target { + return uint16_t4(0, 0, 0, 0); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/constantcolorminprecision.hlsl b/tools/clang/test/HLSLFileCheck/pix/constantcolorminprecision.hlsl new file mode 100644 index 0000000000..08d0fcae82 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/constantcolorminprecision.hlsl @@ -0,0 +1,18 @@ +// RUN: %dxc -Emain -Tps_6_0 %s | %opt -S -hlsl-dxil-constantColor,constant-red=0.5,constant-green=0.25,constant-blue=0.125,constant-alpha=1 | %FileCheck %s + +// A min16float SV_Target lowers to dx.op.storeOutput.f16 at ps_6_0 without +// -enable-16bit-types. + +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 0, half 0xH3800) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 1, half 0xH3400) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 2, half 0xH3000) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 3, half 0xH3C00) + +// CHECK-NOT: declare void @dx.op.storeOutput.f32 +// CHECK-NOT: declare void @dx.op.storeOutput.i16 +// CHECK-NOT: declare void @dx.op.storeOutput.i32 + +[RootSignature("")] +min16float4 main() : SV_Target { + return min16float4(0, 0, 0, 0); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/constantcolorminprecisionFromCB.hlsl b/tools/clang/test/HLSLFileCheck/pix/constantcolorminprecisionFromCB.hlsl new file mode 100644 index 0000000000..1a15bdcc59 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/constantcolorminprecisionFromCB.hlsl @@ -0,0 +1,37 @@ +// RUN: %dxc -Emain -Tps_6_0 %s | %opt -S -hlsl-dxil-constantColor,mod-mode=1 | %FileCheck %s + +// From-constant-buffer mode against a min16float SV_Target0 at ps_6_0 +// without -enable-16bit-types. The tools constant buffer is four 32-bit +// components; loaded values narrow to half. + +// CB return type is f32: +// CHECK: %dx.types.CBufRet.f32 = type { float, float, float, float } + +// Create handle: +// CHECK: %PIX_Constant_Color_CB_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 2, i32 0, i32 0, i1 false) + +// Load the row: +// CHECK: %PIX_Constant_Color_Value = call %dx.types.CBufRet.f32 @dx.op.cbufferLoadLegacy.f32(i32 59, %dx.types.Handle %PIX_Constant_Color_CB_Handle, i32 0) + +// Extract components: +// CHECK: %PIX_Constant_Color_Value0 = extractvalue %dx.types.CBufRet.f32 %PIX_Constant_Color_Value, 0 +// CHECK: %PIX_Constant_Color_Value1 = extractvalue %dx.types.CBufRet.f32 %PIX_Constant_Color_Value, 1 +// CHECK: %PIX_Constant_Color_Value2 = extractvalue %dx.types.CBufRet.f32 %PIX_Constant_Color_Value, 2 +// CHECK: %PIX_Constant_Color_Value3 = extractvalue %dx.types.CBufRet.f32 %PIX_Constant_Color_Value, 3 + +// Narrow to half: +// CHECK: %PIX_Constant_Color_ValueNarrowed0 = fptrunc float %PIX_Constant_Color_Value0 to half +// CHECK: %PIX_Constant_Color_ValueNarrowed1 = fptrunc float %PIX_Constant_Color_Value1 to half +// CHECK: %PIX_Constant_Color_ValueNarrowed2 = fptrunc float %PIX_Constant_Color_Value2 to half +// CHECK: %PIX_Constant_Color_ValueNarrowed3 = fptrunc float %PIX_Constant_Color_Value3 to half + +// Store SV_Target0: +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 0, half %PIX_Constant_Color_ValueNarrowed0) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 1, half %PIX_Constant_Color_ValueNarrowed1) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 2, half %PIX_Constant_Color_ValueNarrowed2) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 3, half %PIX_Constant_Color_ValueNarrowed3) + +[RootSignature("")] +min16float4 main() : SV_Target { + return min16float4(0, 0, 0, 0); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/constantcolorminprecisionMRT.hlsl b/tools/clang/test/HLSLFileCheck/pix/constantcolorminprecisionMRT.hlsl new file mode 100644 index 0000000000..1addc05a48 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/constantcolorminprecisionMRT.hlsl @@ -0,0 +1,29 @@ +// RUN: %dxc -Emain -Tps_6_0 %s | %opt -S -hlsl-dxil-constantColor | %FileCheck %s + +// MRT at ps_6_0 without -enable-16bit-types: RTV0 is min16float4, RTV1 is +// float4. The override applies to SV_Target0 only. + +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 0, half 0xH3C00) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 1, half 0xH3C00) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 2, half 0xH3C00) +// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 3, half 0xH3C00) + +// RTV1 stays unchanged: +// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 1, i32 0, i8 0, float 0.000000e+00) +// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 1, i32 0, i8 1, float 0.000000e+00) +// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 1, i32 0, i8 2, float 0.000000e+00) +// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 1, i32 0, i8 3, float 0.000000e+00) + +struct RTOut +{ + min16float4 h : SV_Target; + float4 c : SV_Target1; +}; + +[RootSignature("")] +RTOut main() { + RTOut rtOut; + rtOut.h = min16float4(0, 0, 0, 0); + rtOut.c = float4(0.f, 0.f, 0.f, 0.f); + return rtOut; +} diff --git a/tools/clang/test/HLSLFileCheck/pix/constantcolorminprecisionint.hlsl b/tools/clang/test/HLSLFileCheck/pix/constantcolorminprecisionint.hlsl new file mode 100644 index 0000000000..f18c542636 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/constantcolorminprecisionint.hlsl @@ -0,0 +1,18 @@ +// RUN: %dxc -Emain -Tps_6_0 %s | %opt -S -hlsl-dxil-constantColor,constant-red=8,constant-green=7,constant-blue=6,constant-alpha=5 | %FileCheck %s + +// A min16int SV_Target lowers to dx.op.storeOutput.i16 at ps_6_0 without +// -enable-16bit-types. + +// CHECK: call void @dx.op.storeOutput.i16(i32 5, i32 0, i32 0, i8 0, i16 8) +// CHECK: call void @dx.op.storeOutput.i16(i32 5, i32 0, i32 0, i8 1, i16 7) +// CHECK: call void @dx.op.storeOutput.i16(i32 5, i32 0, i32 0, i8 2, i16 6) +// CHECK: call void @dx.op.storeOutput.i16(i32 5, i32 0, i32 0, i8 3, i16 5) + +// CHECK-NOT: declare void @dx.op.storeOutput.f16 +// CHECK-NOT: declare void @dx.op.storeOutput.f32 +// CHECK-NOT: declare void @dx.op.storeOutput.i32 + +[RootSignature("")] +min16int4 main() : SV_Target { + return min16int4(1, 2, 3, 4); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/pixelCounterLegacyRowOptionIsAHint.hlsl b/tools/clang/test/HLSLFileCheck/pix/pixelCounterLegacyRowOptionIsAHint.hlsl new file mode 100644 index 0000000000..40faede3fc --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/pixelCounterLegacyRowOptionIsAHint.hlsl @@ -0,0 +1,37 @@ +// RUN: %dxc -Emain -Tps_6_0 %s | %opt -S -hlsl-dxil-add-pixel-hit-instrmentation,rt-width=16,num-pixels=64,upstream-sv-position-row=0 | %FileCheck %s + +// PIX ships dxcompiler.dll separately from the PIX executable, so an older PIX +// talking to a newer compiler is routine. Older PIX sends row 0 both when the +// upstream stage really uses row 0 and when it could not read the upstream +// signature at all, which means the value carries no promise. Acting on it +// would evict a real interpolant -- one that is still linkage-bound to whatever +// the upstream stage actually is -- on the strength of a guess, breaking the +// pipeline the relocation exists to keep working. +// +// "upstream-sv-position-row" is the pre-rename spelling of +// preferred-sv-position-row, kept as an accepted alias so older PIX builds +// keep working. Both spellings mean a hint: use the row if it happens to be +// free, never move anything to clear it. Only the required-sv-position-row +// spelling licenses eviction; see +// pixelCounterRelocationRepacksIntoSharedRow.hlsl for the same shader under +// that option. + +struct PSInput +{ + float2 firstUV : TEXCOORD0; + float2 secondUV : TEXCOORD1; + float4 color : COLOR0; +}; + +float4 main(PSInput input) : SV_Target +{ + return input.color + float4(input.firstUV, input.secondUV); +} + +// Every declared input keeps the register the front end gave it. +// CHECK-DAG: !{i32 0, !"TEXCOORD", i8 9, i8 0, !{{[0-9]+}}, i8 2, i32 1, i8 2, i32 0, i8 0, {{.*}}} +// CHECK-DAG: !{i32 1, !"TEXCOORD", i8 9, i8 0, !{{[0-9]+}}, i8 2, i32 1, i8 2, i32 0, i8 2, {{.*}}} +// CHECK-DAG: !{i32 2, !"COLOR", i8 9, i8 0, !{{[0-9]+}}, i8 2, i32 1, i8 4, i32 1, i8 0, {{.*}}} + +// SV_Position goes to the first register that can hold it instead. +// CHECK-DAG: !{i32 {{[0-9]+}}, !"SV_Position", i8 9, i8 3, !{{[0-9]+}}, i8 4, i32 1, i8 4, i32 2, i8 0, null} diff --git a/tools/clang/test/HLSLFileCheck/pix/pixelCounterPreferredRowOptionIsAHint.hlsl b/tools/clang/test/HLSLFileCheck/pix/pixelCounterPreferredRowOptionIsAHint.hlsl new file mode 100644 index 0000000000..39b846b8e4 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/pixelCounterPreferredRowOptionIsAHint.hlsl @@ -0,0 +1,30 @@ +// RUN: %dxc -Emain -Tps_6_0 %s | %opt -S -hlsl-dxil-add-pixel-hit-instrmentation,rt-width=16,num-pixels=64,preferred-sv-position-row=0 | %FileCheck %s + +// The canonical spelling of the hint option (see +// pixelCounterLegacyRowOptionIsAHint.hlsl for its pre-rename alias). Row 0 +// carries no promise, so acting on it would evict a real interpolant on the +// strength of a guess. preferred-sv-position-row must not license eviction: +// use the row if it happens to be free, never move anything to clear it. +// Only the required-sv-position-row spelling does that; see +// pixelCounterRelocationRepacksIntoSharedRow.hlsl for the same shader under +// that option. + +struct PSInput +{ + float2 firstUV : TEXCOORD0; + float2 secondUV : TEXCOORD1; + float4 color : COLOR0; +}; + +float4 main(PSInput input) : SV_Target +{ + return input.color + float4(input.firstUV, input.secondUV); +} + +// Every declared input keeps the register the front end gave it. +// CHECK-DAG: !{i32 0, !"TEXCOORD", i8 9, i8 0, !{{[0-9]+}}, i8 2, i32 1, i8 2, i32 0, i8 0, {{.*}}} +// CHECK-DAG: !{i32 1, !"TEXCOORD", i8 9, i8 0, !{{[0-9]+}}, i8 2, i32 1, i8 2, i32 0, i8 2, {{.*}}} +// CHECK-DAG: !{i32 2, !"COLOR", i8 9, i8 0, !{{[0-9]+}}, i8 2, i32 1, i8 4, i32 1, i8 0, {{.*}}} + +// SV_Position goes to the first register that can hold it instead. +// CHECK-DAG: !{i32 {{[0-9]+}}, !"SV_Position", i8 9, i8 3, !{{[0-9]+}}, i8 4, i32 1, i8 4, i32 2, i8 0, null} diff --git a/tools/clang/test/HLSLFileCheck/pix/pixelCounterPreferredRowWinsOverLegacyRow.hlsl b/tools/clang/test/HLSLFileCheck/pix/pixelCounterPreferredRowWinsOverLegacyRow.hlsl new file mode 100644 index 0000000000..8d6815362c --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/pixelCounterPreferredRowWinsOverLegacyRow.hlsl @@ -0,0 +1,29 @@ +// RUN: %dxc -Emain -Tps_6_0 %s | %opt -S -hlsl-dxil-add-pixel-hit-instrmentation,rt-width=16,num-pixels=64,preferred-sv-position-row=3,upstream-sv-position-row=2 | %FileCheck %s + +// When both spellings are supplied, preferred-sv-position-row must win +// deterministically over the legacy upstream-sv-position-row alias. Row 2 and +// row 3 are both free here, so if the legacy value won instead, SV_Position +// would land on row 2, not row 3; this test pins the row down to prove which +// spelling was actually read. + +struct PSInput +{ + float2 firstUV : TEXCOORD0; + float2 secondUV : TEXCOORD1; + float4 color : COLOR0; +}; + +float4 main(PSInput input) : SV_Target +{ + return input.color + float4(input.firstUV, input.secondUV); +} + +// TEXCOORD0/1 share row 0, COLOR0 occupies row 1, leaving rows 2 and 3 free. +// CHECK-DAG: !{i32 0, !"TEXCOORD", i8 9, i8 0, !{{[0-9]+}}, i8 2, i32 1, i8 2, i32 0, i8 0, {{.*}}} +// CHECK-DAG: !{i32 1, !"TEXCOORD", i8 9, i8 0, !{{[0-9]+}}, i8 2, i32 1, i8 2, i32 0, i8 2, {{.*}}} +// CHECK-DAG: !{i32 2, !"COLOR", i8 9, i8 0, !{{[0-9]+}}, i8 2, i32 1, i8 4, i32 1, i8 0, {{.*}}} + +// SV_Position lands on row 3 -- the preferred-sv-position-row value -- never +// row 2, which is what the legacy upstream-sv-position-row value would have +// produced had it won instead. +// CHECK-DAG: !{i32 {{[0-9]+}}, !"SV_Position", i8 9, i8 3, !{{[0-9]+}}, i8 4, i32 1, i8 4, i32 3, i8 0, null} diff --git a/tools/clang/test/HLSLFileCheck/pix/pixelCounterRelocationAtSignatureLimit.hlsl b/tools/clang/test/HLSLFileCheck/pix/pixelCounterRelocationAtSignatureLimit.hlsl new file mode 100644 index 0000000000..cbeb50bb9d --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/pixelCounterRelocationAtSignatureLimit.hlsl @@ -0,0 +1,45 @@ +// RUN: %dxc -Emain -Tps_6_0 %s | %opt -S -hlsl-dxil-add-pixel-hit-instrmentation,rt-width=16,num-pixels=64,required-sv-position-row=30 | %FileCheck %s + +// The pathological case for the relocation: a pixel shader that has already +// used 31 of the 32 available input registers. ATTR occupies rows 0-29 as a +// single indexed element, and the two rasterizer system values share row 30 -- +// which is where the upstream stage put SV_Position, so both of them have to +// be evicted. +// +// There is exactly one spare register left, and both evicted elements are one +// component wide, so both of them fit in it. An allocator that hands each +// evicted element a fresh row instead needs two, runs off the end of the +// register file, and emits register 32 -- a number no D3D signature can hold +// and that PIX ships straight to the driver, because it does not re-run the +// validator over the modules it patches. + +struct DensePSInput +{ + float4 attributes[30] : ATTR; + uint primitiveId : SV_PrimitiveID; + bool isFrontFace : SV_IsFrontFace; +}; + +float4 main(DensePSInput input) : SV_Target +{ + float4 accumulated = 0; + [unroll] for (uint index = 0; index < 30; ++index) + { + accumulated += input.attributes[index]; + } + + accumulated.a = input.primitiveId + (input.isFrontFace ? 1.0f : 0.0f); + return accumulated; +} + +// The 30-row array is not on the target row and must stay exactly where the +// front end packed it. +// CHECK-DAG: !{i32 0, !"ATTR", i8 9, i8 0, !{{[0-9]+}}, i8 2, i32 30, i8 4, i32 0, i8 0, {{.*}}} + +// SV_Position takes the register the upstream stage used. +// CHECK-DAG: !{i32 {{[0-9]+}}, !"SV_Position", i8 9, i8 3, !{{[0-9]+}}, i8 4, i32 1, i8 4, i32 30, i8 0, null} + +// Both evicted system values land in the last available register, packed into +// separate components of it rather than taking a row each. +// CHECK-DAG: !{i32 1, !"SV_PrimitiveID", i8 5, i8 10, !{{[0-9]+}}, i8 1, i32 1, i8 1, i32 31, i8 0, {{.*}}} +// CHECK-DAG: !{i32 2, !"SV_IsFrontFace", i8 5, i8 13, !{{[0-9]+}}, i8 1, i32 1, i8 1, i32 31, i8 1, {{.*}}} diff --git a/tools/clang/test/HLSLFileCheck/pix/pixelCounterRelocationRepacksIntoSharedRow.hlsl b/tools/clang/test/HLSLFileCheck/pix/pixelCounterRelocationRepacksIntoSharedRow.hlsl new file mode 100644 index 0000000000..563e229924 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/pixelCounterRelocationRepacksIntoSharedRow.hlsl @@ -0,0 +1,34 @@ +// RUN: %dxc -Emain -Tps_6_0 %s | %opt -S -hlsl-dxil-add-pixel-hit-instrmentation,rt-width=16,num-pixels=64,required-sv-position-row=0 | %FileCheck %s + +// The instrumentation has to put SV_Position on the row the upstream stage +// used, so the two TEXCOORDs packed into that row are the ones that move. The +// question this test pins down is where they move to. +// +// Both are two components wide and were sharing a single register, so a +// correct packer puts them back into a single register rather than into two +// separate rows. On a signature already near the 32-register limit, two +// separate rows would push an element off the end of the register file. + +struct PSInput +{ + float2 firstUV : TEXCOORD0; + float2 secondUV : TEXCOORD1; + float4 color : COLOR0; +}; + +float4 main(PSInput input) : SV_Target +{ + return input.color + float4(input.firstUV, input.secondUV); +} + +// SV_Position lands on the requested row, with the noperspective +// interpolation mode the front end gives a declared SV_Position. +// CHECK-DAG: !{i32 {{[0-9]+}}, !"SV_Position", i8 9, i8 3, !{{[0-9]+}}, i8 4, i32 1, i8 4, i32 0, i8 0, null} + +// COLOR is not on the target row, so it must not have been touched. +// CHECK-DAG: !{i32 2, !"COLOR", i8 9, i8 0, !{{[0-9]+}}, i8 2, i32 1, i8 4, i32 1, i8 0, {{.*}}} + +// Both evicted TEXCOORDs share row 2, in the same two-component halves they +// occupied before. Row 3 is never reached. +// CHECK-DAG: !{i32 0, !"TEXCOORD", i8 9, i8 0, !{{[0-9]+}}, i8 2, i32 1, i8 2, i32 2, i8 0, {{.*}}} +// CHECK-DAG: !{i32 1, !"TEXCOORD", i8 9, i8 0, !{{[0-9]+}}, i8 2, i32 1, i8 2, i32 2, i8 2, {{.*}}} diff --git a/tools/clang/unittests/HLSL/PixTest.cpp b/tools/clang/unittests/HLSL/PixTest.cpp index eb466f41ec..c7c6556881 100644 --- a/tools/clang/unittests/HLSL/PixTest.cpp +++ b/tools/clang/unittests/HLSL/PixTest.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -38,6 +39,7 @@ #include "dxc/DXIL/DxilModule.h" #include "dxc/DXIL/DxilOperations.h" #include "dxc/DXIL/DxilSubobject.h" +#include "dxc/DxilPIXPasses/DxilPIXPasses.h" #include "dxc/Test/DxcTestUtils.h" #include "dxc/Test/HLSLTestData.h" @@ -53,6 +55,7 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringSwitch.h" +#include "llvm/AsmParser/Parser.h" #include "llvm/Bitcode/ReaderWriter.h" #include "llvm/IR/Constants.h" #include "llvm/IR/DebugInfo.h" @@ -63,10 +66,12 @@ #include "llvm/IR/Module.h" #include "llvm/IR/ModuleSlotTracker.h" #include "llvm/IR/Operator.h" +#include "llvm/Pass.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/MSFileSystem.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Path.h" +#include "llvm/Support/SourceMgr.h" #include #include <../lib/DxilDia/DxcPixLiveVariables_FragmentIterator.h> @@ -123,6 +128,12 @@ class PixTest : public ::testing::Test { TEST_METHOD(AccessTracking_ModificationReport_Read) TEST_METHOD(AccessTracking_ModificationReport_Write) TEST_METHOD(AccessTracking_ModificationReport_SM66) + TEST_METHOD(AccessTracking_MultipleDynamicRangesSameTypeAndSpace) + TEST_METHOD(AccessTracking_DynamicRangeRegisterIndex_SM66) + TEST_METHOD(AccessTracking_ConstantIndexAtRangeLimit) + TEST_METHOD(AccessTracking_SamplerAccessInLibrary) + TEST_METHOD(AccessTracking_OobBindlessUsesFunctionShaderKind) + TEST_METHOD(AccessTracking_LibraryNonEntryFunction) TEST_METHOD(PixStructAnnotation_Lib_DualRaygen) @@ -143,6 +154,11 @@ class PixTest : public ::testing::Test { TEST_METHOD(PixStructAnnotation_Inheritance) TEST_METHOD(PixStructAnnotation_ResourceAsMember) TEST_METHOD(PixStructAnnotation_WheresMyDbgValue) + TEST_METHOD(DbgValueToDbgDeclare_BackwardLayout) + TEST_METHOD(DebugInstrumentation_DynamicIndexSpanMatchesAllocaRegisterCount) + TEST_METHOD(PixDbgValueToDbgDeclare_MultiDimensionalStaticGlobalArray) + TEST_METHOD(AllocaRegisterWrite_DeepAggregateChainIsAnnotated) + TEST_METHOD(EntryBlockInjection_HandlesLabelledAndUnlabelledFirstBlock) TEST_METHOD(VirtualRegisters_InstructionCounts) TEST_METHOD(VirtualRegisters_AlignedOffsets) @@ -158,7 +174,10 @@ class PixTest : public ::testing::Test { TEST_METHOD(ToolsUav_ExtendingRootSignaturePreservesUnrelatedParameterFlags) TEST_METHOD(ConstantColor_UnusedIntOverloadIsErased) TEST_METHOD(ConstantColor_NoTargetOverloadsAreErased) + TEST_METHOD(ConstantColor_FromConstantBufferIsWellFormed) TEST_METHOD(RemoveDiscards_UnusedDiscardOverloadIsErased) + TEST_METHOD(ReduceMSAAToSingleSample_SM66) + TEST_METHOD(ReduceMSAAToSingleSample_HalfLoad) TEST_METHOD(OperationCacheCleanup_RemovesErasedFunctions) TEST_METHOD(DynamicResourceCleanup_VisitorStopsEarly) TEST_METHOD(MeshOutput_NoIndicesDeclarationIsErased) @@ -175,6 +194,22 @@ class PixTest : public ::testing::Test { TEST_METHOD(DebugInstrumentation_VectorAllocaWrite_Structs) + // Tests for the pixel-hit and debug instrumentation passes' SV_Position + // signature handling. + TEST_METHOD(PixelHitInstrumentation_ReturnOutsideEntryBlock) + TEST_METHOD(PixelHitInstrumentation_SVPositionRowAlreadyOccupied) + TEST_METHOD(PixelHitInstrumentation_SVPositionRowUnknown) + TEST_METHOD(PixelHitInstrumentation_SVPositionRowOccupiedBySystemValue) + TEST_METHOD(PixelHitInstrumentation_RejectsUnrepresentableDimensions) + TEST_METHOD(PixelHitInstrumentation_ClampsCounterIndexForSmallBuffer) + TEST_METHOD( + PixelHitInstrumentation_ClampOrderingSurvivesElementOffsetOverflow) + TEST_METHOD(PixelHitInstrumentation_RejectsAuthoritativeRowWithNoRoomToEvict) + TEST_METHOD( + PixelHitInstrumentation_ClearsStaleViewIdStateAfterSignatureGrowth) + TEST_METHOD(DebugInstrumentation_ClearsStaleViewIdStateAfterVSSignatureGrowth) + TEST_METHOD(Validation_PixelHit_PixelShader) + TEST_METHOD(DebugBreakInstrumentation_Basic) TEST_METHOD(DebugBreakInstrumentation_NoDebugBreak) TEST_METHOD(DebugBreakInstrumentation_Multiple) @@ -190,6 +225,9 @@ class PixTest : public ::testing::Test { TEST_METHOD(Validation_ControlInvalidModuleFails) TEST_METHOD(Validation_ControlNonPixUnusedMetadataIsRejected) TEST_METHOD(Validation_ControlInvalidPixMetadataIsRejected) + TEST_METHOD(Validation_ControlBoilerplateOnlyFailureIsRejected) + TEST_METHOD(Validation_NonUniformResourceIndex_WaveOpsFlag) + TEST_METHOD(Validation_ShaderAccessTracking_DynamicallyIndexedResource) dxc::DxCompilerDllLoader m_dllSupport; VersionSupportInfo m_ver; @@ -248,6 +286,38 @@ class PixTest : public ::testing::Test { std::move(pOptimizedModule), {}, Tokenize(outputText.c_str(), "\n")}; } + // std::nullopt omits the option entirely, which is how PIX signals that it + // could not read the previous stage's signature. + PassOutput RunPixelHitPass( + IDxcBlob *dxil, int RTWidth, int NumPixels, + std::optional RequiredSVPositionRow = std::nullopt) { + CComPtr pOptimizer; + VERIFY_SUCCEEDED( + m_dllSupport.CreateInstance(CLSID_DxcOptimizer, &pOptimizer)); + std::vector Options; + Options.push_back(L"-opt-mod-passes"); + std::wstring pixelHitArg = + L"-hlsl-dxil-add-pixel-hit-instrmentation,rt-width=" + + std::to_wstring(RTWidth) + L",num-pixels=" + std::to_wstring(NumPixels); + if (RequiredSVPositionRow.has_value()) { + // The required spelling: these tests know the row because they + // choose it, which is what entitles the pass to relocate an occupant. + pixelHitArg += L",required-sv-position-row=" + + std::to_wstring(*RequiredSVPositionRow); + } + Options.push_back(pixelHitArg.c_str()); + + CComPtr pOptimizedModule; + CComPtr pText; + VERIFY_SUCCEEDED(pOptimizer->RunOptimizer( + dxil, Options.data(), Options.size(), &pOptimizedModule, &pText)); + + std::string outputText = BlobToUtf8(pText); + + return { + std::move(pOptimizedModule), {}, Tokenize(outputText.c_str(), "\n")}; + } + PassOutput RunDebugPass(IDxcBlob *dxil, int UAVSize = 1024 * 1024) { CComPtr pOptimizer; VERIFY_SUCCEEDED( @@ -301,6 +371,42 @@ class PixTest : public ::testing::Test { std::vector Lines; }; + // Runs the virtual-register annotation pass over textual IR and returns the + // pass report. Textual IR builds a module shape that HLSL does not express. + std::vector RunAnnotationPassOnText(const std::string &irText) { + CComPtr pSource; + CreateBlobFromText(m_dllSupport, irText.c_str(), &pSource); + + CComPtr pOptimizer; + VERIFY_SUCCEEDED( + m_dllSupport.CreateInstance(CLSID_DxcOptimizer, &pOptimizer)); + std::vector Options; + Options.push_back(L"-S"); + Options.push_back(L"-opt-mod-passes"); + Options.push_back(L"-dxil-annotate-with-virtual-regs"); + + CComPtr pOptimizedModule; + CComPtr pText; + VERIFY_SUCCEEDED(pOptimizer->RunOptimizer( + pSource, Options.data(), Options.size(), &pOptimizedModule, &pText)); + + return Tokenize(BlobToUtf8(pText).c_str(), "\n"); + } + + // Replaces the one occurrence of needle, and fails the test when the text + // does not hold exactly one. + static std::string ReplaceOnlyOccurrence(const std::string &text, + const std::string &needle, + const std::string &replacement) { + auto position = text.find(needle); + VERIFY_IS_TRUE(position != std::string::npos); + VERIFY_IS_TRUE(text.find(needle, position + needle.size()) == + std::string::npos); + std::string result = text; + result.replace(position, needle.size(), replacement); + return result; + } + SinglePassOutput runSinglePass(IDxcBlob *Dxil, LPCWSTR PassOption) { CComPtr Optimizer; VERIFY_SUCCEEDED( @@ -751,7 +857,8 @@ class PixTest : public ::testing::Test { const wchar_t *profile = L"as_6_5"); void ValidateAllocaWrite(std::vector const &allocaWrites, size_t index, const char *name); - PassOutput RunShaderAccessTrackingPass(IDxcBlob *blob); + PassOutput RunShaderAccessTrackingPass( + IDxcBlob *blob, const wchar_t *config = L"U0:0:10i0;U0:1:2i0;.0;0;0."); CComPtr RunDxilPIXAddTidToAmplificationShaderPayloadPass(IDxcBlob *blob); CComPtr RunDxilPIXMeshShaderOutputPass(IDxcBlob *blob); @@ -1036,14 +1143,17 @@ TEST_F(PixTest, CompileDebugDisasmPDB) { VERIFY_SUCCEEDED(pCompiler->Disassemble(pPdbBlob, &pDisasm)); } -PassOutput PixTest::RunShaderAccessTrackingPass(IDxcBlob *blob) { +PassOutput PixTest::RunShaderAccessTrackingPass(IDxcBlob *blob, + const wchar_t *config) { CComPtr pOptimizer; VERIFY_SUCCEEDED( m_dllSupport.CreateInstance(CLSID_DxcOptimizer, &pOptimizer)); std::vector Options; Options.push_back(L"-opt-mod-passes"); - Options.push_back(L"-hlsl-dxil-pix-shader-access-instrumentation,config=U0:0:" - L"10i0;U0:1:2i0;.0;0;0."); + std::wstring passOption = + L"-hlsl-dxil-pix-shader-access-instrumentation,config="; + passOption += config; + Options.push_back(passOption.c_str()); CComPtr pOptimizedModule; CComPtr pText; @@ -1402,6 +1512,207 @@ float main() : SV_Target ValidateAccessTrackingMods(hlsl, true); } +std::vector Split(std::string str, char delimeter); + +static std::string JoinLines(std::vector const &lines) { + std::string joined; + for (auto const &line : lines) { + joined += line; + joined += '\n'; + } + return joined; +} + +static bool HasBufferStoreWithByteOffset(std::vector const &lines, + unsigned byteOffset) { + std::string needle = "i32 " + std::to_string(byteOffset); + for (auto const &line : lines) { + if (line.find("dx.op.bufferStore") != std::string::npos && + line.find(needle) != std::string::npos) { + return true; + } + } + return false; +} + +static bool +HasBufferStoreValueMatchingMask(std::vector const &lines, + uint32_t mask, uint32_t maskedValue) { + for (auto const &line : lines) { + if (line.find("dx.op.bufferStore") == std::string::npos) { + continue; + } + + size_t position = 0; + while ((position = line.find("i32 ", position)) != std::string::npos) { + position += 4; + char *end = nullptr; + uint32_t value = + static_cast(strtoul(line.c_str() + position, &end, 10)); + if (end != line.c_str() + position && (value & mask) == maskedValue) { + return true; + } + } + } + return false; +} + +TEST_F(PixTest, AccessTracking_MultipleDynamicRangesSameTypeAndSpace) { + const char *hlsl = R"( +ByteAddressBuffer g_indices : register(t0); +RWByteAddressBuffer g_firstRange[2] : register(u4); +RWByteAddressBuffer g_secondRange[2] : register(u6); + +[numthreads(1, 1, 1)] +void CSMain() +{ + uint index = g_indices.Load(0); + g_firstRange[index].Store(0, 1); + g_secondRange[index].Store(0, 2); +} +)"; + + auto compiled = Compile(m_dllSupport, hlsl, L"cs_6_0", {L"-Od"}, L"CSMain"); + auto output = + RunShaderAccessTrackingPass(compiled, L"S0:0:2i0;U0:0:10i0;.0;0;0."); + auto text = JoinLines(output.lines); + VERIFY_IS_TRUE(text.find("U0:4;") != std::string::npos); + VERIFY_IS_TRUE(text.find("U0:6;") != std::string::npos); + verifyInstrumentedModuleIsValid(output.blob, + "shader access tracking of two dynamic UAV " + "ranges in the same register space"); +} + +TEST_F(PixTest, AccessTracking_DynamicRangeRegisterIndex_SM66) { + if (m_ver.SkipDxilVersion(1, 6)) { + return; + } + + const char *hlsl = R"( +RWByteAddressBuffer g_buffers[] : register(u5); + +[numthreads(1, 1, 1)] +void CSMain(uint3 dispatchThreadId : SV_DispatchThreadID) +{ + g_buffers[dispatchThreadId.x].Store(0, 1); +} +)"; + + auto compiled = Compile(m_dllSupport, hlsl, L"cs_6_6", {L"-Od"}, L"CSMain"); + auto output = RunShaderAccessTrackingPass(compiled, L"U0:0:10i0;.0;0;0."); + auto text = JoinLines(output.lines); + VERIFY_IS_TRUE(text.find("U0:5;") != std::string::npos); + VERIFY_IS_TRUE(text.find("U0:0;") == std::string::npos); + verifyInstrumentedModuleIsValid( + output.blob, "shader access tracking of an SM 6.6 dynamic UAV range"); +} + +TEST_F(PixTest, AccessTracking_ConstantIndexAtRangeLimit) { + const char *hlsl = R"( +RWByteAddressBuffer g_buffers[] : register(u0); + +[numthreads(1, 1, 1)] +void CSMain() +{ + g_buffers[1].Store(0, 1); +} +)"; + + auto compiled = Compile(m_dllSupport, hlsl, L"cs_6_0", {L"-Od"}, L"CSMain"); + auto output = RunShaderAccessTrackingPass(compiled, L"U0:0:1i0;.0;0;0."); + auto lines = Split(Disassemble(output.blob), '\n'); + VERIFY_IS_TRUE(HasBufferStoreWithByteOffset(lines, 4)); + VERIFY_IS_TRUE(!HasBufferStoreWithByteOffset(lines, 16)); + verifyInstrumentedModuleIsValid( + output.blob, + "shader access tracking of a constant index at the range limit"); +} + +TEST_F(PixTest, AccessTracking_SamplerAccessInLibrary) { + if (m_ver.SkipDxilVersion(1, 6)) { + return; + } + + const char *hlsl = R"( +Texture2D g_texture : register(t0); +SamplerState g_sampler : register(s2); +RWByteAddressBuffer g_output : register(u0); + +[shader("raygeneration")] +void RayGen() +{ + float4 value = g_texture.SampleLevel(g_sampler, float2(0, 0), 0); + g_output.Store(0, asuint(value.x)); +} +)"; + + auto compiled = Compile(m_dllSupport, hlsl, L"lib_6_6", {L"-Od"}); + auto output = RunShaderAccessTrackingPass( + compiled, L"S0:0:4i0;M0:20:4i0;U0:40:4i0;.0;0;0."); + auto lines = Split(Disassemble(output.blob), '\n'); + VERIFY_IS_TRUE(HasBufferStoreWithByteOffset(lines, 264)); + verifyInstrumentedModuleIsValid( + output.blob, "shader access tracking of a library sampler access"); +} + +TEST_F(PixTest, AccessTracking_OobBindlessUsesFunctionShaderKind) { + if (m_ver.SkipDxilVersion(1, 6)) { + return; + } + + const char *hlsl = R"( +[shader("raygeneration")] +void RayGen() +{ + RWByteAddressBuffer output = ResourceDescriptorHeap[1]; + output.Store(0, 1); +} +)"; + + auto compiled = Compile(m_dllSupport, hlsl, L"lib_6_6", {L"-Od"}); + auto output = RunShaderAccessTrackingPass(compiled, L".0;0;0."); + auto lines = Split(Disassemble(output.blob), '\n'); + VERIFY_IS_TRUE( + HasBufferStoreValueMatchingMask(lines, 0xF8000000, 0x78000000)); + VERIFY_IS_TRUE( + !HasBufferStoreValueMatchingMask(lines, 0xF8000000, 0x68000000)); + verifyInstrumentedModuleIsValid( + output.blob, + "shader access tracking of an out-of-bounds bindless access"); +} + +TEST_F(PixTest, AccessTracking_LibraryNonEntryFunction) { + if (m_ver.SkipDxilVersion(1, 6)) { + return; + } + + const char *hlsl = R"( +Texture2D g_texture : register(t0); +RWByteAddressBuffer g_output : register(u0); + +export float4 Helper(uint index) +{ + float4 value = g_texture.Load(int3(index, 0, 0)); + g_output.Store(0, asuint(value.x)); + return value; +} + +[shader("raygeneration")] +void RayGen() +{ + Helper(0); +} +)"; + + auto compiled = Compile(m_dllSupport, hlsl, L"lib_6_6", {L"-Od"}); + auto output = + RunShaderAccessTrackingPass(compiled, L"S0:0:4i0;U0:4:4i0;.0;0;0."); + auto text = JoinLines(output.lines); + VERIFY_IS_TRUE(text.find("NotModified") == std::string::npos); + verifyInstrumentedModuleIsValid( + output.blob, "shader access tracking of a library helper function"); +} + TEST_F(PixTest, AddToASGroupSharedPayload) { const char *hlsl = R"( @@ -1996,9 +2307,9 @@ void main() auto Testables = TestStructAnnotationCase(hlsl, optimization); - // 2 in unoptimized case (one for each instance of smallPayload) - // 1 in optimized case (cuz p2 aliases over p) - VERIFY_IS_TRUE(Testables.OffsetAndSizes.size() >= 1); + // Each unoptimized source variable has storage. Optimized copies alias. + const size_t ExpectedCount = choice.IsOptimized ? 1u : 2u; + VERIFY_ARE_EQUAL(ExpectedCount, Testables.OffsetAndSizes.size()); for (const auto &os : Testables.OffsetAndSizes) { VERIFY_ARE_EQUAL(1u, os.countOfMembers); @@ -2006,10 +2317,78 @@ void main() VERIFY_ARE_EQUAL(32u, os.size); } - VERIFY_ARE_EQUAL(1u, Testables.AllocaWrites.size()); + VERIFY_ARE_EQUAL(ExpectedCount, Testables.AllocaWrites.size()); + for (size_t i = 0; i < ExpectedCount; ++i) { + ValidateAllocaWrite(Testables.AllocaWrites, i, "dummy"); + } } } +TEST_F(PixTest, DbgValueToDbgDeclare_BackwardLayout) { + const char *IR = R"( + %BadStruct = type { i64, i32 } + + define void @main() !dbg !5 { + entry: + %var = alloca %BadStruct, align 4 + call void @llvm.dbg.value(metadata %BadStruct* %var, i64 0, metadata !10, metadata !15), !dbg !16 + ret void + } + + declare void @llvm.dbg.value(metadata, i64, metadata, metadata) + + !llvm.dbg.cu = !{!0} + !llvm.module.flags = !{!3, !4} + + !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: 1, subprograms: !2) + !1 = !DIFile(filename: "test.hlsl", directory: "/") + !2 = !{!5} + !3 = !{i32 2, !"Dwarf Version", i32 4} + !4 = !{i32 2, !"Debug Info Version", i32 3} + !5 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 1, type: !6, isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: false, function: void ()* @main) + !6 = !DISubroutineType(types: !7) + !7 = !{null} + !8 = !DIBasicType(name: "int64", size: 64, align: 32, encoding: DW_ATE_signed) + !9 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed) + !10 = !DILocalVariable(tag: DW_TAG_auto_variable, name: "var", scope: !5, file: !1, line: 2, type: !11) + !11 = !DICompositeType(tag: DW_TAG_structure_type, name: "BadStruct", file: !1, line: 1, size: 96, align: 32, elements: !12) + !12 = !{!13, !14} + !13 = !DIDerivedType(tag: DW_TAG_member, name: "First", scope: !11, file: !1, line: 2, baseType: !8, size: 64, align: 32, offset: 0) + !14 = !DIDerivedType(tag: DW_TAG_member, name: "Second", scope: !11, file: !1, line: 3, baseType: !9, size: 16, align: 32, offset: 32) + !15 = !DIExpression() + !16 = !DILocation(line: 2, column: 1, scope: !5) + )"; + + llvm::LLVMContext Context; + llvm::SMDiagnostic Error; + std::unique_ptr Module = + llvm::parseAssemblyString(IR, Error, Context); + VERIFY_IS_NOT_NULL(Module.get()); + + std::unique_ptr Pass( + llvm::createDxilDbgValueToDbgDeclarePass()); + VERIFY_IS_TRUE(Pass->runOnModule(*Module)); + + std::vector> Pieces; + for (llvm::BasicBlock &Block : *Module->getFunction("main")) { + for (llvm::Instruction &Instruction : Block) { + if (auto *Declare = llvm::dyn_cast(&Instruction)) { + llvm::DIExpression *Expression = Declare->getExpression(); + VERIFY_IS_TRUE(Expression->isBitPiece()); + Pieces.emplace_back(Expression->getBitPieceOffset(), + Expression->getBitPieceSize()); + } + } + } + + std::sort(Pieces.begin(), Pieces.end()); + VERIFY_ARE_EQUAL(size_t(2), Pieces.size()); + VERIFY_ARE_EQUAL(uint64_t(0), Pieces[0].first); + VERIFY_ARE_EQUAL(uint64_t(64), Pieces[0].second); + VERIFY_ARE_EQUAL(uint64_t(64), Pieces[1].first); + VERIFY_ARE_EQUAL(uint64_t(16), Pieces[1].second); +} + TEST_F(PixTest, PixStructAnnotation_MixedSizes) { if (m_ver.SkipDxilVersion(1, 5)) return; @@ -3812,6 +4191,121 @@ float4 main() : SV_Target "discard removal with no discard"); } +TEST_F(PixTest, ConstantColor_FromConstantBufferIsWellFormed) { + const char *source = R"x( +float4 main(float4 position : SV_Position) : SV_Target +{ + return position; +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {L"-Od"}); + auto output = runSinglePass(compiled, L"-hlsl-dxil-constantColor,mod-mode=1"); + + // The CBuffer symbol must be a pointer to the struct so ValidateCBuffer + // can reach the annotation. + CComPtr pAssembler; + VERIFY_SUCCEEDED( + m_dllSupport.CreateInstance(CLSID_DxcAssembler, &pAssembler)); + CComPtr pAssembleResult; + VERIFY_SUCCEEDED( + pAssembler->AssembleToContainer(output.Module, &pAssembleResult)); + HRESULT assembleStatus; + VERIFY_SUCCEEDED(pAssembleResult->GetStatus(&assembleStatus)); + VERIFY_SUCCEEDED(assembleStatus); + + CComPtr pNewContainer; + VERIFY_SUCCEEDED(pAssembleResult->GetResult(&pNewContainer)); + + // The CBuffer resource record field 6 is size in bytes; a float4 row + // is 16 bytes. + auto lines = Tokenize(Disassemble(pNewContainer).c_str(), "\n"); + bool foundConstantColorCBuffer = false; + for (auto const &line : lines) { + if (line.find("!\"PIX_ConstantColorCBName\"") == std::string::npos) + continue; + auto fields = Tokenize(line.c_str(), ","); + VERIFY_IS_TRUE(fields.size() > 6); + // Field 1 is the global symbol; it must be a pointer to the CB struct. + VERIFY_ARE_NOT_EQUAL(std::string::npos, fields[1].find('*')); + VERIFY_ARE_EQUAL(16, atoi(fields[6].c_str() + fields[6].find("i32 ") + 4)); + foundConstantColorCBuffer = true; + } + VERIFY_IS_TRUE(foundConstantColorCBuffer); + + // The struct annotation names the float4 row in the reflection header. + bool foundStructAnnotation = false; + for (auto const &line : lines) { + if (line.find("struct PIX_ConstantColorCB_Type") != std::string::npos) + foundStructAnnotation = true; + } + VERIFY_IS_TRUE(foundStructAnnotation); + + verifyInstrumentedModuleIsValid(pNewContainer, + "constant-colour from constant buffer"); +} + +static void +VerifyMSAALoadSampleWasReduced(std::vector const &lines, + const char *textureLoadOverload, + const char *originalSampleIndex) { + bool foundTextureLoad = false; + for (auto const &line : lines) { + if (line.find(" call ") == std::string::npos || + line.find(textureLoadOverload) == std::string::npos) { + continue; + } + + foundTextureLoad = true; + VERIFY_ARE_EQUAL(std::string::npos, line.find(originalSampleIndex)); + VERIFY_ARE_NOT_EQUAL(std::string::npos, line.find(", i32 0,")); + } + VERIFY_IS_TRUE(foundTextureLoad); +} + +TEST_F(PixTest, ReduceMSAAToSingleSample_SM66) { + if (m_ver.SkipDxilVersion(1, 6)) + return; + + // SM 6.6 lowers the resource handle through annotateHandle. + const char *source = R"x( +Texture2DMS tex : register(t0); +float4 main(float4 position : SV_Position) : SV_Target +{ + return tex.Load(int2(position.xy), 3); +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_6", {L"-Od"}); + auto output = runSinglePass(compiled, L"-hlsl-dxil-reduce-msaa-to-single"); + auto lines = Tokenize(Disassemble(output.Module).c_str(), "\n"); + + VerifyMSAALoadSampleWasReduced(lines, "dx.op.textureLoad.f32", ", i32 3,"); + verifyInstrumentedModuleIsValid(output.Module, + "MSAA reduction on SM 6.6 handle"); +} + +TEST_F(PixTest, ReduceMSAAToSingleSample_HalfLoad) { + if (m_ver.SkipDxilVersion(1, 2)) + return; + + // Texture2DMS.Load lowers to dx.op.textureLoad.f16. + const char *source = R"x( +Texture2DMS tex : register(t0); +float4 main(float4 position : SV_Position) : SV_Target +{ + half4 color = tex.Load(int2(position.xy), 2); + return float4(color); +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_2", + {L"-Od", L"-enable-16bit-types"}); + auto output = runSinglePass(compiled, L"-hlsl-dxil-reduce-msaa-to-single"); + auto lines = Tokenize(Disassemble(output.Module).c_str(), "\n"); + + VerifyMSAALoadSampleWasReduced(lines, "dx.op.textureLoad.f16", ", i32 2,"); + verifyInstrumentedModuleIsValid(output.Module, + "MSAA reduction on 16-bit texture load"); +} + TEST_F(PixTest, OperationCacheCleanup_RemovesErasedFunctions) { const char *Source = R"x( float4 main() : SV_Target @@ -4625,6 +5119,13 @@ float main() : SV_Target SinglePassOutput Output = runSinglePass(Compiled, L"-dxil-annotate-with-virtual-regs"); + // Confirm the baseline validates before corrupting it, so the failure + // below is caused by the corruption and nothing else. + verifyInstrumentedModuleIsValid( + Output.Module, + "virtual-register annotation of a trivial pixel shader, uncorrupted " + "baseline (validation harness control)"); + // Mislabel the shader stage, so the container carries both the // harness's permitted PIX metadata and a real defect. std::string Disassembly = Disassemble(Output.Module); @@ -4768,3 +5269,830 @@ float main() : SV_Target VERIFY_IS_TRUE(AddedMalformedMetadata); VERIFY_IS_FALSE(validateInstrumentedModule(WithMalformedMetadata).Valid); } + +TEST_F(PixTest, Validation_ControlBoilerplateOnlyFailureIsRejected) { + const std::string boilerplateOnly = + getSignificantValidationDiagnostics("Validation failed.\n"); + VERIFY_IS_TRUE(boilerplateOnly.empty()); + + const std::string realDiagnostic = + getSignificantValidationDiagnostics("Validation failed.\n" + "Some real validator diagnostic.\n"); + VERIFY_IS_FALSE(realDiagnostic.empty()); + VERIFY_IS_TRUE(realDiagnostic.find("Some real validator diagnostic.") != + std::string::npos); +} + +TEST_F(PixTest, Validation_NonUniformResourceIndex_WaveOpsFlag) { + if (m_ver.SkipDxilVersion(1, 6)) + return; + + const char *source = R"x( +Texture2D textures[] : register(t0); +SamplerState samp : register(s0); + +cbuffer Constants : register(b0) +{ + uint index; +}; + +float4 main(float4 pos : SV_Position) : SV_Target +{ + return textures[index].Sample(samp, pos.xy); +})x"; + + // This index is dynamic and unmarked, so the pass instruments it; an + // index already marked NonUniformResourceIndex would be skipped. + // Instrumentation inserts WaveActiveAllEqual, which requires the WaveOps + // shader flag. + CComPtr compiled = + Compile(m_dllSupport, source, L"ps_6_6", {L"-Od"}); + CComPtr dxil = FindModule(DFCC_ShaderDebugInfoDXIL, compiled); + + CComPtr pOptimizer; + VERIFY_SUCCEEDED( + m_dllSupport.CreateInstance(CLSID_DxcOptimizer, &pOptimizer)); + std::array Options = { + L"-opt-mod-passes", L"-dxil-dbg-value-to-dbg-declare", + L"-dxil-annotate-with-virtual-regs", + L"-hlsl-dxil-non-uniform-resource-index-instrumentation"}; + + CComPtr pOptimizedModule; + CComPtr pText; + VERIFY_SUCCEEDED(pOptimizer->RunOptimizer( + dxil, Options.data(), Options.size(), &pOptimizedModule, &pText)); + + verifyInstrumentedModuleIsValid(pOptimizedModule, + "non-uniform resource index instrumentation"); + + VERIFY_ARE_NOT_EQUAL( + std::string::npos, + Disassemble(pOptimizedModule).find("dx.op.waveActiveAllEqual")); +} + +TEST_F(PixTest, Validation_ShaderAccessTracking_DynamicallyIndexedResource) { + const char *source = R"x( +Texture2D textures[8] : register(t0); +SamplerState samp : register(s0); + +cbuffer Constants : register(b0) +{ + uint index; +}; + +float4 main(float4 pos : SV_Position) : SV_Target +{ + return textures[index].Sample(samp, pos.xy); +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {L"-Od"}); + auto output = RunShaderAccessTrackingPass(compiled); + verifyInstrumentedModuleIsValid( + output.blob, "shader access tracking of a dynamically indexed resource"); +} + +// Pulls the register span out of every dynamically-indexed alloca write the +// debug instrumentation pass reported. The per-block records it emits are +// semicolon-separated and a dynamic alloca write looks like +// +// ,,,d,- +// +// where the span is how many virtual registers the write could land in. +static std::vector +FindDynamicAllocaWriteSpans(std::vector const &passOutputLines) { + std::vector spans; + for (auto const &line : passOutputLines) { + for (auto const &record : Split(line, ';')) { + auto tokens = Split(record, ','); + if (tokens.size() < 5 || tokens[3] != "d") { + continue; + } + auto const dash = tokens[4].find('-'); + if (dash == std::string::npos) { + continue; + } + spans.push_back(atoi(tokens[4].substr(dash + 1).c_str())); + } + } + return spans; +} + +// PIX clamps a dynamic index to the span this record reports, so a span that +// undercounts the alloca hides every element past it. The span comes from the +// !pix-alloca-reg-write metadata the annotation pass attaches to the +// instruction, not from the alloca's LLVM array length, so it always matches +// the virtual-register numbering. +// +// DXC's SROA flattens every aggregate the front end emits, so today's shapes +// keep both derivations in agreement. This test guards against that ceasing +// to be true. +TEST_F(PixTest, + DebugInstrumentation_DynamicIndexSpanMatchesAllocaRegisterCount) { + struct Case { + char const *description; + char const *source; + int expectedSpan; + }; + + const Case cases[] = { + {"one-dimensional float array", R"x( +RWByteAddressBuffer RawUAV : register(u0); +[numthreads(1, 1, 1)] +void main() +{ + float values[8]; + for (uint i = 0; i < 8; ++i) values[i] = 0; + values[RawUAV.Load(0)] = 7; + RawUAV.Store(4, asuint(values[RawUAV.Load(8)])); +})x", + 8}, + {"two-dimensional array is flattened to one register run", R"x( +RWByteAddressBuffer RawUAV : register(u0); +[numthreads(1, 1, 1)] +void main() +{ + float m[4][4]; + for (uint i = 0; i < 4; ++i) for (uint j = 0; j < 4; ++j) m[i][j] = 0; + m[RawUAV.Load(0)][RawUAV.Load(4)] = 7; + RawUAV.Store(8, asuint(m[RawUAV.Load(12)][RawUAV.Load(16)])); +})x", + 16}, + {"array member of a struct", R"x( +RWByteAddressBuffer RawUAV : register(u0); +struct Container { float before; float values[8]; float after; }; +[numthreads(1, 1, 1)] +void main() +{ + Container c; + c.before = 1; + c.after = 2; + for (uint i = 0; i < 8; ++i) c.values[i] = 0; + c.values[RawUAV.Load(0)] = 7; + RawUAV.Store(4, asuint(c.values[RawUAV.Load(8)] + c.before + c.after)); +})x", + 8}, + {"dynamically indexed vector", R"x( +RWByteAddressBuffer RawUAV : register(u0); +[numthreads(1, 1, 1)] +void main() +{ + float3 v = float3(1, 2, 3); + v[RawUAV.Load(0)] = 7; + RawUAV.Store(4, asuint(v.x + v.y + v.z)); +})x", + 3}, + }; + + for (auto const &testCase : cases) { + WEX::Logging::Log::Comment( + WEX::Common::String().Format(L"%S", testCase.description)); + + auto compiled = Compile(m_dllSupport, testCase.source, L"cs_6_0", {L"-Od"}); + auto output = RunDebugPass(compiled); + auto spans = FindDynamicAllocaWriteSpans(output.lines); + + // If DXC ever stops emitting a dynamically-indexed alloca store for these + // shaders the test would otherwise quietly become a test of nothing. + VERIFY_IS_TRUE(spans.size() > 0); + for (int span : spans) { + VERIFY_ARE_EQUAL(testCase.expectedSpan, span); + } + } +} + +// Counts stores of the given value into a shadow alloca, i.e. stores whose +// destination is a local pointer rather than a module-scope global. +static uint32_t +CountStoresToAllocaOfValue(std::vector const &disassemblyLines, + const char *value) { + uint32_t count = 0; + for (auto const &line : disassemblyLines) { + if (line.find("store ") == std::string::npos) { + continue; + } + if (line.find(value) == std::string::npos) { + continue; + } + // A store into the original global names the global; the shadow stores this + // pass emits target an alloca reached through a local GEP. + if (line.find('@') != std::string::npos) { + continue; + } + count++; + } + return count; +} + +// A flattened multi-dimensional array member renames the module global but +// not the debug variable, so the pass gathers shadow storage by linkage +// name. Keying it on the debug name instead loses the shadow store for every +// write into the flattened array. +TEST_F(PixTest, PixDbgValueToDbgDeclare_MultiDimensionalStaticGlobalArray) { + const char *source = R"x( +RWByteAddressBuffer RawUAV : register(u0); +struct StaticGlobalHolder +{ + float twoD[2][3]; + float oneD[3]; + float count; +}; +static StaticGlobalHolder g_staticGlobalHolder; +[numthreads(1, 1, 1)] +void main() +{ + g_staticGlobalHolder.oneD[0] = 4.0; + g_staticGlobalHolder.oneD[1] = 5.0; + g_staticGlobalHolder.oneD[2] = 6.0; + g_staticGlobalHolder.twoD[1][0] = 40.0; + g_staticGlobalHolder.twoD[1][2] = 42.0; + g_staticGlobalHolder.count = 1; + + float accumulator = 0; + uint index = 0; + [loop] + while (true) + { + accumulator += g_staticGlobalHolder.twoD[index % 2][index % 3]; + accumulator += g_staticGlobalHolder.oneD[index % 3]; + if (index++ == 4) + { + break; + } + } + RawUAV.Store(64, asuint(accumulator + g_staticGlobalHolder.count)); +})x"; + + auto compiled = Compile(m_dllSupport, source, L"cs_6_0", {L"-Od"}); + CComPtr dxilPart = FindModule(DFCC_ShaderDebugInfoDXIL, compiled); + auto output = RunValueToDeclarePass(dxilPart); + auto lines = Split(Disassemble(output.blob), '\n'); + + // The one-dimensional array in the same struct is the control: it is handled + // correctly whether or not the multi-dimensional case is. + VERIFY_ARE_EQUAL(1u, CountStoresToAllocaOfValue(lines, "4.000000e+00")); + // The two writes into the two-dimensional array are the point of the test. + VERIFY_ARE_EQUAL(1u, CountStoresToAllocaOfValue(lines, "4.000000e+01")); + VERIFY_ARE_EQUAL(1u, CountStoresToAllocaOfValue(lines, "4.200000e+01")); +} + +// Returns the module with the given instructions at the start of the entry +// point's first block. The disassembler prints a label for that block only +// when the block is named, and instructions placed ahead of a label would form +// a block with no terminator, so the injection follows the label when there is +// one and the definition's brace when there is not. +static std::string InjectIntoEntryBlock(const std::string &disassembly, + const std::string &instructions) { + const std::string definition = "define void @main() {"; + const std::string labelledDefinition = definition + "\nentry:"; + const std::string &anchor = + disassembly.find(labelledDefinition) != std::string::npos + ? labelledDefinition + : definition; + return PixTest::ReplaceOnlyOccurrence(disassembly, anchor, + anchor + "\n" + instructions); +} + +// Whether the disassembler labels an entry point's first block depends on +// whether the module kept the block's name, so the injection below pins its +// point against both forms rather than against the one this build produces. +TEST_F(PixTest, EntryBlockInjection_HandlesLabelledAndUnlabelledFirstBlock) { + const std::string instruction = " %injected = alloca float"; + + const std::string labelled = "define void @main() {\nentry:\n ret void\n}\n"; + VERIFY_ARE_EQUAL("define void @main() {\nentry:\n" + instruction + + "\n ret void\n}\n", + InjectIntoEntryBlock(labelled, instruction)); + + const std::string unlabelled = "define void @main() {\n ret void\n}\n"; + VERIFY_ARE_EQUAL("define void @main() {\n" + instruction + + "\n ret void\n}\n", + InjectIntoEntryBlock(unlabelled, instruction)); +} + +// A value nested three GEPs below the alloca needs the annotator to walk the +// whole ancestor chain, not just one level, before it can record the store's +// !pix-alloca-reg-write. DXC's SROA flattens aggregates before this pass +// runs, so this shape does not arise from HLSL; the module is constructed +// directly here. +TEST_F(PixTest, AllocaRegisterWrite_DeepAggregateChainIsAnnotated) { + auto compiled = Compile(m_dllSupport, R"x( +RWByteAddressBuffer RawUAV : register(u0); +[numthreads(1, 1, 1)] +void main() +{ + RawUAV.Store(0, 0); +})x", + L"cs_6_0", {L"-Od"}); + std::string disassembly = Disassemble(compiled); + + // An alloca of a struct nested three levels deep, then a GEP chain that + // descends every level to select a scalar, then a store into it. The store's + // pointer is three GEPs removed from the alloca. + std::string withDeepStore = InjectIntoEntryBlock( + disassembly, + " %deep = alloca { { { float, float } } }\n" + " %deep.l0 = getelementptr { { { float, float } } }, { { { " + "float, float } } }* %deep, i32 0, i32 0\n" + " %deep.l1 = getelementptr { { float, float } }, { { float, " + "float } }* %deep.l0, i32 0, i32 0\n" + " %deep.l2 = getelementptr { float, float }, { float, float " + "}* %deep.l1, i32 0, i32 1\n" + " store float 1.000000e+00, float* %deep.l2\n"); + + std::vector lines = RunAnnotationPassOnText(withDeepStore); + + bool allocaRegistered = false; + bool storeFound = false; + bool storeAnnotated = false; + for (const std::string &line : lines) { + if (line.find("%deep = alloca") != std::string::npos && + line.find("pix-alloca-reg") != std::string::npos) { + allocaRegistered = true; + } + if (line.find("store float 1.000000e+00, float* %deep.l2") != + std::string::npos) { + storeFound = true; + if (line.find("pix-alloca-reg-write") != std::string::npos) { + storeAnnotated = true; + } + } + } + + // The alloca is registered, so the shape reached the pass and the store below + // is the thing under test rather than an artifact of it being skipped. + VERIFY_IS_TRUE(allocaRegistered); + VERIFY_IS_TRUE(storeFound); + // The store three GEPs deep still carries its alloca-register-write. + VERIFY_IS_TRUE(storeAnnotated); +} + +/////////////////////////////////////////////////////////////////////////////// +// Tests for the pixel-hit and debug instrumentation passes' SV_Position +// signature handling: finding the shader's return in every block, safe +// counter arithmetic, and relocating the row occupant (rather than +// SV_Position itself) when the upstream stage's row is already taken. + +// Pulls a named input-signature element's start row out of the DXIL signature +// metadata, whose fields are +// {ID, Name, ComponentType, SemanticKind, SemanticIndexes, InterpolationMode, +// Rows, Cols, StartRow, StartCol, NameValueList}. +static int FindSignatureElementStartRow(std::vector const &lines, + char const *name) { + std::string const needle = std::string("!\"") + name + "\""; + for (auto const &line : lines) { + if (line.find(needle) == std::string::npos) + continue; + auto fields = Split(line, ','); + if (fields.size() < 10) + continue; + constexpr size_t StartRowField = 8; + auto const &startRowField = fields[StartRowField]; + auto valueStart = startRowField.find("i32 "); + if (valueStart == std::string::npos) + continue; + return atoi(startRowField.c_str() + valueStart + 4); + } + return -1; +} + +// Counts the pixel-hit counter increments the instrumentation emitted. Every +// increment is an atomic add against the pass's own counter UAV, so keying off +// that handle name keeps any atomic the shader itself performs out of the +// count. +static int CountPixelHitIncrements(std::vector const &lines) { + int increments = 0; + for (auto const &line : lines) { + if (line.find("dx.op.atomicBinOp") != std::string::npos && + line.find("%PIX_CountUAV_Handle") != std::string::npos) + increments++; + } + return increments; +} + +// A pixel shader containing a loop ends its entry block in a branch, not a +// return. The pass scans every block in the function for a return +// instruction, so this shader's counter still increments once per exit point. +TEST_F(PixTest, PixelHitInstrumentation_ReturnOutsideEntryBlock) { + const char *source = R"x( +float4 main(float4 pos : SV_Position, nointerpolation uint count : COUNT) + : SV_Target +{ + float4 accumulated = 0; + [loop] for (uint index = 0; index < count; ++index) + { + accumulated += pos * index; + } + return accumulated; +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {}); + + // The whole point of the shader is that its return does not live in the entry + // block, so confirm the loop really did survive into the DXIL rather than + // being flattened away. + auto uninstrumentedLines = Split(Disassemble(compiled), '\n'); + int labelCount = 0; + for (auto const &line : uninstrumentedLines) { + if (line.find("; preds = ") != std::string::npos) + labelCount++; + } + VERIFY_IS_TRUE(labelCount > 0); + + auto output = RunPixelHitPass(compiled, 16, 64); + auto lines = Split(Disassemble(output.blob), '\n'); + + // The shader has one return, however many blocks control flow crosses + // to reach it, so the instrumented module increments the counter once. + VERIFY_ARE_EQUAL(1, CountPixelHitIncrements(lines)); + + verifyInstrumentedModuleIsValid(output.blob, "pixel-hit instrumentation"); +} + +// The pixel-hit pass has to place SV_Position on the row the upstream stage +// used, because D3D12 pairs the stages by register. When the pixel shader +// has already packed one of its own inputs at that row, appending on top of +// it leaves two elements overlapping the same register and the validator +// rejects the module. +TEST_F(PixTest, PixelHitInstrumentation_SVPositionRowAlreadyOccupied) { + const char *source = R"x( +float4 main(float4 col : COLOR) : SV_Target +{ + return col; +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {}); + + // Row 0 is both COLOR's row and, here, the upstream stage's SV_Position row. + auto output = RunPixelHitPass(compiled, 16, 64, 0 /*requiredSVPositionRow*/); + auto lines = Split(Disassemble(output.blob), '\n'); + + int const colorRow = FindSignatureElementStartRow(lines, "COLOR"); + int const positionRow = FindSignatureElementStartRow(lines, "SV_Position"); + + // It has to land on the upstream row and nowhere else, because D3D12 pairs + // the stages by register and an SV_Position on any other row fails pipeline + // creation outright. So the occupant is the element that moves. + VERIFY_ARE_EQUAL(0, positionRow); + VERIFY_ARE_NOT_EQUAL(colorRow, positionRow); + + verifyInstrumentedModuleIsValid(output.blob, "pixel-hit instrumentation"); +} + +// PIX omits the option when it cannot read the previous stage's signature. The +// pass must not relocate anything on the strength of a guessed row: the +// shader's own attributes are still linkage-bound to whatever the real upstream +// stage is, so the injected SV_Position goes on a row of its own and leaves +// them alone. +TEST_F(PixTest, PixelHitInstrumentation_SVPositionRowUnknown) { + const char *source = R"x( +float4 main(float4 col : COLOR) : SV_Target +{ + return col; +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {}); + + auto output = RunPixelHitPass(compiled, 16, 64); + auto lines = Split(Disassemble(output.blob), '\n'); + + VERIFY_ARE_EQUAL(0, FindSignatureElementStartRow(lines, "COLOR")); + VERIFY_ARE_EQUAL(1, FindSignatureElementStartRow(lines, "SV_Position")); + + verifyInstrumentedModuleIsValid(output.blob, "pixel-hit instrumentation"); +} + +// The collision that actually occurs in the wild: a pixel shader reading a +// strict subset of the vertex shader's outputs plus a system value the +// rasterizer supplies. SV_PrimitiveID needs no vertex shader counterpart, so it +// packs onto row 2 -- exactly where the vertex shader here writes SV_Position. +// +// Relocating SV_Position off row 2 desynchronises the stages and D3D12 rejects +// the pipeline with "Semantic 'SV_Position', Index '0' is defined for +// mismatched hardware registers between the output stage and input stage". +// SV_PrimitiveID has no such constraint, so it is the one that moves. +TEST_F(PixTest, PixelHitInstrumentation_SVPositionRowOccupiedBySystemValue) { + const char *source = R"x( +struct PSInput +{ + float2 uv : TEXCOORD0; + float4 color : COLOR0; + uint primitiveId : SV_PrimitiveID; +}; + +float4 main(PSInput input) : SV_Target +{ + return float4(input.color.rgb, input.uv.x + input.primitiveId); +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {}); + + auto output = RunPixelHitPass(compiled, 16, 64, 2 /*requiredSVPositionRow*/); + auto lines = Split(Disassemble(output.blob), '\n'); + + VERIFY_ARE_EQUAL(2, FindSignatureElementStartRow(lines, "SV_Position")); + VERIFY_ARE_NOT_EQUAL(2, + FindSignatureElementStartRow(lines, "SV_PrimitiveID")); + VERIFY_ARE_EQUAL(0, FindSignatureElementStartRow(lines, "TEXCOORD")); + VERIFY_ARE_EQUAL(1, FindSignatureElementStartRow(lines, "COLOR")); + + verifyInstrumentedModuleIsValid(output.blob, "pixel-hit instrumentation"); +} + +// rt-width and num-pixels size the counter UAV and convert SV_Position into a +// byte offset into it. A num-pixels large enough that its pixel-cost high +// water mark (num-pixels * 2 * 4 bytes) does not fit in 32 bits has no buffer +// layout to compute offsets against, so the pass rejects it. +TEST_F(PixTest, PixelHitInstrumentation_RejectsUnrepresentableDimensions) { + const char *source = R"x( +float4 main(float4 pos : SV_Position) : SV_Target +{ + return pos; +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {}); + + CComPtr pOptimizer; + VERIFY_SUCCEEDED( + m_dllSupport.CreateInstance(CLSID_DxcOptimizer, &pOptimizer)); + std::vector Options; + Options.push_back(L"-opt-mod-passes"); + Options.push_back(L"-hlsl-dxil-add-pixel-hit-instrmentation,rt-width=16," + L"num-pixels=536870912,add-pixel-cost=1"); + + CComPtr pOptimizedModule; + CComPtr pText; + HRESULT hr = pOptimizer->RunOptimizer( + compiled, Options.data(), Options.size(), &pOptimizedModule, &pText); + VERIFY_FAILED(hr); +} + +// A viewport offset from the render target's origin, or a counter buffer +// smaller than rt-width * rt-height, lets SV_Position describe a pixel +// outside the rectangle num-pixels was sized for. The counter index is +// clamped into the buffer, so the atomic add cannot land outside it. +TEST_F(PixTest, PixelHitInstrumentation_ClampsCounterIndexForSmallBuffer) { + const char *source = R"x( +float4 main(float4 pos : SV_Position) : SV_Target +{ + return pos; +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {}); + + // A small render target and a small pixel count stand in for a viewport that + // does not cover the whole surface the shader was told about. + const int RTWidth = 4; + const int NumPixels = 16; + auto output = RunPixelHitPass(compiled, RTWidth, NumPixels); + auto lines = Split(Disassemble(output.blob), '\n'); + + // The clamp is a UMin (DXIL binary opcode 40) against the last valid + // element in the counter's first half, applied before the element count is + // scaled to a byte offset. + const std::string expectedClamp = + "= call i32 @dx.op.binary.i32(i32 40, i32 %ElementOffset, i32 " + + std::to_string(NumPixels - 1) + ")"; + bool foundClamp = false; + bool incrementUsesClampedIndex = false; + for (auto const &line : lines) { + if (line.find(expectedClamp) != std::string::npos) + foundClamp = true; + if (line.find("dx.op.atomicBinOp") != std::string::npos && + line.find("%PIX_CountUAV_Handle") != std::string::npos && + line.find("%ByteIndex") != std::string::npos) + incrementUsesClampedIndex = true; + } + VERIFY_IS_TRUE(foundClamp); + VERIFY_IS_TRUE(incrementUsesClampedIndex); + + verifyInstrumentedModuleIsValid(output.blob, "pixel-hit instrumentation"); +} + +// applyOptions bounds num-pixels but not rt-width, so a caller can pass a +// rt-width that num-pixels has no room for. SV_Position's Y coordinate then +// drives the element count past num-pixels by enough that scaling it to a +// byte offset first, then clamping, lets the 32-bit multiply wrap before the +// clamp ever sees it. Clamping the element count first never wraps: +// (NumPixels-1)*4 is always in range, so the clamped count is always safe to +// scale by 4. +TEST_F(PixTest, + PixelHitInstrumentation_ClampOrderingSurvivesElementOffsetOverflow) { + // YIndex=1 alone drives ElementOffset to RTWidth, and scaling that by 4 + // wraps a 32-bit value to 0 before any clamp can bound it. + const uint32_t RTWidth = 0x40000000; + const uint32_t NumPixels = 64; + const uint32_t YIndex = 1; + const uint32_t XIndex = 0; + const uint32_t ElementOffset = XIndex + YIndex * RTWidth; + const uint32_t MaxCounterByteIndex = (NumPixels - 1) * 4; + + // Scaling first, then clamping the byte offset: the multiply wraps + // ElementOffset to 0, and UMin of 0 against the limit is still 0 -- the + // increment lands on pixel 0's slot instead of the last one. + const uint32_t byteIndexScaledFirst = ElementOffset * 4u; + const uint32_t clampedAfterScaling = + std::min(byteIndexScaledFirst, MaxCounterByteIndex); + VERIFY_ARE_EQUAL(0u, byteIndexScaledFirst); + VERIFY_ARE_EQUAL(0u, clampedAfterScaling); + + // Clamping the element count first, then scaling: the clamp can only + // shrink ElementOffset, so the multiply that follows is always within the + // range applyOptions already guarantees is safe. + const uint32_t clampedElementOffset = std::min(ElementOffset, NumPixels - 1); + const uint32_t byteIndexClampedFirst = clampedElementOffset * 4u; + VERIFY_ARE_EQUAL(MaxCounterByteIndex, byteIndexClampedFirst); + + VERIFY_ARE_NOT_EQUAL(clampedAfterScaling, byteIndexClampedFirst); + + // The pass itself clamps %ElementOffset, not a byte offset derived from it: + // confirm both the clamp's operand and the final increment's operand match + // that ordering for this exact hazard. + const char *source = R"x( +float4 main(float4 pos : SV_Position) : SV_Target +{ + return pos; +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {}); + auto output = RunPixelHitPass(compiled, static_cast(RTWidth), + static_cast(NumPixels)); + auto lines = Split(Disassemble(output.blob), '\n'); + + const std::string expectedClamp = + "= call i32 @dx.op.binary.i32(i32 40, i32 %ElementOffset, i32 " + + std::to_string(NumPixels - 1) + ")"; + bool foundClamp = false; + bool incrementUsesByteIndex = false; + for (auto const &line : lines) { + if (line.find(expectedClamp) != std::string::npos) + foundClamp = true; + if (line.find("dx.op.atomicBinOp") != std::string::npos && + line.find("%PIX_CountUAV_Handle") != std::string::npos && + line.find("%ByteIndex") != std::string::npos) + incrementUsesByteIndex = true; + } + VERIFY_IS_TRUE(foundClamp); + VERIFY_IS_TRUE(incrementUsesByteIndex); + + verifyInstrumentedModuleIsValid(output.blob, "pixel-hit instrumentation"); +} + +// A pixel shader input signature that already fills every one of the 32 +// available registers: ATTR occupies rows 0-30, and the two rasterizer +// system values share row 31 with each other -- and with the upstream +// stage's SV_Position. Evicting them to make room for SV_Position leaves +// nowhere for either of them to go, so placing SV_Position at its +// authoritative row cannot succeed. The pass rejects the module rather than +// emitting a register past the end of the signature, and leaves both +// evicted elements at their original row instead of one of them stranded +// mid-repack. +TEST_F(PixTest, + PixelHitInstrumentation_RejectsAuthoritativeRowWithNoRoomToEvict) { + const char *source = R"x( +struct DensePSInput +{ + float4 attributes[31] : ATTR; + uint primitiveId : SV_PrimitiveID; + bool isFrontFace : SV_IsFrontFace; +}; + +float4 main(DensePSInput input) : SV_Target +{ + float4 accumulated = 0; + [unroll] for (uint index = 0; index < 31; ++index) + { + accumulated += input.attributes[index]; + } + + accumulated.a = input.primitiveId + (input.isFrontFace ? 1.0f : 0.0f); + return accumulated; +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {}); + + CComPtr pOptimizer; + VERIFY_SUCCEEDED( + m_dllSupport.CreateInstance(CLSID_DxcOptimizer, &pOptimizer)); + std::vector Options; + Options.push_back(L"-opt-mod-passes"); + Options.push_back( + L"-hlsl-dxil-add-pixel-hit-instrmentation,rt-width=16,num-pixels=64," + L"required-sv-position-row=31"); + + CComPtr pOptimizedModule; + CComPtr pText; + HRESULT hr = pOptimizer->RunOptimizer( + compiled, Options.data(), Options.size(), &pOptimizedModule, &pText); + VERIFY_FAILED(hr); +} + +// A normal compile embeds a ViewID dependency table sized for the shader's +// declared signature. Appending SV_Position grows the signature, so the +// table must be cleared: container assembly reconciles the module's +// per-register data against the table's size, and a table sized for the +// smaller signature describes the wrong one. +TEST_F(PixTest, + PixelHitInstrumentation_ClearsStaleViewIdStateAfterSignatureGrowth) { + const char *source = R"x( +float4 main(float4 col : COLOR) : SV_Target +{ + return col; +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {}); + + // Confirm the premise: an ordinary compile of this shader really does + // embed a ViewID dependency table, so the pass has something stale to + // clear. + auto uninstrumentedLines = Split(Disassemble(compiled), '\n'); + bool hadViewIdState = false; + for (auto const &line : uninstrumentedLines) { + if (line.find("dx.viewIdState") != std::string::npos) + hadViewIdState = true; + } + VERIFY_IS_TRUE(hadViewIdState); + + auto output = RunPixelHitPass(compiled, 16, 64, 0 /*requiredSVPositionRow*/); + auto lines = Split(Disassemble(output.blob), '\n'); + + // The table describing the old, smaller signature must not survive. + bool stillHasViewIdState = false; + for (auto const &line : lines) { + if (line.find("dx.viewIdState") != std::string::npos) + stillHasViewIdState = true; + } + VERIFY_IS_FALSE(stillHasViewIdState); + + // Reassembling and validating exercises exactly this: a stale table must + // not reach container assembly. + verifyInstrumentedModuleIsValid(output.blob, "pixel-hit instrumentation"); +} + +// A normal compile embeds a ViewID dependency table sized for the shader's +// declared signature. Appending SV_VertexID or SV_InstanceID grows the +// signature, so the table must be cleared: container assembly reconciles +// the module's per-register data against the table's size, and a table +// sized for the smaller signature describes the wrong one. +TEST_F(PixTest, + DebugInstrumentation_ClearsStaleViewIdStateAfterVSSignatureGrowth) { + const char *source = R"x( +float4 main(float4 pos : POSITION) : SV_Position +{ + return pos; +})x"; + + auto compiled = Compile(m_dllSupport, source, L"vs_6_2", {}); + + // Confirm the premise: an ordinary compile of this shader really does + // embed a ViewID dependency table, so the pass has something stale to + // clear. + auto uninstrumentedLines = Split(Disassemble(compiled), '\n'); + bool hadViewIdState = false; + for (auto const &line : uninstrumentedLines) { + if (line.find("dx.viewIdState") != std::string::npos) + hadViewIdState = true; + } + VERIFY_IS_TRUE(hadViewIdState); + + CComPtr pOptimizer; + VERIFY_SUCCEEDED( + m_dllSupport.CreateInstance(CLSID_DxcOptimizer, &pOptimizer)); + std::vector Options; + Options.push_back(L"-opt-mod-passes"); + Options.push_back( + L"-hlsl-dxil-debug-instrumentation,parameter0=1,parameter1=2"); + Options.push_back(L"-hlsl-dxilemit"); + + CComPtr pOptimizedModule; + CComPtr pText; + VERIFY_SUCCEEDED(pOptimizer->RunOptimizer( + compiled, Options.data(), Options.size(), &pOptimizedModule, &pText)); + + auto lines = Split(Disassemble(pOptimizedModule), '\n'); + + // The table describing the old, smaller signature must not survive. + bool stillHasViewIdState = false; + for (auto const &line : lines) { + if (line.find("dx.viewIdState") != std::string::npos) + stillHasViewIdState = true; + } + VERIFY_IS_FALSE(stillHasViewIdState); + + // Reassembling and validating exercises exactly this: a stale table must + // not reach container assembly. + verifyInstrumentedModuleIsValid(pOptimizedModule, "debug instrumentation"); +} + +// Control test for the pixel-hit pass' own use of the validation harness: a +// straightforward pixel shader, instrumented and confirmed to still validate. +TEST_F(PixTest, Validation_PixelHit_PixelShader) { + const char *source = R"x( +float4 main(float4 pos : SV_Position) : SV_Target +{ + return pos; +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {}); + auto output = RunPixelHitPass(compiled, 16, 64, 0 /*requiredSVPositionRow*/); + verifyInstrumentedModuleIsValid(output.blob, "pixel-hit instrumentation"); +} diff --git a/utils/hct/hctdb.py b/utils/hct/hctdb.py index 5e72afcfb9..4eccca941b 100644 --- a/utils/hct/hctdb.py +++ b/utils/hct/hctdb.py @@ -7056,6 +7056,10 @@ def add_pass(name, type_name, doc, opts): {"n": "add-pixel-cost", "t": "int", "c": 1}, {"n": "rt-width", "t": "int", "c": 1}, {"n": "num-pixels", "t": "int", "c": 1}, + {"n": "preferred-sv-position-row", "t": "int", "c": 1}, + {"n": "required-sv-position-row", "t": "int", "c": 1}, + # Pre-rename spelling of preferred-sv-position-row, kept so + # PIX builds older than this rename keep working. {"n": "upstream-sv-position-row", "t": "int", "c": 1}, ], ) @@ -7115,6 +7119,7 @@ def add_pass(name, type_name, doc, opts): {"n": "parameter1", "t": "int", "c": 1}, {"n": "parameter2", "t": "int", "c": 1}, {"n": "upstreamSVPositionRow", "t": "int", "c": 1}, + {"n": "authoritativeSVPositionRow", "t": "int", "c": 1}, ], ) add_pass(