From cfc5204a692a48884817e8a7a78d981e6eeea24a Mon Sep 17 00:00:00 2001 From: baldurk Date: Tue, 30 Apr 2024 15:01:16 +0100 Subject: [PATCH] Don't require dxc for compiling RT patching shaders * We do some manual uint64 emulation with uint2 which compiles on fxc, re-used from the execute indirect patching. --- renderdoc/data/hlsl/hlsl_cbuffers.h | 74 +++++++++++++++- renderdoc/data/hlsl/misc.hlsl | 67 ++------------- renderdoc/data/hlsl/raytracing.hlsl | 14 ++- renderdoc/driver/d3d12/d3d12_common.cpp | 104 +++++++++++++++++++++++ renderdoc/driver/d3d12/d3d12_manager.cpp | 2 +- renderdoc/maths/vec.h | 17 ++++ 6 files changed, 206 insertions(+), 72 deletions(-) diff --git a/renderdoc/data/hlsl/hlsl_cbuffers.h b/renderdoc/data/hlsl/hlsl_cbuffers.h index c20eb71af..196f4f49b 100644 --- a/renderdoc/data/hlsl/hlsl_cbuffers.h +++ b/renderdoc/data/hlsl/hlsl_cbuffers.h @@ -34,6 +34,7 @@ #define float2 Vec2f #define float3 Vec3f #define uint4 Vec4u +#define uint2 Vec2u #define int4 Vec4i #define float4 Vec4f #define float4x4 Matrix4f @@ -195,8 +196,76 @@ cbuffer AccStructPatchInfo REG(b0) uint addressCount; }; -#if defined(SHADER_MODEL_MIN_6_0_REQUIRED) || defined(__cplusplus) + // INCLUDE_GPUADDRESS_HELPERS should only be set for unit tests to check these functions below, + // otherwise it pollutes the interface + +#if defined(__cplusplus) && !defined(INCLUDE_GPUADDRESS_HELPERS) +// on the GPU this will be uint2 {.x = LSB, .y = MSB} to match uint64 order typedef uint64_t GPUAddress; +#else +typedef uint2 GPUAddress; +#endif + +// don't define the helpers in C++ by default, unless we're using them for unit tests +#if !defined(__cplusplus) || defined(INCLUDE_GPUADDRESS_HELPERS) + +#if defined(__cplusplus) +#define max RDCMAX +#define min RDCMIN +#endif + +bool lessThan(GPUAddress a, GPUAddress b) +{ + // either MSB is less, or MSB is equal and LSB is less-equal + return a.y < b.y || (a.y == b.y && a.x < b.x); +} + +bool lessEqual(GPUAddress a, GPUAddress b) +{ + return lessThan(a, b) || (a.y == b.y && a.x == b.x); +} + +GPUAddress add(GPUAddress a, GPUAddress b) +{ + uint msb = 0, lsb = 0; + if(b.x > 0 && a.x > 0xffffffff - b.x) + { + uint x = max(a.x, b.x) - 0x80000000; + uint y = min(a.x, b.x); + + uint sum = x + y; + + msb = a.y + b.y + 1; + lsb = sum - 0x80000000; + } + else + { + msb = a.y + b.y; + lsb = a.x + b.x; + } + + return GPUAddress(lsb, msb); +} + +GPUAddress sub(GPUAddress a, GPUAddress b) +{ + uint msb = 0, lsb = 0; + if(a.x < b.x) + { + uint diff = b.x - a.x; + + msb = a.y - b.y - 1; + lsb = 0xffffffff - (diff - 1); + } + else + { + msb = a.y - b.y; + lsb = a.x - b.x; + } + + return GPUAddress(lsb, msb); +} +#endif struct BlasAddressRange { @@ -213,10 +282,9 @@ struct BlasAddressPair // This corresponds to D3D12_RAYTRACING_INSTANCE_DESC structure struct InstanceDesc { - uint64_t padding[7]; + uint2 padding[7]; GPUAddress blasAddress; }; -#endif cbuffer DebugSampleOperation REG(b0) { diff --git a/renderdoc/data/hlsl/misc.hlsl b/renderdoc/data/hlsl/misc.hlsl index 0aea922ae..a9a71bfe5 100644 --- a/renderdoc/data/hlsl/misc.hlsl +++ b/renderdoc/data/hlsl/misc.hlsl @@ -154,77 +154,24 @@ cbuffer countbuffer : register(b2) struct buffermapping { - // {.x = LSB, .y = MSB} to match uint64 order - uint2 origBase; - uint2 origEnd; - uint2 newBase; - uint2 pad; + GPUAddress origBase; + GPUAddress origEnd; + GPUAddress newBase; + GPUAddress pad; }; StructuredBuffer buffers : register(t0); RWByteAddressBuffer arguments : register(u0); -bool uint64LessThan(uint2 a, uint2 b) -{ - // either MSB is less, or MSB is equal and LSB is less-equal - return a.y < b.y || (a.y == b.y && a.x < b.x); -} - -bool uint64LessEqual(uint2 a, uint2 b) -{ - return uint64LessThan(a, b) || (a.y == b.y && a.x == b.x); -} - -uint2 uint64Add(uint2 a, uint2 b) -{ - uint msb = 0, lsb = 0; - if(b.x > 0 && a.x > 0xffffffff - b.x) - { - uint x = max(a.x, b.x) - 0x80000000; - uint y = min(a.x, b.x); - - uint sum = x + y; - - msb = a.y + b.y + 1; - lsb = sum - 0x80000000; - } - else - { - msb = a.y + b.y; - lsb = a.x + b.x; - } - - return uint2(lsb, msb); -} - -uint2 uint64Sub(uint2 a, uint2 b) -{ - uint msb = 0, lsb = 0; - if(a.x < b.x) - { - uint diff = b.x - a.x; - - msb = a.y - b.y - 1; - lsb = 0xffffffff - (diff - 1); - } - else - { - msb = a.y - b.y; - lsb = a.x - b.x; - } - - return uint2(lsb, msb); -} - -uint2 PatchAddress(uint2 addr) +GPUAddress PatchAddress(GPUAddress addr) { for(uint i = 0; i < bufCount; i++) { buffermapping b = buffers[i]; - if(uint64LessEqual(b.origBase, addr) && uint64LessThan(addr, b.origEnd)) + if(lessEqual(b.origBase, addr) && lessThan(addr, b.origEnd)) { - return uint64Add(b.newBase, uint64Sub(addr, b.origBase)); + return add(b.newBase, sub(addr, b.origBase)); } } diff --git a/renderdoc/data/hlsl/raytracing.hlsl b/renderdoc/data/hlsl/raytracing.hlsl index 56e695da6..4dc60108c 100644 --- a/renderdoc/data/hlsl/raytracing.hlsl +++ b/renderdoc/data/hlsl/raytracing.hlsl @@ -22,17 +22,14 @@ * THE SOFTWARE. ******************************************************************************/ -#ifndef SHADER_MODEL_MIN_6_0_REQUIRED -#define SHADER_MODEL_MIN_6_0_REQUIRED -#endif #include "hlsl_cbuffers.h" -RWStructuredBuffer instanceDescs : register(u0, space0); -StructuredBuffer oldNewAddressesPair : register(t0, space0); +RWStructuredBuffer instanceDescs : register(u0); +StructuredBuffer oldNewAddressesPair : register(t0); bool InRange(BlasAddressRange addressRange, GPUAddress address) { - if(addressRange.start <= address && address <= addressRange.end) + if(lessEqual(addressRange.start, address) && lessThan(address, addressRange.end)) { return true; } @@ -49,8 +46,9 @@ bool InRange(BlasAddressRange addressRange, GPUAddress address) { if(InRange(oldNewAddressesPair[i].oldAddress, instanceBlasAddress)) { - uint64_t offset = instanceBlasAddress - oldNewAddressesPair[i].oldAddress.start; - instanceDescs[dispatchGroup.x].blasAddress = oldNewAddressesPair[i].newAddress.start + offset; + GPUAddress offset = sub(instanceBlasAddress, oldNewAddressesPair[i].oldAddress.start); + instanceDescs[dispatchGroup.x].blasAddress = + add(oldNewAddressesPair[i].newAddress.start, offset); return; } } diff --git a/renderdoc/driver/d3d12/d3d12_common.cpp b/renderdoc/driver/d3d12/d3d12_common.cpp index 09daa89c5..f0a692655 100644 --- a/renderdoc/driver/d3d12/d3d12_common.cpp +++ b/renderdoc/driver/d3d12/d3d12_common.cpp @@ -1814,3 +1814,107 @@ D3D12_PACKED_PIPELINE_STATE_STREAM_DESC &D3D12_PACKED_PIPELINE_STATE_STREAM_DESC return *this; } + +#if ENABLED(ENABLE_UNIT_TESTS) +#include "catch/catch.hpp" + +#define INCLUDE_GPUADDRESS_HELPERS + +#include "data/hlsl/hlsl_cbuffers.h" + +GPUAddress toaddr(uint64_t addr) +{ + GPUAddress ret; + RDCCOMPILE_ASSERT(sizeof(ret) == sizeof(addr), "GPU address isn't 64-bit"); + memcpy(&ret, &addr, sizeof(ret)); + return ret; +} + +uint64_t fromaddr(GPUAddress addr) +{ + uint64_t ret; + RDCCOMPILE_ASSERT(sizeof(ret) == sizeof(addr), "GPU address isn't 64-bit"); + memcpy(&ret, &addr, sizeof(ret)); + return ret; +} + +TEST_CASE("HLSL uint64 helpers", "[d3d]") +{ + rdcarray testValues = { + 0, + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10, + 11, + 100, + 128, + 1000, + + 0xfffffffa, + 0xfffffffb, + 0xfffffffc, + 0xfffffffd, + 0xfffffffe, + 0xffffffff, + + 0x100000000ULL, + 0x100000001ULL, + 0x100000002ULL, + 0x100000003ULL, + 0x100000004ULL, + 0x100000005ULL, + 0x100000006ULL, + + 0x1000000000001000ULL, + 0x100000000fffffffULL, + 0x1000000010000000ULL, + 0x1000000010000001ULL, + 0x1000000010000002ULL, + 0x1000000010000002ULL, + + 0x4000000000001000ULL, + 0x400000000fffffffULL, + 0x4000000010000000ULL, + 0x4000000010000001ULL, + 0x4000000010000002ULL, + 0x4000000010000002ULL, + // don't test anything that could overflow if summed together for simplicity + }; + + for(uint64_t first : testValues) + { + for(uint64_t second : testValues) + { + GPUAddress a, b; + a = toaddr(first); + b = toaddr(second); + + // sanity check + CHECK(fromaddr(a) == first); + CHECK(fromaddr(b) == second); + + CHECK(lessThan(a, b) == (first < second)); + CHECK(lessEqual(a, b) == (first <= second)); + + CHECK(lessThan(b, a) == (second < first)); + CHECK(lessEqual(b, a) == (second <= first)); + + CHECK(fromaddr(add(a, b)) == (first + second)); + CHECK(fromaddr(add(b, a)) == (first + second)); + + if(first >= second) + CHECK(fromaddr(sub(a, b)) == (first - second)); + else + CHECK(fromaddr(sub(b, a)) == (second - first)); + } + } +}; + +#endif // ENABLED(ENABLE_UNIT_TESTS) diff --git a/renderdoc/driver/d3d12/d3d12_manager.cpp b/renderdoc/driver/d3d12/d3d12_manager.cpp index 9f0287c0e..e88ca2c44 100644 --- a/renderdoc/driver/d3d12/d3d12_manager.cpp +++ b/renderdoc/driver/d3d12/d3d12_manager.cpp @@ -840,7 +840,7 @@ void D3D12RaytracingResourceAndUtilHandler::InitReplayBlasPatchingResources() ID3DBlob *shader = NULL; rdcstr hlsl = GetEmbeddedResource(raytracing_hlsl); shaderCache->GetShaderBlob(hlsl.c_str(), "RENDERDOC_PatchAccStructAddressCS", - D3DCOMPILE_WARNINGS_ARE_ERRORS, {}, "cs_6_0", &shader); + D3DCOMPILE_WARNINGS_ARE_ERRORS, {}, "cs_5_0", &shader); if(shader) { diff --git a/renderdoc/maths/vec.h b/renderdoc/maths/vec.h index 37c448bdc..1c5643652 100644 --- a/renderdoc/maths/vec.h +++ b/renderdoc/maths/vec.h @@ -167,6 +167,23 @@ inline Vec4f operator+=(Vec4f &a, const Vec4f &b) return a; } +struct Vec2u +{ + Vec2u(uint32_t X = 0, uint32_t Y = 0) + { + x = X; + y = Y; + } + union + { + struct + { + uint32_t x, y; + }; + uint32_t uv[2]; + }; +}; + struct Vec4u { Vec4u(uint32_t X = 0, uint32_t Y = 0, uint32_t Z = 0, uint32_t W = 0)