From 332c784354a8c54488ab8850cd5e8eacaf9c6588 Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Tue, 8 Oct 2024 10:15:50 -0700 Subject: [PATCH 1/4] SAT dynamic TLAS check and test --- .../DxilDebugInstrumentation.cpp | 4 +- .../DxilShaderAccessTracking.cpp | 11 ++- .../pix/AccessTrackingDXRDynamicTLAS.hlsl | 77 +++++++++++++++++++ .../test/HLSLFileCheck/pix/DebugBasic.hlsl | 2 +- 4 files changed, 89 insertions(+), 5 deletions(-) create mode 100644 tools/clang/test/HLSLFileCheck/pix/AccessTrackingDXRDynamicTLAS.hlsl diff --git a/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp b/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp index 21c1199f91..a7d7e72cb4 100644 --- a/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp @@ -795,12 +795,12 @@ void DxilDebugInstrumentation::determineLimitANDAndInitializeCounter( auto *PHIForCounterOffset = BC.Builder.CreatePHI(Type::getInt32Ty(BC.Ctx), 2, "PIXCounterLocation"); const uint32_t InterestingCounterOffset = - static_cast(m_UAVSize / 2 - 1); + static_cast(m_UAVSize / 2 - sizeof(uint32_t)); PHIForCounterOffset->addIncoming( BC.HlslOP->GetU32Const(InterestingCounterOffset), InterestingInvocationBlock); const uint32_t UninterestingCounterOffsetValue = - static_cast(m_UAVSize - 1); + static_cast(m_UAVSize - sizeof(uint32_t)); PHIForCounterOffset->addIncoming( BC.HlslOP->GetU32Const(UninterestingCounterOffsetValue), NonInterestingInvocationBlock); diff --git a/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp b/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp index 8299ff87d2..037637d609 100644 --- a/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp +++ b/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp @@ -982,8 +982,15 @@ bool DxilShaderAccessTracking::runOnModule(Module &M) { break; case DXIL::OpCode::TraceRay: // Read of AccelerationStructure; doesn't match function attribute - // readWrite = ShaderAccessFlags::Read; // TODO: Support - continue; + auto res = GetResourceFromHandle(Call->getArgOperand(1), DM); + if (res.accessStyle == AccessStyle::None) { + continue; + } + if (EmitResourceAccess(DM, res, Call, HlslOP, Ctx, + ShaderAccessFlags::Read)) { + Modified = true; + } + continue; case DXIL::OpCode::RayQuery_TraceRayInline: { // Read of AccelerationStructure; doesn't match function attribute auto res = GetResourceFromHandle(Call->getArgOperand(2), DM); diff --git a/tools/clang/test/HLSLFileCheck/pix/AccessTrackingDXRDynamicTLAS.hlsl b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingDXRDynamicTLAS.hlsl new file mode 100644 index 0000000000..4bfb4d6e8c --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingDXRDynamicTLAS.hlsl @@ -0,0 +1,77 @@ +// RUN: %dxc -T lib_6_6 %s | %opt -S -hlsl-dxil-pix-shader-access-instrumentation,config=.256;512;1024. | %FileCheck %s + +// This file is checking for the correct access tracking for a descriptor-heap-indexed TLAS for TraceRay. + +// First advance through the output text to where the handle for heap index 7 is created: +// CHECK: call %dx.types.Handle @dx.op.createHandleFromHeap(i32 218, i32 7 + +// The next buffer store should be for this resource. +// See DxilShaderAccessTracking::EmitResourceAccess for how this index is calculated. +// It's the descriptor heap index (7, as seen in the HLSL below) plus 1 (to skip +// over the "out-of-bounds" entry in the output UAV) times 8 DWORDs per record +// (the first DWORD for write, the second for read), plus 4 to offset to the "read" record. +// Read access for descriptor 7 is therefore at (7+1)*8+4 = 68. +// This is then added to the base address for dynamic writes, which is 256 +// (from the config=.256 in the command-line above), for a total of 324. + +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %2, i32 324, +// ("%2" should be stable: the PIX output UAV is always added at the start of the shader.) + +RWTexture2D RTOutput : register(u0); + +struct PayloadData +{ + uint index : INDEX; +}; + +struct AttributeData +{ + float2 barycentrics; +}; + +struct ColorConstant +{ + uint3 color; +}; + +struct AlphaConstant +{ + uint alpha; +}; + + +ConstantBuffer color : register(b0); +ConstantBuffer alpha : register(b1); + +[shader("raygeneration")] +void RayGenMain() +{ + uint2 index = DispatchRaysIndex().xy; + uint2 dim = DispatchRaysDimensions().xy; + + PayloadData payload; + payload.index = index.y * dim.x + index.x; + + RayDesc ray; + ray.Origin.x = 2.0 * (index.x + 0.5) / dim.x - 1.0; + ray.Origin.y = 1.0 - 2.0 * (index.y + 0.5) / dim.y; + ray.Origin.z = 0.0; + ray.Direction = float3(0, 0, -1); + ray.TMin = 0.01; + ray.TMax = 100.0; + + RaytracingAccelerationStructure scene = ResourceDescriptorHeap[7]; + + TraceRay( + scene, // Acceleration structure + 0, // Ray flags + 0xFF, // Instance inclusion mask + 0, // RayContributionToHitGroupIndex + 1, // MultiplierForGeometryContributionToHitGroupIndex + 0, // MissShaderIndex + ray, + payload); + + RTOutput[index] = float4(color.color.r / 255.0f, color.color.g / 255.0f, color.color.b / 255.0f, alpha.alpha / 255.0f); +} + diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugBasic.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugBasic.hlsl index 6d6cd5454d..b211e22959 100644 --- a/tools/clang/test/HLSLFileCheck/pix/DebugBasic.hlsl +++ b/tools/clang/test/HLSLFileCheck/pix/DebugBasic.hlsl @@ -16,7 +16,7 @@ // Check for branches-for-interest and AND value and counter location for a UAV size of 128 // CHECK: br i1 %ComparePos, label %PIXInterestingBlock, label %PIXNonInterestingBlock // CHECK: %PIXOffsetOr = phi i32 [ 0, %PIXInterestingBlock ], [ 64, %PIXNonInterestingBlock ] -// CHECK: %PIXCounterLocation = phi i32 [ 63, %PIXInterestingBlock ], [ 127, %PIXNonInterestingBlock ] +// CHECK: %PIXCounterLocation = phi i32 [ 60, %PIXInterestingBlock ], [ 124, %PIXNonInterestingBlock ] // Check the first block header was emitted: (increment, AND + OR) // CHECK: call i32 @dx.op.atomicBinOp.i32(i32 78, %dx.types.Handle %PIX_DebugUAV_Handle, i32 0 From 590218e1e837a5794ecab336cbf5b434bce2e2c9 Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Tue, 8 Oct 2024 10:22:23 -0700 Subject: [PATCH 2/4] clang-format --- lib/DxilPIXPasses/DxilShaderAccessTracking.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp b/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp index 037637d609..06fda75905 100644 --- a/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp +++ b/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp @@ -982,15 +982,15 @@ bool DxilShaderAccessTracking::runOnModule(Module &M) { break; case DXIL::OpCode::TraceRay: // Read of AccelerationStructure; doesn't match function attribute - auto res = GetResourceFromHandle(Call->getArgOperand(1), DM); - if (res.accessStyle == AccessStyle::None) { - continue; - } - if (EmitResourceAccess(DM, res, Call, HlslOP, Ctx, - ShaderAccessFlags::Read)) { - Modified = true; - } + auto res = GetResourceFromHandle(Call->getArgOperand(1), DM); + if (res.accessStyle == AccessStyle::None) { continue; + } + if (EmitResourceAccess(DM, res, Call, HlslOP, Ctx, + ShaderAccessFlags::Read)) { + Modified = true; + } + continue; case DXIL::OpCode::RayQuery_TraceRayInline: { // Read of AccelerationStructure; doesn't match function attribute auto res = GetResourceFromHandle(Call->getArgOperand(2), DM); From 070ad52995bbba016493621dcec6b7206431e3da Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Tue, 8 Oct 2024 10:54:31 -0700 Subject: [PATCH 3/4] *nix build fix --- lib/DxilPIXPasses/DxilShaderAccessTracking.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp b/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp index 06fda75905..03ab62c50b 100644 --- a/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp +++ b/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp @@ -980,7 +980,7 @@ bool DxilShaderAccessTracking::runOnModule(Module &M) { case DXIL::OpCode::BufferUpdateCounter: readWrite = ShaderAccessFlags::Counter; break; - case DXIL::OpCode::TraceRay: + case DXIL::OpCode::TraceRay: { // Read of AccelerationStructure; doesn't match function attribute auto res = GetResourceFromHandle(Call->getArgOperand(1), DM); if (res.accessStyle == AccessStyle::None) { @@ -990,6 +990,7 @@ bool DxilShaderAccessTracking::runOnModule(Module &M) { ShaderAccessFlags::Read)) { Modified = true; } + } continue; case DXIL::OpCode::RayQuery_TraceRayInline: { // Read of AccelerationStructure; doesn't match function attribute From af34b06fac69009be6b6aa2b677b6ba85c9d6dfa Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Wed, 9 Oct 2024 09:41:20 -0700 Subject: [PATCH 4/4] Fix Value name instability on Mac/*nix test runs --- .../test/HLSLFileCheck/pix/AccessTrackingDXRDynamicTLAS.hlsl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/clang/test/HLSLFileCheck/pix/AccessTrackingDXRDynamicTLAS.hlsl b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingDXRDynamicTLAS.hlsl index 4bfb4d6e8c..5e7a23d60e 100644 --- a/tools/clang/test/HLSLFileCheck/pix/AccessTrackingDXRDynamicTLAS.hlsl +++ b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingDXRDynamicTLAS.hlsl @@ -14,8 +14,7 @@ // This is then added to the base address for dynamic writes, which is 256 // (from the config=.256 in the command-line above), for a total of 324. -// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %2, i32 324, -// ("%2" should be stable: the PIX output UAV is always added at the start of the shader.) +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %[[UAV:[0-9+]]], i32 324, RWTexture2D RTOutput : register(u0);