From 0e1a37420e2111a2b4fb72808707075cb5121dfc Mon Sep 17 00:00:00 2001 From: Damyan Pepper Date: Mon, 24 Aug 2026 19:58:17 -0700 Subject: [PATCH 1/5] [PIX] Fix shared instrumentation resource handling More than one PIX pass can run on the same module. Each pass adds its own tools UAV, so a pipeline with two passes makes two resources at the same register and space. The code that adds this UAV to a root signature has three unsafe paths. Root signature serialization can fail and give a null blob, which the code then reads. An empty result can replace a correct root signature. Only the first global root-signature subobject gets the new parameter, so a state object that declares more than one is left in a mixed state. The tools UAV is a raw buffer, so it changes the shader flags of the module. The code does not compute the flags again. Some passes make a dx.op overload declaration and then find no callers for it. They leave the declaration in the module, and the validator refuses a module that has an unused declaration. When serialization fails, the helpers keep the original root signature. A valid signature is better than an empty one. Assisted-by: Copilot Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 40dc9de3-617e-4caf-ab0d-fba0a033ed93 --- .../DxilDebugBreakInstrumentation.cpp | 6 +- .../DxilDebugInstrumentation.cpp | 17 - ...NonUniformResourceIndexInstrumentation.cpp | 3 + .../DxilOutputColorBecomesConstant.cpp | 5 + ...lPIXAddTidToAmplificationShaderPayload.cpp | 3 + ...DxilPIXMeshShaderOutputInstrumentation.cpp | 9 + lib/DxilPIXPasses/DxilRemoveDiscards.cpp | 4 + lib/DxilPIXPasses/PixPassHelpers.cpp | 115 ++-- lib/DxilPIXPasses/PixPassHelpers.h | 2 +- .../HLSLFileCheck/pix/pixelCounterEarlyZ.hlsl | 6 +- tools/clang/unittests/HLSL/PixTest.cpp | 595 +++++++++++++++++- 11 files changed, 684 insertions(+), 81 deletions(-) diff --git a/lib/DxilPIXPasses/DxilDebugBreakInstrumentation.cpp b/lib/DxilPIXPasses/DxilDebugBreakInstrumentation.cpp index 30799c4bdc..dc27f4b469 100644 --- a/lib/DxilPIXPasses/DxilDebugBreakInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilDebugBreakInstrumentation.cpp @@ -120,10 +120,8 @@ bool DxilDebugBreakInstrumentation::runOnModule(Module &M) { CI->eraseFromParent(); } - // Clean up the now-unused declaration. Not strictly required for - // correctness, but keeps the module free of dead references. - if (DebugBreakFunc->use_empty()) - DebugBreakFunc->eraseFromParent(); + PIXPassHelpers::EraseIfUnused(DM, DebugBreakFunc); + PIXPassHelpers::EraseIfUnused(DM, AtomicOpFunc); const bool modified = (PixUAVResource != nullptr); diff --git a/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp b/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp index 4dd43b07cc..a40acfe860 100644 --- a/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp @@ -1436,23 +1436,6 @@ bool DxilDebugInstrumentation::RunOnFunction(Module &M, DxilModule &DM, auto &values = m_FunctionToValues[BC.Builder.GetInsertBlock()->getParent()]; - // PIX binds two UAVs when running this instrumentation: one for raygen - // shaders and another for the hitgroups and miss shaders. Since PIX invokes - // this pass at the library level, which may contain examples of both types, - // PIX can't really specify which UAV index to use per-shader. This pass - // therefore just has to know this: - constexpr unsigned int RayGenUAVRegister = 0; - constexpr unsigned int HitGroupAndMissUAVRegister = 1; - unsigned int UAVRegisterId = RayGenUAVRegister; - switch (shaderKind) { - case DXIL::ShaderKind::ClosestHit: - case DXIL::ShaderKind::Intersection: - case DXIL::ShaderKind::AnyHit: - case DXIL::ShaderKind::Miss: - UAVRegisterId = HitGroupAndMissUAVRegister; - break; - } - values.UAVHandle = PIXPassHelpers::CreateHandleForResource( DM, Builder, uav, "PIX_DebugUAV_Handle"); diff --git a/lib/DxilPIXPasses/DxilNonUniformResourceIndexInstrumentation.cpp b/lib/DxilPIXPasses/DxilNonUniformResourceIndexInstrumentation.cpp index a442bfabed..3c7a542648 100644 --- a/lib/DxilPIXPasses/DxilNonUniformResourceIndexInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilNonUniformResourceIndexInstrumentation.cpp @@ -149,6 +149,9 @@ bool DxilNonUniformResourceIndexInstrumentation::runOnModule(Module &M) { const bool modified = (PixUAVResource != nullptr); + PIXPassHelpers::EraseIfUnused(DM, WaveActiveAllEqualFunc); + PIXPassHelpers::EraseIfUnused(DM, AtomicOpFunc); + if (modified) { DM.ReEmitDxilResources(); diff --git a/lib/DxilPIXPasses/DxilOutputColorBecomesConstant.cpp b/lib/DxilPIXPasses/DxilOutputColorBecomesConstant.cpp index a0749fbe0f..227cc890a7 100644 --- a/lib/DxilPIXPasses/DxilOutputColorBecomesConstant.cpp +++ b/lib/DxilPIXPasses/DxilOutputColorBecomesConstant.cpp @@ -125,6 +125,8 @@ bool DxilOutputColorBecomesConstant::runOnModule(Module &M) { [&hasIntOutputs](CallInst *) { hasIntOutputs = true; }); if (!hasFloatOutputs && !hasIntOutputs) { + PIXPassHelpers::EraseIfUnused(DM, FloatOutputFunction); + PIXPassHelpers::EraseIfUnused(DM, IntOutputFunction); return false; } @@ -251,6 +253,9 @@ bool DxilOutputColorBecomesConstant::runOnModule(Module &M) { }); } + PIXPassHelpers::EraseIfUnused(DM, FloatOutputFunction); + PIXPassHelpers::EraseIfUnused(DM, IntOutputFunction); + return Modified; } diff --git a/lib/DxilPIXPasses/DxilPIXAddTidToAmplificationShaderPayload.cpp b/lib/DxilPIXPasses/DxilPIXAddTidToAmplificationShaderPayload.cpp index e756e98781..c459d4e768 100644 --- a/lib/DxilPIXPasses/DxilPIXAddTidToAmplificationShaderPayload.cpp +++ b/lib/DxilPIXPasses/DxilPIXAddTidToAmplificationShaderPayload.cpp @@ -183,8 +183,11 @@ bool DxilPIXAddTidToAmplificationShaderPayload::runOnModule(Module &M) { {DispatchMeshOpcode, DispatchMesh.get_threadGroupCountX(), DispatchMesh.get_threadGroupCountY(), DispatchMesh.get_threadGroupCountZ(), NewStructAlloca}); + llvm::Function *OriginalDispatchMeshFn = + cast(&*I)->getCalledFunction(); I->removeFromParent(); delete &*I; + PIXPassHelpers::EraseIfUnused(DM, OriginalDispatchMeshFn); // Validation requires exactly one DispatchMesh in an AS, so we can exit // after the first one: DM.ReEmitDxilResources(); diff --git a/lib/DxilPIXPasses/DxilPIXMeshShaderOutputInstrumentation.cpp b/lib/DxilPIXPasses/DxilPIXMeshShaderOutputInstrumentation.cpp index 507a718a4c..418c285576 100644 --- a/lib/DxilPIXPasses/DxilPIXMeshShaderOutputInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilPIXMeshShaderOutputInstrumentation.cpp @@ -310,6 +310,8 @@ bool DxilPIXMeshShaderOutputInstrumentation::runOnModule(Module &M) { } if (getMeshPayloadInstructions != nullptr) { + llvm::Function *OriginalGetMeshPayloadFunction = + cast(getMeshPayloadInstructions)->getCalledFunction(); Function *DxilFunc = HlslOP->GetOpFunc( OP::OpCode::GetMeshPayload, expanded.ExpandedPayloadStructPtrType); @@ -326,6 +328,7 @@ bool DxilPIXMeshShaderOutputInstrumentation::runOnModule(Module &M) { ReplaceAllUsesOfInstructionWithNewValueAndDeleteInstruction( getMeshPayloadInstructions, payload, expanded.ExpandedPayloadStructType); + PIXPassHelpers::EraseIfUnused(DM, OriginalGetMeshPayloadFunction); } } @@ -378,9 +381,11 @@ bool DxilPIXMeshShaderOutputInstrumentation::runOnModule(Module &M) { {Type::getInt16Ty(Ctx), int16ValueIndicator}, {Type::getFloatTy(Ctx), floatValueIndicator}, {Type::getHalfTy(Ctx), float16ValueIndicator}}; + SmallVector StoreVertexOutputFunctions; for (auto const &Overload : StoreVertexOutputOverloads) { F = HlslOP->GetOpFunc(DXIL::OpCode::StoreVertexOutput, Overload.type); + StoreVertexOutputFunctions.push_back(F); FunctionUses = F->uses(); for (auto FI = FunctionUses.begin(); FI != FunctionUses.end();) { auto &FunctionUse = *FI++; @@ -419,6 +424,10 @@ bool DxilPIXMeshShaderOutputInstrumentation::runOnModule(Module &M) { } } + for (Function *StoreVertexOutputFunction : StoreVertexOutputFunctions) { + PIXPassHelpers::EraseIfUnused(DM, StoreVertexOutputFunction); + } + DM.ReEmitDxilResources(); return true; diff --git a/lib/DxilPIXPasses/DxilRemoveDiscards.cpp b/lib/DxilPIXPasses/DxilRemoveDiscards.cpp index 5f8d9263f0..72b2bf88cd 100644 --- a/lib/DxilPIXPasses/DxilRemoveDiscards.cpp +++ b/lib/DxilPIXPasses/DxilRemoveDiscards.cpp @@ -17,6 +17,8 @@ #include "llvm/IR/Instructions.h" #include "llvm/IR/PassManager.h" +#include "PixPassHelpers.h" + using namespace llvm; using namespace hlsl; @@ -53,6 +55,8 @@ bool DxilRemoveDiscards::runOnModule(Module &M) { Modified = true; } + PIXPassHelpers::EraseIfUnused(DM, DiscardFunction); + return Modified; } diff --git a/lib/DxilPIXPasses/PixPassHelpers.cpp b/lib/DxilPIXPasses/PixPassHelpers.cpp index c7c99cf763..1137fe6cf8 100644 --- a/lib/DxilPIXPasses/PixPassHelpers.cpp +++ b/lib/DxilPIXPasses/PixPassHelpers.cpp @@ -185,6 +185,9 @@ static std::vector SerializeRootSignatureToVector( SerializeRootSignature(rootSignature, &serializedRootSignature, &errorBlob, allowReservedRegisterSpace); std::vector ret; + if (serializedRootSignature == nullptr) { + return ret; + } auto const *serializedData = reinterpret_cast( serializedRootSignature->GetBufferPointer()); ret.assign(serializedData, @@ -194,10 +197,9 @@ static std::vector SerializeRootSignatureToVector( } constexpr uint32_t toolsRegisterSpace = static_cast(-2); -constexpr uint32_t toolsUAVRegister = 0; template -void ExtendRootSig(RootSigDesc &rootSigDesc) { +void ExtendRootSig(RootSigDesc &rootSigDesc, uint32_t toolsUAVRegister) { auto *existingParams = rootSigDesc.pParameters; for (uint32_t i = 0; i < rootSigDesc.NumParameters; ++i) { if (rootSigDesc.pParameters[i].ParameterType == @@ -229,17 +231,20 @@ void ExtendRootSig(RootSigDesc &rootSigDesc) { rootSigDesc.NumParameters++; } -static std::vector AddUAVParamterToRootSignature(const void *Data, - uint32_t Size) { +static std::vector +AddUAVParamterToRootSignature(const void *Data, uint32_t Size, + uint32_t toolsUAVRegister) { DxilVersionedRootSignature rootSignature; DeserializeRootSignature(Data, Size, rootSignature.get_address_of()); auto *rs = rootSignature.get_mutable(); switch (rootSignature->Version) { case DxilRootSignatureVersion::Version_1_0: - ExtendRootSig(rs->Desc_1_0); + ExtendRootSig(rs->Desc_1_0, + toolsUAVRegister); break; case DxilRootSignatureVersion::Version_1_1: - ExtendRootSig(rs->Desc_1_1); + ExtendRootSig(rs->Desc_1_1, + toolsUAVRegister); rs->Desc_1_1.pParameters[rs->Desc_1_1.NumParameters - 1].Descriptor.Flags = hlsl::DxilRootDescriptorFlags::None; break; @@ -247,16 +252,26 @@ static std::vector AddUAVParamterToRootSignature(const void *Data, return SerializeRootSignatureToVector(rs); } -static void AddUAVToShaderAttributeRootSignature(DxilModule &DM) { +static void AddUAVToShaderAttributeRootSignature(DxilModule &DM, + uint32_t toolsUAVRegister) { auto rs = DM.GetSerializedRootSignature(); if (!rs.empty()) { std::vector asVector = AddUAVParamterToRootSignature( - rs.data(), static_cast(rs.size())); - DM.ResetSerializedRootSignature(asVector); + rs.data(), static_cast(rs.size()), toolsUAVRegister); + if (!asVector.empty()) { + DM.ResetSerializedRootSignature(asVector); + } } } -static void AddUAVToDxilDefinedGlobalRootSignatures(DxilModule &DM) { +static void AddUAVToDxilDefinedGlobalRootSignatures(DxilModule &DM, + uint32_t toolsUAVRegister) { + struct ReplacementRootSignature { + std::string Name; + std::vector Data; + }; + + std::vector replacementRootSignatures; auto *subObjects = DM.GetSubobjects(); if (subObjects != nullptr) { for (auto const &subObject : subObjects->GetSubobjects()) { @@ -267,16 +282,24 @@ static void AddUAVToDxilDefinedGlobalRootSignatures(DxilModule &DM) { constexpr bool notALocalRS = false; if (subObject.second->GetRootSignature(notALocalRS, Data, Size, nullptr)) { - auto extendedRootSig = AddUAVParamterToRootSignature(Data, Size); - auto rootSignatureSubObjectName = subObject.first; - subObjects->RemoveSubobject(rootSignatureSubObjectName); - subObjects->CreateRootSignature( - rootSignatureSubObjectName, notALocalRS, extendedRootSig.data(), - static_cast(extendedRootSig.size())); - break; + std::vector extended = + AddUAVParamterToRootSignature(Data, Size, toolsUAVRegister); + if (!extended.empty()) { + replacementRootSignatures.push_back( + {subObject.first.str(), std::move(extended)}); + } } } } + + constexpr bool notALocalRS = false; + for (auto const &replacementRootSignature : replacementRootSignatures) { + subObjects->RemoveSubobject(replacementRootSignature.Name); + subObjects->CreateRootSignature( + replacementRootSignature.Name, notALocalRS, + replacementRootSignature.Data.data(), + static_cast(replacementRootSignature.Data.size())); + } } } @@ -286,6 +309,13 @@ hlsl::DxilResource *CreateGlobalUAVResource(hlsl::DxilModule &DM, const char *name) { LLVMContext &Ctx = DM.GetModule()->getContext(); + for (auto const &existingUAV : DM.GetUAVs()) { + if (existingUAV->GetSpaceID() == toolsRegisterSpace && + existingUAV->GetLowerBound() == hlslBindIndex) { + return existingUAV.get(); + } + } + const char *PIXStructTypeName = ShaderModelHandleTypeName(DM); llvm::StructType *UAVStructTy = DM.GetModule()->getTypeByName(PIXStructTypeName); @@ -295,10 +325,8 @@ hlsl::DxilResource *CreateGlobalUAVResource(hlsl::DxilModule &DM, UAVStructTy = llvm::StructType::create(Elements, PIXStructTypeName); } - // Since this function should only be called once per module, - // we can modify the root sig at the same time: - AddUAVToDxilDefinedGlobalRootSignatures(DM); - AddUAVToShaderAttributeRootSignature(DM); + AddUAVToDxilDefinedGlobalRootSignatures(DM, hlslBindIndex); + AddUAVToShaderAttributeRootSignature(DM, hlslBindIndex); unsigned int Id = static_cast(DM.GetUAVs().size()); std::unique_ptr pUAV = llvm::make_unique(); @@ -320,8 +348,7 @@ hlsl::DxilResource *CreateGlobalUAVResource(hlsl::DxilModule &DM, } pUAV->SetGlobalName(name); pUAV->SetRW(true); // sets UAV class - pUAV->SetSpaceID( - (unsigned int)-2); // This is the reserved-for-tools register space + pUAV->SetSpaceID(toolsRegisterSpace); // reserved-for-tools register space pUAV->SetSampleCount(0); // This is what compiler generates for a raw UAV pUAV->SetGloballyCoherent(false); pUAV->SetReorderCoherent(false); @@ -351,9 +378,17 @@ hlsl::DxilResource *CreateGlobalUAVResource(hlsl::DxilModule &DM, auto *ret = pUAV.get(); DM.AddUAV(std::move(pUAV)); + DM.CollectShaderFlagsForModule(); return ret; } +void EraseIfUnused(hlsl::DxilModule &DM, llvm::Function *OpFunction) { + if (OpFunction != nullptr && OpFunction->user_empty()) { + DM.GetOP()->RemoveFunction(OpFunction); + OpFunction->eraseFromParent(); + } +} + // Set up a UAV with structure of a single int llvm::CallInst *CreateUAVOnceForModule(hlsl::DxilModule &DM, llvm::IRBuilder<> &Builder, @@ -401,18 +436,6 @@ hlsl::DXIL::ShaderKind GetFunctionShaderKind(hlsl::DxilModule &DM, return shaderKind; } -std::vector GetAllBlocks(hlsl::DxilModule &DM) { - std::vector ret; - auto entryPoints = DM.GetExportedFunctions(); - for (auto &fn : entryPoints) { - auto &blocks = fn->getBasicBlockList(); - for (auto &block : blocks) { - ret.push_back(&block); - } - } - return ret; -} - ExpandedStruct ExpandStructType(LLVMContext &Ctx, Type *OriginalPayloadStructType) { SmallVector Elements; @@ -547,6 +570,24 @@ void ForEachDynamicallyIndexedResource( auto CreateHandleFn = HlslOP->GetOpFunc(DXIL::OpCode::CreateHandle, Type::getVoidTy(Ctx)); + auto CreateHandleFromBindingFn = HlslOP->GetOpFunc( + DXIL::OpCode::CreateHandleFromBinding, Type::getVoidTy(Ctx)); + auto CreateHandleFromHeapFn = HlslOP->GetOpFunc( + DXIL::OpCode::CreateHandleFromHeap, Type::getVoidTy(Ctx)); + + struct UnusedDeclarationCleanup { + hlsl::DxilModule &DM; + llvm::Function *CreateHandleFn; + llvm::Function *CreateHandleFromBindingFn; + llvm::Function *CreateHandleFromHeapFn; + ~UnusedDeclarationCleanup() { + EraseIfUnused(DM, CreateHandleFn); + EraseIfUnused(DM, CreateHandleFromBindingFn); + EraseIfUnused(DM, CreateHandleFromHeapFn); + } + } cleanup{DM, CreateHandleFn, CreateHandleFromBindingFn, + CreateHandleFromHeapFn}; + for (auto FI = CreateHandleFn->user_begin(); FI != CreateHandleFn->user_end();) { auto *FunctionUser = *FI++; @@ -562,8 +603,6 @@ void ForEachDynamicallyIndexedResource( } } - auto CreateHandleFromBindingFn = HlslOP->GetOpFunc( - DXIL::OpCode::CreateHandleFromBinding, Type::getVoidTy(Ctx)); for (auto FI = CreateHandleFromBindingFn->user_begin(); FI != CreateHandleFromBindingFn->user_end();) { auto *FunctionUser = *FI++; @@ -579,8 +618,6 @@ void ForEachDynamicallyIndexedResource( } } - auto CreateHandleFromHeapFn = HlslOP->GetOpFunc( - DXIL::OpCode::CreateHandleFromHeap, Type::getVoidTy(Ctx)); for (auto FI = CreateHandleFromHeapFn->user_begin(); FI != CreateHandleFromHeapFn->user_end();) { auto *FunctionUser = *FI++; diff --git a/lib/DxilPIXPasses/PixPassHelpers.h b/lib/DxilPIXPasses/PixPassHelpers.h index d7b0b40af8..3d6e24d22f 100644 --- a/lib/DxilPIXPasses/PixPassHelpers.h +++ b/lib/DxilPIXPasses/PixPassHelpers.h @@ -48,7 +48,7 @@ llvm::CallInst *CreateHandleForResource(hlsl::DxilModule &DM, hlsl::DxilResourceBase *resource, const char *name); llvm::Function *GetEntryFunction(hlsl::DxilModule &DM); -std::vector GetAllBlocks(hlsl::DxilModule &DM); +void EraseIfUnused(hlsl::DxilModule &DM, llvm::Function *OpFunction); std::vector GetAllInstrumentableFunctions(hlsl::DxilModule &DM); hlsl::DXIL::ShaderKind GetFunctionShaderKind(hlsl::DxilModule &DM, diff --git a/tools/clang/test/HLSLFileCheck/pix/pixelCounterEarlyZ.hlsl b/tools/clang/test/HLSLFileCheck/pix/pixelCounterEarlyZ.hlsl index 86d6d498b8..63968a3375 100644 --- a/tools/clang/test/HLSLFileCheck/pix/pixelCounterEarlyZ.hlsl +++ b/tools/clang/test/HLSLFileCheck/pix/pixelCounterEarlyZ.hlsl @@ -3,9 +3,11 @@ // Check the write to the UAV was emitted: // CHECK: %UAVIncResult = call i32 @dx.op.atomicBinOp.i32(i32 78, %dx.types.Handle %PIX_CountUAV_Handle, i32 0, i32 %ByteIndex, i32 undef, i32 undef, i32 1) -// Early z flag value is 8. The flags are stored in an entry in the entry function description record. See: +// The flags are stored in an entry in the entry function description record. See: // https://github.com/Microsoft/DirectXShaderCompiler/blob/main/docs/DXIL.rst#shader-properties-and-capabilities -// CHECK: !{i32 0, i64 8} +// 8 is force-early-z. 16 is EnableRawAndStructuredBuffers, set for the +// RWByteAddressBuffer counter this pass adds. 8 | 16 = 24. +// CHECK: !{i32 0, i64 24} float4 main(float4 pos : SV_Position) : SV_Target { return pos; diff --git a/tools/clang/unittests/HLSL/PixTest.cpp b/tools/clang/unittests/HLSL/PixTest.cpp index e21546c9ac..7f1b4cf365 100644 --- a/tools/clang/unittests/HLSL/PixTest.cpp +++ b/tools/clang/unittests/HLSL/PixTest.cpp @@ -33,7 +33,10 @@ #include #endif +#include "dxc/DXIL/DxilConstants.h" #include "dxc/DXIL/DxilModule.h" +#include "dxc/DXIL/DxilOperations.h" +#include "dxc/DXIL/DxilSubobject.h" #include "dxc/Test/DxcTestUtils.h" #include "dxc/Test/HLSLTestData.h" @@ -65,6 +68,7 @@ #include #include <../lib/DxilDia/DxcPixLiveVariables_FragmentIterator.h> +#include <../lib/DxilPIXPasses/PixPassHelpers.h> #include #include "PixTestUtils.h" @@ -143,6 +147,16 @@ class PixTest : public ::testing::Test { TEST_METHOD(RootSignatureUpgrade_SubObjects) TEST_METHOD(RootSignatureUpgrade_Annotation) + TEST_METHOD(ToolsUav_TwoPixPassesShareOneResource) + TEST_METHOD(ToolsUav_LibraryWithTwoEntryPointsCreatesOnePair) + TEST_METHOD(ToolsUav_ExtendsEveryGlobalRootSignatureSubobject) + TEST_METHOD(DebugInstrumentation_RawBufferShaderFlagDeclared) + TEST_METHOD(ToolsUav_RootSignatureSerializationFailurePreservesSignature) + TEST_METHOD(ConstantColor_UnusedIntOverloadIsErased) + TEST_METHOD(ConstantColor_NoTargetOverloadsAreErased) + TEST_METHOD(RemoveDiscards_UnusedDiscardOverloadIsErased) + TEST_METHOD(OperationCacheCleanup_RemovesErasedFunctions) + TEST_METHOD(DynamicResourceCleanup_VisitorStopsEarly) TEST_METHOD(DxilPIXDXRInvocationsLog_SanityTest) TEST_METHOD(DxilPIXDXRInvocationsLog_EmbeddedRootSigs) @@ -157,6 +171,7 @@ class PixTest : public ::testing::Test { TEST_METHOD(DebugBreakInstrumentation_Multiple) TEST_METHOD(NonUniformResourceIndex_Resource) + TEST_METHOD(NonUniformResourceIndex_QualifiedCleanupValidates) TEST_METHOD(NonUniformResourceIndex_DescriptorHeap) TEST_METHOD(NonUniformResourceIndex_Raytracing) @@ -283,6 +298,7 @@ class PixTest : public ::testing::Test { std::vector Options; Options.push_back(L"-opt-mod-passes"); Options.push_back(passOption); + Options.push_back(L"-hlsl-dxilemit"); CComPtr pOptimizedModule; CComPtr pText; @@ -528,6 +544,12 @@ class PixTest : public ::testing::Test { } void ValidateAccessTrackingMods(const char *hlsl, bool modsExpected); + void LoadSubobjectsFromContainerIntoModule(IDxcBlob *container, + DxilModule &DM); + void VerifyGlobalRootSignaturesHaveToolsUAVs( + DxilSubobjects *subObjects, + const std::vector &expectedRootSignatureNames, + const std::vector &expectedShaderRegisters); class ModuleAndHangersOn { std::unique_ptr llvmContext; @@ -611,10 +633,11 @@ class PixTest : public ::testing::Test { void ValidateAllocaWrite(std::vector const &allocaWrites, size_t index, const char *name); PassOutput RunShaderAccessTrackingPass(IDxcBlob *blob); - std::string RunDxilPIXAddTidToAmplificationShaderPayloadPass(IDxcBlob *blob); + CComPtr + RunDxilPIXAddTidToAmplificationShaderPayloadPass(IDxcBlob *blob); CComPtr RunDxilPIXMeshShaderOutputPass(IDxcBlob *blob); CComPtr RunDxilPIXDXRInvocationsLog(IDxcBlob *blob); - std::vector + PassOutput RunDxilNonUniformResourceIndexInstrumentation(IDxcBlob *blob, std::string &outputText); void TestNuriCase(const char *source, const wchar_t *target, @@ -632,6 +655,122 @@ bool PixTest::InitSupport() { return true; } +static unsigned CountToolsUAVs(DxilModule &DM) { + unsigned count = 0; + for (auto const &uav : DM.GetUAVs()) { + if (uav->GetSpaceID() == static_cast(-2)) { + count++; + } + } + return count; +} + +static int CountToolsUAVRecords(std::vector const &lines) { + int count = 0; + for (auto const &line : lines) { + if (!line.empty() && line[0] == '!' && + line.find(", i32 -2, i32 ") != std::string::npos) { + count++; + } + } + return count; +} + +static bool +RootSignatureHasToolsUAV(const DxilVersionedRootSignatureDesc *rootSignature, + uint32_t shaderRegister) { + switch (rootSignature->Version) { + case DxilRootSignatureVersion::Version_1_0: { + const DxilRootSignatureDesc &desc = rootSignature->Desc_1_0; + for (uint32_t i = 0; i < desc.NumParameters; ++i) { + const DxilRootParameter ¶m = desc.pParameters[i]; + if (param.ParameterType == DxilRootParameterType::UAV && + param.Descriptor.RegisterSpace == static_cast(-2) && + param.Descriptor.ShaderRegister == shaderRegister) { + return true; + } + } + break; + } + case DxilRootSignatureVersion::Version_1_1: { + const DxilRootSignatureDesc1 &desc = rootSignature->Desc_1_1; + for (uint32_t i = 0; i < desc.NumParameters; ++i) { + const DxilRootParameter1 ¶m = desc.pParameters[i]; + if (param.ParameterType == DxilRootParameterType::UAV && + param.Descriptor.RegisterSpace == static_cast(-2) && + param.Descriptor.ShaderRegister == shaderRegister) { + return true; + } + } + break; + } + } + return false; +} + +void PixTest::LoadSubobjectsFromContainerIntoModule(IDxcBlob *container, + DxilModule &DM) { + const char *blobContent = + reinterpret_cast(container->GetBufferPointer()); + const unsigned blobSize = container->GetBufferSize(); + const hlsl::DxilContainerHeader *containerHeader = + hlsl::IsDxilContainerLike(blobContent, blobSize); + VERIFY_ARE_NOT_EQUAL(containerHeader, nullptr); + + const hlsl::DxilPartHeader *partHeader = + GetDxilPartByType(containerHeader, hlsl::DFCC_RuntimeData); + VERIFY_ARE_NOT_EQUAL(partHeader, nullptr); + + hlsl::RDAT::DxilRuntimeData rdat(GetDxilPartData(partHeader), + partHeader->PartSize); + std::unique_ptr subObjects(new DxilSubobjects()); + VERIFY_IS_TRUE(LoadSubobjectsFromRDAT(*subObjects, rdat)); + DM.ResetSubobjects(subObjects.release()); +} + +void PixTest::VerifyGlobalRootSignaturesHaveToolsUAVs( + DxilSubobjects *subObjects, + const std::vector &expectedRootSignatureNames, + const std::vector &expectedShaderRegisters) { + VERIFY_IS_NOT_NULL(subObjects); + + std::map foundRootSignatures; + for (const std::string &rootSignatureName : expectedRootSignatureNames) { + foundRootSignatures[rootSignatureName] = false; + } + + for (auto const &subObject : subObjects->GetSubobjects()) { + if (subObject.second->GetKind() != + hlsl::DXIL::SubobjectKind::GlobalRootSignature) { + continue; + } + + const std::string subObjectName = subObject.first.str(); + if (foundRootSignatures.find(subObjectName) == foundRootSignatures.end()) { + continue; + } + + const void *data = nullptr; + uint32_t size = 0; + constexpr bool notALocalRS = false; + VERIFY_IS_TRUE( + subObject.second->GetRootSignature(notALocalRS, data, size, nullptr)); + + DxilVersionedRootSignatureDesc const *rootSignature = nullptr; + DeserializeRootSignature(data, size, &rootSignature); + for (uint32_t expectedShaderRegister : expectedShaderRegisters) { + VERIFY_IS_TRUE( + RootSignatureHasToolsUAV(rootSignature, expectedShaderRegister)); + } + DeleteRootSignature(rootSignature); + foundRootSignatures[subObjectName] = true; + } + + for (const auto &foundRootSignature : foundRootSignatures) { + VERIFY_IS_TRUE(foundRootSignature.second); + } +} + void PixTest::TestPixUAVCase(char const *hlsl, wchar_t const *model, wchar_t const *entry) { auto mod = Compile(m_dllSupport, hlsl, model, {}, entry); @@ -846,7 +985,7 @@ CComPtr PixTest::RunDxilPIXDXRInvocationsLog(IDxcBlob *blob) { return pOptimizedModule; } -std::vector PixTest::RunDxilNonUniformResourceIndexInstrumentation( +PassOutput PixTest::RunDxilNonUniformResourceIndexInstrumentation( IDxcBlob *blob, std::string &outputText) { CComPtr dxil = FindModule(DFCC_ShaderDebugInfoDXIL, blob); @@ -865,11 +1004,13 @@ std::vector PixTest::RunDxilNonUniformResourceIndexInstrumentation( outputText = BlobToUtf8(pText); - const std::string disassembly = Disassemble(pOptimizedModule); - return Tokenize(disassembly, "\n"); + PassOutput result; + result.blob = pOptimizedModule; + result.lines = Tokenize(Disassemble(pOptimizedModule), "\n"); + return result; } -std::string +CComPtr PixTest::RunDxilPIXAddTidToAmplificationShaderPayloadPass(IDxcBlob *blob) { CComPtr dxil = FindModule(DFCC_ShaderDebugInfoDXIL, blob); CComPtr pOptimizer; @@ -885,14 +1026,16 @@ PixTest::RunDxilPIXAddTidToAmplificationShaderPayloadPass(IDxcBlob *blob) { VERIFY_SUCCEEDED(pOptimizer->RunOptimizer( dxil, Options.data(), Options.size(), &pOptimizedModule, &pText)); - std::string outputText; - if (pText->GetBufferSize() != 0) { - outputText = reinterpret_cast(pText->GetBufferPointer()); - } - - return outputText; + return pOptimizedModule; } +static bool HasDeclaration(const std::string &disassembly, + const std::string &functionName); +static std::string FindDeclarationLine(const std::string &disassembly, + const std::string &functionName); +static bool HasDeclarationLine(const std::string &disassembly, + const std::string &declaration); + TEST_F(PixTest, AddToASPayload) { const char *hlsl = R"( @@ -934,10 +1077,29 @@ void MSMain( )"; auto as = Compile(m_dllSupport, hlsl, L"as_6_6", {}, L"ASMain"); - RunDxilPIXAddTidToAmplificationShaderPayloadPass(as); + const std::string originalDispatchMeshDeclaration = + FindDeclarationLine(Disassemble(as), "dx.op.dispatchMesh"); + VERIFY_IS_FALSE(originalDispatchMeshDeclaration.empty()); + + auto asOutput = RunDxilPIXAddTidToAmplificationShaderPayloadPass(as); + VERIFY_IS_FALSE(HasDeclarationLine(Disassemble(asOutput), + originalDispatchMeshDeclaration)); auto ms = Compile(m_dllSupport, hlsl, L"ms_6_6", {}, L"MSMain"); - RunDxilPIXMeshShaderOutputPass(ms); + const std::string originalGetMeshPayloadDeclaration = + FindDeclarationLine(Disassemble(ms), "dx.op.getMeshPayload"); + VERIFY_IS_FALSE(originalGetMeshPayloadDeclaration.empty()); + + auto msOutput = RunDxilPIXMeshShaderOutputPass(ms); + const std::string meshDisassembly = Disassemble(msOutput); + VERIFY_IS_FALSE( + HasDeclarationLine(meshDisassembly, originalGetMeshPayloadDeclaration)); + VERIFY_IS_FALSE( + HasDeclaration(meshDisassembly, "dx.op.storeVertexOutput.i32")); + VERIFY_IS_FALSE( + HasDeclaration(meshDisassembly, "dx.op.storeVertexOutput.i16")); + VERIFY_IS_FALSE( + HasDeclaration(meshDisassembly, "dx.op.storeVertexOutput.f16")); } unsigned FindOrAddVSInSignatureElementForInstanceOrVertexID( hlsl::DxilSignature &InputSignature, hlsl::DXIL::SemanticKind semanticKind); @@ -3015,6 +3177,373 @@ float4 main(int i : A, float j : B) : SV_TARGET VERIFY_IS_TRUE(foundGlobalRS); } +TEST_F(PixTest, ToolsUav_TwoPixPassesShareOneResource) { + const char *source = R"x( +RWByteAddressBuffer output : register(u0); + +[numthreads(1, 1, 1)] +void main(uint3 tid : SV_DispatchThreadID) +{ + output.Store(4 * tid.x, tid.x); +})x"; + + auto compiled = Compile(m_dllSupport, source, L"cs_6_2", {L"-Od"}); + auto debugOutput = RunDebugPass(compiled); + auto accessOutput = RunShaderAccessTrackingPass(debugOutput.blob); + + ModuleAndHangersOn moduleEtc(accessOutput.blob); + VERIFY_ARE_EQUAL(1u, CountToolsUAVs(moduleEtc.GetDxilModule())); + VerifyInstrumentedModuleIsValid( + accessOutput.blob, + "debug instrumentation followed by shader access tracking"); +} + +TEST_F(PixTest, ToolsUav_LibraryWithTwoEntryPointsCreatesOnePair) { + const char *source = R"x( +struct [raypayload] MyPayload +{ + float2 barycentrics : read(caller) : write(caller,anyhit); + uint primitiveIndex : read(caller) : write(caller,anyhit); +}; + +[shader("miss")] +void MissOne(inout MyPayload payload) +{ + payload.primitiveIndex = 1; +} + +[shader("miss")] +void MissTwo(inout MyPayload payload) +{ + payload.primitiveIndex = 2; +} +)x"; + + auto compiled = Compile(m_dllSupport, source, L"lib_6_6", {}); + auto output = RunDxilPIXDXRInvocationsLog(compiled); + + auto lines = Tokenize(Disassemble(output), "\n"); + VERIFY_ARE_EQUAL(2, CountToolsUAVRecords(lines)); +} + +TEST_F(PixTest, ToolsUav_ExtendsEveryGlobalRootSignatureSubobject) { + const char *source = R"x( +GlobalRootSignature firstRootSignature = {"CBV(b0)"}; +GlobalRootSignature secondRootSignature = {"SRV(t0)"}; + +SubobjectToExportsAssociation firstAssociation = +{ + "firstRootSignature", + "MyClosestHit" +}; + +SubobjectToExportsAssociation secondAssociation = +{ + "secondRootSignature", + "MyMiss" +}; + +struct MyPayload +{ + float4 color; +}; + +[shader("raygeneration")] +void MyRayGen() +{ +} + +[shader("closesthit")] +void MyClosestHit(inout MyPayload payload, + in BuiltInTriangleIntersectionAttributes attr) +{ +} + +[shader("miss")] +void MyMiss(inout MyPayload payload) +{ +} +)x"; + + auto compiled = Compile(m_dllSupport, source, L"lib_6_6", {}); + ModuleAndHangersOn moduleEtc(compiled); + DxilModule &DM = moduleEtc.GetDxilModule(); + LoadSubobjectsFromContainerIntoModule(compiled, DM); + PIXPassHelpers::CreateGlobalUAVResource(DM, 0, "PIX_CountUAV_Handle"); + PIXPassHelpers::CreateGlobalUAVResource(DM, 1, "PIX_LogUAV_Handle"); + + VerifyGlobalRootSignaturesHaveToolsUAVs( + DM.GetSubobjects(), {"firstRootSignature", "secondRootSignature"}, + {0, 1}); +} + +TEST_F(PixTest, DebugInstrumentation_RawBufferShaderFlagDeclared) { + const char *source = R"x( +[numthreads(1, 1, 1)] +void main(uint threadId : SV_DispatchThreadID) +{ +})x"; + + auto compiled = Compile(m_dllSupport, source, L"cs_6_2", {L"-Od"}); + auto output = RunDebugPass(compiled); + auto lines = Tokenize(Disassemble(output.blob), "\n"); + + constexpr uint64_t EnableRawAndStructuredBuffers = 0x10; + bool foundShaderFlags = false; + uint64_t shaderFlags = 0; + const std::string tagPrefix = "!{i32 0, i64 "; + for (auto const &line : lines) { + const auto tagStart = line.find(tagPrefix); + if (tagStart == std::string::npos) { + continue; + } + shaderFlags = + strtoull(line.c_str() + tagStart + tagPrefix.length(), nullptr, 10); + foundShaderFlags = true; + break; + } + + VERIFY_IS_TRUE(foundShaderFlags); + VERIFY_ARE_EQUAL(EnableRawAndStructuredBuffers, + shaderFlags & EnableRawAndStructuredBuffers); + VerifyInstrumentedModuleIsValid(output.blob, + "debug instrumentation shader flags"); +} + +TEST_F(PixTest, ToolsUav_RootSignatureSerializationFailurePreservesSignature) { + const char *source = R"x( +[numthreads(1, 1, 1)] +void main() +{ +})x"; + + DxilDescriptorRange range = {}; + range.RangeType = DxilDescriptorRangeType::UAV; + range.NumDescriptors = 1; + range.BaseShaderRegister = 0; + range.RegisterSpace = static_cast(-2); + range.OffsetInDescriptorsFromTableStart = DxilDescriptorRangeOffsetAppend; + + DxilRootParameter parameter = {}; + parameter.ParameterType = DxilRootParameterType::DescriptorTable; + parameter.DescriptorTable.NumDescriptorRanges = 1; + parameter.DescriptorTable.pDescriptorRanges = ⦥ + parameter.ShaderVisibility = DxilShaderVisibility::All; + + DxilVersionedRootSignatureDesc rootSignature = {}; + rootSignature.Version = DxilRootSignatureVersion::Version_1_0; + rootSignature.Desc_1_0.NumParameters = 1; + rootSignature.Desc_1_0.pParameters = ¶meter; + rootSignature.Desc_1_0.Flags = DxilRootSignatureFlags::None; + + CComPtr serializedRootSignature; + CComPtr errorBlob; + SerializeRootSignature(&rootSignature, &serializedRootSignature, &errorBlob, + true); + VERIFY_IS_NOT_NULL(serializedRootSignature); + + auto serializedData = + static_cast(serializedRootSignature->GetBufferPointer()); + std::vector originalRootSignature( + serializedData, + serializedData + serializedRootSignature->GetBufferSize()); + + auto compiled = Compile(m_dllSupport, source, L"cs_6_0", {}); + ModuleAndHangersOn moduleEtc(compiled); + DxilModule &DM = moduleEtc.GetDxilModule(); + DM.ResetSerializedRootSignature(originalRootSignature); + + std::unique_ptr subObjects(new DxilSubobjects()); + constexpr bool notALocalRootSignature = false; + subObjects->CreateRootSignature( + "testRootSignature", notALocalRootSignature, originalRootSignature.data(), + static_cast(originalRootSignature.size())); + DM.ResetSubobjects(subObjects.release()); + + PIXPassHelpers::CreateGlobalUAVResource(DM, 0, "PIX_TestUAV"); + + const std::vector &actualRootSignature = + DM.GetSerializedRootSignature(); + VERIFY_ARE_EQUAL(originalRootSignature.size(), actualRootSignature.size()); + VERIFY_IS_TRUE(std::equal(originalRootSignature.begin(), + originalRootSignature.end(), + actualRootSignature.begin())); + + bool foundRootSignature = false; + for (auto const &subObject : DM.GetSubobjects()->GetSubobjects()) { + if (subObject.first != "testRootSignature") { + continue; + } + + const void *data = nullptr; + uint32_t size = 0; + VERIFY_IS_TRUE(subObject.second->GetRootSignature(notALocalRootSignature, + data, size, nullptr)); + VERIFY_ARE_EQUAL(originalRootSignature.size(), static_cast(size)); + VERIFY_IS_TRUE(std::equal(originalRootSignature.begin(), + originalRootSignature.end(), + static_cast(data))); + foundRootSignature = true; + } + VERIFY_IS_TRUE(foundRootSignature); +} + +static bool HasUnusedDeclaration(std::vector const &lines, + std::string const &functionName) { + bool declared = false; + for (auto const &line : lines) { + if (line.find("declare") != std::string::npos && + line.find(functionName) != std::string::npos) { + declared = true; + } + if (line.find("call") != std::string::npos && + line.find(functionName) != std::string::npos) { + return false; + } + } + return declared; +} + +static bool HasDeclaration(const std::string &disassembly, + const std::string &functionName) { + for (const std::string &line : Tokenize(disassembly, "\n")) { + if (line.find("declare") != std::string::npos && + line.find(functionName) != std::string::npos) { + return true; + } + } + return false; +} + +static std::string FindDeclarationLine(const std::string &disassembly, + const std::string &functionName) { + for (const std::string &line : Tokenize(disassembly, "\n")) { + if (line.find("declare") != std::string::npos && + line.find(functionName) != std::string::npos) { + return line; + } + } + return {}; +} + +static bool HasDeclarationLine(const std::string &disassembly, + const std::string &declaration) { + for (const std::string &line : Tokenize(disassembly, "\n")) { + if (line == declaration) { + return true; + } + } + return false; +} + +TEST_F(PixTest, ConstantColor_UnusedIntOverloadIsErased) { + const char *source = R"x( +float4 main() : SV_Target +{ + return float4(1, 2, 3, 4); +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {L"-Od"}); + auto output = RunSinglePass(compiled, L"-hlsl-dxil-constantColor"); + + VERIFY_IS_FALSE(HasUnusedDeclaration(output.Lines, "dx.op.storeOutput.i32")); + VerifyInstrumentedModuleIsValid(output.Module, + "constant-colour substitution"); +} + +TEST_F(PixTest, ConstantColor_NoTargetOverloadsAreErased) { + const char *source = R"x( +[numthreads(1, 1, 1)] +void main() +{ +})x"; + + auto compiled = Compile(m_dllSupport, source, L"cs_6_0", {L"-Od"}); + auto output = RunSinglePass(compiled, L"-hlsl-dxil-constantColor"); + const std::string disassembly = Disassemble(output.Module); + + VerifyInstrumentedModuleIsValid( + output.Module, "constant-colour substitution with no target"); + VERIFY_IS_FALSE(HasDeclaration(disassembly, "dx.op.storeOutput.f32")); + VERIFY_IS_FALSE(HasDeclaration(disassembly, "dx.op.storeOutput.i32")); +} + +TEST_F(PixTest, RemoveDiscards_UnusedDiscardOverloadIsErased) { + const char *source = R"x( +float4 main() : SV_Target +{ + return float4(1, 2, 3, 4); +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {L"-Od"}); + auto output = RunSinglePass(compiled, L"-hlsl-dxil-remove-discards"); + + VERIFY_IS_FALSE(HasUnusedDeclaration(output.Lines, "dx.op.discard")); + VerifyInstrumentedModuleIsValid(output.Module, + "discard removal with no discard"); +} + +TEST_F(PixTest, OperationCacheCleanup_RemovesErasedFunctions) { + const char *source = R"x( +float4 main() : SV_Target +{ + return float4(1, 2, 3, 4); +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {}); + ModuleAndHangersOn moduleEtc(compiled); + DxilModule &DM = moduleEtc.GetDxilModule(); + OP *HlslOP = DM.GetOP(); + llvm::Function *discard = + HlslOP->GetOpFunc(DXIL::OpCode::Discard, + llvm::Type::getVoidTy(DM.GetModule()->getContext())); + + VERIFY_ARE_EQUAL(1u, + static_cast( + HlslOP->GetOpFuncList(DXIL::OpCode::Discard).size())); + PIXPassHelpers::EraseIfUnused(DM, discard); + VERIFY_ARE_EQUAL(0u, + static_cast( + HlslOP->GetOpFuncList(DXIL::OpCode::Discard).size())); + + llvm::Function *recreated = + HlslOP->GetOpFunc(DXIL::OpCode::Discard, + llvm::Type::getVoidTy(DM.GetModule()->getContext())); + VERIFY_IS_NOT_NULL(recreated); + PIXPassHelpers::EraseIfUnused(DM, recreated); +} + +TEST_F(PixTest, DynamicResourceCleanup_VisitorStopsEarly) { + const char *source = R"x( +Texture2D textures[] : register(t0); + +float4 main(float2 uv : TEXCOORD0) : SV_Target +{ + return textures[(uint)uv.x].Load(int3(0, 0, 0)); +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {L"-Od"}); + ModuleAndHangersOn moduleEtc(compiled); + DxilModule &DM = moduleEtc.GetDxilModule(); + bool visitorCalled = false; + PIXPassHelpers::ForEachDynamicallyIndexedResource( + DM, [&visitorCalled](bool, llvm::Instruction *, llvm::Value *) { + visitorCalled = true; + return false; + }); + + VERIFY_IS_TRUE(visitorCalled); + OP *HlslOP = DM.GetOP(); + VERIFY_ARE_EQUAL( + 0u, + static_cast( + HlslOP->GetOpFuncList(DXIL::OpCode::CreateHandleFromBinding).size())); + VERIFY_ARE_EQUAL( + 0u, + static_cast( + HlslOP->GetOpFuncList(DXIL::OpCode::CreateHandleFromHeap).size())); +} + TEST_F(PixTest, DxilPIXDXRInvocationsLog_SanityTest) { const char *source = R"x( @@ -3108,8 +3637,9 @@ void PixTest::TestNuriCase(const char *source, const wchar_t *target, Compile(m_dllSupport, source, target, compilationOptions); std::string outputText; - const std::vector dxilLines = + PassOutput output = RunDxilNonUniformResourceIndexInstrumentation(compiledLib, outputText); + const std::vector &dxilLines = output.lines; VERIFY_ARE_EQUAL(NuriGetWaveInstructionCount(dxilLines), expectedResult); @@ -3155,6 +3685,33 @@ float4 main(float2 uv : TEXCOORD0) : SV_TARGET TestNuriCase(sourceWithNuri, L"ps_6_6", 0); } +TEST_F(PixTest, NonUniformResourceIndex_QualifiedCleanupValidates) { + if (m_ver.SkipDxilVersion(1, 6)) { + return; + } + + const char *source = R"x( +Texture2D textures[] : register(t0); + +float4 main(float2 uv : TEXCOORD0) : SV_Target +{ + uint index = (uint)uv.x; + return textures[NonUniformResourceIndex(index)].Load(int3(0, 0, 0)); +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_6", {L"-Od"}); + std::string outputText; + PassOutput output = + RunDxilNonUniformResourceIndexInstrumentation(compiled, outputText); + const std::string disassembly = Disassemble(output.blob); + + VerifyInstrumentedModuleIsValid( + output.blob, "qualified non-uniform resource index instrumentation"); + VERIFY_ARE_EQUAL(0u, NuriGetWaveInstructionCount(output.lines)); + VERIFY_IS_FALSE(HasDeclaration(disassembly, "dx.op.waveActiveAllEqual.i32")); + VERIFY_IS_FALSE(HasDeclaration(disassembly, "dx.op.atomicBinOp.i32")); +} + TEST_F(PixTest, NonUniformResourceIndex_DescriptorHeap) { if (m_ver.SkipDxilVersion(1, 6)) { @@ -3551,15 +4108,15 @@ void main() { } TEST_F(PixTest, DebugBreakInstrumentation_NoDebugBreak) { + if (m_ver.SkipDxilVersion(1, 10)) + return; const char *source = R"x( -RWByteAddressBuffer buf : register(u0); [numthreads(1, 1, 1)] void main() { - buf.Store(0, 1); })x"; - auto compiled = Compile(m_dllSupport, source, L"cs_6_0", {}); + auto compiled = Compile(m_dllSupport, source, L"cs_6_10", {}); auto output = RunDebugBreakPass(compiled); bool foundDebugBreak = false; for (auto const &line : output.lines) { @@ -3567,6 +4124,8 @@ void main() { foundDebugBreak = true; } VERIFY_IS_FALSE(foundDebugBreak); + VerifyInstrumentedModuleIsValid(output.blob, + "debug-break instrumentation with no call"); } TEST_F(PixTest, DebugBreakInstrumentation_Multiple) { From c3bc4656cab74fec176fd2583f0ddf6ce9bf6fdb Mon Sep 17 00:00:00 2001 From: Damyan Pepper Date: Wed, 2 Sep 2026 11:34:47 -0700 Subject: [PATCH 2/5] [PIX] Set the root descriptor flags only on append ExtendRootSig returns early, without appending a parameter, when a version 1.1 root signature already holds the requested tools UAV. The caller still unconditionally cleared Descriptor.Flags on the last parameter in that case, which may be an unrelated, pre-existing parameter whose flags the caller must not disturb. Have ExtendRootSig report whether it actually appended a parameter, and only clear flags on that path. Add a regression test: a v1.1 root signature with the tools UAV already present as parameter 0 and an unrelated root descriptor as parameter 1 with DataVolatile flags. Re-requesting the same register leaves parameter 1's flags untouched; appending a genuinely new register still leaves parameter 1 untouched and gives the new parameter None flags. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- lib/DxilPIXPasses/PixPassHelpers.cpp | 15 +++-- tools/clang/unittests/HLSL/PixTest.cpp | 78 ++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 6 deletions(-) diff --git a/lib/DxilPIXPasses/PixPassHelpers.cpp b/lib/DxilPIXPasses/PixPassHelpers.cpp index 1137fe6cf8..709aa2286d 100644 --- a/lib/DxilPIXPasses/PixPassHelpers.cpp +++ b/lib/DxilPIXPasses/PixPassHelpers.cpp @@ -198,8 +198,9 @@ static std::vector SerializeRootSignatureToVector( constexpr uint32_t toolsRegisterSpace = static_cast(-2); +// Returns whether a parameter was appended. template -void ExtendRootSig(RootSigDesc &rootSigDesc, uint32_t toolsUAVRegister) { +bool ExtendRootSig(RootSigDesc &rootSigDesc, uint32_t toolsUAVRegister) { auto *existingParams = rootSigDesc.pParameters; for (uint32_t i = 0; i < rootSigDesc.NumParameters; ++i) { if (rootSigDesc.pParameters[i].ParameterType == @@ -209,7 +210,7 @@ void ExtendRootSig(RootSigDesc &rootSigDesc, uint32_t toolsUAVRegister) { rootSigDesc.pParameters[i].Descriptor.ShaderRegister == toolsUAVRegister) { // Already added - return; + return false; } } } @@ -229,6 +230,7 @@ void ExtendRootSig(RootSigDesc &rootSigDesc, uint32_t toolsUAVRegister) { rootSigDesc.pParameters[rootSigDesc.NumParameters].ShaderVisibility = DxilShaderVisibility::All; rootSigDesc.NumParameters++; + return true; } static std::vector @@ -243,10 +245,11 @@ AddUAVParamterToRootSignature(const void *Data, uint32_t Size, toolsUAVRegister); break; case DxilRootSignatureVersion::Version_1_1: - ExtendRootSig(rs->Desc_1_1, - toolsUAVRegister); - rs->Desc_1_1.pParameters[rs->Desc_1_1.NumParameters - 1].Descriptor.Flags = - hlsl::DxilRootDescriptorFlags::None; + if (ExtendRootSig( + rs->Desc_1_1, toolsUAVRegister)) { + rs->Desc_1_1.pParameters[rs->Desc_1_1.NumParameters - 1] + .Descriptor.Flags = hlsl::DxilRootDescriptorFlags::None; + } break; } return SerializeRootSignatureToVector(rs); diff --git a/tools/clang/unittests/HLSL/PixTest.cpp b/tools/clang/unittests/HLSL/PixTest.cpp index 9e38390162..e03f5c5c0f 100644 --- a/tools/clang/unittests/HLSL/PixTest.cpp +++ b/tools/clang/unittests/HLSL/PixTest.cpp @@ -152,6 +152,7 @@ class PixTest : public ::testing::Test { TEST_METHOD(ToolsUav_ExtendsEveryGlobalRootSignatureSubobject) TEST_METHOD(DebugInstrumentation_RawBufferShaderFlagDeclared) TEST_METHOD(ToolsUav_RootSignatureSerializationFailurePreservesSignature) + TEST_METHOD(ToolsUav_ExtendingRootSignaturePreservesUnrelatedParameterFlags) TEST_METHOD(ConstantColor_UnusedIntOverloadIsErased) TEST_METHOD(ConstantColor_NoTargetOverloadsAreErased) TEST_METHOD(RemoveDiscards_UnusedDiscardOverloadIsErased) @@ -3422,6 +3423,83 @@ void main() VERIFY_IS_TRUE(foundRootSignature); } +TEST_F(PixTest, + ToolsUav_ExtendingRootSignaturePreservesUnrelatedParameterFlags) { + const char *source = R"x( +[numthreads(1, 1, 1)] +void main() +{ +})x"; + + DxilRootParameter1 parameters[2] = {}; + parameters[0].ParameterType = DxilRootParameterType::UAV; + parameters[0].Descriptor.RegisterSpace = static_cast(-2); + parameters[0].Descriptor.ShaderRegister = 0; + parameters[0].Descriptor.Flags = DxilRootDescriptorFlags::None; + parameters[0].ShaderVisibility = DxilShaderVisibility::All; + + parameters[1].ParameterType = DxilRootParameterType::CBV; + parameters[1].Descriptor.RegisterSpace = 0; + parameters[1].Descriptor.ShaderRegister = 0; + parameters[1].Descriptor.Flags = DxilRootDescriptorFlags::DataVolatile; + parameters[1].ShaderVisibility = DxilShaderVisibility::All; + + DxilVersionedRootSignatureDesc rootSignature = {}; + rootSignature.Version = DxilRootSignatureVersion::Version_1_1; + rootSignature.Desc_1_1.NumParameters = 2; + rootSignature.Desc_1_1.pParameters = parameters; + rootSignature.Desc_1_1.Flags = DxilRootSignatureFlags::None; + + CComPtr serializedRootSignature; + CComPtr errorBlob; + SerializeRootSignature(&rootSignature, &serializedRootSignature, &errorBlob, + true); + VERIFY_IS_NOT_NULL(serializedRootSignature); + + const uint8_t *serializedData = + static_cast(serializedRootSignature->GetBufferPointer()); + std::vector originalRootSignature( + serializedData, + serializedData + serializedRootSignature->GetBufferSize()); + + CComPtr compiled = Compile(m_dllSupport, source, L"cs_6_0", {}); + ModuleAndHangersOn moduleEtc(compiled); + DxilModule &DM = moduleEtc.GetDxilModule(); + DM.ResetSerializedRootSignature(originalRootSignature); + + PIXPassHelpers::CreateGlobalUAVResource(DM, 0, "PIX_TestUAV0"); + + { + const std::vector &bytes = DM.GetSerializedRootSignature(); + DxilVersionedRootSignatureDesc const *afterNoOp = nullptr; + DeserializeRootSignature(bytes.data(), static_cast(bytes.size()), + &afterNoOp); + VERIFY_ARE_EQUAL(afterNoOp->Desc_1_1.NumParameters, 2u); + VERIFY_IS_TRUE(afterNoOp->Desc_1_1.pParameters[1].Descriptor.Flags == + DxilRootDescriptorFlags::DataVolatile); + DeleteRootSignature(afterNoOp); + } + + PIXPassHelpers::CreateGlobalUAVResource(DM, 1, "PIX_TestUAV1"); + + { + const std::vector &bytes = DM.GetSerializedRootSignature(); + DxilVersionedRootSignatureDesc const *afterAdd = nullptr; + DeserializeRootSignature(bytes.data(), static_cast(bytes.size()), + &afterAdd); + VERIFY_ARE_EQUAL(afterAdd->Desc_1_1.NumParameters, 3u); + VERIFY_IS_TRUE(afterAdd->Desc_1_1.pParameters[1].Descriptor.Flags == + DxilRootDescriptorFlags::DataVolatile); + VERIFY_ARE_EQUAL(afterAdd->Desc_1_1.pParameters[2].Descriptor.RegisterSpace, + static_cast(-2)); + VERIFY_ARE_EQUAL( + afterAdd->Desc_1_1.pParameters[2].Descriptor.ShaderRegister, 1u); + VERIFY_IS_TRUE(afterAdd->Desc_1_1.pParameters[2].Descriptor.Flags == + DxilRootDescriptorFlags::None); + DeleteRootSignature(afterAdd); + } +} + static bool HasUnusedDeclaration(std::vector const &lines, std::string const &functionName) { bool declared = false; From 27008ea2c4b40b4d0747c105f9b69281ef864e77 Mon Sep 17 00:00:00 2001 From: Damyan Pepper Date: Wed, 2 Sep 2026 19:00:57 -0700 Subject: [PATCH 3/5] [NFC] Spell out L2 local types Replace the stable, nameable auto-deduced types introduced by L2. Retain only the two DxilSubobjects MapVector ranges whose underlying pair type is implementation-dependent and obscures the loop. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- lib/DxilPIXPasses/PixPassHelpers.cpp | 9 ++-- tools/clang/unittests/HLSL/PixTest.cpp | 72 +++++++++++++++----------- 2 files changed, 47 insertions(+), 34 deletions(-) diff --git a/lib/DxilPIXPasses/PixPassHelpers.cpp b/lib/DxilPIXPasses/PixPassHelpers.cpp index 709aa2286d..b83f195e32 100644 --- a/lib/DxilPIXPasses/PixPassHelpers.cpp +++ b/lib/DxilPIXPasses/PixPassHelpers.cpp @@ -296,7 +296,8 @@ static void AddUAVToDxilDefinedGlobalRootSignatures(DxilModule &DM, } constexpr bool notALocalRS = false; - for (auto const &replacementRootSignature : replacementRootSignatures) { + for (const ReplacementRootSignature &replacementRootSignature : + replacementRootSignatures) { subObjects->RemoveSubobject(replacementRootSignature.Name); subObjects->CreateRootSignature( replacementRootSignature.Name, notALocalRS, @@ -312,7 +313,7 @@ hlsl::DxilResource *CreateGlobalUAVResource(hlsl::DxilModule &DM, const char *name) { LLVMContext &Ctx = DM.GetModule()->getContext(); - for (auto const &existingUAV : DM.GetUAVs()) { + for (const std::unique_ptr &existingUAV : DM.GetUAVs()) { if (existingUAV->GetSpaceID() == toolsRegisterSpace && existingUAV->GetLowerBound() == hlslBindIndex) { return existingUAV.get(); @@ -573,9 +574,9 @@ void ForEachDynamicallyIndexedResource( auto CreateHandleFn = HlslOP->GetOpFunc(DXIL::OpCode::CreateHandle, Type::getVoidTy(Ctx)); - auto CreateHandleFromBindingFn = HlslOP->GetOpFunc( + llvm::Function *CreateHandleFromBindingFn = HlslOP->GetOpFunc( DXIL::OpCode::CreateHandleFromBinding, Type::getVoidTy(Ctx)); - auto CreateHandleFromHeapFn = HlslOP->GetOpFunc( + llvm::Function *CreateHandleFromHeapFn = HlslOP->GetOpFunc( DXIL::OpCode::CreateHandleFromHeap, Type::getVoidTy(Ctx)); struct UnusedDeclarationCleanup { diff --git a/tools/clang/unittests/HLSL/PixTest.cpp b/tools/clang/unittests/HLSL/PixTest.cpp index e03f5c5c0f..c2cefea7bc 100644 --- a/tools/clang/unittests/HLSL/PixTest.cpp +++ b/tools/clang/unittests/HLSL/PixTest.cpp @@ -692,7 +692,7 @@ bool PixTest::InitSupport() { static unsigned CountToolsUAVs(DxilModule &DM) { unsigned count = 0; - for (auto const &uav : DM.GetUAVs()) { + for (const std::unique_ptr &uav : DM.GetUAVs()) { if (uav->GetSpaceID() == static_cast(-2)) { count++; } @@ -702,7 +702,7 @@ static unsigned CountToolsUAVs(DxilModule &DM) { static int CountToolsUAVRecords(std::vector const &lines) { int count = 0; - for (auto const &line : lines) { + for (const std::string &line : lines) { if (!line.empty() && line[0] == '!' && line.find(", i32 -2, i32 ") != std::string::npos) { count++; @@ -801,7 +801,8 @@ void PixTest::VerifyGlobalRootSignaturesHaveToolsUAVs( foundRootSignatures[subObjectName] = true; } - for (const auto &foundRootSignature : foundRootSignatures) { + for (const std::map::value_type &foundRootSignature : + foundRootSignatures) { VERIFY_IS_TRUE(foundRootSignature.second); } } @@ -1116,7 +1117,8 @@ void MSMain( FindDeclarationLine(Disassemble(as), "dx.op.dispatchMesh"); VERIFY_IS_FALSE(originalDispatchMeshDeclaration.empty()); - auto asOutput = RunDxilPIXAddTidToAmplificationShaderPayloadPass(as); + CComPtr asOutput = + RunDxilPIXAddTidToAmplificationShaderPayloadPass(as); VERIFY_IS_FALSE(HasDeclarationLine(Disassemble(asOutput), originalDispatchMeshDeclaration)); @@ -1125,7 +1127,7 @@ void MSMain( FindDeclarationLine(Disassemble(ms), "dx.op.getMeshPayload"); VERIFY_IS_FALSE(originalGetMeshPayloadDeclaration.empty()); - auto msOutput = RunDxilPIXMeshShaderOutputPass(ms); + CComPtr msOutput = RunDxilPIXMeshShaderOutputPass(ms); const std::string meshDisassembly = Disassemble(msOutput); VERIFY_IS_FALSE( HasDeclarationLine(meshDisassembly, originalGetMeshPayloadDeclaration)); @@ -3222,9 +3224,10 @@ void main(uint3 tid : SV_DispatchThreadID) output.Store(4 * tid.x, tid.x); })x"; - auto compiled = Compile(m_dllSupport, source, L"cs_6_2", {L"-Od"}); - auto debugOutput = RunDebugPass(compiled); - auto accessOutput = RunShaderAccessTrackingPass(debugOutput.blob); + CComPtr compiled = + Compile(m_dllSupport, source, L"cs_6_2", {L"-Od"}); + PassOutput debugOutput = RunDebugPass(compiled); + PassOutput accessOutput = RunShaderAccessTrackingPass(debugOutput.blob); ModuleAndHangersOn moduleEtc(accessOutput.blob); VERIFY_ARE_EQUAL(1u, CountToolsUAVs(moduleEtc.GetDxilModule())); @@ -3254,10 +3257,10 @@ void MissTwo(inout MyPayload payload) } )x"; - auto compiled = Compile(m_dllSupport, source, L"lib_6_6", {}); - auto output = RunDxilPIXDXRInvocationsLog(compiled); + CComPtr compiled = Compile(m_dllSupport, source, L"lib_6_6", {}); + CComPtr output = RunDxilPIXDXRInvocationsLog(compiled); - auto lines = Tokenize(Disassemble(output), "\n"); + std::vector lines = Tokenize(Disassemble(output), "\n"); VERIFY_ARE_EQUAL(2, CountToolsUAVRecords(lines)); } @@ -3300,7 +3303,7 @@ void MyMiss(inout MyPayload payload) } )x"; - auto compiled = Compile(m_dllSupport, source, L"lib_6_6", {}); + CComPtr compiled = Compile(m_dllSupport, source, L"lib_6_6", {}); ModuleAndHangersOn moduleEtc(compiled); DxilModule &DM = moduleEtc.GetDxilModule(); LoadSubobjectsFromContainerIntoModule(compiled, DM); @@ -3319,16 +3322,17 @@ void main(uint threadId : SV_DispatchThreadID) { })x"; - auto compiled = Compile(m_dllSupport, source, L"cs_6_2", {L"-Od"}); - auto output = RunDebugPass(compiled); - auto lines = Tokenize(Disassemble(output.blob), "\n"); + CComPtr compiled = + Compile(m_dllSupport, source, L"cs_6_2", {L"-Od"}); + PassOutput output = RunDebugPass(compiled); + std::vector lines = Tokenize(Disassemble(output.blob), "\n"); constexpr uint64_t EnableRawAndStructuredBuffers = 0x10; bool foundShaderFlags = false; uint64_t shaderFlags = 0; const std::string tagPrefix = "!{i32 0, i64 "; - for (auto const &line : lines) { - const auto tagStart = line.find(tagPrefix); + for (const std::string &line : lines) { + const std::string::size_type tagStart = line.find(tagPrefix); if (tagStart == std::string::npos) { continue; } @@ -3377,13 +3381,13 @@ void main() true); VERIFY_IS_NOT_NULL(serializedRootSignature); - auto serializedData = + const uint8_t *serializedData = static_cast(serializedRootSignature->GetBufferPointer()); std::vector originalRootSignature( serializedData, serializedData + serializedRootSignature->GetBufferSize()); - auto compiled = Compile(m_dllSupport, source, L"cs_6_0", {}); + CComPtr compiled = Compile(m_dllSupport, source, L"cs_6_0", {}); ModuleAndHangersOn moduleEtc(compiled); DxilModule &DM = moduleEtc.GetDxilModule(); DM.ResetSerializedRootSignature(originalRootSignature); @@ -3503,7 +3507,7 @@ void main() static bool HasUnusedDeclaration(std::vector const &lines, std::string const &functionName) { bool declared = false; - for (auto const &line : lines) { + for (const std::string &line : lines) { if (line.find("declare") != std::string::npos && line.find(functionName) != std::string::npos) { declared = true; @@ -3555,8 +3559,10 @@ float4 main() : SV_Target return float4(1, 2, 3, 4); })x"; - auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {L"-Od"}); - auto output = RunSinglePass(compiled, L"-hlsl-dxil-constantColor"); + CComPtr compiled = + Compile(m_dllSupport, source, L"ps_6_0", {L"-Od"}); + SinglePassOutput output = + RunSinglePass(compiled, L"-hlsl-dxil-constantColor"); VERIFY_IS_FALSE(HasUnusedDeclaration(output.Lines, "dx.op.storeOutput.i32")); VerifyInstrumentedModuleIsValid(output.Module, @@ -3570,8 +3576,10 @@ void main() { })x"; - auto compiled = Compile(m_dllSupport, source, L"cs_6_0", {L"-Od"}); - auto output = RunSinglePass(compiled, L"-hlsl-dxil-constantColor"); + CComPtr compiled = + Compile(m_dllSupport, source, L"cs_6_0", {L"-Od"}); + SinglePassOutput output = + RunSinglePass(compiled, L"-hlsl-dxil-constantColor"); const std::string disassembly = Disassemble(output.Module); VerifyInstrumentedModuleIsValid( @@ -3587,8 +3595,10 @@ float4 main() : SV_Target return float4(1, 2, 3, 4); })x"; - auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {L"-Od"}); - auto output = RunSinglePass(compiled, L"-hlsl-dxil-remove-discards"); + CComPtr compiled = + Compile(m_dllSupport, source, L"ps_6_0", {L"-Od"}); + SinglePassOutput output = + RunSinglePass(compiled, L"-hlsl-dxil-remove-discards"); VERIFY_IS_FALSE(HasUnusedDeclaration(output.Lines, "dx.op.discard")); VerifyInstrumentedModuleIsValid(output.Module, @@ -3602,7 +3612,7 @@ float4 main() : SV_Target return float4(1, 2, 3, 4); })x"; - auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {}); + CComPtr compiled = Compile(m_dllSupport, source, L"ps_6_0", {}); ModuleAndHangersOn moduleEtc(compiled); DxilModule &DM = moduleEtc.GetDxilModule(); OP *HlslOP = DM.GetOP(); @@ -3634,7 +3644,8 @@ float4 main(float2 uv : TEXCOORD0) : SV_Target return textures[(uint)uv.x].Load(int3(0, 0, 0)); })x"; - auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {L"-Od"}); + CComPtr compiled = + Compile(m_dllSupport, source, L"ps_6_0", {L"-Od"}); ModuleAndHangersOn moduleEtc(compiled); DxilModule &DM = moduleEtc.GetDxilModule(); bool visitorCalled = false; @@ -3811,7 +3822,8 @@ float4 main(float2 uv : TEXCOORD0) : SV_Target return textures[NonUniformResourceIndex(index)].Load(int3(0, 0, 0)); })x"; - auto compiled = Compile(m_dllSupport, source, L"ps_6_6", {L"-Od"}); + CComPtr compiled = + Compile(m_dllSupport, source, L"ps_6_6", {L"-Od"}); std::string outputText; PassOutput output = RunDxilNonUniformResourceIndexInstrumentation(compiled, outputText); @@ -4209,7 +4221,7 @@ void main() { DebugBreak(); })x"; - auto compiled = Compile(m_dllSupport, source, L"cs_6_10", {}); + CComPtr compiled = Compile(m_dllSupport, source, L"cs_6_10", {}); auto output = RunDebugBreakPass(compiled); bool foundDebugBreak = false; for (auto const &line : output.lines) { From d1257a5fa5b0b988199564c320c2055360d81b4b Mon Sep 17 00:00:00 2001 From: Damyan Pepper Date: Thu, 3 Sep 2026 10:44:14 -0700 Subject: [PATCH 4/5] [PIX] Apply LLVM naming to L2 additions Rename only the C++ declarations introduced by L2 and their required references. Preserve framework test names, established LLVM abbreviations, pre-existing identifiers, and embedded HLSL fixture content. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../DxilDebugBreakInstrumentation.cpp | 4 +- ...NonUniformResourceIndexInstrumentation.cpp | 4 +- .../DxilOutputColorBecomesConstant.cpp | 8 +- ...lPIXAddTidToAmplificationShaderPayload.cpp | 2 +- ...DxilPIXMeshShaderOutputInstrumentation.cpp | 4 +- lib/DxilPIXPasses/DxilRemoveDiscards.cpp | 2 +- lib/DxilPIXPasses/PixPassHelpers.cpp | 61 +- lib/DxilPIXPasses/PixPassHelpers.h | 2 +- tools/clang/unittests/HLSL/PixTest.cpp | 670 +++++++++--------- 9 files changed, 379 insertions(+), 378 deletions(-) diff --git a/lib/DxilPIXPasses/DxilDebugBreakInstrumentation.cpp b/lib/DxilPIXPasses/DxilDebugBreakInstrumentation.cpp index dc27f4b469..13cbf7c6c8 100644 --- a/lib/DxilPIXPasses/DxilDebugBreakInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilDebugBreakInstrumentation.cpp @@ -120,8 +120,8 @@ bool DxilDebugBreakInstrumentation::runOnModule(Module &M) { CI->eraseFromParent(); } - PIXPassHelpers::EraseIfUnused(DM, DebugBreakFunc); - PIXPassHelpers::EraseIfUnused(DM, AtomicOpFunc); + PIXPassHelpers::eraseIfUnused(DM, DebugBreakFunc); + PIXPassHelpers::eraseIfUnused(DM, AtomicOpFunc); const bool modified = (PixUAVResource != nullptr); diff --git a/lib/DxilPIXPasses/DxilNonUniformResourceIndexInstrumentation.cpp b/lib/DxilPIXPasses/DxilNonUniformResourceIndexInstrumentation.cpp index 3c7a542648..cbeb778c41 100644 --- a/lib/DxilPIXPasses/DxilNonUniformResourceIndexInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilNonUniformResourceIndexInstrumentation.cpp @@ -149,8 +149,8 @@ bool DxilNonUniformResourceIndexInstrumentation::runOnModule(Module &M) { const bool modified = (PixUAVResource != nullptr); - PIXPassHelpers::EraseIfUnused(DM, WaveActiveAllEqualFunc); - PIXPassHelpers::EraseIfUnused(DM, AtomicOpFunc); + PIXPassHelpers::eraseIfUnused(DM, WaveActiveAllEqualFunc); + PIXPassHelpers::eraseIfUnused(DM, AtomicOpFunc); if (modified) { DM.ReEmitDxilResources(); diff --git a/lib/DxilPIXPasses/DxilOutputColorBecomesConstant.cpp b/lib/DxilPIXPasses/DxilOutputColorBecomesConstant.cpp index 227cc890a7..ff389f9348 100644 --- a/lib/DxilPIXPasses/DxilOutputColorBecomesConstant.cpp +++ b/lib/DxilPIXPasses/DxilOutputColorBecomesConstant.cpp @@ -125,8 +125,8 @@ bool DxilOutputColorBecomesConstant::runOnModule(Module &M) { [&hasIntOutputs](CallInst *) { hasIntOutputs = true; }); if (!hasFloatOutputs && !hasIntOutputs) { - PIXPassHelpers::EraseIfUnused(DM, FloatOutputFunction); - PIXPassHelpers::EraseIfUnused(DM, IntOutputFunction); + PIXPassHelpers::eraseIfUnused(DM, FloatOutputFunction); + PIXPassHelpers::eraseIfUnused(DM, IntOutputFunction); return false; } @@ -253,8 +253,8 @@ bool DxilOutputColorBecomesConstant::runOnModule(Module &M) { }); } - PIXPassHelpers::EraseIfUnused(DM, FloatOutputFunction); - PIXPassHelpers::EraseIfUnused(DM, IntOutputFunction); + PIXPassHelpers::eraseIfUnused(DM, FloatOutputFunction); + PIXPassHelpers::eraseIfUnused(DM, IntOutputFunction); return Modified; } diff --git a/lib/DxilPIXPasses/DxilPIXAddTidToAmplificationShaderPayload.cpp b/lib/DxilPIXPasses/DxilPIXAddTidToAmplificationShaderPayload.cpp index c459d4e768..10122f748f 100644 --- a/lib/DxilPIXPasses/DxilPIXAddTidToAmplificationShaderPayload.cpp +++ b/lib/DxilPIXPasses/DxilPIXAddTidToAmplificationShaderPayload.cpp @@ -187,7 +187,7 @@ bool DxilPIXAddTidToAmplificationShaderPayload::runOnModule(Module &M) { cast(&*I)->getCalledFunction(); I->removeFromParent(); delete &*I; - PIXPassHelpers::EraseIfUnused(DM, OriginalDispatchMeshFn); + PIXPassHelpers::eraseIfUnused(DM, OriginalDispatchMeshFn); // Validation requires exactly one DispatchMesh in an AS, so we can exit // after the first one: DM.ReEmitDxilResources(); diff --git a/lib/DxilPIXPasses/DxilPIXMeshShaderOutputInstrumentation.cpp b/lib/DxilPIXPasses/DxilPIXMeshShaderOutputInstrumentation.cpp index 418c285576..de04a246e1 100644 --- a/lib/DxilPIXPasses/DxilPIXMeshShaderOutputInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilPIXMeshShaderOutputInstrumentation.cpp @@ -328,7 +328,7 @@ bool DxilPIXMeshShaderOutputInstrumentation::runOnModule(Module &M) { ReplaceAllUsesOfInstructionWithNewValueAndDeleteInstruction( getMeshPayloadInstructions, payload, expanded.ExpandedPayloadStructType); - PIXPassHelpers::EraseIfUnused(DM, OriginalGetMeshPayloadFunction); + PIXPassHelpers::eraseIfUnused(DM, OriginalGetMeshPayloadFunction); } } @@ -425,7 +425,7 @@ bool DxilPIXMeshShaderOutputInstrumentation::runOnModule(Module &M) { } for (Function *StoreVertexOutputFunction : StoreVertexOutputFunctions) { - PIXPassHelpers::EraseIfUnused(DM, StoreVertexOutputFunction); + PIXPassHelpers::eraseIfUnused(DM, StoreVertexOutputFunction); } DM.ReEmitDxilResources(); diff --git a/lib/DxilPIXPasses/DxilRemoveDiscards.cpp b/lib/DxilPIXPasses/DxilRemoveDiscards.cpp index 72b2bf88cd..492dfebdb4 100644 --- a/lib/DxilPIXPasses/DxilRemoveDiscards.cpp +++ b/lib/DxilPIXPasses/DxilRemoveDiscards.cpp @@ -55,7 +55,7 @@ bool DxilRemoveDiscards::runOnModule(Module &M) { Modified = true; } - PIXPassHelpers::EraseIfUnused(DM, DiscardFunction); + PIXPassHelpers::eraseIfUnused(DM, DiscardFunction); return Modified; } diff --git a/lib/DxilPIXPasses/PixPassHelpers.cpp b/lib/DxilPIXPasses/PixPassHelpers.cpp index b83f195e32..2c572e4cf4 100644 --- a/lib/DxilPIXPasses/PixPassHelpers.cpp +++ b/lib/DxilPIXPasses/PixPassHelpers.cpp @@ -200,7 +200,7 @@ constexpr uint32_t toolsRegisterSpace = static_cast(-2); // Returns whether a parameter was appended. template -bool ExtendRootSig(RootSigDesc &rootSigDesc, uint32_t toolsUAVRegister) { +bool ExtendRootSig(RootSigDesc &rootSigDesc, uint32_t ToolsUAVRegister) { auto *existingParams = rootSigDesc.pParameters; for (uint32_t i = 0; i < rootSigDesc.NumParameters; ++i) { if (rootSigDesc.pParameters[i].ParameterType == @@ -208,7 +208,7 @@ bool ExtendRootSig(RootSigDesc &rootSigDesc, uint32_t toolsUAVRegister) { if (rootSigDesc.pParameters[i].Descriptor.RegisterSpace == toolsRegisterSpace && rootSigDesc.pParameters[i].Descriptor.ShaderRegister == - toolsUAVRegister) { + ToolsUAVRegister) { // Already added return false; } @@ -226,7 +226,7 @@ bool ExtendRootSig(RootSigDesc &rootSigDesc, uint32_t toolsUAVRegister) { rootSigDesc.pParameters[rootSigDesc.NumParameters].Descriptor.RegisterSpace = toolsRegisterSpace; rootSigDesc.pParameters[rootSigDesc.NumParameters].Descriptor.ShaderRegister = - toolsUAVRegister; + ToolsUAVRegister; rootSigDesc.pParameters[rootSigDesc.NumParameters].ShaderVisibility = DxilShaderVisibility::All; rootSigDesc.NumParameters++; @@ -235,18 +235,18 @@ bool ExtendRootSig(RootSigDesc &rootSigDesc, uint32_t toolsUAVRegister) { static std::vector AddUAVParamterToRootSignature(const void *Data, uint32_t Size, - uint32_t toolsUAVRegister) { + uint32_t ToolsUAVRegister) { DxilVersionedRootSignature rootSignature; DeserializeRootSignature(Data, Size, rootSignature.get_address_of()); auto *rs = rootSignature.get_mutable(); switch (rootSignature->Version) { case DxilRootSignatureVersion::Version_1_0: ExtendRootSig(rs->Desc_1_0, - toolsUAVRegister); + ToolsUAVRegister); break; case DxilRootSignatureVersion::Version_1_1: if (ExtendRootSig( - rs->Desc_1_1, toolsUAVRegister)) { + rs->Desc_1_1, ToolsUAVRegister)) { rs->Desc_1_1.pParameters[rs->Desc_1_1.NumParameters - 1] .Descriptor.Flags = hlsl::DxilRootDescriptorFlags::None; } @@ -256,11 +256,11 @@ AddUAVParamterToRootSignature(const void *Data, uint32_t Size, } static void AddUAVToShaderAttributeRootSignature(DxilModule &DM, - uint32_t toolsUAVRegister) { + uint32_t ToolsUAVRegister) { auto rs = DM.GetSerializedRootSignature(); if (!rs.empty()) { std::vector asVector = AddUAVParamterToRootSignature( - rs.data(), static_cast(rs.size()), toolsUAVRegister); + rs.data(), static_cast(rs.size()), ToolsUAVRegister); if (!asVector.empty()) { DM.ResetSerializedRootSignature(asVector); } @@ -268,13 +268,13 @@ static void AddUAVToShaderAttributeRootSignature(DxilModule &DM, } static void AddUAVToDxilDefinedGlobalRootSignatures(DxilModule &DM, - uint32_t toolsUAVRegister) { + uint32_t ToolsUAVRegister) { struct ReplacementRootSignature { std::string Name; std::vector Data; }; - std::vector replacementRootSignatures; + std::vector ReplacementRootSignatures; auto *subObjects = DM.GetSubobjects(); if (subObjects != nullptr) { for (auto const &subObject : subObjects->GetSubobjects()) { @@ -285,24 +285,23 @@ static void AddUAVToDxilDefinedGlobalRootSignatures(DxilModule &DM, constexpr bool notALocalRS = false; if (subObject.second->GetRootSignature(notALocalRS, Data, Size, nullptr)) { - std::vector extended = - AddUAVParamterToRootSignature(Data, Size, toolsUAVRegister); - if (!extended.empty()) { - replacementRootSignatures.push_back( - {subObject.first.str(), std::move(extended)}); + std::vector ExtendedRootSignature = + AddUAVParamterToRootSignature(Data, Size, ToolsUAVRegister); + if (!ExtendedRootSignature.empty()) { + ReplacementRootSignatures.push_back( + {subObject.first.str(), std::move(ExtendedRootSignature)}); } } } } - constexpr bool notALocalRS = false; - for (const ReplacementRootSignature &replacementRootSignature : - replacementRootSignatures) { - subObjects->RemoveSubobject(replacementRootSignature.Name); + constexpr bool NotALocalRootSignature = false; + for (const ReplacementRootSignature &Replacement : + ReplacementRootSignatures) { + subObjects->RemoveSubobject(Replacement.Name); subObjects->CreateRootSignature( - replacementRootSignature.Name, notALocalRS, - replacementRootSignature.Data.data(), - static_cast(replacementRootSignature.Data.size())); + Replacement.Name, NotALocalRootSignature, Replacement.Data.data(), + static_cast(Replacement.Data.size())); } } } @@ -313,10 +312,10 @@ hlsl::DxilResource *CreateGlobalUAVResource(hlsl::DxilModule &DM, const char *name) { LLVMContext &Ctx = DM.GetModule()->getContext(); - for (const std::unique_ptr &existingUAV : DM.GetUAVs()) { - if (existingUAV->GetSpaceID() == toolsRegisterSpace && - existingUAV->GetLowerBound() == hlslBindIndex) { - return existingUAV.get(); + for (const std::unique_ptr &ExistingUAV : DM.GetUAVs()) { + if (ExistingUAV->GetSpaceID() == toolsRegisterSpace && + ExistingUAV->GetLowerBound() == hlslBindIndex) { + return ExistingUAV.get(); } } @@ -386,7 +385,7 @@ hlsl::DxilResource *CreateGlobalUAVResource(hlsl::DxilModule &DM, return ret; } -void EraseIfUnused(hlsl::DxilModule &DM, llvm::Function *OpFunction) { +void eraseIfUnused(hlsl::DxilModule &DM, llvm::Function *OpFunction) { if (OpFunction != nullptr && OpFunction->user_empty()) { DM.GetOP()->RemoveFunction(OpFunction); OpFunction->eraseFromParent(); @@ -585,11 +584,11 @@ void ForEachDynamicallyIndexedResource( llvm::Function *CreateHandleFromBindingFn; llvm::Function *CreateHandleFromHeapFn; ~UnusedDeclarationCleanup() { - EraseIfUnused(DM, CreateHandleFn); - EraseIfUnused(DM, CreateHandleFromBindingFn); - EraseIfUnused(DM, CreateHandleFromHeapFn); + eraseIfUnused(DM, CreateHandleFn); + eraseIfUnused(DM, CreateHandleFromBindingFn); + eraseIfUnused(DM, CreateHandleFromHeapFn); } - } cleanup{DM, CreateHandleFn, CreateHandleFromBindingFn, + } Cleanup{DM, CreateHandleFn, CreateHandleFromBindingFn, CreateHandleFromHeapFn}; for (auto FI = CreateHandleFn->user_begin(); diff --git a/lib/DxilPIXPasses/PixPassHelpers.h b/lib/DxilPIXPasses/PixPassHelpers.h index 3d6e24d22f..5efdc4940c 100644 --- a/lib/DxilPIXPasses/PixPassHelpers.h +++ b/lib/DxilPIXPasses/PixPassHelpers.h @@ -48,7 +48,7 @@ llvm::CallInst *CreateHandleForResource(hlsl::DxilModule &DM, hlsl::DxilResourceBase *resource, const char *name); llvm::Function *GetEntryFunction(hlsl::DxilModule &DM); -void EraseIfUnused(hlsl::DxilModule &DM, llvm::Function *OpFunction); +void eraseIfUnused(hlsl::DxilModule &DM, llvm::Function *OpFunction); std::vector GetAllInstrumentableFunctions(hlsl::DxilModule &DM); hlsl::DXIL::ShaderKind GetFunctionShaderKind(hlsl::DxilModule &DM, diff --git a/tools/clang/unittests/HLSL/PixTest.cpp b/tools/clang/unittests/HLSL/PixTest.cpp index 443f734f5d..f4ce5bf855 100644 --- a/tools/clang/unittests/HLSL/PixTest.cpp +++ b/tools/clang/unittests/HLSL/PixTest.cpp @@ -578,12 +578,12 @@ class PixTest : public ::testing::Test { } void ValidateAccessTrackingMods(const char *hlsl, bool modsExpected); - void LoadSubobjectsFromContainerIntoModule(IDxcBlob *container, + void loadSubobjectsFromContainerIntoModule(IDxcBlob *Container, DxilModule &DM); - void VerifyGlobalRootSignaturesHaveToolsUAVs( - DxilSubobjects *subObjects, - const std::vector &expectedRootSignatureNames, - const std::vector &expectedShaderRegisters); + void verifyGlobalRootSignaturesHaveToolsUAVs( + DxilSubobjects *Subobjects, + const std::vector &ExpectedRootSignatureNames, + const std::vector &ExpectedShaderRegisters); class ModuleAndHangersOn { std::unique_ptr llvmContext; @@ -689,50 +689,52 @@ bool PixTest::InitSupport() { return true; } -static unsigned CountToolsUAVs(DxilModule &DM) { - unsigned count = 0; - for (const std::unique_ptr &uav : DM.GetUAVs()) { - if (uav->GetSpaceID() == static_cast(-2)) { - count++; +static unsigned countToolsUAVs(DxilModule &DM) { + unsigned Count = 0; + for (const std::unique_ptr &UAV : DM.GetUAVs()) { + if (UAV->GetSpaceID() == static_cast(-2)) { + Count++; } } - return count; + return Count; } -static int CountToolsUAVRecords(std::vector const &lines) { - int count = 0; - for (const std::string &line : lines) { - if (!line.empty() && line[0] == '!' && - line.find(", i32 -2, i32 ") != std::string::npos) { - count++; +static int countToolsUAVRecords(std::vector const &Lines) { + int Count = 0; + for (const std::string &Line : Lines) { + if (!Line.empty() && Line[0] == '!' && + Line.find(", i32 -2, i32 ") != std::string::npos) { + Count++; } } - return count; + return Count; } static bool -RootSignatureHasToolsUAV(const DxilVersionedRootSignatureDesc *rootSignature, - uint32_t shaderRegister) { - switch (rootSignature->Version) { +rootSignatureHasToolsUAV(const DxilVersionedRootSignatureDesc *RootSignature, + uint32_t ShaderRegister) { + switch (RootSignature->Version) { case DxilRootSignatureVersion::Version_1_0: { - const DxilRootSignatureDesc &desc = rootSignature->Desc_1_0; - for (uint32_t i = 0; i < desc.NumParameters; ++i) { - const DxilRootParameter ¶m = desc.pParameters[i]; - if (param.ParameterType == DxilRootParameterType::UAV && - param.Descriptor.RegisterSpace == static_cast(-2) && - param.Descriptor.ShaderRegister == shaderRegister) { + const DxilRootSignatureDesc &Desc = RootSignature->Desc_1_0; + for (uint32_t ParameterIndex = 0; ParameterIndex < Desc.NumParameters; + ++ParameterIndex) { + const DxilRootParameter &Parameter = Desc.pParameters[ParameterIndex]; + if (Parameter.ParameterType == DxilRootParameterType::UAV && + Parameter.Descriptor.RegisterSpace == static_cast(-2) && + Parameter.Descriptor.ShaderRegister == ShaderRegister) { return true; } } break; } case DxilRootSignatureVersion::Version_1_1: { - const DxilRootSignatureDesc1 &desc = rootSignature->Desc_1_1; - for (uint32_t i = 0; i < desc.NumParameters; ++i) { - const DxilRootParameter1 ¶m = desc.pParameters[i]; - if (param.ParameterType == DxilRootParameterType::UAV && - param.Descriptor.RegisterSpace == static_cast(-2) && - param.Descriptor.ShaderRegister == shaderRegister) { + const DxilRootSignatureDesc1 &Desc = RootSignature->Desc_1_1; + for (uint32_t ParameterIndex = 0; ParameterIndex < Desc.NumParameters; + ++ParameterIndex) { + const DxilRootParameter1 &Parameter = Desc.pParameters[ParameterIndex]; + if (Parameter.ParameterType == DxilRootParameterType::UAV && + Parameter.Descriptor.RegisterSpace == static_cast(-2) && + Parameter.Descriptor.ShaderRegister == ShaderRegister) { return true; } } @@ -742,67 +744,67 @@ RootSignatureHasToolsUAV(const DxilVersionedRootSignatureDesc *rootSignature, return false; } -void PixTest::LoadSubobjectsFromContainerIntoModule(IDxcBlob *container, +void PixTest::loadSubobjectsFromContainerIntoModule(IDxcBlob *Container, DxilModule &DM) { - const char *blobContent = - reinterpret_cast(container->GetBufferPointer()); - const unsigned blobSize = container->GetBufferSize(); - const hlsl::DxilContainerHeader *containerHeader = - hlsl::IsDxilContainerLike(blobContent, blobSize); - VERIFY_ARE_NOT_EQUAL(containerHeader, nullptr); - - const hlsl::DxilPartHeader *partHeader = - GetDxilPartByType(containerHeader, hlsl::DFCC_RuntimeData); - VERIFY_ARE_NOT_EQUAL(partHeader, nullptr); - - hlsl::RDAT::DxilRuntimeData rdat(GetDxilPartData(partHeader), - partHeader->PartSize); - std::unique_ptr subObjects(new DxilSubobjects()); - VERIFY_IS_TRUE(LoadSubobjectsFromRDAT(*subObjects, rdat)); - DM.ResetSubobjects(subObjects.release()); -} - -void PixTest::VerifyGlobalRootSignaturesHaveToolsUAVs( - DxilSubobjects *subObjects, - const std::vector &expectedRootSignatureNames, - const std::vector &expectedShaderRegisters) { - VERIFY_IS_NOT_NULL(subObjects); - - std::map foundRootSignatures; - for (const std::string &rootSignatureName : expectedRootSignatureNames) { - foundRootSignatures[rootSignatureName] = false; + const char *BlobContent = + reinterpret_cast(Container->GetBufferPointer()); + const unsigned BlobSize = Container->GetBufferSize(); + const hlsl::DxilContainerHeader *ContainerHeader = + hlsl::IsDxilContainerLike(BlobContent, BlobSize); + VERIFY_ARE_NOT_EQUAL(ContainerHeader, nullptr); + + const hlsl::DxilPartHeader *PartHeader = + GetDxilPartByType(ContainerHeader, hlsl::DFCC_RuntimeData); + VERIFY_ARE_NOT_EQUAL(PartHeader, nullptr); + + hlsl::RDAT::DxilRuntimeData RuntimeData(GetDxilPartData(PartHeader), + PartHeader->PartSize); + std::unique_ptr Subobjects(new DxilSubobjects()); + VERIFY_IS_TRUE(LoadSubobjectsFromRDAT(*Subobjects, RuntimeData)); + DM.ResetSubobjects(Subobjects.release()); +} + +void PixTest::verifyGlobalRootSignaturesHaveToolsUAVs( + DxilSubobjects *Subobjects, + const std::vector &ExpectedRootSignatureNames, + const std::vector &ExpectedShaderRegisters) { + VERIFY_IS_NOT_NULL(Subobjects); + + std::map FoundRootSignatures; + for (const std::string &RootSignatureName : ExpectedRootSignatureNames) { + FoundRootSignatures[RootSignatureName] = false; } - for (auto const &subObject : subObjects->GetSubobjects()) { - if (subObject.second->GetKind() != + for (auto const &Subobject : Subobjects->GetSubobjects()) { + if (Subobject.second->GetKind() != hlsl::DXIL::SubobjectKind::GlobalRootSignature) { continue; } - const std::string subObjectName = subObject.first.str(); - if (foundRootSignatures.find(subObjectName) == foundRootSignatures.end()) { + const std::string SubobjectName = Subobject.first.str(); + if (FoundRootSignatures.find(SubobjectName) == FoundRootSignatures.end()) { continue; } - const void *data = nullptr; - uint32_t size = 0; - constexpr bool notALocalRS = false; - VERIFY_IS_TRUE( - subObject.second->GetRootSignature(notALocalRS, data, size, nullptr)); + const void *Data = nullptr; + uint32_t Size = 0; + constexpr bool NotALocalRootSignature = false; + VERIFY_IS_TRUE(Subobject.second->GetRootSignature(NotALocalRootSignature, + Data, Size, nullptr)); - DxilVersionedRootSignatureDesc const *rootSignature = nullptr; - DeserializeRootSignature(data, size, &rootSignature); - for (uint32_t expectedShaderRegister : expectedShaderRegisters) { + DxilVersionedRootSignatureDesc const *RootSignature = nullptr; + DeserializeRootSignature(Data, Size, &RootSignature); + for (uint32_t ExpectedShaderRegister : ExpectedShaderRegisters) { VERIFY_IS_TRUE( - RootSignatureHasToolsUAV(rootSignature, expectedShaderRegister)); + rootSignatureHasToolsUAV(RootSignature, ExpectedShaderRegister)); } - DeleteRootSignature(rootSignature); - foundRootSignatures[subObjectName] = true; + DeleteRootSignature(RootSignature); + FoundRootSignatures[SubobjectName] = true; } - for (const std::map::value_type &foundRootSignature : - foundRootSignatures) { - VERIFY_IS_TRUE(foundRootSignature.second); + for (const std::map::value_type &FoundRootSignature : + FoundRootSignatures) { + VERIFY_IS_TRUE(FoundRootSignature.second); } } @@ -1039,10 +1041,10 @@ PassOutput PixTest::RunDxilNonUniformResourceIndexInstrumentation( outputText = BlobToUtf8(pText); - PassOutput result; - result.blob = pOptimizedModule; - result.lines = Tokenize(Disassemble(pOptimizedModule), "\n"); - return result; + PassOutput Result; + Result.blob = pOptimizedModule; + Result.lines = Tokenize(Disassemble(pOptimizedModule), "\n"); + return Result; } CComPtr @@ -1064,12 +1066,12 @@ PixTest::RunDxilPIXAddTidToAmplificationShaderPayloadPass(IDxcBlob *blob) { return pOptimizedModule; } -static bool HasDeclaration(const std::string &disassembly, - const std::string &functionName); -static std::string FindDeclarationLine(const std::string &disassembly, - const std::string &functionName); -static bool HasDeclarationLine(const std::string &disassembly, - const std::string &declaration); +static bool hasDeclaration(const std::string &Disassembly, + const std::string &FunctionName); +static std::string findDeclarationLine(const std::string &Disassembly, + const std::string &FunctionName); +static bool hasDeclarationLine(const std::string &Disassembly, + const std::string &Declaration); TEST_F(PixTest, AddToASPayload) { @@ -1112,30 +1114,30 @@ void MSMain( )"; auto as = Compile(m_dllSupport, hlsl, L"as_6_6", {}, L"ASMain"); - const std::string originalDispatchMeshDeclaration = - FindDeclarationLine(Disassemble(as), "dx.op.dispatchMesh"); - VERIFY_IS_FALSE(originalDispatchMeshDeclaration.empty()); + const std::string OriginalDispatchMeshDeclaration = + findDeclarationLine(Disassemble(as), "dx.op.dispatchMesh"); + VERIFY_IS_FALSE(OriginalDispatchMeshDeclaration.empty()); - CComPtr asOutput = + CComPtr ASOutput = RunDxilPIXAddTidToAmplificationShaderPayloadPass(as); - VERIFY_IS_FALSE(HasDeclarationLine(Disassemble(asOutput), - originalDispatchMeshDeclaration)); + VERIFY_IS_FALSE(hasDeclarationLine(Disassemble(ASOutput), + OriginalDispatchMeshDeclaration)); auto ms = Compile(m_dllSupport, hlsl, L"ms_6_6", {}, L"MSMain"); - const std::string originalGetMeshPayloadDeclaration = - FindDeclarationLine(Disassemble(ms), "dx.op.getMeshPayload"); - VERIFY_IS_FALSE(originalGetMeshPayloadDeclaration.empty()); + const std::string OriginalGetMeshPayloadDeclaration = + findDeclarationLine(Disassemble(ms), "dx.op.getMeshPayload"); + VERIFY_IS_FALSE(OriginalGetMeshPayloadDeclaration.empty()); - CComPtr msOutput = RunDxilPIXMeshShaderOutputPass(ms); - const std::string meshDisassembly = Disassemble(msOutput); + CComPtr MSOutput = RunDxilPIXMeshShaderOutputPass(ms); + const std::string MeshDisassembly = Disassemble(MSOutput); VERIFY_IS_FALSE( - HasDeclarationLine(meshDisassembly, originalGetMeshPayloadDeclaration)); + hasDeclarationLine(MeshDisassembly, OriginalGetMeshPayloadDeclaration)); VERIFY_IS_FALSE( - HasDeclaration(meshDisassembly, "dx.op.storeVertexOutput.i32")); + hasDeclaration(MeshDisassembly, "dx.op.storeVertexOutput.i32")); VERIFY_IS_FALSE( - HasDeclaration(meshDisassembly, "dx.op.storeVertexOutput.i16")); + hasDeclaration(MeshDisassembly, "dx.op.storeVertexOutput.i16")); VERIFY_IS_FALSE( - HasDeclaration(meshDisassembly, "dx.op.storeVertexOutput.f16")); + hasDeclaration(MeshDisassembly, "dx.op.storeVertexOutput.f16")); } unsigned FindOrAddVSInSignatureElementForInstanceOrVertexID( hlsl::DxilSignature &InputSignature, hlsl::DXIL::SemanticKind semanticKind); @@ -3214,7 +3216,7 @@ float4 main(int i : A, float j : B) : SV_TARGET } TEST_F(PixTest, ToolsUav_TwoPixPassesShareOneResource) { - const char *source = R"x( + const char *Source = R"x( RWByteAddressBuffer output : register(u0); [numthreads(1, 1, 1)] @@ -3223,20 +3225,20 @@ void main(uint3 tid : SV_DispatchThreadID) output.Store(4 * tid.x, tid.x); })x"; - CComPtr compiled = - Compile(m_dllSupport, source, L"cs_6_2", {L"-Od"}); - PassOutput debugOutput = RunDebugPass(compiled); - PassOutput accessOutput = RunShaderAccessTrackingPass(debugOutput.blob); + CComPtr Compiled = + Compile(m_dllSupport, Source, L"cs_6_2", {L"-Od"}); + PassOutput DebugOutput = RunDebugPass(Compiled); + PassOutput AccessOutput = RunShaderAccessTrackingPass(DebugOutput.blob); - ModuleAndHangersOn moduleEtc(accessOutput.blob); - VERIFY_ARE_EQUAL(1u, CountToolsUAVs(moduleEtc.GetDxilModule())); - VerifyInstrumentedModuleIsValid( - accessOutput.blob, + ModuleAndHangersOn ModuleEtc(AccessOutput.blob); + VERIFY_ARE_EQUAL(1u, countToolsUAVs(ModuleEtc.GetDxilModule())); + verifyInstrumentedModuleIsValid( + AccessOutput.blob, "debug instrumentation followed by shader access tracking"); } TEST_F(PixTest, ToolsUav_LibraryWithTwoEntryPointsCreatesOnePair) { - const char *source = R"x( + const char *Source = R"x( struct [raypayload] MyPayload { float2 barycentrics : read(caller) : write(caller,anyhit); @@ -3256,15 +3258,15 @@ void MissTwo(inout MyPayload payload) } )x"; - CComPtr compiled = Compile(m_dllSupport, source, L"lib_6_6", {}); - CComPtr output = RunDxilPIXDXRInvocationsLog(compiled); + CComPtr Compiled = Compile(m_dllSupport, Source, L"lib_6_6", {}); + CComPtr Output = RunDxilPIXDXRInvocationsLog(Compiled); - std::vector lines = Tokenize(Disassemble(output), "\n"); - VERIFY_ARE_EQUAL(2, CountToolsUAVRecords(lines)); + std::vector Lines = Tokenize(Disassemble(Output), "\n"); + VERIFY_ARE_EQUAL(2, countToolsUAVRecords(Lines)); } TEST_F(PixTest, ToolsUav_ExtendsEveryGlobalRootSignatureSubobject) { - const char *source = R"x( + const char *Source = R"x( GlobalRootSignature firstRootSignature = {"CBV(b0)"}; GlobalRootSignature secondRootSignature = {"SRV(t0)"}; @@ -3302,249 +3304,249 @@ void MyMiss(inout MyPayload payload) } )x"; - CComPtr compiled = Compile(m_dllSupport, source, L"lib_6_6", {}); - ModuleAndHangersOn moduleEtc(compiled); - DxilModule &DM = moduleEtc.GetDxilModule(); - LoadSubobjectsFromContainerIntoModule(compiled, DM); + CComPtr Compiled = Compile(m_dllSupport, Source, L"lib_6_6", {}); + ModuleAndHangersOn ModuleEtc(Compiled); + DxilModule &DM = ModuleEtc.GetDxilModule(); + loadSubobjectsFromContainerIntoModule(Compiled, DM); PIXPassHelpers::CreateGlobalUAVResource(DM, 0, "PIX_CountUAV_Handle"); PIXPassHelpers::CreateGlobalUAVResource(DM, 1, "PIX_LogUAV_Handle"); - VerifyGlobalRootSignaturesHaveToolsUAVs( + verifyGlobalRootSignaturesHaveToolsUAVs( DM.GetSubobjects(), {"firstRootSignature", "secondRootSignature"}, {0, 1}); } TEST_F(PixTest, DebugInstrumentation_RawBufferShaderFlagDeclared) { - const char *source = R"x( + const char *Source = R"x( [numthreads(1, 1, 1)] void main(uint threadId : SV_DispatchThreadID) { })x"; - CComPtr compiled = - Compile(m_dllSupport, source, L"cs_6_2", {L"-Od"}); - PassOutput output = RunDebugPass(compiled); - std::vector lines = Tokenize(Disassemble(output.blob), "\n"); + CComPtr Compiled = + Compile(m_dllSupport, Source, L"cs_6_2", {L"-Od"}); + PassOutput Output = RunDebugPass(Compiled); + std::vector Lines = Tokenize(Disassemble(Output.blob), "\n"); constexpr uint64_t EnableRawAndStructuredBuffers = 0x10; - bool foundShaderFlags = false; - uint64_t shaderFlags = 0; - const std::string tagPrefix = "!{i32 0, i64 "; - for (const std::string &line : lines) { - const std::string::size_type tagStart = line.find(tagPrefix); - if (tagStart == std::string::npos) { + bool FoundShaderFlags = false; + uint64_t ShaderFlags = 0; + const std::string TagPrefix = "!{i32 0, i64 "; + for (const std::string &Line : Lines) { + const std::string::size_type TagStart = Line.find(TagPrefix); + if (TagStart == std::string::npos) { continue; } - shaderFlags = - strtoull(line.c_str() + tagStart + tagPrefix.length(), nullptr, 10); - foundShaderFlags = true; + ShaderFlags = + strtoull(Line.c_str() + TagStart + TagPrefix.length(), nullptr, 10); + FoundShaderFlags = true; break; } - VERIFY_IS_TRUE(foundShaderFlags); + VERIFY_IS_TRUE(FoundShaderFlags); VERIFY_ARE_EQUAL(EnableRawAndStructuredBuffers, - shaderFlags & EnableRawAndStructuredBuffers); - VerifyInstrumentedModuleIsValid(output.blob, + ShaderFlags & EnableRawAndStructuredBuffers); + verifyInstrumentedModuleIsValid(Output.blob, "debug instrumentation shader flags"); } TEST_F(PixTest, ToolsUav_RootSignatureSerializationFailurePreservesSignature) { - const char *source = R"x( + const char *Source = R"x( [numthreads(1, 1, 1)] void main() { })x"; - DxilDescriptorRange range = {}; - range.RangeType = DxilDescriptorRangeType::UAV; - range.NumDescriptors = 1; - range.BaseShaderRegister = 0; - range.RegisterSpace = static_cast(-2); - range.OffsetInDescriptorsFromTableStart = DxilDescriptorRangeOffsetAppend; - - DxilRootParameter parameter = {}; - parameter.ParameterType = DxilRootParameterType::DescriptorTable; - parameter.DescriptorTable.NumDescriptorRanges = 1; - parameter.DescriptorTable.pDescriptorRanges = ⦥ - parameter.ShaderVisibility = DxilShaderVisibility::All; - - DxilVersionedRootSignatureDesc rootSignature = {}; - rootSignature.Version = DxilRootSignatureVersion::Version_1_0; - rootSignature.Desc_1_0.NumParameters = 1; - rootSignature.Desc_1_0.pParameters = ¶meter; - rootSignature.Desc_1_0.Flags = DxilRootSignatureFlags::None; - - CComPtr serializedRootSignature; - CComPtr errorBlob; - SerializeRootSignature(&rootSignature, &serializedRootSignature, &errorBlob, + DxilDescriptorRange Range = {}; + Range.RangeType = DxilDescriptorRangeType::UAV; + Range.NumDescriptors = 1; + Range.BaseShaderRegister = 0; + Range.RegisterSpace = static_cast(-2); + Range.OffsetInDescriptorsFromTableStart = DxilDescriptorRangeOffsetAppend; + + DxilRootParameter Parameter = {}; + Parameter.ParameterType = DxilRootParameterType::DescriptorTable; + Parameter.DescriptorTable.NumDescriptorRanges = 1; + Parameter.DescriptorTable.pDescriptorRanges = &Range; + Parameter.ShaderVisibility = DxilShaderVisibility::All; + + DxilVersionedRootSignatureDesc RootSignature = {}; + RootSignature.Version = DxilRootSignatureVersion::Version_1_0; + RootSignature.Desc_1_0.NumParameters = 1; + RootSignature.Desc_1_0.pParameters = &Parameter; + RootSignature.Desc_1_0.Flags = DxilRootSignatureFlags::None; + + CComPtr SerializedRootSignature; + CComPtr ErrorBlob; + SerializeRootSignature(&RootSignature, &SerializedRootSignature, &ErrorBlob, true); - VERIFY_IS_NOT_NULL(serializedRootSignature); - - const uint8_t *serializedData = - static_cast(serializedRootSignature->GetBufferPointer()); - std::vector originalRootSignature( - serializedData, - serializedData + serializedRootSignature->GetBufferSize()); - - CComPtr compiled = Compile(m_dllSupport, source, L"cs_6_0", {}); - ModuleAndHangersOn moduleEtc(compiled); - DxilModule &DM = moduleEtc.GetDxilModule(); - DM.ResetSerializedRootSignature(originalRootSignature); - - std::unique_ptr subObjects(new DxilSubobjects()); - constexpr bool notALocalRootSignature = false; - subObjects->CreateRootSignature( - "testRootSignature", notALocalRootSignature, originalRootSignature.data(), - static_cast(originalRootSignature.size())); - DM.ResetSubobjects(subObjects.release()); + VERIFY_IS_NOT_NULL(SerializedRootSignature); + + const uint8_t *SerializedData = + static_cast(SerializedRootSignature->GetBufferPointer()); + std::vector OriginalRootSignature( + SerializedData, + SerializedData + SerializedRootSignature->GetBufferSize()); + + CComPtr Compiled = Compile(m_dllSupport, Source, L"cs_6_0", {}); + ModuleAndHangersOn ModuleEtc(Compiled); + DxilModule &DM = ModuleEtc.GetDxilModule(); + DM.ResetSerializedRootSignature(OriginalRootSignature); + + std::unique_ptr Subobjects(new DxilSubobjects()); + constexpr bool NotALocalRootSignature = false; + Subobjects->CreateRootSignature( + "testRootSignature", NotALocalRootSignature, OriginalRootSignature.data(), + static_cast(OriginalRootSignature.size())); + DM.ResetSubobjects(Subobjects.release()); PIXPassHelpers::CreateGlobalUAVResource(DM, 0, "PIX_TestUAV"); - const std::vector &actualRootSignature = + const std::vector &ActualRootSignature = DM.GetSerializedRootSignature(); - VERIFY_ARE_EQUAL(originalRootSignature.size(), actualRootSignature.size()); - VERIFY_IS_TRUE(std::equal(originalRootSignature.begin(), - originalRootSignature.end(), - actualRootSignature.begin())); - - bool foundRootSignature = false; - for (auto const &subObject : DM.GetSubobjects()->GetSubobjects()) { - if (subObject.first != "testRootSignature") { + VERIFY_ARE_EQUAL(OriginalRootSignature.size(), ActualRootSignature.size()); + VERIFY_IS_TRUE(std::equal(OriginalRootSignature.begin(), + OriginalRootSignature.end(), + ActualRootSignature.begin())); + + bool FoundRootSignature = false; + for (auto const &Subobject : DM.GetSubobjects()->GetSubobjects()) { + if (Subobject.first != "testRootSignature") { continue; } - const void *data = nullptr; - uint32_t size = 0; - VERIFY_IS_TRUE(subObject.second->GetRootSignature(notALocalRootSignature, - data, size, nullptr)); - VERIFY_ARE_EQUAL(originalRootSignature.size(), static_cast(size)); - VERIFY_IS_TRUE(std::equal(originalRootSignature.begin(), - originalRootSignature.end(), - static_cast(data))); - foundRootSignature = true; + const void *Data = nullptr; + uint32_t Size = 0; + VERIFY_IS_TRUE(Subobject.second->GetRootSignature(NotALocalRootSignature, + Data, Size, nullptr)); + VERIFY_ARE_EQUAL(OriginalRootSignature.size(), static_cast(Size)); + VERIFY_IS_TRUE(std::equal(OriginalRootSignature.begin(), + OriginalRootSignature.end(), + static_cast(Data))); + FoundRootSignature = true; } - VERIFY_IS_TRUE(foundRootSignature); + VERIFY_IS_TRUE(FoundRootSignature); } TEST_F(PixTest, ToolsUav_ExtendingRootSignaturePreservesUnrelatedParameterFlags) { - const char *source = R"x( + const char *Source = R"x( [numthreads(1, 1, 1)] void main() { })x"; - DxilRootParameter1 parameters[2] = {}; - parameters[0].ParameterType = DxilRootParameterType::UAV; - parameters[0].Descriptor.RegisterSpace = static_cast(-2); - parameters[0].Descriptor.ShaderRegister = 0; - parameters[0].Descriptor.Flags = DxilRootDescriptorFlags::None; - parameters[0].ShaderVisibility = DxilShaderVisibility::All; - - parameters[1].ParameterType = DxilRootParameterType::CBV; - parameters[1].Descriptor.RegisterSpace = 0; - parameters[1].Descriptor.ShaderRegister = 0; - parameters[1].Descriptor.Flags = DxilRootDescriptorFlags::DataVolatile; - parameters[1].ShaderVisibility = DxilShaderVisibility::All; - - DxilVersionedRootSignatureDesc rootSignature = {}; - rootSignature.Version = DxilRootSignatureVersion::Version_1_1; - rootSignature.Desc_1_1.NumParameters = 2; - rootSignature.Desc_1_1.pParameters = parameters; - rootSignature.Desc_1_1.Flags = DxilRootSignatureFlags::None; - - CComPtr serializedRootSignature; - CComPtr errorBlob; - SerializeRootSignature(&rootSignature, &serializedRootSignature, &errorBlob, + DxilRootParameter1 Parameters[2] = {}; + Parameters[0].ParameterType = DxilRootParameterType::UAV; + Parameters[0].Descriptor.RegisterSpace = static_cast(-2); + Parameters[0].Descriptor.ShaderRegister = 0; + Parameters[0].Descriptor.Flags = DxilRootDescriptorFlags::None; + Parameters[0].ShaderVisibility = DxilShaderVisibility::All; + + Parameters[1].ParameterType = DxilRootParameterType::CBV; + Parameters[1].Descriptor.RegisterSpace = 0; + Parameters[1].Descriptor.ShaderRegister = 0; + Parameters[1].Descriptor.Flags = DxilRootDescriptorFlags::DataVolatile; + Parameters[1].ShaderVisibility = DxilShaderVisibility::All; + + DxilVersionedRootSignatureDesc RootSignature = {}; + RootSignature.Version = DxilRootSignatureVersion::Version_1_1; + RootSignature.Desc_1_1.NumParameters = 2; + RootSignature.Desc_1_1.pParameters = Parameters; + RootSignature.Desc_1_1.Flags = DxilRootSignatureFlags::None; + + CComPtr SerializedRootSignature; + CComPtr ErrorBlob; + SerializeRootSignature(&RootSignature, &SerializedRootSignature, &ErrorBlob, true); - VERIFY_IS_NOT_NULL(serializedRootSignature); + VERIFY_IS_NOT_NULL(SerializedRootSignature); - const uint8_t *serializedData = - static_cast(serializedRootSignature->GetBufferPointer()); - std::vector originalRootSignature( - serializedData, - serializedData + serializedRootSignature->GetBufferSize()); + const uint8_t *SerializedData = + static_cast(SerializedRootSignature->GetBufferPointer()); + std::vector OriginalRootSignature( + SerializedData, + SerializedData + SerializedRootSignature->GetBufferSize()); - CComPtr compiled = Compile(m_dllSupport, source, L"cs_6_0", {}); - ModuleAndHangersOn moduleEtc(compiled); - DxilModule &DM = moduleEtc.GetDxilModule(); - DM.ResetSerializedRootSignature(originalRootSignature); + CComPtr Compiled = Compile(m_dllSupport, Source, L"cs_6_0", {}); + ModuleAndHangersOn ModuleEtc(Compiled); + DxilModule &DM = ModuleEtc.GetDxilModule(); + DM.ResetSerializedRootSignature(OriginalRootSignature); PIXPassHelpers::CreateGlobalUAVResource(DM, 0, "PIX_TestUAV0"); { - const std::vector &bytes = DM.GetSerializedRootSignature(); - DxilVersionedRootSignatureDesc const *afterNoOp = nullptr; - DeserializeRootSignature(bytes.data(), static_cast(bytes.size()), - &afterNoOp); - VERIFY_ARE_EQUAL(afterNoOp->Desc_1_1.NumParameters, 2u); - VERIFY_IS_TRUE(afterNoOp->Desc_1_1.pParameters[1].Descriptor.Flags == + const std::vector &Bytes = DM.GetSerializedRootSignature(); + DxilVersionedRootSignatureDesc const *AfterNoOp = nullptr; + DeserializeRootSignature(Bytes.data(), static_cast(Bytes.size()), + &AfterNoOp); + VERIFY_ARE_EQUAL(AfterNoOp->Desc_1_1.NumParameters, 2u); + VERIFY_IS_TRUE(AfterNoOp->Desc_1_1.pParameters[1].Descriptor.Flags == DxilRootDescriptorFlags::DataVolatile); - DeleteRootSignature(afterNoOp); + DeleteRootSignature(AfterNoOp); } PIXPassHelpers::CreateGlobalUAVResource(DM, 1, "PIX_TestUAV1"); { - const std::vector &bytes = DM.GetSerializedRootSignature(); - DxilVersionedRootSignatureDesc const *afterAdd = nullptr; - DeserializeRootSignature(bytes.data(), static_cast(bytes.size()), - &afterAdd); - VERIFY_ARE_EQUAL(afterAdd->Desc_1_1.NumParameters, 3u); - VERIFY_IS_TRUE(afterAdd->Desc_1_1.pParameters[1].Descriptor.Flags == + const std::vector &Bytes = DM.GetSerializedRootSignature(); + DxilVersionedRootSignatureDesc const *AfterAdd = nullptr; + DeserializeRootSignature(Bytes.data(), static_cast(Bytes.size()), + &AfterAdd); + VERIFY_ARE_EQUAL(AfterAdd->Desc_1_1.NumParameters, 3u); + VERIFY_IS_TRUE(AfterAdd->Desc_1_1.pParameters[1].Descriptor.Flags == DxilRootDescriptorFlags::DataVolatile); - VERIFY_ARE_EQUAL(afterAdd->Desc_1_1.pParameters[2].Descriptor.RegisterSpace, + VERIFY_ARE_EQUAL(AfterAdd->Desc_1_1.pParameters[2].Descriptor.RegisterSpace, static_cast(-2)); VERIFY_ARE_EQUAL( - afterAdd->Desc_1_1.pParameters[2].Descriptor.ShaderRegister, 1u); - VERIFY_IS_TRUE(afterAdd->Desc_1_1.pParameters[2].Descriptor.Flags == + AfterAdd->Desc_1_1.pParameters[2].Descriptor.ShaderRegister, 1u); + VERIFY_IS_TRUE(AfterAdd->Desc_1_1.pParameters[2].Descriptor.Flags == DxilRootDescriptorFlags::None); - DeleteRootSignature(afterAdd); + DeleteRootSignature(AfterAdd); } } -static bool HasUnusedDeclaration(std::vector const &lines, - std::string const &functionName) { - bool declared = false; - for (const std::string &line : lines) { - if (line.find("declare") != std::string::npos && - line.find(functionName) != std::string::npos) { - declared = true; +static bool hasUnusedDeclaration(std::vector const &Lines, + std::string const &FunctionName) { + bool Declared = false; + for (const std::string &Line : Lines) { + if (Line.find("declare") != std::string::npos && + Line.find(FunctionName) != std::string::npos) { + Declared = true; } - if (line.find("call") != std::string::npos && - line.find(functionName) != std::string::npos) { + if (Line.find("call") != std::string::npos && + Line.find(FunctionName) != std::string::npos) { return false; } } - return declared; + return Declared; } -static bool HasDeclaration(const std::string &disassembly, - const std::string &functionName) { - for (const std::string &line : Tokenize(disassembly, "\n")) { - if (line.find("declare") != std::string::npos && - line.find(functionName) != std::string::npos) { +static bool hasDeclaration(const std::string &Disassembly, + const std::string &FunctionName) { + for (const std::string &Line : Tokenize(Disassembly, "\n")) { + if (Line.find("declare") != std::string::npos && + Line.find(FunctionName) != std::string::npos) { return true; } } return false; } -static std::string FindDeclarationLine(const std::string &disassembly, - const std::string &functionName) { - for (const std::string &line : Tokenize(disassembly, "\n")) { - if (line.find("declare") != std::string::npos && - line.find(functionName) != std::string::npos) { - return line; +static std::string findDeclarationLine(const std::string &Disassembly, + const std::string &FunctionName) { + for (const std::string &Line : Tokenize(Disassembly, "\n")) { + if (Line.find("declare") != std::string::npos && + Line.find(FunctionName) != std::string::npos) { + return Line; } } return {}; } -static bool HasDeclarationLine(const std::string &disassembly, - const std::string &declaration) { - for (const std::string &line : Tokenize(disassembly, "\n")) { - if (line == declaration) { +static bool hasDeclarationLine(const std::string &Disassembly, + const std::string &Declaration) { + for (const std::string &Line : Tokenize(Disassembly, "\n")) { + if (Line == Declaration) { return true; } } @@ -3552,90 +3554,90 @@ static bool HasDeclarationLine(const std::string &disassembly, } TEST_F(PixTest, ConstantColor_UnusedIntOverloadIsErased) { - const char *source = R"x( + const char *Source = R"x( float4 main() : SV_Target { return float4(1, 2, 3, 4); })x"; - CComPtr compiled = - Compile(m_dllSupport, source, L"ps_6_0", {L"-Od"}); - SinglePassOutput output = - RunSinglePass(compiled, L"-hlsl-dxil-constantColor"); + CComPtr Compiled = + Compile(m_dllSupport, Source, L"ps_6_0", {L"-Od"}); + SinglePassOutput Output = + runSinglePass(Compiled, L"-hlsl-dxil-constantColor"); - VERIFY_IS_FALSE(HasUnusedDeclaration(output.Lines, "dx.op.storeOutput.i32")); - VerifyInstrumentedModuleIsValid(output.Module, + VERIFY_IS_FALSE(hasUnusedDeclaration(Output.Lines, "dx.op.storeOutput.i32")); + verifyInstrumentedModuleIsValid(Output.Module, "constant-colour substitution"); } TEST_F(PixTest, ConstantColor_NoTargetOverloadsAreErased) { - const char *source = R"x( + const char *Source = R"x( [numthreads(1, 1, 1)] void main() { })x"; - CComPtr compiled = - Compile(m_dllSupport, source, L"cs_6_0", {L"-Od"}); - SinglePassOutput output = - RunSinglePass(compiled, L"-hlsl-dxil-constantColor"); - const std::string disassembly = Disassemble(output.Module); + CComPtr Compiled = + Compile(m_dllSupport, Source, L"cs_6_0", {L"-Od"}); + SinglePassOutput Output = + runSinglePass(Compiled, L"-hlsl-dxil-constantColor"); + const std::string Disassembly = Disassemble(Output.Module); - VerifyInstrumentedModuleIsValid( - output.Module, "constant-colour substitution with no target"); - VERIFY_IS_FALSE(HasDeclaration(disassembly, "dx.op.storeOutput.f32")); - VERIFY_IS_FALSE(HasDeclaration(disassembly, "dx.op.storeOutput.i32")); + verifyInstrumentedModuleIsValid( + Output.Module, "constant-colour substitution with no target"); + VERIFY_IS_FALSE(hasDeclaration(Disassembly, "dx.op.storeOutput.f32")); + VERIFY_IS_FALSE(hasDeclaration(Disassembly, "dx.op.storeOutput.i32")); } TEST_F(PixTest, RemoveDiscards_UnusedDiscardOverloadIsErased) { - const char *source = R"x( + const char *Source = R"x( float4 main() : SV_Target { return float4(1, 2, 3, 4); })x"; - CComPtr compiled = - Compile(m_dllSupport, source, L"ps_6_0", {L"-Od"}); - SinglePassOutput output = - RunSinglePass(compiled, L"-hlsl-dxil-remove-discards"); + CComPtr Compiled = + Compile(m_dllSupport, Source, L"ps_6_0", {L"-Od"}); + SinglePassOutput Output = + runSinglePass(Compiled, L"-hlsl-dxil-remove-discards"); - VERIFY_IS_FALSE(HasUnusedDeclaration(output.Lines, "dx.op.discard")); - VerifyInstrumentedModuleIsValid(output.Module, + VERIFY_IS_FALSE(hasUnusedDeclaration(Output.Lines, "dx.op.discard")); + verifyInstrumentedModuleIsValid(Output.Module, "discard removal with no discard"); } TEST_F(PixTest, OperationCacheCleanup_RemovesErasedFunctions) { - const char *source = R"x( + const char *Source = R"x( float4 main() : SV_Target { return float4(1, 2, 3, 4); })x"; - CComPtr compiled = Compile(m_dllSupport, source, L"ps_6_0", {}); - ModuleAndHangersOn moduleEtc(compiled); - DxilModule &DM = moduleEtc.GetDxilModule(); + CComPtr Compiled = Compile(m_dllSupport, Source, L"ps_6_0", {}); + ModuleAndHangersOn ModuleEtc(Compiled); + DxilModule &DM = ModuleEtc.GetDxilModule(); OP *HlslOP = DM.GetOP(); - llvm::Function *discard = + llvm::Function *Discard = HlslOP->GetOpFunc(DXIL::OpCode::Discard, llvm::Type::getVoidTy(DM.GetModule()->getContext())); VERIFY_ARE_EQUAL(1u, static_cast( HlslOP->GetOpFuncList(DXIL::OpCode::Discard).size())); - PIXPassHelpers::EraseIfUnused(DM, discard); + PIXPassHelpers::eraseIfUnused(DM, Discard); VERIFY_ARE_EQUAL(0u, static_cast( HlslOP->GetOpFuncList(DXIL::OpCode::Discard).size())); - llvm::Function *recreated = + llvm::Function *Recreated = HlslOP->GetOpFunc(DXIL::OpCode::Discard, llvm::Type::getVoidTy(DM.GetModule()->getContext())); - VERIFY_IS_NOT_NULL(recreated); - PIXPassHelpers::EraseIfUnused(DM, recreated); + VERIFY_IS_NOT_NULL(Recreated); + PIXPassHelpers::eraseIfUnused(DM, Recreated); } TEST_F(PixTest, DynamicResourceCleanup_VisitorStopsEarly) { - const char *source = R"x( + const char *Source = R"x( Texture2D textures[] : register(t0); float4 main(float2 uv : TEXCOORD0) : SV_Target @@ -3643,18 +3645,18 @@ float4 main(float2 uv : TEXCOORD0) : SV_Target return textures[(uint)uv.x].Load(int3(0, 0, 0)); })x"; - CComPtr compiled = - Compile(m_dllSupport, source, L"ps_6_0", {L"-Od"}); - ModuleAndHangersOn moduleEtc(compiled); - DxilModule &DM = moduleEtc.GetDxilModule(); - bool visitorCalled = false; + CComPtr Compiled = + Compile(m_dllSupport, Source, L"ps_6_0", {L"-Od"}); + ModuleAndHangersOn ModuleEtc(Compiled); + DxilModule &DM = ModuleEtc.GetDxilModule(); + bool VisitorCalled = false; PIXPassHelpers::ForEachDynamicallyIndexedResource( - DM, [&visitorCalled](bool, llvm::Instruction *, llvm::Value *) { - visitorCalled = true; + DM, [&VisitorCalled](bool, llvm::Instruction *, llvm::Value *) { + VisitorCalled = true; return false; }); - VERIFY_IS_TRUE(visitorCalled); + VERIFY_IS_TRUE(VisitorCalled); OP *HlslOP = DM.GetOP(); VERIFY_ARE_EQUAL( 0u, @@ -3759,9 +3761,9 @@ void PixTest::TestNuriCase(const char *source, const wchar_t *target, Compile(m_dllSupport, source, target, compilationOptions); std::string outputText; - PassOutput output = + PassOutput Output = RunDxilNonUniformResourceIndexInstrumentation(compiledLib, outputText); - const std::vector &dxilLines = output.lines; + const std::vector &dxilLines = Output.lines; VERIFY_ARE_EQUAL(NuriGetWaveInstructionCount(dxilLines), expectedResult); @@ -3812,7 +3814,7 @@ TEST_F(PixTest, NonUniformResourceIndex_QualifiedCleanupValidates) { return; } - const char *source = R"x( + const char *Source = R"x( Texture2D textures[] : register(t0); float4 main(float2 uv : TEXCOORD0) : SV_Target @@ -3821,18 +3823,18 @@ float4 main(float2 uv : TEXCOORD0) : SV_Target return textures[NonUniformResourceIndex(index)].Load(int3(0, 0, 0)); })x"; - CComPtr compiled = - Compile(m_dllSupport, source, L"ps_6_6", {L"-Od"}); - std::string outputText; - PassOutput output = - RunDxilNonUniformResourceIndexInstrumentation(compiled, outputText); - const std::string disassembly = Disassemble(output.blob); + CComPtr Compiled = + Compile(m_dllSupport, Source, L"ps_6_6", {L"-Od"}); + std::string OutputText; + PassOutput Output = + RunDxilNonUniformResourceIndexInstrumentation(Compiled, OutputText); + const std::string Disassembly = Disassemble(Output.blob); - VerifyInstrumentedModuleIsValid( - output.blob, "qualified non-uniform resource index instrumentation"); - VERIFY_ARE_EQUAL(0u, NuriGetWaveInstructionCount(output.lines)); - VERIFY_IS_FALSE(HasDeclaration(disassembly, "dx.op.waveActiveAllEqual.i32")); - VERIFY_IS_FALSE(HasDeclaration(disassembly, "dx.op.atomicBinOp.i32")); + verifyInstrumentedModuleIsValid( + Output.blob, "qualified non-uniform resource index instrumentation"); + VERIFY_ARE_EQUAL(0u, NuriGetWaveInstructionCount(Output.lines)); + VERIFY_IS_FALSE(hasDeclaration(Disassembly, "dx.op.waveActiveAllEqual.i32")); + VERIFY_IS_FALSE(hasDeclaration(Disassembly, "dx.op.atomicBinOp.i32")); } TEST_F(PixTest, NonUniformResourceIndex_DescriptorHeap) { @@ -4247,7 +4249,7 @@ void main() { foundDebugBreak = true; } VERIFY_IS_FALSE(foundDebugBreak); - VerifyInstrumentedModuleIsValid(output.blob, + verifyInstrumentedModuleIsValid(output.blob, "debug-break instrumentation with no call"); } From ad562073cd114bde22319bca376afea2b096742f Mon Sep 17 00:00:00 2001 From: Damyan Pepper Date: Thu, 3 Sep 2026 11:27:25 -0700 Subject: [PATCH 5/5] [PIX] Name the root signature parameter Apply the explicitly approved naming exception to ExtendRootSig's existing parameter without changing any other pre-existing identifier. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- lib/DxilPIXPasses/PixPassHelpers.cpp | 34 ++++++++++++++-------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/DxilPIXPasses/PixPassHelpers.cpp b/lib/DxilPIXPasses/PixPassHelpers.cpp index 2c572e4cf4..c16d8a2fa3 100644 --- a/lib/DxilPIXPasses/PixPassHelpers.cpp +++ b/lib/DxilPIXPasses/PixPassHelpers.cpp @@ -200,36 +200,36 @@ constexpr uint32_t toolsRegisterSpace = static_cast(-2); // Returns whether a parameter was appended. template -bool ExtendRootSig(RootSigDesc &rootSigDesc, uint32_t ToolsUAVRegister) { - auto *existingParams = rootSigDesc.pParameters; - for (uint32_t i = 0; i < rootSigDesc.NumParameters; ++i) { - if (rootSigDesc.pParameters[i].ParameterType == +bool ExtendRootSig(RootSigDesc &RootSignatureDesc, uint32_t ToolsUAVRegister) { + auto *existingParams = RootSignatureDesc.pParameters; + for (uint32_t i = 0; i < RootSignatureDesc.NumParameters; ++i) { + if (RootSignatureDesc.pParameters[i].ParameterType == DxilRootParameterType::UAV) { - if (rootSigDesc.pParameters[i].Descriptor.RegisterSpace == + if (RootSignatureDesc.pParameters[i].Descriptor.RegisterSpace == toolsRegisterSpace && - rootSigDesc.pParameters[i].Descriptor.ShaderRegister == + RootSignatureDesc.pParameters[i].Descriptor.ShaderRegister == ToolsUAVRegister) { // Already added return false; } } } - auto *newParams = new RootParameterDesc[rootSigDesc.NumParameters + 1]; + auto *newParams = new RootParameterDesc[RootSignatureDesc.NumParameters + 1]; if (existingParams != nullptr) { memcpy(newParams, existingParams, - rootSigDesc.NumParameters * sizeof(RootParameterDesc)); + RootSignatureDesc.NumParameters * sizeof(RootParameterDesc)); delete[] existingParams; } - rootSigDesc.pParameters = newParams; - rootSigDesc.pParameters[rootSigDesc.NumParameters].ParameterType = + RootSignatureDesc.pParameters = newParams; + RootSignatureDesc.pParameters[RootSignatureDesc.NumParameters].ParameterType = DxilRootParameterType::UAV; - rootSigDesc.pParameters[rootSigDesc.NumParameters].Descriptor.RegisterSpace = - toolsRegisterSpace; - rootSigDesc.pParameters[rootSigDesc.NumParameters].Descriptor.ShaderRegister = - ToolsUAVRegister; - rootSigDesc.pParameters[rootSigDesc.NumParameters].ShaderVisibility = - DxilShaderVisibility::All; - rootSigDesc.NumParameters++; + RootSignatureDesc.pParameters[RootSignatureDesc.NumParameters] + .Descriptor.RegisterSpace = toolsRegisterSpace; + RootSignatureDesc.pParameters[RootSignatureDesc.NumParameters] + .Descriptor.ShaderRegister = ToolsUAVRegister; + RootSignatureDesc.pParameters[RootSignatureDesc.NumParameters] + .ShaderVisibility = DxilShaderVisibility::All; + RootSignatureDesc.NumParameters++; return true; }