From 2a293ba64197f20e3186ac3950ab40374b0986ab Mon Sep 17 00:00:00 2001 From: Damyan Pepper Date: Thu, 1 Oct 2026 14:52:17 -0700 Subject: [PATCH 1/2] [PIX] Recompute shader flags after NURI instrumentation The non-uniform resource index pass inserts WaveActiveAllEqual after the shader flags were computed, so the module could contain wave operations it does not declare and fail validation. Recompute the flags when the pass changes the module. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 40dc9de3-617e-4caf-ab0d-fba0a033ed93 --- ...NonUniformResourceIndexInstrumentation.cpp | 2 + tools/clang/unittests/HLSL/PixTest.cpp | 69 +++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/lib/DxilPIXPasses/DxilNonUniformResourceIndexInstrumentation.cpp b/lib/DxilPIXPasses/DxilNonUniformResourceIndexInstrumentation.cpp index 65be5e44b5..ea35002614 100644 --- a/lib/DxilPIXPasses/DxilNonUniformResourceIndexInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilNonUniformResourceIndexInstrumentation.cpp @@ -160,6 +160,8 @@ bool DxilNonUniformResourceIndexInstrumentation::runOnModule(Module &M) { modified |= PIXPassHelpers::eraseIfUnused(DM, AtomicOpFunc); if (modified) { + // Recompute shader flags after inserting WaveActiveAllEqual so the + // declared flags match the module. DM.CollectShaderFlagsForModule(); DM.ReEmitDxilResources(); diff --git a/tools/clang/unittests/HLSL/PixTest.cpp b/tools/clang/unittests/HLSL/PixTest.cpp index eb466f41ec..83585e4991 100644 --- a/tools/clang/unittests/HLSL/PixTest.cpp +++ b/tools/clang/unittests/HLSL/PixTest.cpp @@ -190,6 +190,8 @@ class PixTest : public ::testing::Test { TEST_METHOD(Validation_ControlInvalidModuleFails) TEST_METHOD(Validation_ControlNonPixUnusedMetadataIsRejected) TEST_METHOD(Validation_ControlInvalidPixMetadataIsRejected) + TEST_METHOD(Validation_ControlBoilerplateOnlyFailureIsRejected) + TEST_METHOD(Validation_NonUniformResourceIndex_WaveOpsFlag) dxc::DxCompilerDllLoader m_dllSupport; VersionSupportInfo m_ver; @@ -4625,6 +4627,13 @@ float main() : SV_Target SinglePassOutput Output = runSinglePass(Compiled, L"-dxil-annotate-with-virtual-regs"); + // Confirm the baseline validates before corrupting it, so the failure + // below is caused by the corruption and nothing else. + verifyInstrumentedModuleIsValid( + Output.Module, + "virtual-register annotation of a trivial pixel shader, uncorrupted " + "baseline (validation harness control)"); + // Mislabel the shader stage, so the container carries both the // harness's permitted PIX metadata and a real defect. std::string Disassembly = Disassemble(Output.Module); @@ -4768,3 +4777,63 @@ float main() : SV_Target VERIFY_IS_TRUE(AddedMalformedMetadata); VERIFY_IS_FALSE(validateInstrumentedModule(WithMalformedMetadata).Valid); } + +TEST_F(PixTest, Validation_ControlBoilerplateOnlyFailureIsRejected) { + const std::string boilerplateOnly = + getSignificantValidationDiagnostics("Validation failed.\n"); + VERIFY_IS_TRUE(boilerplateOnly.empty()); + + const std::string realDiagnostic = + getSignificantValidationDiagnostics("Validation failed.\n" + "Some real validator diagnostic.\n"); + VERIFY_IS_FALSE(realDiagnostic.empty()); + VERIFY_IS_TRUE(realDiagnostic.find("Some real validator diagnostic.") != + std::string::npos); +} + +TEST_F(PixTest, Validation_NonUniformResourceIndex_WaveOpsFlag) { + if (m_ver.SkipDxilVersion(1, 6)) + return; + + const char *source = R"x( +Texture2D textures[] : register(t0); +SamplerState samp : register(s0); + +cbuffer Constants : register(b0) +{ + uint index; +}; + +float4 main(float4 pos : SV_Position) : SV_Target +{ + return textures[index].Sample(samp, pos.xy); +})x"; + + // This index is dynamic and unmarked, so the pass instruments it; an + // index already marked NonUniformResourceIndex would be skipped. + // Instrumentation inserts WaveActiveAllEqual, which requires the WaveOps + // shader flag. + CComPtr compiled = + Compile(m_dllSupport, source, L"ps_6_6", {L"-Od"}); + CComPtr dxil = FindModule(DFCC_ShaderDebugInfoDXIL, compiled); + + CComPtr pOptimizer; + VERIFY_SUCCEEDED( + m_dllSupport.CreateInstance(CLSID_DxcOptimizer, &pOptimizer)); + std::array Options = { + L"-opt-mod-passes", L"-dxil-dbg-value-to-dbg-declare", + L"-dxil-annotate-with-virtual-regs", + L"-hlsl-dxil-non-uniform-resource-index-instrumentation"}; + + CComPtr pOptimizedModule; + CComPtr pText; + VERIFY_SUCCEEDED(pOptimizer->RunOptimizer( + dxil, Options.data(), Options.size(), &pOptimizedModule, &pText)); + + verifyInstrumentedModuleIsValid(pOptimizedModule, + "non-uniform resource index instrumentation"); + + VERIFY_ARE_NOT_EQUAL( + std::string::npos, + Disassemble(pOptimizedModule).find("dx.op.waveActiveAllEqual")); +} From 810a923bf472fc74eb03da64b77218ab16d57afd Mon Sep 17 00:00:00 2001 From: Damyan Pepper Date: Thu, 1 Oct 2026 14:57:14 -0700 Subject: [PATCH 2/2] [PIX] Record only real resource accesses, including samplers Shader access tracking no longer records annotateHandle or barrierByMemoryHandle as accesses, still looks through annotated handles to the resource, and matches library handles against samplers so sampler accesses are recorded. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 40dc9de3-617e-4caf-ab0d-fba0a033ed93 --- .../DxilShaderAccessTracking.cpp | 20 +++++ .../AccessTrackingBarrierIsNotAnAccess.hlsl | 27 +++++++ ...rackingLibAnnotateHandleIsNotAnAccess.hlsl | 30 ++++++++ ...ngLibAnnotatedHandleReadStillRecorded.hlsl | 33 ++++++++ tools/clang/unittests/HLSL/PixTest.cpp | 76 ++++++++++++++++++- 5 files changed, 182 insertions(+), 4 deletions(-) create mode 100644 tools/clang/test/HLSLFileCheck/pix/AccessTrackingBarrierIsNotAnAccess.hlsl create mode 100644 tools/clang/test/HLSLFileCheck/pix/AccessTrackingLibAnnotateHandleIsNotAnAccess.hlsl create mode 100644 tools/clang/test/HLSLFileCheck/pix/AccessTrackingLibAnnotatedHandleReadStillRecorded.hlsl diff --git a/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp b/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp index ed1d1b26cc..0774befdf4 100644 --- a/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp +++ b/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp @@ -659,6 +659,17 @@ DxilResourceAndClass DxilShaderAccessTracking::DetermineAccessForHandleForLib( } } } + if (ret.registerType == RegisterType::Invalid) { + auto const &Samplers = DM.GetSamplers(); + for (auto &Sampler : Samplers) { + if (global == Sampler->GetGlobalSymbol()) { + binding = + hlsl::resource_helper::loadBindingFromResourceBase(Sampler.get()); + ret.registerType = RegisterType::Sampler; + break; + } + } + } if (ret.registerType != RegisterType::Invalid) { ret.accessStyle = AccessStyle::FromRootSig; ret.RegisterID = binding.rangeLowerBound; @@ -898,6 +909,15 @@ bool DxilShaderAccessTracking::runOnModule(Module &M) { // Special cases switch (opCode) { + case DXIL::OpCode::AnnotateHandle: + // annotateHandle attaches type information. It is not a resource + // access. GetResourceFromHandle still walks through it when a + // later access uses the annotated handle. + continue; + case DXIL::OpCode::BarrierByMemoryHandle: + // A barrier orders accesses to a resource. It is not itself an + // access. + continue; case DXIL::OpCode::GetDimensions: // readWrite = ShaderAccessFlags::DescriptorRead; // TODO: Support // GetDimensions diff --git a/tools/clang/test/HLSLFileCheck/pix/AccessTrackingBarrierIsNotAnAccess.hlsl b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingBarrierIsNotAnAccess.hlsl new file mode 100644 index 0000000000..73da834b3e --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingBarrierIsNotAnAccess.hlsl @@ -0,0 +1,27 @@ +// RUN: %dxc -T cs_6_8 -E main -Od %s | %opt -S -hlsl-dxil-pix-shader-access-instrumentation,config=U0:0:2i0;.0;256;512. | %FileCheck %s + +// Barrier() on a resource handle orders accesses to that resource. It is +// not itself an access. +// +// The config puts the UAVs of space 0 at slot 0 onwards, so g_out is slot +// 0 and g_rw is slot 1. A slot is three dwords, so g_out's write dword is +// at byte 4 and g_rw's write dword is at byte 16. + +// g_rw is only barriered, never accessed, so nothing is recorded against +// it. +// CHECK-NOT: bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 16, + +// The store to g_out is a genuine write and is recorded. +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 4, + +// CHECK-NOT: bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 16, + +RWByteAddressBuffer g_out : register(u0); +RWTexture2D g_rw : register(u1); + +[numthreads(1, 1, 1)] +void main(uint index : SV_GroupIndex) +{ + Barrier(g_rw, DEVICE_SCOPE); + g_out.Store(0, 1); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/AccessTrackingLibAnnotateHandleIsNotAnAccess.hlsl b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingLibAnnotateHandleIsNotAnAccess.hlsl new file mode 100644 index 0000000000..d81de35182 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingLibAnnotateHandleIsNotAnAccess.hlsl @@ -0,0 +1,30 @@ +// RUN: %dxc -T lib_6_6 -Od %s | %opt -S -hlsl-dxil-pix-shader-access-instrumentation,config=S0:1:1i0;U0:2:1i0;.0;0;0. | %FileCheck %s + +// annotateHandle attaches type information to a handle. It is not a +// memory operation. g_untouched is only passed to GetDimensions, which +// this pass skips, so the annotation is that resource's only handle use. +// Nothing is recorded against it. +// +// The config puts the SRV of space 0 at slot 1 and the UAV of space 0 at +// slot 2. A slot is three dwords, so g_untouched's read dword is at byte +// 12 and its write dword at byte 16. g_output's write dword is at byte 28. + +// CHECK-NOT: bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 12, +// CHECK-NOT: bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 16, + +// The store to g_output is a genuine access and is recorded: +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 28, + +// CHECK-NOT: bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 12, +// CHECK-NOT: bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 16, + +Texture2D g_untouched : register(t0); +RWByteAddressBuffer g_output : register(u0); + +[shader("raygeneration")] +void RayGen() +{ + uint width, height; + g_untouched.GetDimensions(width, height); + g_output.Store(0, width + height); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/AccessTrackingLibAnnotatedHandleReadStillRecorded.hlsl b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingLibAnnotatedHandleReadStillRecorded.hlsl new file mode 100644 index 0000000000..b14588c0a4 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingLibAnnotatedHandleReadStillRecorded.hlsl @@ -0,0 +1,33 @@ +// RUN: %dxc -T lib_6_6 -Od %s | %opt -S -hlsl-dxil-pix-shader-access-instrumentation,config=S0:1:1i0;U0:2:1i0;.256;512;1024. | %FileCheck %s + +// annotateHandle is not an access, but a genuine access that uses an +// annotated handle still records the resource class from that annotation. +// +// Offsets with this config (SRV space 0 at slot 1, UAV space 0 at slot 2, +// three dwords per slot, descriptor-heap records at byte 256): +// g_input read slot 1, read dword -> 12 +// g_input write slot 1, write dword -> 16 (must not appear) +// g_output write slot 2, write dword -> 28 +// heapTexture read descriptor 3 -> 292 +// +// A descriptor-heap record encodes shader kind in its top four bits and +// ResourceAccessStyle in the next four. RayGeneration is 7 and SRVRead is +// 5, so 0x75000000 == 1962934272. + +// CHECK-NOT: bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 16, +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 12, +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 292, i32 undef, i32 1962934272, +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 28, +// CHECK-NOT: bufferStore.i32(i32 69, %dx.types.Handle {{.*}}, i32 16, + +ByteAddressBuffer g_input : register(t0); +RWByteAddressBuffer g_output : register(u0); + +[shader("raygeneration")] +void RayGen() +{ + Texture2D heapTexture = ResourceDescriptorHeap[3]; + uint value = g_input.Load(0); + value += asuint(heapTexture.Load(int3(0, 0, 0)).x); + g_output.Store(0, value); +} diff --git a/tools/clang/unittests/HLSL/PixTest.cpp b/tools/clang/unittests/HLSL/PixTest.cpp index 83585e4991..c29803bb18 100644 --- a/tools/clang/unittests/HLSL/PixTest.cpp +++ b/tools/clang/unittests/HLSL/PixTest.cpp @@ -123,6 +123,7 @@ class PixTest : public ::testing::Test { TEST_METHOD(AccessTracking_ModificationReport_Read) TEST_METHOD(AccessTracking_ModificationReport_Write) TEST_METHOD(AccessTracking_ModificationReport_SM66) + TEST_METHOD(AccessTracking_SamplerAccessInLibrary) TEST_METHOD(PixStructAnnotation_Lib_DualRaygen) @@ -192,6 +193,7 @@ class PixTest : public ::testing::Test { TEST_METHOD(Validation_ControlInvalidPixMetadataIsRejected) TEST_METHOD(Validation_ControlBoilerplateOnlyFailureIsRejected) TEST_METHOD(Validation_NonUniformResourceIndex_WaveOpsFlag) + TEST_METHOD(Validation_ShaderAccessTracking_DynamicallyIndexedResource) dxc::DxCompilerDllLoader m_dllSupport; VersionSupportInfo m_ver; @@ -753,7 +755,8 @@ class PixTest : public ::testing::Test { const wchar_t *profile = L"as_6_5"); void ValidateAllocaWrite(std::vector const &allocaWrites, size_t index, const char *name); - PassOutput RunShaderAccessTrackingPass(IDxcBlob *blob); + PassOutput RunShaderAccessTrackingPass( + IDxcBlob *blob, const wchar_t *config = L"U0:0:10i0;U0:1:2i0;.0;0;0."); CComPtr RunDxilPIXAddTidToAmplificationShaderPayloadPass(IDxcBlob *blob); CComPtr RunDxilPIXMeshShaderOutputPass(IDxcBlob *blob); @@ -1038,14 +1041,17 @@ TEST_F(PixTest, CompileDebugDisasmPDB) { VERIFY_SUCCEEDED(pCompiler->Disassemble(pPdbBlob, &pDisasm)); } -PassOutput PixTest::RunShaderAccessTrackingPass(IDxcBlob *blob) { +PassOutput PixTest::RunShaderAccessTrackingPass(IDxcBlob *blob, + const wchar_t *config) { CComPtr pOptimizer; VERIFY_SUCCEEDED( m_dllSupport.CreateInstance(CLSID_DxcOptimizer, &pOptimizer)); std::vector Options; Options.push_back(L"-opt-mod-passes"); - Options.push_back(L"-hlsl-dxil-pix-shader-access-instrumentation,config=U0:0:" - L"10i0;U0:1:2i0;.0;0;0."); + std::wstring passOption = + L"-hlsl-dxil-pix-shader-access-instrumentation,config="; + passOption += config; + Options.push_back(passOption.c_str()); CComPtr pOptimizedModule; CComPtr pText; @@ -1404,6 +1410,47 @@ float main() : SV_Target ValidateAccessTrackingMods(hlsl, true); } +std::vector Split(std::string str, char delimeter); + +static bool HasBufferStoreWithByteOffset(std::vector const &lines, + unsigned byteOffset) { + std::string needle = "i32 " + std::to_string(byteOffset); + for (auto const &line : lines) { + if (line.find("dx.op.bufferStore") != std::string::npos && + line.find(needle) != std::string::npos) { + return true; + } + } + return false; +} + +TEST_F(PixTest, AccessTracking_SamplerAccessInLibrary) { + if (m_ver.SkipDxilVersion(1, 6)) { + return; + } + + const char *hlsl = R"( +Texture2D g_texture : register(t0); +SamplerState g_sampler : register(s2); +RWByteAddressBuffer g_output : register(u0); + +[shader("raygeneration")] +void RayGen() +{ + float4 value = g_texture.SampleLevel(g_sampler, float2(0, 0), 0); + g_output.Store(0, asuint(value.x)); +} +)"; + + auto compiled = Compile(m_dllSupport, hlsl, L"lib_6_6", {L"-Od"}); + auto output = RunShaderAccessTrackingPass( + compiled, L"S0:0:4i0;M0:20:4i0;U0:40:4i0;.0;0;0."); + auto lines = Split(Disassemble(output.blob), '\n'); + VERIFY_IS_TRUE(HasBufferStoreWithByteOffset(lines, 264)); + verifyInstrumentedModuleIsValid( + output.blob, "shader access tracking of a library sampler access"); +} + TEST_F(PixTest, AddToASGroupSharedPayload) { const char *hlsl = R"( @@ -4837,3 +4884,24 @@ float4 main(float4 pos : SV_Position) : SV_Target std::string::npos, Disassemble(pOptimizedModule).find("dx.op.waveActiveAllEqual")); } + +TEST_F(PixTest, Validation_ShaderAccessTracking_DynamicallyIndexedResource) { + const char *source = R"x( +Texture2D textures[8] : register(t0); +SamplerState samp : register(s0); + +cbuffer Constants : register(b0) +{ + uint index; +}; + +float4 main(float4 pos : SV_Position) : SV_Target +{ + return textures[index].Sample(samp, pos.xy); +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {L"-Od"}); + auto output = RunShaderAccessTrackingPass(compiled); + verifyInstrumentedModuleIsValid( + output.blob, "shader access tracking of a dynamically indexed resource"); +}