From 60e6902b39d43619cbbd85fff4ae949c5a138114 Mon Sep 17 00:00:00 2001 From: Damyan Pepper Date: Mon, 24 Aug 2026 18:54:24 -0700 Subject: [PATCH] [PIX] Fix dynamic bind-point identity and range bounds RSRegisterIdentifier::operator< combines its three fields with &&. This is not a strict weak ordering. Two distinct ranges can compare as neither less than the other, so the map treats them as one key and gives their slot assignments to one of them. Accesses to one range are then reported against the other. RegisterID comes from the position of the resource in the module's resource list. PIX needs the register that the binding occupies. The range check for a constant index is off by one, so the pass accepts an index that addresses the slot after the range. An index outside the range is still recorded at slot zero. The meaning of RegisterID changes. A tool that resolved the old value against the module's resource list must resolve the new value against the root signature. Assisted-by: Copilot Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 40dc9de3-617e-4caf-ab0d-fba0a033ed93 --- .../DxilShaderAccessTracking.cpp | 12 ++- tools/clang/unittests/HLSL/PixTest.cpp | 83 +++++++++++++++++++ 2 files changed, 91 insertions(+), 4 deletions(-) diff --git a/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp b/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp index 8c5317d946..f6ce712b5d 100644 --- a/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp +++ b/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp @@ -174,8 +174,9 @@ struct RSRegisterIdentifier { unsigned Index; bool operator<(const RSRegisterIdentifier &o) const { - return static_cast(Type) < static_cast(o.Type) && - Space < o.Space && Index < o.Index; + return static_cast(Type) < static_cast(o.Type) || + (Type == o.Type && + (Space < o.Space || (Space == o.Space && Index < o.Index))); } }; @@ -428,7 +429,8 @@ bool DxilShaderAccessTracking::EmitResourceAccess(DxilModule &DM, if (isa(res.index) && res.indexDynamicOffset == nullptr) { unsigned index = cast(res.index)->getLimitedValue(); - if (index > slot->second.numSlots) { + // Index is 0-based, so numSlots is the first out-of-range value. + if (index >= slot->second.numSlots) { // out-of-range accesses are written to slot zero: slotIndex = HlslOP->GetU32Const(0); } else { @@ -746,7 +748,8 @@ DxilShaderAccessTracking::GetResourceFromHandle(Value *resHandle, ret.index = createHandle.get_index(); ret.registerType = registerType; ret.accessStyle = AccessStyle::FromRootSig; - ret.RegisterID = resource->GetID(); + // RegisterID is the binding lower bound, not the resource-list ID. + ret.RegisterID = resource->GetLowerBound(); ret.RegisterSpace = resource->GetSpaceID(); } } @@ -768,6 +771,7 @@ DxilShaderAccessTracking::GetResourceFromHandle(Value *resHandle, ret.index = createHandleFromBinding.get_index(); ret.registerType = RegisterTypeFromResourceClass( static_cast(binding.resourceClass)); + ret.RegisterID = binding.rangeLowerBound; ret.RegisterSpace = binding.spaceID; } else if (hlsl::OP::IsDxilOpFuncCallInst( handleCreation, hlsl::OP::OpCode::CreateHandleFromHeap)) { diff --git a/tools/clang/unittests/HLSL/PixTest.cpp b/tools/clang/unittests/HLSL/PixTest.cpp index b6a8cb918f..57be05ff93 100644 --- a/tools/clang/unittests/HLSL/PixTest.cpp +++ b/tools/clang/unittests/HLSL/PixTest.cpp @@ -121,6 +121,9 @@ class PixTest : public ::testing::Test { TEST_METHOD(AccessTracking_ModificationReport_Read) TEST_METHOD(AccessTracking_ModificationReport_Write) TEST_METHOD(AccessTracking_ModificationReport_SM66) + TEST_METHOD(AccessTracking_MultipleDynamicRangesSameTypeAndSpace) + TEST_METHOD(AccessTracking_DynamicRangeRegisterIndex_SM66) + TEST_METHOD(AccessTracking_ConstantIndexAtRangeLimit) TEST_METHOD(AccessTracking_SamplerAccessInLibrary) TEST_METHOD(PixStructAnnotation_Lib_DualRaygen) @@ -1294,6 +1297,15 @@ float main() : SV_Target std::vector Split(std::string str, char delimeter); +static std::string JoinLines(std::vector const &lines) { + std::string joined; + for (auto const &line : lines) { + joined += line; + joined += '\n'; + } + return joined; +} + static bool HasBufferStoreWithByteOffset(std::vector const &lines, unsigned byteOffset) { std::string needle = "i32 " + std::to_string(byteOffset); @@ -1306,6 +1318,77 @@ static bool HasBufferStoreWithByteOffset(std::vector const &lines, return false; } +TEST_F(PixTest, AccessTracking_MultipleDynamicRangesSameTypeAndSpace) { + const char *hlsl = R"( +ByteAddressBuffer g_indices : register(t0); +RWByteAddressBuffer g_firstRange[2] : register(u4); +RWByteAddressBuffer g_secondRange[2] : register(u6); + +[numthreads(1, 1, 1)] +void CSMain() +{ + uint index = g_indices.Load(0); + g_firstRange[index].Store(0, 1); + g_secondRange[index].Store(0, 2); +} +)"; + + auto compiled = Compile(m_dllSupport, hlsl, L"cs_6_0", {L"-Od"}, L"CSMain"); + auto output = + RunShaderAccessTrackingPass(compiled, L"S0:0:2i0;U0:0:10i0;.0;0;0."); + auto text = JoinLines(output.lines); + VERIFY_IS_TRUE(text.find("U0:4;") != std::string::npos); + VERIFY_IS_TRUE(text.find("U0:6;") != std::string::npos); + VerifyInstrumentedModuleIsValid(output.blob, + "shader access tracking of two dynamic UAV " + "ranges in the same register space"); +} + +TEST_F(PixTest, AccessTracking_DynamicRangeRegisterIndex_SM66) { + if (m_ver.SkipDxilVersion(1, 6)) { + return; + } + + const char *hlsl = R"( +RWByteAddressBuffer g_buffers[] : register(u5); + +[numthreads(1, 1, 1)] +void CSMain(uint3 dispatchThreadId : SV_DispatchThreadID) +{ + g_buffers[dispatchThreadId.x].Store(0, 1); +} +)"; + + auto compiled = Compile(m_dllSupport, hlsl, L"cs_6_6", {L"-Od"}, L"CSMain"); + auto output = RunShaderAccessTrackingPass(compiled, L"U0:0:10i0;.0;0;0."); + auto text = JoinLines(output.lines); + VERIFY_IS_TRUE(text.find("U0:5;") != std::string::npos); + VERIFY_IS_TRUE(text.find("U0:0;") == std::string::npos); + VerifyInstrumentedModuleIsValid( + output.blob, "shader access tracking of an SM 6.6 dynamic UAV range"); +} + +TEST_F(PixTest, AccessTracking_ConstantIndexAtRangeLimit) { + const char *hlsl = R"( +RWByteAddressBuffer g_buffers[] : register(u0); + +[numthreads(1, 1, 1)] +void CSMain() +{ + g_buffers[1].Store(0, 1); +} +)"; + + auto compiled = Compile(m_dllSupport, hlsl, L"cs_6_0", {L"-Od"}, L"CSMain"); + auto output = RunShaderAccessTrackingPass(compiled, L"U0:0:1i0;.0;0;0."); + auto lines = Split(Disassemble(output.blob), '\n'); + VERIFY_IS_TRUE(HasBufferStoreWithByteOffset(lines, 4)); + VERIFY_IS_TRUE(!HasBufferStoreWithByteOffset(lines, 16)); + VerifyInstrumentedModuleIsValid( + output.blob, + "shader access tracking of a constant index at the range limit"); +} + TEST_F(PixTest, AccessTracking_SamplerAccessInLibrary) { if (m_ver.SkipDxilVersion(1, 6)) { return;