From a23d58663f9055dea1ddcb32ad4539eaccb08d5b Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Fri, 2 Feb 2024 09:44:32 -0800 Subject: [PATCH] WinPIX: Changes to support bitfields in PIX's shader debugger (#6219) Two related sets of changes: 1) Modify the value-to-declare pass to tolerate bitfields, which are detected by their habit of being smaller than the type in which they are contained. The pass produces dbg.declare statements that map each HLSL variable to some storage. With bitfields, that mapping becomes many-to-one, because many bitfields can be stored in the same basic type. Most of the changes are to preserve the size of the bitfield as the code descends the type hierarchy into the underlying basic type, which is what motivates these new "type overrride" parameters to some functions. Complications to watch out for are min16 types, which may or may not be their own purpose-made (16-bit) types depending on whether or not they are enabled. 2) Modify the PIX debug-info API to return enough data to PIX to allow it to extract (by bit shifting) a bitfield from the underlying type. This bit is also about preserving knowledge of the field's size as opposed to its containing type's size, hence the new dxcapi interface/method. --------- Co-authored-by: github-actions[bot] --- include/dxc/dxcpix.h | 11 + lib/DxilDia/DxcPixEntrypoints.cpp | 29 +- .../DxcPixLiveVariables_FragmentIterator.cpp | 2 +- lib/DxilDia/DxcPixTypes.cpp | 7 + lib/DxilDia/DxcPixTypes.h | 11 + .../DxilDbgValueToDbgDeclare.cpp | 170 +++++++---- tools/clang/unittests/HLSL/PixDiaTest.cpp | 289 +++++++++++++++++- 7 files changed, 437 insertions(+), 82 deletions(-) diff --git a/include/dxc/dxcpix.h b/include/dxc/dxcpix.h index 87edc974d..f1ab1447b 100644 --- a/include/dxc/dxcpix.h +++ b/include/dxc/dxcpix.h @@ -47,12 +47,23 @@ struct __declspec(uuid("9ba0d9d3-457b-426f-8019-9f3849982aa2")) IDxcPixArrayType }; struct __declspec(uuid("6c707d08-7995-4a84-bae5-e6d8291f3b78")) + IDxcPixStructField0 : public IUnknown { + virtual STDMETHODIMP GetName(_Outptr_result_z_ BSTR *Name) = 0; + + virtual STDMETHODIMP GetType(_COM_Outptr_ IDxcPixType **ppType) = 0; + + virtual STDMETHODIMP GetOffsetInBits(_Out_ DWORD *pOffsetInBits) = 0; +}; + +struct __declspec(uuid("de45597c-5869-4f97-a77b-d6650b9a16cf")) IDxcPixStructField : public IUnknown { virtual STDMETHODIMP GetName(_Outptr_result_z_ BSTR *Name) = 0; virtual STDMETHODIMP GetType(_COM_Outptr_ IDxcPixType **ppType) = 0; virtual STDMETHODIMP GetOffsetInBits(_Out_ DWORD *pOffsetInBits) = 0; + + virtual STDMETHODIMP GetFieldSizeInBits(_Out_ DWORD *pFieldSizeInBits) = 0; }; struct __declspec(uuid("24c08c44-684b-4b1c-b41b-f8772383d074")) diff --git a/lib/DxilDia/DxcPixEntrypoints.cpp b/lib/DxilDia/DxcPixEntrypoints.cpp index b32b795d1..6355ab846 100644 --- a/lib/DxilDia/DxcPixEntrypoints.cpp +++ b/lib/DxilDia/DxcPixEntrypoints.cpp @@ -232,7 +232,7 @@ HRESULT CreateEntrypointWrapper(IMalloc *pMalloc, IUnknown *pReal, REFIID iid, // Entrypoint is the base class for all entrypoints, providing // the default QueryInterface implementation, as well as a // more convenient way of calling SetupAndRun. -template class Entrypoint : public I { +template class Entrypoint : public I { protected: using IInterface = I; @@ -251,15 +251,16 @@ public: HRESULT STDMETHODCALLTYPE QueryInterface(REFIID iid, void **ppvObject) override final { - return SetupAndRun(m_pMalloc, - std::mem_fn(&Entrypoint::QueryInterfaceImpl), - ThisPtr(this), iid, CheckNotNull(OutParam(ppvObject))); + return SetupAndRun( + m_pMalloc, + std::mem_fn(&Entrypoint::QueryInterfaceImpl), + ThisPtr(this), iid, CheckNotNull(OutParam(ppvObject))); } HRESULT STDMETHODCALLTYPE QueryInterfaceImpl(REFIID iid, void **ppvObject) { // Special-casing so we don't need to create a new wrapper. - if (iid == __uuidof(IInterface) || iid == __uuidof(IUnknown) || - iid == __uuidof(INoMarshal)) { + if (iid == __uuidof(IInterface) || iid == __uuidof(IParent) || + iid == __uuidof(IUnknown) || iid == __uuidof(INoMarshal)) { this->AddRef(); *ppvObject = this; return S_OK; @@ -275,6 +276,11 @@ public: Name(IMalloc *M, IInterface *pI) : Entrypoint(M, pI) {} \ DXC_MICROCOM_TM_ALLOC(Name) +#define DEFINE_ENTRYPOINT_BOILERPLATE2(Name, ParentIFace) \ + Name(IMalloc *M, IInterface *pI) \ + : Entrypoint(M, pI) {} \ + DXC_MICROCOM_TM_ALLOC(Name) + struct IUnknownEntrypoint : public Entrypoint { DEFINE_ENTRYPOINT_BOILERPLATE(IUnknownEntrypoint); }; @@ -390,8 +396,10 @@ struct IDxcPixArrayTypeEntrypoint : public Entrypoint { }; DEFINE_ENTRYPOINT_WRAPPER_TRAIT(IDxcPixArrayType); -struct IDxcPixStructFieldEntrypoint : public Entrypoint { - DEFINE_ENTRYPOINT_BOILERPLATE(IDxcPixStructFieldEntrypoint); +struct IDxcPixStructFieldEntrypoint + : public Entrypoint { + DEFINE_ENTRYPOINT_BOILERPLATE2(IDxcPixStructFieldEntrypoint, + IDxcPixStructField0); STDMETHODIMP GetName(BSTR *Name) override { return InvokeOnReal(&IInterface::GetName, CheckNotNull(OutParam(Name))); @@ -401,6 +409,11 @@ struct IDxcPixStructFieldEntrypoint : public Entrypoint { return InvokeOnReal(&IInterface::GetType, CheckNotNull(OutParam(ppType))); } + STDMETHODIMP GetFieldSizeInBits(DWORD *pFieldSizeInBits) override { + return InvokeOnReal(&IInterface::GetFieldSizeInBits, + CheckNotNull(OutParam(pFieldSizeInBits))); + } + STDMETHODIMP GetOffsetInBits(DWORD *pOffsetInBits) override { return InvokeOnReal(&IInterface::GetOffsetInBits, CheckNotNull(OutParam(pOffsetInBits))); diff --git a/lib/DxilDia/DxcPixLiveVariables_FragmentIterator.cpp b/lib/DxilDia/DxcPixLiveVariables_FragmentIterator.cpp index d850c6d5e..de210633e 100644 --- a/lib/DxilDia/DxcPixLiveVariables_FragmentIterator.cpp +++ b/lib/DxilDia/DxcPixLiveVariables_FragmentIterator.cpp @@ -99,7 +99,7 @@ InitialOffsetInBitsFromDIExpression(const llvm::DataLayout &DataLayout, } FragmentOffsetInBits = Expression->getBitPieceOffset(); - assert(Expression->getBitPieceSize() == + assert(Expression->getBitPieceSize() <= DataLayout.getTypeAllocSizeInBits(Alloca->getAllocatedType())); } diff --git a/lib/DxilDia/DxcPixTypes.cpp b/lib/DxilDia/DxcPixTypes.cpp index 8b49ee494..30fa0b245 100644 --- a/lib/DxilDia/DxcPixTypes.cpp +++ b/lib/DxilDia/DxcPixTypes.cpp @@ -291,3 +291,10 @@ dxil_debug_info::DxcPixStructField::GetOffsetInBits(DWORD *pOffsetInBits) { *pOffsetInBits = m_pField->getOffsetInBits(); return S_OK; } + +STDMETHODIMP +dxil_debug_info::DxcPixStructField::GetFieldSizeInBits( + DWORD *pFieldSizeInBits) { + *pFieldSizeInBits = m_pField->getSizeInBits(); + return S_OK; +} diff --git a/lib/DxilDia/DxcPixTypes.h b/lib/DxilDia/DxcPixTypes.h index aaa9a5c1f..ab9d5c0b2 100644 --- a/lib/DxilDia/DxcPixTypes.h +++ b/lib/DxilDia/DxcPixTypes.h @@ -208,6 +208,9 @@ public: STDMETHODIMP GetBaseType(IDxcPixType **ppType) override; }; +struct __declspec(uuid("6c707d08-7995-4a84-bae5-e6d8291f3b78")) + PreviousDxcPixStructField {}; + class DxcPixStructField : public IDxcPixStructField { private: DXC_MICROCOM_TM_REF_FIELDS() @@ -228,6 +231,12 @@ public: HRESULT STDMETHODCALLTYPE QueryInterface(REFIID iid, void **ppvObject) override { + if (IsEqualIID(iid, __uuidof(PreviousDxcPixStructField))) { + *(IDxcPixStructField **)ppvObject = this; + this->AddRef(); + return S_OK; + } + return DoBasicQueryInterface(this, iid, ppvObject); } @@ -236,5 +245,7 @@ public: STDMETHODIMP GetType(IDxcPixType **ppType) override; STDMETHODIMP GetOffsetInBits(DWORD *pOffsetInBits) override; + + STDMETHODIMP GetFieldSizeInBits(DWORD *pFieldSizeInBits) override; }; } // namespace dxil_debug_info diff --git a/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp b/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp index 069f3483d..ea93d9968 100644 --- a/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp +++ b/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp @@ -51,6 +51,44 @@ using namespace llvm; namespace { using OffsetInBits = unsigned; using SizeInBits = unsigned; +struct Offsets { + OffsetInBits Aligned; + OffsetInBits Packed; +}; + +// DITypePeelTypeAlias peels const, typedef, and other alias types off of Ty, +// returning the unalised type. +static llvm::DIType *DITypePeelTypeAlias(llvm::DIType *Ty) { + if (auto *DerivedTy = llvm::dyn_cast(Ty)) { + 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: + case llvm::dwarf::DW_TAG_pointer_type: + return DITypePeelTypeAlias(DerivedTy->getBaseType().resolve(EmptyMap)); + case llvm::dwarf::DW_TAG_member: + return DITypePeelTypeAlias(DerivedTy->getBaseType().resolve(EmptyMap)); + } + } + + return Ty; +} + +llvm::DIBasicType *BaseTypeIfItIsBasicAndLarger(llvm::DIType *Ty) { + // Working around problems with bitfield size/alignment: + // For bitfield types, size may be < 32, but the underlying type + // will have the size of that basic type, e.g. 32 for ints. + // By contrast, for min16float, size will be 16, but align will be 16 or 32 + // depending on whether or not 16-bit is enabled. + // So if we find a disparity in size, we can assume it's not e.g. min16float. + auto *baseType = DITypePeelTypeAlias(Ty); + if (Ty->getSizeInBits() != 0 && + Ty->getSizeInBits() < baseType->getSizeInBits()) + return llvm::dyn_cast(baseType); + return nullptr; +} // OffsetManager is used to map between "packed" and aligned offsets. // @@ -63,28 +101,28 @@ using SizeInBits = unsigned; class OffsetManager { unsigned DescendTypeToGetAlignMask(llvm::DIType *Ty) { unsigned AlignMask = Ty->getAlignInBits(); - - auto *DerivedTy = llvm::dyn_cast(Ty); - if (DerivedTy != nullptr) { - // Working around a bug where byte size is stored instead of bit size - if (AlignMask == 4 && Ty->getSizeInBits() == 32) { - AlignMask = 32; - } - if (AlignMask == 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) { - if (baseType->getAlignInBits() == 0) { - (void)baseType->getAlignInBits(); - } - return DescendTypeToGetAlignMask(baseType); - } + if (BaseTypeIfItIsBasicAndLarger(Ty)) + AlignMask = 0; + else { + auto *DerivedTy = llvm::dyn_cast(Ty); + if (DerivedTy != nullptr) { + // Working around a bug where byte size is stored instead of bit size + if (AlignMask == 4 && Ty->getSizeInBits() == 32) { + AlignMask = 32; } + if (AlignMask == 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 DescendTypeToGetAlignMask(baseType); + } + } + } } } } @@ -128,7 +166,7 @@ public: // Add is used to "add" an aggregate element (struct field, array element) // at the current aligned/packed offsets, bumping them by Ty's size. - OffsetInBits Add(llvm::DIBasicType *Ty) { + Offsets Add(llvm::DIBasicType *Ty, unsigned sizeOverride) { VALUE_TO_DECLARE_LOG("Adding known type at aligned %d / packed %d, size %d", m_CurrentAlignedOffset, m_CurrentPackedOffset, Ty->getSizeInBits()); @@ -138,9 +176,10 @@ public: m_AlignedOffsetToPackedOffset[m_CurrentAlignedOffset] = m_CurrentPackedOffset; - const OffsetInBits Ret = m_CurrentAlignedOffset; - m_CurrentPackedOffset += Ty->getSizeInBits(); - m_CurrentAlignedOffset += Ty->getSizeInBits(); + const Offsets Ret = {m_CurrentAlignedOffset, m_CurrentPackedOffset}; + unsigned size = sizeOverride != 0 ? sizeOverride : Ty->getSizeInBits(); + m_CurrentPackedOffset += size; + m_CurrentAlignedOffset += size; return Ret; } @@ -230,7 +269,8 @@ public: private: void PopulateAllocaMap(llvm::DIType *Ty); - void PopulateAllocaMap_BasicType(llvm::DIBasicType *Ty); + void PopulateAllocaMap_BasicType(llvm::DIBasicType *Ty, + unsigned sizeOverride); void PopulateAllocaMap_ArrayType(llvm::DICompositeType *Ty); @@ -239,7 +279,8 @@ private: llvm::DILocation *GetVariableLocation() const; llvm::Value *GetMetadataAsValue(llvm::Metadata *M) const; llvm::DIExpression *GetDIExpression(llvm::DIType *Ty, OffsetInBits Offset, - SizeInBits ParentSize) const; + SizeInBits ParentSize, + unsigned sizeOverride) const; llvm::DebugLoc const &m_dbgLoc; llvm::DIVariable *m_Variable = nullptr; @@ -1054,6 +1095,8 @@ SizeInBits VariableRegisters::GetVariableSizeInbits(DIVariable *Var) { const llvm::DITypeIdentifierMap EmptyMap; DIType *Ty = Var->getType().resolve(EmptyMap); DIDerivedType *DerivedTy = nullptr; + if (BaseTypeIfItIsBasicAndLarger(Ty)) + return Ty->getSizeInBits(); while (Ty && (Ty->getSizeInBits() == 0 && (DerivedTy = dyn_cast(Ty)))) { Ty = DerivedTy->getBaseType().resolve(EmptyMap); @@ -1076,28 +1119,6 @@ VariableRegisters::GetRegisterForAlignedOffset(OffsetInBits Offset) const { return it->second; } -#ifndef NDEBUG -// DITypePeelTypeAlias peels const, typedef, and other alias types off of Ty, -// returning the unalised type. -static llvm::DIType *DITypePeelTypeAlias(llvm::DIType *Ty) { - if (auto *DerivedTy = llvm::dyn_cast(Ty)) { - 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: - case llvm::dwarf::DW_TAG_pointer_type: - return DITypePeelTypeAlias(DerivedTy->getBaseType().resolve(EmptyMap)); - case llvm::dwarf::DW_TAG_member: - return DITypePeelTypeAlias(DerivedTy->getBaseType().resolve(EmptyMap)); - } - } - - return Ty; -} -#endif // NDEBUG - VariableRegisters::VariableRegisters( llvm::DebugLoc const &dbgLoc, llvm::BasicBlock::iterator allocaInsertionPoint, llvm::DIVariable *Variable, @@ -1134,7 +1155,10 @@ void VariableRegisters::PopulateAllocaMap(llvm::DIType *Ty) { PopulateAllocaMap(DerivedTy->getBaseType().resolve(EmptyMap)); return; case llvm::dwarf::DW_TAG_member: - PopulateAllocaMap(DerivedTy->getBaseType().resolve(EmptyMap)); + if (auto *baseType = BaseTypeIfItIsBasicAndLarger(DerivedTy)) + PopulateAllocaMap_BasicType(baseType, DerivedTy->getSizeInBits()); + else + PopulateAllocaMap(DerivedTy->getBaseType().resolve(EmptyMap)); return; case llvm::dwarf::DW_TAG_subroutine_type: // ignore member functions. @@ -1164,7 +1188,7 @@ void VariableRegisters::PopulateAllocaMap(llvm::DIType *Ty) { return; } } else if (auto *BasicTy = llvm::dyn_cast(Ty)) { - PopulateAllocaMap_BasicType(BasicTy); + PopulateAllocaMap_BasicType(BasicTy, 0 /*no size override*/); return; } @@ -1207,24 +1231,28 @@ static llvm::Type *GetLLVMTypeFromDIBasicType(llvm::IRBuilder<> &B, return nullptr; } -void VariableRegisters::PopulateAllocaMap_BasicType(llvm::DIBasicType *Ty) { +void VariableRegisters::PopulateAllocaMap_BasicType(llvm::DIBasicType *Ty, + unsigned sizeOverride) { llvm::Type *AllocaElementTy = GetLLVMTypeFromDIBasicType(m_B, Ty); assert(AllocaElementTy != nullptr); if (AllocaElementTy == nullptr) { return; } - const OffsetInBits AlignedOffset = m_Offsets.Add(Ty); + const auto offsets = m_Offsets.Add(Ty, sizeOverride); llvm::Type *AllocaTy = llvm::ArrayType::get(AllocaElementTy, 1); - llvm::AllocaInst *&Alloca = m_AlignedOffsetToAlloca[AlignedOffset]; - Alloca = m_B.CreateAlloca(AllocaTy, m_B.getInt32(0)); - Alloca->setDebugLoc(llvm::DebugLoc()); + llvm::AllocaInst *&Alloca = m_AlignedOffsetToAlloca[offsets.Aligned]; + if (Alloca == nullptr) { + Alloca = m_B.CreateAlloca(AllocaTy, m_B.getInt32(0)); + Alloca->setDebugLoc(llvm::DebugLoc()); + } auto *Storage = GetMetadataAsValue(llvm::ValueAsMetadata::get(Alloca)); auto *Variable = GetMetadataAsValue(m_Variable); auto *Expression = GetMetadataAsValue( - GetDIExpression(Ty, AlignedOffset, GetVariableSizeInbits(m_Variable))); + GetDIExpression(Ty, sizeOverride == 0 ? offsets.Aligned : offsets.Packed, + GetVariableSizeInbits(m_Variable), sizeOverride)); auto *DbgDeclare = m_B.CreateCall(m_DbgDeclareFn, {Storage, Variable, Expression}); DbgDeclare->setDebugLoc(m_dbgLoc); @@ -1299,14 +1327,22 @@ void VariableRegisters::PopulateAllocaMap_StructType( // should always result in the current aligned offset being the // same as the member's offset. m_Offsets.AlignTo(OffsetAndMember.second); - assert(m_Offsets.GetCurrentAlignedOffset() == - StructStart + OffsetAndMember.first && - "Offset mismatch in DIStructType"); - if (IsResourceObject(OffsetAndMember.second)) { - m_Offsets.AddResourceType(OffsetAndMember.second); + if (BaseTypeIfItIsBasicAndLarger(OffsetAndMember.second)) { + // This is the bitfields case (i.e. a field that is smaller + // than the type in which it resides). If we were to take + // the base type, then the information about the member's + // size would be lost + PopulateAllocaMap(OffsetAndMember.second); } else { - PopulateAllocaMap( - OffsetAndMember.second->getBaseType().resolve(EmptyMap)); + assert(m_Offsets.GetCurrentAlignedOffset() == + StructStart + OffsetAndMember.first && + "Offset mismatch in DIStructType"); + if (IsResourceObject(OffsetAndMember.second)) { + m_Offsets.AddResourceType(OffsetAndMember.second); + } else { + PopulateAllocaMap( + OffsetAndMember.second->getBaseType().resolve(EmptyMap)); + } } } } @@ -1330,12 +1366,14 @@ llvm::Value *VariableRegisters::GetMetadataAsValue(llvm::Metadata *M) const { llvm::DIExpression * VariableRegisters::GetDIExpression(llvm::DIType *Ty, OffsetInBits Offset, - SizeInBits ParentSize) const { + SizeInBits ParentSize, + unsigned sizeOverride) const { llvm::SmallVector ExpElements; if (Offset != 0 || Ty->getSizeInBits() != ParentSize) { ExpElements.emplace_back(llvm::dwarf::DW_OP_bit_piece); ExpElements.emplace_back(Offset); - ExpElements.emplace_back(Ty->getSizeInBits()); + ExpElements.emplace_back(sizeOverride != 0 ? sizeOverride + : Ty->getSizeInBits()); } return llvm::DIExpression::get(m_B.getContext(), ExpElements); } diff --git a/tools/clang/unittests/HLSL/PixDiaTest.cpp b/tools/clang/unittests/HLSL/PixDiaTest.cpp index dede2386d..033ecfbdc 100644 --- a/tools/clang/unittests/HLSL/PixDiaTest.cpp +++ b/tools/clang/unittests/HLSL/PixDiaTest.cpp @@ -12,6 +12,8 @@ // This whole file is win32-only #ifdef _WIN32 +#include + #include "dxc/DxilContainer/DxilContainer.h" #include "dxc/Support/WinIncludes.h" @@ -176,6 +178,14 @@ public: TEST_METHOD(DxcPixDxilDebugInfo_UnnamedField) TEST_METHOD(DxcPixDxilDebugInfo_SubProgramsInNamespaces) TEST_METHOD(DxcPixDxilDebugInfo_SubPrograms) + TEST_METHOD(DxcPixDxilDebugInfo_Alignment_ConstInt) + TEST_METHOD(DxcPixDxilDebugInfo_QIOldFieldInterface) + TEST_METHOD(DxcPixDxilDebugInfo_BitFields_Simple) + TEST_METHOD(DxcPixDxilDebugInfo_BitFields_Derived) + TEST_METHOD(DxcPixDxilDebugInfo_BitFields_Bool) + TEST_METHOD(DxcPixDxilDebugInfo_BitFields_Overlap) + TEST_METHOD(DxcPixDxilDebugInfo_Min16SizesAndOffsets_Enabled) + TEST_METHOD(DxcPixDxilDebugInfo_Min16SizesAndOffsets_Disabled) TEST_METHOD( DxcPixDxilDebugInfo_VariableScopes_InlinedFunctions_TwiceInlinedFunctions) TEST_METHOD( @@ -627,12 +637,13 @@ public: void CompileAndRunAnnotationAndGetDebugPart( dxc::DxcDllSupport &dllSupport, const char *source, const wchar_t *profile, IDxcIncludeHandler *includer, - IDxcBlob **ppDebugPart, std::vector extraArgs = {}); + IDxcBlob **ppDebugPart, + std::vector extraArgs = {L"-Od"}); void CompileAndRunAnnotationAndLoadDiaSource( dxc::DxcDllSupport &dllSupport, const char *source, const wchar_t *profile, IDxcIncludeHandler *includer, IDiaDataSource **ppDataSource, - std::vector extraArgs = {}); + std::vector extraArgs = {L"-Od"}); struct VariableComponentInfo { std::wstring Name; @@ -643,10 +654,15 @@ public: 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"}); DebuggerInterfaces CompileAndCreateDxcDebug(const char *hlsl, const wchar_t *profile, - IDxcIncludeHandler *includer = nullptr); + IDxcIncludeHandler *includer = nullptr, + std::vector extraArgs = {L"-Od"}); CComPtr GetLiveVariablesAt(const char *hlsl, @@ -777,11 +793,12 @@ void PixDiaTest::CompileAndRunAnnotationAndGetDebugPart( DebuggerInterfaces PixDiaTest::CompileAndCreateDxcDebug(const char *hlsl, const wchar_t *profile, - IDxcIncludeHandler *includer) { + IDxcIncludeHandler *includer, + std::vector extraArgs) { CComPtr pDiaDataSource; CompileAndRunAnnotationAndLoadDiaSource(m_dllSupport, hlsl, profile, includer, - &pDiaDataSource, {L"-Od"}); + &pDiaDataSource, extraArgs); CComPtr session; VERIFY_SUCCEEDED(pDiaDataSource->openSession(&session)); @@ -2740,6 +2757,263 @@ void main() RunSubProgramsCase(hlsl); } +TEST_F(PixDiaTest, DxcPixDxilDebugInfo_QIOldFieldInterface) { + const char *hlsl = R"( +struct Struct +{ + unsigned int first; + unsigned int second; +}; + +RWStructuredBuffer UAV: register(u0); + +[numthreads(1, 1, 1)] +void main() +{ + Struct s; + s.second = UAV[0]; + UAV[16] = s.second; //STOP_HERE +} +)"; + + auto debugInfo = CompileAndCreateDxcDebug(hlsl, L"cs_6_5", nullptr).debugInfo; + auto live = GetLiveVariablesAt(hlsl, "STOP_HERE", debugInfo); + CComPtr bf; + VERIFY_SUCCEEDED(live->GetVariableByName(L"s", &bf)); + CComPtr bfType; + VERIFY_SUCCEEDED(bf->GetType(&bfType)); + CComPtr bfStructType; + VERIFY_SUCCEEDED(bfType->QueryInterface(IID_PPV_ARGS(&bfStructType))); + CComPtr field; + VERIFY_SUCCEEDED(bfStructType->GetFieldByIndex(1, &field)); + CComPtr mike; + VERIFY_SUCCEEDED(field->QueryInterface(IID_PPV_ARGS(&mike))); + DWORD secondFieldOffset = 0; + VERIFY_SUCCEEDED(mike->GetOffsetInBits(&secondFieldOffset)); + VERIFY_ARE_EQUAL(32, 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; + auto debugInfo = + CompileAndCreateDxcDebug(hlsl, L"cs_6_5", nullptr, extraArgs).debugInfo; + auto live = GetLiveVariablesAt(hlsl, "STOP_HERE", debugInfo); + CComPtr bf; + VERIFY_SUCCEEDED(live->GetVariableByName(L"bf", &bf)); + CComPtr bfType; + VERIFY_SUCCEEDED(bf->GetType(&bfType)); + CComPtr bfStructType; + VERIFY_SUCCEEDED(bfType->QueryInterface(IID_PPV_ARGS(&bfStructType))); + const wchar_t *memberNames[] = {L"first", L"second", L"third", L"fourth"}; + for (size_t i = 0; i < memberOffsets.size(); ++i) { + CComPtr field; + VERIFY_SUCCEEDED( + bfStructType->GetFieldByIndex(static_cast(i), &field)); + DWORD offsetInBits = 0; + VERIFY_SUCCEEDED(field->GetOffsetInBits(&offsetInBits)); + VERIFY_ARE_EQUAL(memberOffsets[i], offsetInBits); + DWORD sizeInBits = 0; + VERIFY_SUCCEEDED(field->GetFieldSizeInBits(&sizeInBits)); + VERIFY_ARE_EQUAL(memberSizes[i], sizeInBits); + } +} + +TEST_F(PixDiaTest, DxcPixDxilDebugInfo_BitFields_Simple) { + const char *hlsl = R"( +struct Bitfields +{ + unsigned int first : 17; + unsigned int second : 15; // consume all 32 bits of first dword + unsigned int third : 3; // should be at bit offset 32 + unsigned int fourth; // should be at bit offset 64 +}; + +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 +} + +)"; + RunSizeAndOffsetTestCase(hlsl, {0, 17, 32, 64}, {17, 15, 3, 32}); +} + +TEST_F(PixDiaTest, DxcPixDxilDebugInfo_BitFields_Derived) { + const char *hlsl = R"( +struct Bitfields +{ + uint first : 17; + uint second : 15; // consume all 32 bits of first dword + uint third : 3; // should be at bit offset 32 + uint fourth; // should be at bit offset 64 +}; + +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 +} + +)"; + RunSizeAndOffsetTestCase(hlsl, {0, 17, 32, 64}, {17, 15, 3, 32}); +} + +TEST_F(PixDiaTest, DxcPixDxilDebugInfo_BitFields_Bool) { + const char *hlsl = R"( +struct Bitfields +{ + bool first : 1; + bool second : 1; + bool third : 3; // just to be weird + uint fourth; // should be at bit offset 64 +}; + +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 +} + +)"; + RunSizeAndOffsetTestCase(hlsl, {0, 1, 2, 32}, {1, 1, 3, 32}); +} + +TEST_F(PixDiaTest, DxcPixDxilDebugInfo_BitFields_Overlap) { + 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 +}; + +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 +} + +)"; + RunSizeAndOffsetTestCase(hlsl, {0, 32, 52, 64}, {20, 20, 3, 32}); +} + +TEST_F(PixDiaTest, DxcPixDxilDebugInfo_Alignment_ConstInt) { + if (m_ver.SkipDxilVersion(1, 5)) + return; + + const char *hlsl = R"( + +RWStructuredBuffer UAV: register(u0); + +[numthreads(1, 1, 1)] +void main() +{ + const uint c = UAV[0]; + UAV[16] = c; +} + +)"; + CComPtr pDiaDataSource; + CompileAndRunAnnotationAndLoadDiaSource(m_dllSupport, hlsl, L"cs_6_5", + nullptr, &pDiaDataSource, {L"-Od"}); +} + +TEST_F(PixDiaTest, DxcPixDxilDebugInfo_Min16SizesAndOffsets_Enabled) { + if (m_ver.SkipDxilVersion(1, 5)) + return; + + const char *hlsl = R"( +struct Mins +{ + min16uint first; + min16int second; + min12int third; + min16float fourth; +}; + +RWStructuredBuffer UAV: register(u0); + +[numthreads(1, 1, 1)] +void main() +{ + Mins 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 +} + + +)"; + RunSizeAndOffsetTestCase(hlsl, {0, 16, 32, 48}, {16, 16, 16, 16}, + {L"-Od", L"-enable-16bit-types"}); +} + +TEST_F(PixDiaTest, DxcPixDxilDebugInfo_Min16SizesAndOffsets_Disabled) { + if (m_ver.SkipDxilVersion(1, 5)) + return; + + const char *hlsl = R"( +struct Mins +{ + min16uint first; + min16int second; + min12int third; + min16float fourth; +}; + +RWStructuredBuffer UAV: register(u0); + +[numthreads(1, 1, 1)] +void main() +{ + Mins 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 +} + + +)"; + RunSizeAndOffsetTestCase(hlsl, {0, 32, 64, 96}, {16, 16, 16, 16}, {L"-Od"}); +} + TEST_F(PixDiaTest, DxcPixDxilDebugInfo_SubProgramsInNamespaces) { if (m_ver.SkipDxilVersion(1, 2)) return; @@ -3006,7 +3280,8 @@ void ClosestHitShader3(inout RayPayload payload, in BuiltInTriangleIntersectionA dxilDebugger, instructionOffset, L"InlinedFunction"); DWORD callsite0 = GetRegisterNumberForVariable( dxilDebugger, instructionOffset, L"ret", L"x"); - // advance until we're out of InlinedFunction before we call it a second time + // advance until we're out of InlinedFunction before we call it a second + // time instructionOffset = AdvanceUntilFunctionEntered( dxilDebugger, instructionOffset, L"ClosestHitShader3"); instructionOffset = AdvanceUntilFunctionEntered(