From 3cdeeeb897ec327d7eed6ddc1315518cdc6bfc1e Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Tue, 17 Jun 2025 09:22:46 -0700 Subject: [PATCH 1/6] test --- tools/clang/unittests/HLSL/PixTest.cpp | 48 ++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tools/clang/unittests/HLSL/PixTest.cpp b/tools/clang/unittests/HLSL/PixTest.cpp index af7801c7bf..3be49b32da 100644 --- a/tools/clang/unittests/HLSL/PixTest.cpp +++ b/tools/clang/unittests/HLSL/PixTest.cpp @@ -135,6 +135,7 @@ class PixTest : public ::testing::Test { TEST_METHOD(PixStructAnnotation_BigMess) TEST_METHOD(PixStructAnnotation_AlignedFloat4Arrays) TEST_METHOD(PixStructAnnotation_Inheritance) + TEST_METHOD(PixStructAnnotation_Bitfields) TEST_METHOD(PixStructAnnotation_ResourceAsMember) TEST_METHOD(PixStructAnnotation_WheresMyDbgValue) @@ -2321,6 +2322,53 @@ void main() } } +TEST_F(PixTest, PixStructAnnotation_Bitfields) { + if (m_ver.SkipDxilVersion(1, 5)) + return; + + for (auto choice : OptimizationChoices) { + auto optimization = choice.Flag; + + const char *hlsl = R"( +struct Bitfield +{ + uint32_t Field1 : 5; + uint32_t Field2 : 3; + uint32_t Field3 : 16; + uint32_t Field4 : 8; + uint32_t i32 : 32; +}; + +RWStructuredBuffer BitfieldStructure: register(u0); + +RWByteAddressBuffer RawUAV: register(u1); + +[numthreads(1, 1, 1)] +void main() +{ + Bitfield bitfield; + //bitfield.Field1 = RawUAV.Load(1 * 4); + bitfield.Field2 = RawUAV.Load(2 * 4); + //bitfield.Field3 = RawUAV.Load(3 * 4); + //bitfield.Field4 = RawUAV.Load(4 * 4); + //bitfield.i32 = RawUAV.Load(5 * 4); + + //RawUAV.Store(64*4, bitfield.Field1 ); + RawUAV.Store(65*4, bitfield.Field2 ); + //RawUAV.Store(66*4, bitfield.Field3 ); + //RawUAV.Store(67*4, bitfield.Field4 ); + //RawUAV.Store(68*4, bitfield.i32 ); +} +)"; + + CComPtr pBlob = + Compile(m_dllSupport, hlsl, L"cs_6_6", {optimization}); + CComPtr pDxil = FindModule(DFCC_ShaderDebugInfoDXIL, pBlob); + auto outputLines = RunAnnotationPasses(m_dllSupport, pDxil).lines; + + } +} + TEST_F(PixTest, PixStructAnnotation_ResourceAsMember) { if (m_ver.SkipDxilVersion(1, 5)) return; From 5e67cc3d879400d7a3d26c547abda5281849c900 Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Tue, 17 Jun 2025 15:10:40 -0700 Subject: [PATCH 2/6] squish to boundary --- lib/DxilDia/DxcPixDxilStorage.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/DxilDia/DxcPixDxilStorage.cpp b/lib/DxilDia/DxcPixDxilStorage.cpp index 79d21303dc..e1eaf462a4 100644 --- a/lib/DxilDia/DxcPixDxilStorage.cpp +++ b/lib/DxilDia/DxcPixDxilStorage.cpp @@ -185,7 +185,8 @@ dxil_debug_info::DxcPixDxilScalarStorage::Index(DWORD Index, STDMETHODIMP dxil_debug_info::DxcPixDxilScalarStorage::GetRegisterNumber( DWORD *pRegisterNumber) { const auto &ValueLocationMap = m_pVarInfo->m_ValueLocationMap; - auto RegIt = ValueLocationMap.find(m_OffsetFromStorageStartInBits); + // Bitfields will have been packed into their containing unit32: + auto RegIt = ValueLocationMap.find(m_OffsetFromStorageStartInBits & ~0x1f); if (RegIt == ValueLocationMap.end()) { return E_FAIL; From e73bc1ca56d860cb687d09b1040d8a8b2fa46d76 Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Wed, 18 Jun 2025 17:25:24 -0700 Subject: [PATCH 3/6] 64 bit and test --- lib/DxilDia/DxcPixDxilStorage.cpp | 7 +- tools/clang/unittests/HLSL/PixDiaTest.cpp | 169 +++++++++++++++++----- 2 files changed, 139 insertions(+), 37 deletions(-) diff --git a/lib/DxilDia/DxcPixDxilStorage.cpp b/lib/DxilDia/DxcPixDxilStorage.cpp index e1eaf462a4..4b06f472e8 100644 --- a/lib/DxilDia/DxcPixDxilStorage.cpp +++ b/lib/DxilDia/DxcPixDxilStorage.cpp @@ -185,8 +185,11 @@ dxil_debug_info::DxcPixDxilScalarStorage::Index(DWORD Index, STDMETHODIMP dxil_debug_info::DxcPixDxilScalarStorage::GetRegisterNumber( DWORD *pRegisterNumber) { const auto &ValueLocationMap = m_pVarInfo->m_ValueLocationMap; - // Bitfields will have been packed into their containing unit32: - auto RegIt = ValueLocationMap.find(m_OffsetFromStorageStartInBits & ~0x1f); + // Bitfields will have been packed into their containing integer type: + DWORD size; + m_pOriginalType->GetSizeInBits(&size); + auto RegIt = + ValueLocationMap.find(m_OffsetFromStorageStartInBits & ~(size - 1)); if (RegIt == ValueLocationMap.end()) { return E_FAIL; diff --git a/tools/clang/unittests/HLSL/PixDiaTest.cpp b/tools/clang/unittests/HLSL/PixDiaTest.cpp index a4439b998d..8ccb8707a6 100644 --- a/tools/clang/unittests/HLSL/PixDiaTest.cpp +++ b/tools/clang/unittests/HLSL/PixDiaTest.cpp @@ -13,6 +13,7 @@ #ifdef _WIN32 #include +#include #include "dxc/DxilContainer/DxilContainer.h" #include "dxc/Support/WinIncludes.h" @@ -186,6 +187,7 @@ class PixDiaTest { TEST_METHOD(DxcPixDxilDebugInfo_BitFields_Derived) TEST_METHOD(DxcPixDxilDebugInfo_BitFields_Bool) TEST_METHOD(DxcPixDxilDebugInfo_BitFields_Overlap) + TEST_METHOD(DxcPixDxilDebugInfo_BitFields_uint64) TEST_METHOD(DxcPixDxilDebugInfo_Min16SizesAndOffsets_Enabled) TEST_METHOD(DxcPixDxilDebugInfo_Min16SizesAndOffsets_Disabled) TEST_METHOD(DxcPixDxilDebugInfo_Min16VectorOffsets_Enabled) @@ -658,11 +660,11 @@ class PixDiaTest { const char *hlsl, const wchar_t *profile, const char *lineAtWhichToExamineVariables, std::vector const &ExpectedVariables); - void RunSizeAndOffsetTestCase(const char *hlsl, - std::array const &memberOffsets, - std::array const &memberSizes, - std::vector extraArgs = { - L"-Od"}); + CComPtr + RunSizeAndOffsetTestCase(const char *hlsl, + std::array const &memberOffsets, + std::array const &memberSizes, + std::vector extraArgs = {L"-Od"}); void RunVectorSizeAndOffsetTestCase(const char *hlsl, std::array const &memberOffsets, std::vector extraArgs = { @@ -2787,7 +2789,7 @@ class DxcIncludeHandlerForInjectedSourcesForPix : public IDxcIncludeHandler { DxcIncludeHandlerForInjectedSourcesForPix( PixDiaTest *pixTest, std::vector> files) - : m_dwRef(0), m_files(files), m_pixTest(pixTest){}; + : m_dwRef(0), m_files(files), m_pixTest(pixTest) {}; HRESULT STDMETHODCALLTYPE QueryInterface(REFIID iid, void **ppvObject) override { @@ -2948,12 +2950,11 @@ void main() VERIFY_ARE_EQUAL(32u, secondFieldOffset); } -void PixDiaTest::RunSizeAndOffsetTestCase( - const char *hlsl, std::array const &memberOffsets, - std::array const &memberSizes, - std::vector extraArgs) { - if (m_ver.SkipDxilVersion(1, 5)) - return; +CComPtr +PixDiaTest::RunSizeAndOffsetTestCase(const char *hlsl, + std::array const &memberOffsets, + std::array const &memberSizes, + std::vector extraArgs) { auto debugInfo = CompileAndCreateDxcDebug(hlsl, L"cs_6_5", nullptr, extraArgs).debugInfo; auto live = GetLiveVariablesAt(hlsl, "STOP_HERE", debugInfo); @@ -2974,9 +2975,46 @@ void PixDiaTest::RunSizeAndOffsetTestCase( VERIFY_SUCCEEDED(field->GetFieldSizeInBits(&sizeInBits)); VERIFY_ARE_EQUAL(memberSizes[i], sizeInBits); } + // Check that first and second and third are reported as residing in the same + // register (cuz they do!), and that the third does not + + CComPtr bfStorage; + VERIFY_SUCCEEDED(bf->GetStorage(&bfStorage)); + return bfStorage; +} + +void RunBitfieldAdjacencyTest( + IDxcPixDxilStorage *bfStorage, + std::vector> const &adjacentRuns) { + std::vector> registersByRun; + registersByRun.resize(adjacentRuns.size()); + for (size_t run = 0; run < adjacentRuns.size(); ++run) { + for (auto const &field : adjacentRuns[run]) { + CComPtr fieldStorage; + VERIFY_SUCCEEDED(bfStorage->AccessField(field, &fieldStorage)); + DWORD reg; + VERIFY_SUCCEEDED(fieldStorage->GetRegisterNumber(®)); + registersByRun[run].insert(reg); + } + } + for (size_t run = 0; run < registersByRun.size(); ++run) { + { + // Every field in this run should have the same register number, so this + // set should be of size 1: + VERIFY_ARE_EQUAL(1, registersByRun[run].size()); + // Every adjacent run should have different register numbers: + if (run != 0) { + VERIFY_ARE_NOT_EQUAL(*registersByRun[run - 1].begin(), + *registersByRun[run].begin()); + } + } + } } TEST_F(PixDiaTest, DxcPixDxilDebugInfo_BitFields_Simple) { + if (m_ver.SkipDxilVersion(1, 5)) + return; + const char *hlsl = R"( struct Bitfields { @@ -3000,10 +3038,16 @@ void main() } )"; - RunSizeAndOffsetTestCase(hlsl, {0, 17, 32, 64}, {17, 15, 3, 32}); + auto bfStorage = + RunSizeAndOffsetTestCase(hlsl, {0, 17, 32, 64}, {17, 15, 3, 32}); + RunBitfieldAdjacencyTest(bfStorage, + {{L"first", L"second"}, {L"third"}, {L"fourth"}}); } TEST_F(PixDiaTest, DxcPixDxilDebugInfo_BitFields_Derived) { + if (m_ver.SkipDxilVersion(1, 5)) + return; + const char *hlsl = R"( struct Bitfields { @@ -3027,10 +3071,16 @@ void main() } )"; - RunSizeAndOffsetTestCase(hlsl, {0, 17, 32, 64}, {17, 15, 3, 32}); + auto bfStorage = + RunSizeAndOffsetTestCase(hlsl, {0, 17, 32, 64}, {17, 15, 3, 32}); + RunBitfieldAdjacencyTest(bfStorage, + {{L"first", L"second"}, {L"third"}, {L"fourth"}}); } TEST_F(PixDiaTest, DxcPixDxilDebugInfo_BitFields_Bool) { + if (m_ver.SkipDxilVersion(1, 5)) + return; + const char *hlsl = R"( struct Bitfields { @@ -3054,17 +3104,58 @@ void main() } )"; - RunSizeAndOffsetTestCase(hlsl, {0, 1, 2, 32}, {1, 1, 3, 32}); + auto bfStorage = RunSizeAndOffsetTestCase(hlsl, {0, 1, 2, 32}, {1, 1, 3, 32}); + RunBitfieldAdjacencyTest(bfStorage, + {{L"first", L"second", L"third"}, {L"fourth"}}); } TEST_F(PixDiaTest, DxcPixDxilDebugInfo_BitFields_Overlap) { + if (m_ver.SkipDxilVersion(1, 5)) + return; + + const char *hlsl = R"( +struct Bitfields +{ + uint32_t first : 20; + uint32_t second : 20; // should end up in second DWORD + uint32_t third : 3; // should shader second DWORD + uint32_t fourth; // should be in third DWORD +}; + +RWStructuredBuffer UAV: register(u0); + +[numthreads(1, 1, 1)] +void main() +{ + Bitfields bf; + bf.first = UAV[0]; + bf.second = UAV[1]; + bf.third = UAV[2]; + bf.fourth = UAV[3]; + UAV[16] = bf.first + bf.second + bf.third + bf.fourth; //STOP_HERE +} + +)"; + auto bfStorage = + RunSizeAndOffsetTestCase(hlsl, {0, 32, 52, 64}, {20, 20, 3, 32}); + // (PIX #58022343): fields that overlap their storage type are not yet + // reflected properly in terms of their packed offsets as maintained via + // these PixDxc interfaces based on the dbg.declare data + //RunBitfieldAdjacencyTest(bfStorage, + // {{L"first"}, {L"second", L"third"}, {L"fourth"}}); +} + +TEST_F(PixDiaTest, DxcPixDxilDebugInfo_BitFields_uint64) { + if (m_ver.SkipDxilVersion(1, 5)) + return; + const char *hlsl = R"( struct Bitfields { - unsigned int first : 20; - unsigned int second : 20; // should end up in second DWORD - unsigned int third : 3; // should shader second DWORD - unsigned int fourth; // should be in third DWORD + uint64_t first : 20; + uint64_t second : 20; // should end up in first uint64 also + uint64_t third : 24; // in first + uint64_t fourth; // should be in second }; RWStructuredBuffer UAV: register(u0); @@ -3081,7 +3172,10 @@ void main() } )"; - RunSizeAndOffsetTestCase(hlsl, {0, 32, 52, 64}, {20, 20, 3, 32}); + auto bfStorage = + RunSizeAndOffsetTestCase(hlsl, {0, 20, 40, 64}, {20, 20, 24, 64}); + RunBitfieldAdjacencyTest(bfStorage, + {{L"first", L"second", L"third"}, {L"fourth"}}); } TEST_F(PixDiaTest, DxcPixDxilDebugInfo_Alignment_ConstInt) { @@ -3502,9 +3596,10 @@ void ClosestHitShader3(inout RayPayload payload, in BuiltInTriangleIntersectionA // Case: same function called from two places in same top-level function. // In this case, we expect the storage for the variable to be in the same - // place for both "instances" of the function: as a thread proceeds through - // the caller, it will write new values into the variable's storage during - // the second or subsequent invocations of the inlined function. + // place for both "instances" of the function: as a thread proceeds + // through the caller, it will write new values into the variable's + // storage during the second or subsequent invocations of the inlined + // function. DWORD instructionOffset = AdvanceUntilFunctionEntered(dxilDebugger, 0, L"ClosestHitShader3"); instructionOffset = AdvanceUntilFunctionEntered( @@ -3550,9 +3645,10 @@ TEST_F(PixDiaTest, DxcPixDxilDebugInfo_VariableScopes_ForScopes) { // Case: same function called from two places in same top-level function. // In this case, we expect the storage for the variable to be in the same - // place for both "instances" of the function: as a thread proceeds through - // the caller, it will write new values into the variable's storage during - // the second or subsequent invocations of the inlined function. + // place for both "instances" of the function: as a thread proceeds + // through the caller, it will write new values into the variable's + // storage during the second or subsequent invocations of the inlined + // function. DWORD instructionOffset = AdvanceUntilFunctionEntered(dxilDebugger, 0, L"CSMain"); @@ -3597,9 +3693,10 @@ TEST_F(PixDiaTest, DxcPixDxilDebugInfo_VariableScopes_ScopeBraces) { // Case: same function called from two places in same top-level function. // In this case, we expect the storage for the variable to be in the same - // place for both "instances" of the function: as a thread proceeds through - // the caller, it will write new values into the variable's storage during - // the second or subsequent invocations of the inlined function. + // place for both "instances" of the function: as a thread proceeds + // through the caller, it will write new values into the variable's + // storage during the second or subsequent invocations of the inlined + // function. DWORD instructionOffset = AdvanceUntilFunctionEntered(dxilDebugger, 0, L"CSMain"); @@ -3644,9 +3741,10 @@ TEST_F(PixDiaTest, DxcPixDxilDebugInfo_VariableScopes_Function) { // Case: same function called from two places in same top-level function. // In this case, we expect the storage for the variable to be in the same - // place for both "instances" of the function: as a thread proceeds through - // the caller, it will write new values into the variable's storage during - // the second or subsequent invocations of the inlined function. + // place for both "instances" of the function: as a thread proceeds + // through the caller, it will write new values into the variable's + // storage during the second or subsequent invocations of the inlined + // function. DWORD instructionOffset = AdvanceUntilFunctionEntered(dxilDebugger, 0, L"CSMain"); @@ -3692,9 +3790,10 @@ void CSMain() // Case: same function called from two places in same top-level function. // In this case, we expect the storage for the variable to be in the same - // place for both "instances" of the function: as a thread proceeds through - // the caller, it will write new values into the variable's storage during - // the second or subsequent invocations of the inlined function. + // place for both "instances" of the function: as a thread proceeds + // through the caller, it will write new values into the variable's + // storage during the second or subsequent invocations of the inlined + // function. DWORD instructionOffset = AdvanceUntilFunctionEntered(dxilDebugger, 0, L"CSMain"); From 048e72d300a954276dcb922f86cfbbee70615c07 Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Wed, 18 Jun 2025 17:25:57 -0700 Subject: [PATCH 4/6] clang format --- tools/clang/unittests/HLSL/PixDiaTest.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/clang/unittests/HLSL/PixDiaTest.cpp b/tools/clang/unittests/HLSL/PixDiaTest.cpp index 8ccb8707a6..211622261c 100644 --- a/tools/clang/unittests/HLSL/PixDiaTest.cpp +++ b/tools/clang/unittests/HLSL/PixDiaTest.cpp @@ -3140,8 +3140,8 @@ void main() RunSizeAndOffsetTestCase(hlsl, {0, 32, 52, 64}, {20, 20, 3, 32}); // (PIX #58022343): fields that overlap their storage type are not yet // reflected properly in terms of their packed offsets as maintained via - // these PixDxc interfaces based on the dbg.declare data - //RunBitfieldAdjacencyTest(bfStorage, + // these PixDxc interfaces based on the dbg.declare data + // RunBitfieldAdjacencyTest(bfStorage, // {{L"first"}, {L"second", L"third"}, {L"fourth"}}); } From 292b4ae4e36be49cdb8ffa5227e84b07890f31ed Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Fri, 20 Jun 2025 13:17:30 -0700 Subject: [PATCH 5/6] Undo unintentional test change --- tools/clang/unittests/HLSL/PixTest.cpp | 48 -------------------------- 1 file changed, 48 deletions(-) diff --git a/tools/clang/unittests/HLSL/PixTest.cpp b/tools/clang/unittests/HLSL/PixTest.cpp index 0168887cad..c032e9e872 100644 --- a/tools/clang/unittests/HLSL/PixTest.cpp +++ b/tools/clang/unittests/HLSL/PixTest.cpp @@ -135,7 +135,6 @@ class PixTest : public ::testing::Test { TEST_METHOD(PixStructAnnotation_BigMess) TEST_METHOD(PixStructAnnotation_AlignedFloat4Arrays) TEST_METHOD(PixStructAnnotation_Inheritance) - TEST_METHOD(PixStructAnnotation_Bitfields) TEST_METHOD(PixStructAnnotation_ResourceAsMember) TEST_METHOD(PixStructAnnotation_WheresMyDbgValue) @@ -2321,53 +2320,6 @@ void main() } } -TEST_F(PixTest, PixStructAnnotation_Bitfields) { - if (m_ver.SkipDxilVersion(1, 5)) - return; - - for (auto choice : OptimizationChoices) { - auto optimization = choice.Flag; - - const char *hlsl = R"( -struct Bitfield -{ - uint32_t Field1 : 5; - uint32_t Field2 : 3; - uint32_t Field3 : 16; - uint32_t Field4 : 8; - uint32_t i32 : 32; -}; - -RWStructuredBuffer BitfieldStructure: register(u0); - -RWByteAddressBuffer RawUAV: register(u1); - -[numthreads(1, 1, 1)] -void main() -{ - Bitfield bitfield; - //bitfield.Field1 = RawUAV.Load(1 * 4); - bitfield.Field2 = RawUAV.Load(2 * 4); - //bitfield.Field3 = RawUAV.Load(3 * 4); - //bitfield.Field4 = RawUAV.Load(4 * 4); - //bitfield.i32 = RawUAV.Load(5 * 4); - - //RawUAV.Store(64*4, bitfield.Field1 ); - RawUAV.Store(65*4, bitfield.Field2 ); - //RawUAV.Store(66*4, bitfield.Field3 ); - //RawUAV.Store(67*4, bitfield.Field4 ); - //RawUAV.Store(68*4, bitfield.i32 ); -} -)"; - - CComPtr pBlob = - Compile(m_dllSupport, hlsl, L"cs_6_6", {optimization}); - CComPtr pDxil = FindModule(DFCC_ShaderDebugInfoDXIL, pBlob); - auto outputLines = RunAnnotationPasses(m_dllSupport, pDxil).lines; - - } -} - TEST_F(PixTest, PixStructAnnotation_ResourceAsMember) { if (m_ver.SkipDxilVersion(1, 5)) return; From e28106d565f646e4dcd98f6a4dff823784cff169 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 26 Jun 2025 16:52:37 +0000 Subject: [PATCH 6/6] chore: autopublish 2025-06-26T16:52:37Z --- tools/clang/unittests/HLSL/PixDiaTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/clang/unittests/HLSL/PixDiaTest.cpp b/tools/clang/unittests/HLSL/PixDiaTest.cpp index 211622261c..d36e762762 100644 --- a/tools/clang/unittests/HLSL/PixDiaTest.cpp +++ b/tools/clang/unittests/HLSL/PixDiaTest.cpp @@ -2789,7 +2789,7 @@ class DxcIncludeHandlerForInjectedSourcesForPix : public IDxcIncludeHandler { DxcIncludeHandlerForInjectedSourcesForPix( PixDiaTest *pixTest, std::vector> files) - : m_dwRef(0), m_files(files), m_pixTest(pixTest) {}; + : m_dwRef(0), m_files(files), m_pixTest(pixTest){}; HRESULT STDMETHODCALLTYPE QueryInterface(REFIID iid, void **ppvObject) override {