From f13ea562a90eac845b00aabe008fde9514b55b57 Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Wed, 11 Jan 2023 13:09:23 -0800 Subject: [PATCH 1/7] Test and assert fix --- .../DxilDbgValueToDbgDeclare.cpp | 3 +- tools/clang/unittests/HLSL/PixTest.cpp | 123 +++++++++++++++++- 2 files changed, 118 insertions(+), 8 deletions(-) diff --git a/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp b/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp index 34a56b72c5..0db0ee9307 100644 --- a/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp +++ b/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp @@ -365,7 +365,8 @@ static OffsetInBits SplitValue( else { assert(VTy->isFloatTy() || VTy->isDoubleTy() || VTy->isHalfTy() || - VTy->isIntegerTy(32) || VTy->isIntegerTy(64) || VTy->isIntegerTy(16)); + VTy->isIntegerTy(32) || VTy->isIntegerTy(64) || VTy->isIntegerTy(16) || + VTy->isPointerTy()); Values->emplace_back(ValueAndOffset{V, CurrentOffset}); CurrentOffset += VTy->getScalarSizeInBits(); } diff --git a/tools/clang/unittests/HLSL/PixTest.cpp b/tools/clang/unittests/HLSL/PixTest.cpp index b633c400dc..02a2a281a1 100644 --- a/tools/clang/unittests/HLSL/PixTest.cpp +++ b/tools/clang/unittests/HLSL/PixTest.cpp @@ -219,6 +219,7 @@ class PixTest { TEST_METHOD(PixStructAnnotation_BigMess) TEST_METHOD(PixStructAnnotation_AlignedFloat4Arrays) TEST_METHOD(PixStructAnnotation_Inheritance) + TEST_METHOD(PixStructAnnotation_InheritancePointer) TEST_METHOD(PixStructAnnotation_ResourceAsMember) TEST_METHOD(PixStructAnnotation_WheresMyDbgValue) @@ -1131,6 +1132,12 @@ static std::string ToString(std::wstring from) std::string RunDxilPIXAddTidToAmplificationShaderPayloadPass(IDxcBlob * blob); CComPtr RunDxilPIXMeshShaderOutputPass(IDxcBlob* blob); + void CompileAndRunAnnotationAndGetDebugPart( + dxc::DxcDllSupport &dllSupport, const char *source, wchar_t *profile, + IDxcBlob **ppDebugPart); + void CompileAndRunAnnotationAndLoadDiaSource(dxc::DxcDllSupport &dllSupport, + const char *source, wchar_t *profile, + IDiaDataSource **ppDataSource); }; @@ -3212,18 +3219,78 @@ void main() } } +void PixTest::CompileAndRunAnnotationAndGetDebugPart(dxc::DxcDllSupport &dllSupport, + const char *source, wchar_t *profile, + IDxcBlob **ppDebugPart) { + CComPtr pContainer; + CComPtr pLib; + CComPtr pReflection; + UINT32 index; + std::vector args; + args.push_back(L"/Zi"); + args.push_back(L"/Qembed_debug"); + + VerifyCompileOK(dllSupport, source, profile, args, &pContainer); + + auto annotated = RunAnnotationPasses(pContainer); + + CComPtr pNewContainer; + { + CComPtr pAssembler; + IFT(m_dllSupport.CreateInstance(CLSID_DxcAssembler, &pAssembler)); + + CComPtr pAssembleResult; + VERIFY_SUCCEEDED( + pAssembler->AssembleToContainer(annotated.blob, &pAssembleResult)); + + CComPtr pAssembleErrors; + VERIFY_SUCCEEDED(pAssembleResult->GetErrorBuffer(&pAssembleErrors)); + + if (pAssembleErrors && pAssembleErrors->GetBufferSize() != 0) { + OutputDebugStringA( + static_cast(pAssembleErrors->GetBufferPointer())); + VERIFY_SUCCEEDED(E_FAIL); + } + + VERIFY_SUCCEEDED(pAssembleResult->GetResult(&pNewContainer)); + } + + + VERIFY_SUCCEEDED(dllSupport.CreateInstance(CLSID_DxcLibrary, &pLib)); + VERIFY_SUCCEEDED( + dllSupport.CreateInstance(CLSID_DxcContainerReflection, &pReflection)); + VERIFY_SUCCEEDED(pReflection->Load(pNewContainer)); + VERIFY_SUCCEEDED( + pReflection->FindFirstPartKind(hlsl::DFCC_ShaderDebugInfoDXIL, &index)); + VERIFY_SUCCEEDED(pReflection->GetPartContent(index, ppDebugPart)); +} + +void PixTest::CompileAndRunAnnotationAndLoadDiaSource( + dxc::DxcDllSupport &dllSupport, + const char *source, wchar_t *profile, + IDiaDataSource **ppDataSource) { + CComPtr pDebugContent; + CComPtr pStream; + CComPtr pDiaSource; + CComPtr pLib; + VERIFY_SUCCEEDED(dllSupport.CreateInstance(CLSID_DxcLibrary, &pLib)); + + CompileAndRunAnnotationAndGetDebugPart(dllSupport, source, profile, + &pDebugContent); + VERIFY_SUCCEEDED(pLib->CreateStreamFromBlobReadOnly(pDebugContent, &pStream)); + VERIFY_SUCCEEDED( + dllSupport.CreateInstance(CLSID_DxcDiaDataSource, &pDiaSource)); + VERIFY_SUCCEEDED(pDiaSource->loadDataFromIStream(pStream)); + if (ppDataSource) { + *ppDataSource = pDiaSource.Detach(); + } +} + TEST_F(PixTest, PixStructAnnotation_Inheritance) { if (m_ver.SkipDxilVersion(1, 5)) return; for (auto choice : OptimizationChoices) { auto optimization = choice.Flag; - auto IsOptimized = choice.IsOptimized; - - // don't run -Od test until -Od inheritance is fixed (#3274: - // https://github.com/microsoft/DirectXShaderCompiler/issues/3274) - if (!IsOptimized) - continue; - const char* hlsl = R"( struct Base @@ -3254,6 +3321,48 @@ void main() } } +TEST_F(PixTest, PixStructAnnotation_InheritancePointer) { + if (m_ver.SkipDxilVersion(1, 5)) return; + + const char* hlsl = R"( +struct Base +{ + float floatValue; +}; +typedef Base BaseTypedef; + +struct Derived : BaseTypedef +{ + int intValue; +}; + +RaytracingAccelerationStructure Scene : register(t0, space0); + +[shader("raygeneration")] +void main() +{ + RayDesc ray; + ray.Origin = float3(0,0,0); + ray.Direction = float3(0,0,1); + // Set TMin to a non-zero small value to avoid aliasing issues due to floating - point errors. + // TMin should be kept small to prevent missing geometry at close contact areas. + ray.TMin = 0.001; + ray.TMax = 10000.0; + Derived payload; + payload.floatValue = 1; + payload.intValue = 2; + TraceRay(Scene, RAY_FLAG_CULL_BACK_FACING_TRIANGLES, ~0, 0, 1, 0, ray, payload);} + +)"; + + CComPtr pDiaDataSource; + CComPtr pDiaSession; + CompileAndRunAnnotationAndLoadDiaSource(m_dllSupport, hlsl, L"lib_6_6", + &pDiaDataSource); + + VERIFY_SUCCEEDED(pDiaDataSource->openSession(&pDiaSession)); +} + TEST_F(PixTest, PixStructAnnotation_ResourceAsMember) { if (m_ver.SkipDxilVersion(1, 5)) return; From 5dfd5c61c842ce319cbdcd62599f7f37aea73091 Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Thu, 12 Jan 2023 08:59:01 -0800 Subject: [PATCH 2/7] Offset for inherited class --- lib/DxilDia/DxilDiaSymbolManager.cpp | 42 ++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/lib/DxilDia/DxilDiaSymbolManager.cpp b/lib/DxilDia/DxilDiaSymbolManager.cpp index abe757d0cb..9e01bd95d5 100644 --- a/lib/DxilDia/DxilDiaSymbolManager.cpp +++ b/lib/DxilDia/DxilDiaSymbolManager.cpp @@ -607,6 +607,7 @@ class SymbolManagerInit { void Embed(const TypeInfo &TI); void AddBasicType(llvm::DIBasicType *BT); + void PrependBaseClassSize(uint64_t baseSize); private: DWORD m_dwTypeID; @@ -1151,6 +1152,12 @@ void dxil_dia::hlsl_symbols::SymbolManagerInit::TypeInfo::Embed(const TypeInfo & m_dwCurrentSizeInBytes += TI.m_dwCurrentSizeInBytes; } +void dxil_dia::hlsl_symbols::SymbolManagerInit::TypeInfo::PrependBaseClassSize( + uint64_t baseSize) { + static constexpr DWORD kNumBitsPerByte = 8; + m_dwCurrentSizeInBytes += baseSize / kNumBitsPerByte; +} + void dxil_dia::hlsl_symbols::SymbolManagerInit::TypeInfo::AddBasicType(llvm::DIBasicType *BT) { m_Layout.emplace_back(BT); @@ -1439,6 +1446,33 @@ HRESULT dxil_dia::hlsl_symbols::SymbolManagerInit::CreateBasicType(DWORD dwParen return S_OK; } +static uint64_t getBaseClassSize(llvm::DIType * Ty) +{ + uint64_t sizeInBits = Ty->getSizeInBits(); + auto *DerivedTy = llvm::dyn_cast(Ty); + if (DerivedTy != nullptr) { + // Working around a bug where byte size is stored instead of bit size + if (sizeInBits == 4 && Ty->getSizeInBits() == 32) { + sizeInBits = 32; + } + if (sizeInBits == 0) { + const llvm::DITypeIdentifierMap EmptyMap; + switch (DerivedTy->getTag()) { + case llvm::dwarf::DW_TAG_restrict_type: + case llvm::dwarf::DW_TAG_reference_type: + case llvm::dwarf::DW_TAG_const_type: + case llvm::dwarf::DW_TAG_typedef: { + llvm::DIType *baseType = DerivedTy->getBaseType().resolve(EmptyMap); + if (baseType != nullptr) { + return getBaseClassSize(baseType); + } + } + } + } + } + return sizeInBits; +} + HRESULT dxil_dia::hlsl_symbols::SymbolManagerInit::CreateCompositeType(DWORD dwParentID, llvm::DICompositeType *CT, DWORD *pNewTypeID) { switch (CT->getTag()) { case llvm::dwarf::DW_TAG_array_type: { @@ -1533,6 +1567,14 @@ HRESULT dxil_dia::hlsl_symbols::SymbolManagerInit::CreateCompositeType(DWORD dwP if (auto *Field = llvm::dyn_cast(N)) { DWORD dwUnusedFieldID; IFR(CreateType(Field, &dwUnusedFieldID)); + if (Field->getTag() == llvm::dwarf::DW_TAG_inheritance) { + // The base class is a type of its own, so will have contributed to its own TypeInfo. + // But we still need to remember the size that it contributed to this type: + auto *DerivedType = llvm::dyn_cast(Field); + const llvm::DITypeIdentifierMap EmptyMap; + llvm::DIType *BaseType = DerivedType->getBaseType().resolve(EmptyMap); + udtTI->PrependBaseClassSize(getBaseClassSize(BaseType)); + } } } From f132b9fff7260cfd343ba48daf500865a60ae875 Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Thu, 12 Jan 2023 08:59:43 -0800 Subject: [PATCH 3/7] Tests --- tools/clang/unittests/HLSL/PixTest.cpp | 309 ++++++++++++++++--------- 1 file changed, 198 insertions(+), 111 deletions(-) diff --git a/tools/clang/unittests/HLSL/PixTest.cpp b/tools/clang/unittests/HLSL/PixTest.cpp index 02a2a281a1..0b5813fc81 100644 --- a/tools/clang/unittests/HLSL/PixTest.cpp +++ b/tools/clang/unittests/HLSL/PixTest.cpp @@ -219,11 +219,14 @@ class PixTest { TEST_METHOD(PixStructAnnotation_BigMess) TEST_METHOD(PixStructAnnotation_AlignedFloat4Arrays) TEST_METHOD(PixStructAnnotation_Inheritance) - TEST_METHOD(PixStructAnnotation_InheritancePointer) TEST_METHOD(PixStructAnnotation_ResourceAsMember) TEST_METHOD(PixStructAnnotation_WheresMyDbgValue) - TEST_METHOD(VirtualRegisters_InstructionCounts) + TEST_METHOD(PixTypeManager_InheritancePointerStruct) + TEST_METHOD(PixTypeManager_InheritancePointerTypedef) + TEST_METHOD(PixTypeManager_MatricesInBase) + + TEST_METHOD(VirtualRegisters_InstructionCounts) TEST_METHOD(VirtualRegisters_AlignedOffsets) TEST_METHOD(RootSignatureUpgrade_SubObjects) @@ -1797,6 +1800,199 @@ TEST_F(PixTest, PixDebugCompileInfo) { VERIFY_ARE_EQUAL(std::wstring(profile), std::wstring(hlslTarget)); } +void PixTest::CompileAndRunAnnotationAndGetDebugPart( + dxc::DxcDllSupport &dllSupport, const char *source, wchar_t *profile, + IDxcBlob **ppDebugPart) { + CComPtr pContainer; + CComPtr pLib; + CComPtr pReflection; + UINT32 index; + std::vector args; + args.push_back(L"/Zi"); + args.push_back(L"/Qembed_debug"); + + VerifyCompileOK(dllSupport, source, profile, args, &pContainer); + + auto annotated = RunAnnotationPasses(pContainer); + + CComPtr pNewContainer; + { + CComPtr pAssembler; + IFT(m_dllSupport.CreateInstance(CLSID_DxcAssembler, &pAssembler)); + + CComPtr pAssembleResult; + VERIFY_SUCCEEDED( + pAssembler->AssembleToContainer(annotated.blob, &pAssembleResult)); + + CComPtr pAssembleErrors; + VERIFY_SUCCEEDED(pAssembleResult->GetErrorBuffer(&pAssembleErrors)); + + if (pAssembleErrors && pAssembleErrors->GetBufferSize() != 0) { + OutputDebugStringA( + static_cast(pAssembleErrors->GetBufferPointer())); + VERIFY_SUCCEEDED(E_FAIL); + } + + VERIFY_SUCCEEDED(pAssembleResult->GetResult(&pNewContainer)); + } + + VERIFY_SUCCEEDED(dllSupport.CreateInstance(CLSID_DxcLibrary, &pLib)); + VERIFY_SUCCEEDED( + dllSupport.CreateInstance(CLSID_DxcContainerReflection, &pReflection)); + VERIFY_SUCCEEDED(pReflection->Load(pNewContainer)); + VERIFY_SUCCEEDED( + pReflection->FindFirstPartKind(hlsl::DFCC_ShaderDebugInfoDXIL, &index)); + VERIFY_SUCCEEDED(pReflection->GetPartContent(index, ppDebugPart)); +} + +void PixTest::CompileAndRunAnnotationAndLoadDiaSource( + dxc::DxcDllSupport &dllSupport, const char *source, wchar_t *profile, + IDiaDataSource **ppDataSource) { + CComPtr pDebugContent; + CComPtr pStream; + CComPtr pDiaSource; + CComPtr pLib; + VERIFY_SUCCEEDED(dllSupport.CreateInstance(CLSID_DxcLibrary, &pLib)); + + CompileAndRunAnnotationAndGetDebugPart(dllSupport, source, profile, + &pDebugContent); + VERIFY_SUCCEEDED(pLib->CreateStreamFromBlobReadOnly(pDebugContent, &pStream)); + VERIFY_SUCCEEDED( + dllSupport.CreateInstance(CLSID_DxcDiaDataSource, &pDiaSource)); + VERIFY_SUCCEEDED(pDiaSource->loadDataFromIStream(pStream)); + if (ppDataSource) { + *ppDataSource = pDiaSource.Detach(); + } +} + +TEST_F(PixTest, PixTypeManager_InheritancePointerStruct) { + if (m_ver.SkipDxilVersion(1, 5)) + return; + + const char *hlsl = R"( +struct Base +{ + float floatValue; +}; + +struct Derived : Base +{ + int intValue; +}; + +RaytracingAccelerationStructure Scene : register(t0, space0); + +[shader("raygeneration")] +void main() +{ + RayDesc ray; + ray.Origin = float3(0,0,0); + ray.Direction = float3(0,0,1); + // Set TMin to a non-zero small value to avoid aliasing issues due to floating - point errors. + // TMin should be kept small to prevent missing geometry at close contact areas. + ray.TMin = 0.001; + ray.TMax = 10000.0; + Derived payload; + payload.floatValue = 1; + payload.intValue = 2; + TraceRay(Scene, RAY_FLAG_CULL_BACK_FACING_TRIANGLES, ~0, 0, 1, 0, ray, payload);} + +)"; + + CComPtr pDiaDataSource; + CComPtr pDiaSession; + CompileAndRunAnnotationAndLoadDiaSource(m_dllSupport, hlsl, L"lib_6_6", + &pDiaDataSource); + + VERIFY_SUCCEEDED(pDiaDataSource->openSession(&pDiaSession)); +} + +TEST_F(PixTest, PixTypeManager_InheritancePointerTypedef) { + if (m_ver.SkipDxilVersion(1, 5)) + return; + + const char *hlsl = R"( +struct Base +{ + float floatValue; +}; +typedef Base BaseTypedef; + +struct Derived : BaseTypedef +{ + int intValue; +}; + +RaytracingAccelerationStructure Scene : register(t0, space0); + +[shader("raygeneration")] +void main() +{ + RayDesc ray; + ray.Origin = float3(0,0,0); + ray.Direction = float3(0,0,1); + // Set TMin to a non-zero small value to avoid aliasing issues due to floating - point errors. + // TMin should be kept small to prevent missing geometry at close contact areas. + ray.TMin = 0.001; + ray.TMax = 10000.0; + Derived payload; + payload.floatValue = 1; + payload.intValue = 2; + TraceRay(Scene, RAY_FLAG_CULL_BACK_FACING_TRIANGLES, ~0, 0, 1, 0, ray, payload);} + +)"; + + CComPtr pDiaDataSource; + CComPtr pDiaSession; + CompileAndRunAnnotationAndLoadDiaSource(m_dllSupport, hlsl, L"lib_6_6", + &pDiaDataSource); + + VERIFY_SUCCEEDED(pDiaDataSource->openSession(&pDiaSession)); +} + +TEST_F(PixTest, PixTypeManager_MatricesInBase) { + if (m_ver.SkipDxilVersion(1, 5)) + return; + + const char *hlsl = R"( +struct Base +{ + float4x4 mat; +}; +typedef Base BaseTypedef; + +struct Derived : BaseTypedef +{ + int intValue; +}; + +RaytracingAccelerationStructure Scene : register(t0, space0); + +[shader("raygeneration")] +void main() +{ + RayDesc ray; + ray.Origin = float3(0,0,0); + ray.Direction = float3(0,0,1); + // Set TMin to a non-zero small value to avoid aliasing issues due to floating - point errors. + // TMin should be kept small to prevent missing geometry at close contact areas. + ray.TMin = 0.001; + ray.TMax = 10000.0; + Derived payload; + payload.mat[0][0] = 1; + payload.intValue = 2; + TraceRay(Scene, RAY_FLAG_CULL_BACK_FACING_TRIANGLES, ~0, 0, 1, 0, ray, payload);} + +)"; + + CComPtr pDiaDataSource; + CComPtr pDiaSession; + CompileAndRunAnnotationAndLoadDiaSource(m_dllSupport, hlsl, L"lib_6_6", + &pDiaDataSource); + + VERIFY_SUCCEEDED(pDiaDataSource->openSession(&pDiaSession)); +} + CComPtr PixTest::RunShaderAccessTrackingPass(IDxcBlob *blob) { CComPtr pOptimizer; VERIFY_SUCCEEDED( @@ -3219,73 +3415,6 @@ void main() } } -void PixTest::CompileAndRunAnnotationAndGetDebugPart(dxc::DxcDllSupport &dllSupport, - const char *source, wchar_t *profile, - IDxcBlob **ppDebugPart) { - CComPtr pContainer; - CComPtr pLib; - CComPtr pReflection; - UINT32 index; - std::vector args; - args.push_back(L"/Zi"); - args.push_back(L"/Qembed_debug"); - - VerifyCompileOK(dllSupport, source, profile, args, &pContainer); - - auto annotated = RunAnnotationPasses(pContainer); - - CComPtr pNewContainer; - { - CComPtr pAssembler; - IFT(m_dllSupport.CreateInstance(CLSID_DxcAssembler, &pAssembler)); - - CComPtr pAssembleResult; - VERIFY_SUCCEEDED( - pAssembler->AssembleToContainer(annotated.blob, &pAssembleResult)); - - CComPtr pAssembleErrors; - VERIFY_SUCCEEDED(pAssembleResult->GetErrorBuffer(&pAssembleErrors)); - - if (pAssembleErrors && pAssembleErrors->GetBufferSize() != 0) { - OutputDebugStringA( - static_cast(pAssembleErrors->GetBufferPointer())); - VERIFY_SUCCEEDED(E_FAIL); - } - - VERIFY_SUCCEEDED(pAssembleResult->GetResult(&pNewContainer)); - } - - - VERIFY_SUCCEEDED(dllSupport.CreateInstance(CLSID_DxcLibrary, &pLib)); - VERIFY_SUCCEEDED( - dllSupport.CreateInstance(CLSID_DxcContainerReflection, &pReflection)); - VERIFY_SUCCEEDED(pReflection->Load(pNewContainer)); - VERIFY_SUCCEEDED( - pReflection->FindFirstPartKind(hlsl::DFCC_ShaderDebugInfoDXIL, &index)); - VERIFY_SUCCEEDED(pReflection->GetPartContent(index, ppDebugPart)); -} - -void PixTest::CompileAndRunAnnotationAndLoadDiaSource( - dxc::DxcDllSupport &dllSupport, - const char *source, wchar_t *profile, - IDiaDataSource **ppDataSource) { - CComPtr pDebugContent; - CComPtr pStream; - CComPtr pDiaSource; - CComPtr pLib; - VERIFY_SUCCEEDED(dllSupport.CreateInstance(CLSID_DxcLibrary, &pLib)); - - CompileAndRunAnnotationAndGetDebugPart(dllSupport, source, profile, - &pDebugContent); - VERIFY_SUCCEEDED(pLib->CreateStreamFromBlobReadOnly(pDebugContent, &pStream)); - VERIFY_SUCCEEDED( - dllSupport.CreateInstance(CLSID_DxcDiaDataSource, &pDiaSource)); - VERIFY_SUCCEEDED(pDiaSource->loadDataFromIStream(pStream)); - if (ppDataSource) { - *ppDataSource = pDiaSource.Detach(); - } -} - TEST_F(PixTest, PixStructAnnotation_Inheritance) { if (m_ver.SkipDxilVersion(1, 5)) return; @@ -3321,48 +3450,6 @@ void main() } } -TEST_F(PixTest, PixStructAnnotation_InheritancePointer) { - if (m_ver.SkipDxilVersion(1, 5)) return; - - const char* hlsl = R"( -struct Base -{ - float floatValue; -}; -typedef Base BaseTypedef; - -struct Derived : BaseTypedef -{ - int intValue; -}; - -RaytracingAccelerationStructure Scene : register(t0, space0); - -[shader("raygeneration")] -void main() -{ - RayDesc ray; - ray.Origin = float3(0,0,0); - ray.Direction = float3(0,0,1); - // Set TMin to a non-zero small value to avoid aliasing issues due to floating - point errors. - // TMin should be kept small to prevent missing geometry at close contact areas. - ray.TMin = 0.001; - ray.TMax = 10000.0; - Derived payload; - payload.floatValue = 1; - payload.intValue = 2; - TraceRay(Scene, RAY_FLAG_CULL_BACK_FACING_TRIANGLES, ~0, 0, 1, 0, ray, payload);} - -)"; - - CComPtr pDiaDataSource; - CComPtr pDiaSession; - CompileAndRunAnnotationAndLoadDiaSource(m_dllSupport, hlsl, L"lib_6_6", - &pDiaDataSource); - - VERIFY_SUCCEEDED(pDiaDataSource->openSession(&pDiaSession)); -} - TEST_F(PixTest, PixStructAnnotation_ResourceAsMember) { if (m_ver.SkipDxilVersion(1, 5)) return; From f39b6b3e9a390cbf4ee24eb2689454078ffed686 Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Thu, 12 Jan 2023 13:17:17 -0800 Subject: [PATCH 4/7] Add resource sizes --- lib/DxilDia/DxilDiaSymbolManager.cpp | 41 ++++++++++++++++++---------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/lib/DxilDia/DxilDiaSymbolManager.cpp b/lib/DxilDia/DxilDiaSymbolManager.cpp index 9e01bd95d5..c9a09f947a 100644 --- a/lib/DxilDia/DxilDiaSymbolManager.cpp +++ b/lib/DxilDia/DxilDiaSymbolManager.cpp @@ -607,7 +607,7 @@ class SymbolManagerInit { void Embed(const TypeInfo &TI); void AddBasicType(llvm::DIBasicType *BT); - void PrependBaseClassSize(uint64_t baseSize); + void AppendSize(uint64_t baseSize); private: DWORD m_dwTypeID; @@ -1152,7 +1152,7 @@ void dxil_dia::hlsl_symbols::SymbolManagerInit::TypeInfo::Embed(const TypeInfo & m_dwCurrentSizeInBytes += TI.m_dwCurrentSizeInBytes; } -void dxil_dia::hlsl_symbols::SymbolManagerInit::TypeInfo::PrependBaseClassSize( +void dxil_dia::hlsl_symbols::SymbolManagerInit::TypeInfo::AppendSize( uint64_t baseSize) { static constexpr DWORD kNumBitsPerByte = 8; m_dwCurrentSizeInBytes += baseSize / kNumBitsPerByte; @@ -1558,26 +1558,39 @@ HRESULT dxil_dia::hlsl_symbols::SymbolManagerInit::CreateCompositeType(DWORD dwP return S_OK; }; + if (CT->getTag() == llvm::dwarf::DW_TAG_structure_type && + CT->getName() == "SamplerState") { + CT->getName(); + } IFR(AddType(dwParentID, CT, pNewTypeID, CT->getAlignInBits(), CT, LazyName)); TypeInfo *udtTI; IFR(GetTypeInfo(CT, &udtTI)); auto udtScope = BeginUDTScope(udtTI); - for (llvm::DINode *N : CT->getElements()) { - if (auto *Field = llvm::dyn_cast(N)) { - DWORD dwUnusedFieldID; - IFR(CreateType(Field, &dwUnusedFieldID)); - if (Field->getTag() == llvm::dwarf::DW_TAG_inheritance) { - // The base class is a type of its own, so will have contributed to its own TypeInfo. - // But we still need to remember the size that it contributed to this type: - auto *DerivedType = llvm::dyn_cast(Field); - const llvm::DITypeIdentifierMap EmptyMap; - llvm::DIType *BaseType = DerivedType->getBaseType().resolve(EmptyMap); - udtTI->PrependBaseClassSize(getBaseClassSize(BaseType)); + if (CT->getElements().size() == 0) { + // "Resources" (textures, samplers, etc.) are composite types without any elements, + // but they do have a size. + udtTI->AppendSize(CT->getSizeInBits()); + } else { + for (llvm::DINode *N : CT->getElements()) { + if (auto *Field = llvm::dyn_cast(N)) { + DWORD dwUnusedFieldID; + if (Field->getName() == "MaterialTextureBilinearWrapedSampler") { + Field->getName(); + } + IFR(CreateType(Field, &dwUnusedFieldID)); + if (Field->getTag() == llvm::dwarf::DW_TAG_inheritance) { + // The base class is a type of its own, so will have contributed to + // its own TypeInfo. But we still need to remember the size that it + // contributed to this type: + auto *DerivedType = llvm::dyn_cast(Field); + const llvm::DITypeIdentifierMap EmptyMap; + llvm::DIType *BaseType = DerivedType->getBaseType().resolve(EmptyMap); + udtTI->AppendSize(getBaseClassSize(BaseType)); + } } } } - return S_OK; } From 243c782ee9e0e6e09965cfe09e73c1c5586fd6d0 Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Thu, 12 Jan 2023 13:59:00 -0800 Subject: [PATCH 5/7] cleanup --- lib/DxilDia/DxilDiaSymbolManager.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/DxilDia/DxilDiaSymbolManager.cpp b/lib/DxilDia/DxilDiaSymbolManager.cpp index c9a09f947a..284a27db2f 100644 --- a/lib/DxilDia/DxilDiaSymbolManager.cpp +++ b/lib/DxilDia/DxilDiaSymbolManager.cpp @@ -1558,10 +1558,6 @@ HRESULT dxil_dia::hlsl_symbols::SymbolManagerInit::CreateCompositeType(DWORD dwP return S_OK; }; - if (CT->getTag() == llvm::dwarf::DW_TAG_structure_type && - CT->getName() == "SamplerState") { - CT->getName(); - } IFR(AddType(dwParentID, CT, pNewTypeID, CT->getAlignInBits(), CT, LazyName)); TypeInfo *udtTI; From c2793a4a5dd694a4d235fb37d8e191f067254baa Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Thu, 12 Jan 2023 13:59:15 -0800 Subject: [PATCH 6/7] Test for samplers n resources --- tools/clang/unittests/HLSL/PixTest.cpp | 96 +++++++++++++++++++++++++- 1 file changed, 95 insertions(+), 1 deletion(-) diff --git a/tools/clang/unittests/HLSL/PixTest.cpp b/tools/clang/unittests/HLSL/PixTest.cpp index 0b5813fc81..a4db865d40 100644 --- a/tools/clang/unittests/HLSL/PixTest.cpp +++ b/tools/clang/unittests/HLSL/PixTest.cpp @@ -225,6 +225,7 @@ class PixTest { TEST_METHOD(PixTypeManager_InheritancePointerStruct) TEST_METHOD(PixTypeManager_InheritancePointerTypedef) TEST_METHOD(PixTypeManager_MatricesInBase) + TEST_METHOD(PixTypeManager_SamplersAndResources) TEST_METHOD(VirtualRegisters_InstructionCounts) TEST_METHOD(VirtualRegisters_AlignedOffsets) @@ -1800,6 +1801,30 @@ TEST_F(PixTest, PixDebugCompileInfo) { VERIFY_ARE_EQUAL(std::wstring(profile), std::wstring(hlslTarget)); } +static void CompileAndLogErrors(dxc::DxcDllSupport &dllSupport, LPCSTR pText, + LPWSTR pTargetProfile, std::vector &args, + _Outptr_ IDxcBlob **ppResult) { + CComPtr pCompiler; + CComPtr pSource; + CComPtr pResult; + HRESULT hrCompile; + *ppResult = nullptr; + VERIFY_SUCCEEDED(dllSupport.CreateInstance(CLSID_DxcCompiler, &pCompiler)); + Utf8ToBlob(dllSupport, pText, &pSource); + VERIFY_SUCCEEDED(pCompiler->Compile(pSource, L"source.hlsl", L"main", + pTargetProfile, args.data(), args.size(), + nullptr, 0, nullptr, &pResult)); + + VERIFY_SUCCEEDED(pResult->GetStatus(&hrCompile)); + if (FAILED(hrCompile)) { + CComPtr textBlob; + VERIFY_SUCCEEDED(pResult->GetErrorBuffer(&textBlob)); + std::wstring text = BlobToWide(textBlob); + WEX::Logging::Log::Comment(text.c_str()); + } + VERIFY_SUCCEEDED(hrCompile); + VERIFY_SUCCEEDED(pResult->GetResult(ppResult)); +} void PixTest::CompileAndRunAnnotationAndGetDebugPart( dxc::DxcDllSupport &dllSupport, const char *source, wchar_t *profile, IDxcBlob **ppDebugPart) { @@ -1811,7 +1836,7 @@ void PixTest::CompileAndRunAnnotationAndGetDebugPart( args.push_back(L"/Zi"); args.push_back(L"/Qembed_debug"); - VerifyCompileOK(dllSupport, source, profile, args, &pContainer); + CompileAndLogErrors(dllSupport, source, profile, args, &pContainer); auto annotated = RunAnnotationPasses(pContainer); @@ -1993,6 +2018,75 @@ void main() VERIFY_SUCCEEDED(pDiaDataSource->openSession(&pDiaSession)); } +TEST_F(PixTest, PixTypeManager_SamplersAndResources) { + if (m_ver.SkipDxilVersion(1, 5)) + return; + + const char *hlsl = R"( + +static const SamplerState SamplerRef = SamplerDescriptorHeap[1]; + +Texture3D Tex3DTemplated ; +Texture3D Tex3d ; +Texture2D Tex2D ; +Texture2D Tex2DTemplated ; +StructuredBuffer StructBuf ; +Texture2DArray Tex2DArray ; +Buffer Buff ; + +static const struct +{ + float AFloat; + SamplerState Samp1; + Texture3D Tex1; + Texture3D Tex2; + Texture2D Tex3; + Texture2D Tex4; + StructuredBuffer Buff1; + Texture2DArray Tex5; + Buffer Buff2; + float AnotherFloat; +} View = { +1, +SamplerRef, +Tex3DTemplated, +Tex3d, +Tex2D, +Tex2DTemplated, +StructBuf, +Tex2DArray, +Buff, +2 +}; + +struct Payload +{ + int intValue; +}; + +RaytracingAccelerationStructure Scene : register(t0, space0); + +[shader("raygeneration")] +void main() +{ + RayDesc ray; + ray.Origin = float3(0,0,0); + ray.Direction = float3(0,0,1); + ray.TMin = 0.001; + ray.TMax = 10000.0; + Payload payload; + payload.intValue = View.AFloat; + TraceRay(Scene, RAY_FLAG_CULL_BACK_FACING_TRIANGLES, ~0, 0, 1, 0, ray, payload);} +)"; + + CComPtr pDiaDataSource; + CComPtr pDiaSession; + CompileAndRunAnnotationAndLoadDiaSource(m_dllSupport, hlsl, L"lib_6_6", + &pDiaDataSource); + + VERIFY_SUCCEEDED(pDiaDataSource->openSession(&pDiaSession)); +} + CComPtr PixTest::RunShaderAccessTrackingPass(IDxcBlob *blob) { CComPtr pOptimizer; VERIFY_SUCCEEDED( From 06baf973a147d92a36131243e104d44d961f2ec0 Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Thu, 12 Jan 2023 18:04:31 -0800 Subject: [PATCH 7/7] Adam's comments --- lib/DxilDia/DxilDiaSymbolManager.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/DxilDia/DxilDiaSymbolManager.cpp b/lib/DxilDia/DxilDiaSymbolManager.cpp index 284a27db2f..3efced6ffd 100644 --- a/lib/DxilDia/DxilDiaSymbolManager.cpp +++ b/lib/DxilDia/DxilDiaSymbolManager.cpp @@ -1571,15 +1571,12 @@ HRESULT dxil_dia::hlsl_symbols::SymbolManagerInit::CreateCompositeType(DWORD dwP for (llvm::DINode *N : CT->getElements()) { if (auto *Field = llvm::dyn_cast(N)) { DWORD dwUnusedFieldID; - if (Field->getName() == "MaterialTextureBilinearWrapedSampler") { - Field->getName(); - } IFR(CreateType(Field, &dwUnusedFieldID)); if (Field->getTag() == llvm::dwarf::DW_TAG_inheritance) { // The base class is a type of its own, so will have contributed to // its own TypeInfo. But we still need to remember the size that it // contributed to this type: - auto *DerivedType = llvm::dyn_cast(Field); + auto *DerivedType = llvm::cast(Field); const llvm::DITypeIdentifierMap EmptyMap; llvm::DIType *BaseType = DerivedType->getBaseType().resolve(EmptyMap); udtTI->AppendSize(getBaseClassSize(BaseType));