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;