From 0abacfcb1e5b0602cd5b535cd224768028337077 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Thu, 24 Apr 2025 21:54:56 +0000 Subject: [PATCH 01/35] adding support for Root Descriptors --- llvm/include/llvm/BinaryFormat/DXContainer.h | 28 ++++- .../BinaryFormat/DXContainerConstants.def | 15 +++ .../llvm/MC/DXContainerRootSignature.h | 4 +- llvm/include/llvm/Object/DXContainer.h | 47 +++++++- .../include/llvm/ObjectYAML/DXContainerYAML.h | 34 +++++- llvm/lib/MC/DXContainerRootSignature.cpp | 25 +++++ llvm/lib/ObjectYAML/DXContainerEmitter.cpp | 17 +++ llvm/lib/ObjectYAML/DXContainerYAML.cpp | 53 ++++++++- .../RootSignature-Descriptor1.0.yaml | 45 ++++++++ .../RootSignature-Descriptor1.1.yaml | 47 ++++++++ .../RootSignature-MultipleParameters.yaml | 20 +++- llvm/unittests/Object/DXContainerTest.cpp | 91 ++++++++++++++++ .../ObjectYAML/DXContainerYAMLTest.cpp | 103 ++++++++++++++++++ 13 files changed, 514 insertions(+), 15 deletions(-) create mode 100644 llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.0.yaml create mode 100644 llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.1.yaml diff --git a/llvm/include/llvm/BinaryFormat/DXContainer.h b/llvm/include/llvm/BinaryFormat/DXContainer.h index 455657980bf40..439bf7b40f31b 100644 --- a/llvm/include/llvm/BinaryFormat/DXContainer.h +++ b/llvm/include/llvm/BinaryFormat/DXContainer.h @@ -158,6 +158,11 @@ enum class RootElementFlag : uint32_t { #include "DXContainerConstants.def" }; +#define ROOT_DESCRIPTOR_FLAG(Num, Val) Val = 1ull << Num, +enum class RootDescriptorFlag : uint32_t { +#include "DXContainerConstants.def" +}; + #define ROOT_PARAMETER(Val, Enum) Enum = Val, enum class RootParameterType : uint32_t { #include "DXContainerConstants.def" @@ -422,7 +427,6 @@ struct SignatureElement { static_assert(sizeof(SignatureElement) == 4 * sizeof(uint32_t), "PSV Signature elements must fit in 16 bytes."); - } // namespace v0 namespace v1 { @@ -463,7 +467,6 @@ struct RuntimeInfo : public v0::RuntimeInfo { sys::swapByteOrder(GeomData.MaxVertexCount); } }; - } // namespace v1 namespace v2 { @@ -580,7 +583,28 @@ struct ProgramSignatureElement { static_assert(sizeof(ProgramSignatureElement) == 32, "ProgramSignatureElement is misaligned"); +namespace RST0 { +namespace v0 { +struct RootDescriptor { + uint32_t ShaderRegister; + uint32_t RegisterSpace; + void swapBytes() { + sys::swapByteOrder(ShaderRegister); + sys::swapByteOrder(RegisterSpace); + } +}; +} // namespace v0 +namespace v1 { +struct RootDescriptor : public v0::RootDescriptor { + uint32_t Flags; + void swapBytes() { + v0::RootDescriptor::swapBytes(); + sys::swapByteOrder(Flags); + } +}; +} // namespace v1 +} // namespace RST0 // following dx12 naming // https://learn.microsoft.com/en-us/windows/win32/api/d3d12/ns-d3d12-d3d12_root_constants struct RootConstants { diff --git a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def index 590ded5e8c899..bd9bd760547dc 100644 --- a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def +++ b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def @@ -72,9 +72,24 @@ ROOT_ELEMENT_FLAG(11, SamplerHeapDirectlyIndexed) #undef ROOT_ELEMENT_FLAG #endif // ROOT_ELEMENT_FLAG + +// ROOT_ELEMENT_FLAG(bit offset for the flag, name). +#ifdef ROOT_DESCRIPTOR_FLAG + +ROOT_DESCRIPTOR_FLAG(0, NONE) +ROOT_DESCRIPTOR_FLAG(1, DATA_VOLATILE) +ROOT_DESCRIPTOR_FLAG(2, DATA_STATIC_WHILE_SET_AT_EXECUTE) +ROOT_DESCRIPTOR_FLAG(3, DATA_STATIC) +#undef ROOT_DESCRIPTOR_FLAG +#endif // ROOT_DESCRIPTOR_FLAG + + #ifdef ROOT_PARAMETER ROOT_PARAMETER(1, Constants32Bit) +ROOT_PARAMETER(2, CBV) +ROOT_PARAMETER(3, SRV) +ROOT_PARAMETER(4, UAV) #undef ROOT_PARAMETER #endif // ROOT_PARAMETER diff --git a/llvm/include/llvm/MC/DXContainerRootSignature.h b/llvm/include/llvm/MC/DXContainerRootSignature.h index fee799249b255..ac062a375818c 100644 --- a/llvm/include/llvm/MC/DXContainerRootSignature.h +++ b/llvm/include/llvm/MC/DXContainerRootSignature.h @@ -19,13 +19,15 @@ struct RootParameter { dxbc::RootParameterHeader Header; union { dxbc::RootConstants Constants; + dxbc::RST0::v0::RootDescriptor Descriptor_V10; + dxbc::RST0::v1::RootDescriptor Descriptor_V11; }; }; struct RootSignatureDesc { uint32_t Version = 2U; uint32_t Flags = 0U; - uint32_t RootParameterOffset = 0U; + uint32_t RootParameterOffset = 24U; uint32_t StaticSamplersOffset = 0u; uint32_t NumStaticSamplers = 0u; SmallVector Parameters; diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h index e8287ce078365..ba261a9e42aea 100644 --- a/llvm/include/llvm/Object/DXContainer.h +++ b/llvm/include/llvm/Object/DXContainer.h @@ -120,9 +120,10 @@ template struct ViewArray { namespace DirectX { struct RootParameterView { const dxbc::RootParameterHeader &Header; + uint32_t Version; StringRef ParamData; - RootParameterView(const dxbc::RootParameterHeader &H, StringRef P) - : Header(H), ParamData(P) {} + RootParameterView(uint32_t V, const dxbc::RootParameterHeader &H, StringRef P) + : Header(H), Version(V), ParamData(P) {} template Expected readParameter() { T Struct; @@ -149,6 +150,38 @@ struct RootConstantView : RootParameterView { } }; +struct RootDescriptorView_V1_0 : RootParameterView { + static bool classof(const RootParameterView *V) { + return (V->Version == 1 && + (V->Header.ParameterType == + llvm::to_underlying(dxbc::RootParameterType::CBV) || + V->Header.ParameterType == + llvm::to_underlying(dxbc::RootParameterType::SRV) || + V->Header.ParameterType == + llvm::to_underlying(dxbc::RootParameterType::UAV))); + } + + llvm::Expected read() { + return readParameter(); + } +}; + +struct RootDescriptorView_V1_1 : RootParameterView { + static bool classof(const RootParameterView *V) { + return (V->Version == 2 && + (V->Header.ParameterType == + llvm::to_underlying(dxbc::RootParameterType::CBV) || + V->Header.ParameterType == + llvm::to_underlying(dxbc::RootParameterType::SRV) || + V->Header.ParameterType == + llvm::to_underlying(dxbc::RootParameterType::UAV))); + } + + llvm::Expected read() { + return readParameter(); + } +}; + static Error parseFailed(const Twine &Msg) { return make_error(Msg.str(), object_error::parse_failed); } @@ -192,6 +225,14 @@ class RootSignature { case dxbc::RootParameterType::Constants32Bit: DataSize = sizeof(dxbc::RootConstants); break; + case dxbc::RootParameterType::CBV: + case dxbc::RootParameterType::SRV: + case dxbc::RootParameterType::UAV: + if (Version == 1) + DataSize = sizeof(dxbc::RST0::v0::RootDescriptor); + else + DataSize = sizeof(dxbc::RST0::v1::RootDescriptor); + break; } size_t EndOfSectionByte = getNumStaticSamplers() == 0 ? PartData.size() @@ -201,7 +242,7 @@ class RootSignature { return parseFailed("Reading structure out of file bounds"); StringRef Buff = PartData.substr(Header.ParameterOffset, DataSize); - RootParameterView View = RootParameterView(Header, Buff); + RootParameterView View = RootParameterView(Version, Header, Buff); return View; } }; diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h index 393bba9c79bf8..e86a869da99bc 100644 --- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h +++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h @@ -73,24 +73,50 @@ struct ShaderHash { std::vector Digest; }; -#define ROOT_ELEMENT_FLAG(Num, Val) bool Val = false; - struct RootConstantsYaml { uint32_t ShaderRegister; uint32_t RegisterSpace; uint32_t Num32BitValues; }; +#define ROOT_DESCRIPTOR_FLAG(Num, Val) bool Val = false; +struct RootDescriptorYaml { + RootDescriptorYaml() = default; + + uint32_t ShaderRegister; + uint32_t RegisterSpace; + + uint32_t getEncodedFlags() const; + +#include "llvm/BinaryFormat/DXContainerConstants.def" +}; + struct RootParameterYamlDesc { uint32_t Type; uint32_t Visibility; uint32_t Offset; + RootParameterYamlDesc(){}; + RootParameterYamlDesc(uint32_t T) : Type(T) { + switch (T) { + + case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): + Constants = RootConstantsYaml(); + break; + case llvm::to_underlying(dxbc::RootParameterType::CBV): + case llvm::to_underlying(dxbc::RootParameterType::SRV): + case llvm::to_underlying(dxbc::RootParameterType::UAV): + Descriptor = RootDescriptorYaml(); + break; + } + } union { RootConstantsYaml Constants; + RootDescriptorYaml Descriptor; }; }; +#define ROOT_ELEMENT_FLAG(Num, Val) bool Val = false; struct RootSignatureYamlDesc { RootSignatureYamlDesc() = default; @@ -298,6 +324,10 @@ template <> struct MappingTraits { static void mapping(IO &IO, llvm::DXContainerYAML::RootConstantsYaml &C); }; +template <> struct MappingTraits { + static void mapping(IO &IO, llvm::DXContainerYAML::RootDescriptorYaml &D); +}; + } // namespace yaml } // namespace llvm diff --git a/llvm/lib/MC/DXContainerRootSignature.cpp b/llvm/lib/MC/DXContainerRootSignature.cpp index c2731d95c955e..a5210f4768f16 100644 --- a/llvm/lib/MC/DXContainerRootSignature.cpp +++ b/llvm/lib/MC/DXContainerRootSignature.cpp @@ -37,6 +37,15 @@ size_t RootSignatureDesc::getSize() const { case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): Size += sizeof(dxbc::RootConstants); break; + case llvm::to_underlying(dxbc::RootParameterType::CBV): + case llvm::to_underlying(dxbc::RootParameterType::SRV): + case llvm::to_underlying(dxbc::RootParameterType::UAV): + if (Version == 1) + Size += sizeof(dxbc::RST0::v0::RootDescriptor); + else + Size += sizeof(dxbc::RST0::v1::RootDescriptor); + + break; } } return Size; @@ -80,6 +89,22 @@ void RootSignatureDesc::write(raw_ostream &OS) const { support::endian::write(BOS, P.Constants.Num32BitValues, llvm::endianness::little); break; + case llvm::to_underlying(dxbc::RootParameterType::CBV): + case llvm::to_underlying(dxbc::RootParameterType::SRV): + case llvm::to_underlying(dxbc::RootParameterType::UAV): + if (Version == 1) { + support::endian::write(BOS, P.Descriptor_V10.ShaderRegister, + llvm::endianness::little); + support::endian::write(BOS, P.Descriptor_V10.RegisterSpace, + llvm::endianness::little); + } else { + support::endian::write(BOS, P.Descriptor_V11.ShaderRegister, + llvm::endianness::little); + support::endian::write(BOS, P.Descriptor_V11.RegisterSpace, + llvm::endianness::little); + support::endian::write(BOS, P.Descriptor_V11.Flags, + llvm::endianness::little); + } } } assert(Storage.size() == getSize()); diff --git a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp index 86e24eae4abc6..be0e52fef04f5 100644 --- a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp +++ b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp @@ -283,6 +283,23 @@ void DXContainerWriter::writeParts(raw_ostream &OS) { NewParam.Constants.Num32BitValues = Param.Constants.Num32BitValues; NewParam.Constants.RegisterSpace = Param.Constants.RegisterSpace; NewParam.Constants.ShaderRegister = Param.Constants.ShaderRegister; + break; + case llvm::to_underlying(dxbc::RootParameterType::SRV): + case llvm::to_underlying(dxbc::RootParameterType::UAV): + case llvm::to_underlying(dxbc::RootParameterType::CBV): + if (RS.Version == 1) { + NewParam.Descriptor_V10.RegisterSpace = + Param.Descriptor.RegisterSpace; + NewParam.Descriptor_V10.ShaderRegister = + Param.Descriptor.ShaderRegister; + } else { + NewParam.Descriptor_V11.RegisterSpace = + Param.Descriptor.RegisterSpace; + NewParam.Descriptor_V11.ShaderRegister = + Param.Descriptor.ShaderRegister; + NewParam.Descriptor_V11.Flags = Param.Descriptor.getEncodedFlags(); + } + break; } diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp index 59914fe30082d..ef86da85989e6 100644 --- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp +++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp @@ -15,6 +15,7 @@ #include "llvm/ADT/STLForwardCompat.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/BinaryFormat/DXContainer.h" +#include "llvm/Object/DXContainer.h" #include "llvm/Support/Error.h" #include "llvm/Support/ScopedPrinter.h" #include @@ -48,13 +49,12 @@ DXContainerYAML::RootSignatureYamlDesc::create( uint32_t Flags = Data.getFlags(); for (const dxbc::RootParameterHeader &PH : Data.param_headers()) { - RootParameterYamlDesc NewP; - NewP.Offset = PH.ParameterOffset; - if (!dxbc::isValidParameterType(PH.ParameterType)) return createStringError(std::errc::invalid_argument, "Invalid value for parameter type"); + RootParameterYamlDesc NewP(PH.ParameterType); + NewP.Offset = PH.ParameterOffset; NewP.Type = PH.ParameterType; if (!dxbc::isValidShaderVisibility(PH.ShaderVisibility)) @@ -79,7 +79,32 @@ DXContainerYAML::RootSignatureYamlDesc::create( NewP.Constants.Num32BitValues = Constants.Num32BitValues; NewP.Constants.ShaderRegister = Constants.ShaderRegister; NewP.Constants.RegisterSpace = Constants.RegisterSpace; + } else if (auto *RDV = dyn_cast( + &ParamView)) { + llvm::Expected DescriptorOrErr = + RDV->read(); + if (Error E = DescriptorOrErr.takeError()) + return std::move(E); + auto Descriptor = *DescriptorOrErr; + + NewP.Descriptor.ShaderRegister = Descriptor.ShaderRegister; + NewP.Descriptor.RegisterSpace = Descriptor.RegisterSpace; + } else if (auto *RDV = dyn_cast( + &ParamView)) { + llvm::Expected DescriptorOrErr = + RDV->read(); + if (Error E = DescriptorOrErr.takeError()) + return std::move(E); + auto Descriptor = *DescriptorOrErr; + NewP.Descriptor.ShaderRegister = Descriptor.ShaderRegister; + NewP.Descriptor.RegisterSpace = Descriptor.RegisterSpace; +#define ROOT_DESCRIPTOR_FLAG(Num, Val) \ + NewP.Descriptor.Val = \ + (Descriptor.Flags & \ + llvm::to_underlying(dxbc::RootDescriptorFlag::Val)) > 0; +#include "llvm/BinaryFormat/DXContainerConstants.def" } + RootSigDesc.Parameters.push_back(NewP); } #define ROOT_ELEMENT_FLAG(Num, Val) \ @@ -89,6 +114,15 @@ DXContainerYAML::RootSignatureYamlDesc::create( return RootSigDesc; } +uint32_t DXContainerYAML::RootDescriptorYaml::getEncodedFlags() const { + uint64_t Flag = 0; +#define ROOT_DESCRIPTOR_FLAG(Num, Val) \ + if (Val) \ + Flag |= (uint32_t)dxbc::RootDescriptorFlag::Val; +#include "llvm/BinaryFormat/DXContainerConstants.def" + return Flag; +} + uint32_t DXContainerYAML::RootSignatureYamlDesc::getEncodedFlags() { uint64_t Flag = 0; #define ROOT_ELEMENT_FLAG(Num, Val) \ @@ -276,6 +310,14 @@ void MappingTraits::mapping( IO.mapRequired("ShaderRegister", C.ShaderRegister); } +void MappingTraits::mapping( + IO &IO, llvm::DXContainerYAML::RootDescriptorYaml &D) { + IO.mapRequired("RegisterSpace", D.RegisterSpace); + IO.mapRequired("ShaderRegister", D.ShaderRegister); +#define ROOT_DESCRIPTOR_FLAG(Num, Val) IO.mapOptional(#Val, D.Val, false); +#include "llvm/BinaryFormat/DXContainerConstants.def" +} + void MappingTraits::mapping( IO &IO, llvm::DXContainerYAML::RootParameterYamlDesc &P) { IO.mapRequired("ParameterType", P.Type); @@ -285,6 +327,11 @@ void MappingTraits::mapping( case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): IO.mapRequired("Constants", P.Constants); break; + case llvm::to_underlying(dxbc::RootParameterType::CBV): + case llvm::to_underlying(dxbc::RootParameterType::SRV): + case llvm::to_underlying(dxbc::RootParameterType::UAV): + IO.mapRequired("Descriptor", P.Descriptor); + break; } } diff --git a/llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.0.yaml b/llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.0.yaml new file mode 100644 index 0000000000000..46cdf416ffcae --- /dev/null +++ b/llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.0.yaml @@ -0,0 +1,45 @@ +# RUN: yaml2obj %s | obj2yaml | FileCheck %s + +--- !dxcontainer +Header: + Hash: [ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, + 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 ] + Version: + Major: 1 + Minor: 0 + PartCount: 1 + PartOffsets: [ 60 ] +Parts: + - Name: RTS0 + Size: 96 + RootSignature: + Version: 1 + NumRootParameters: 1 + RootParametersOffset: 24 + NumStaticSamplers: 0 + StaticSamplersOffset: 60 + Parameters: + - ParameterType: 2 # SRV + ShaderVisibility: 3 # Domain + Descriptor: + ShaderRegister: 31 + RegisterSpace: 32 + AllowInputAssemblerInputLayout: true + DenyGeometryShaderRootAccess: true + +# CHECK: - Name: RTS0 +# CHECK-NEXT: Size: 96 +# CHECK-NEXT: RootSignature: +# CHECK-NEXT: Version: 1 +# CHECK-NEXT: NumRootParameters: 1 +# CHECK-NEXT: RootParametersOffset: 24 +# CHECK-NEXT: NumStaticSamplers: 0 +# CHECK-NEXT: StaticSamplersOffset: 60 +# CHECK-NEXT: Parameters: +# CHECK-NEXT: - ParameterType: 2 +# CHECK-NEXT: ShaderVisibility: 3 +# CHECK-NEXT: Descriptor: +# CHECK-NEXT: RegisterSpace: 32 +# CHECK-NEXT: ShaderRegister: 31 +# CHECK-NEXT: AllowInputAssemblerInputLayout: true +# CHECK-NEXT: DenyGeometryShaderRootAccess: true diff --git a/llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.1.yaml b/llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.1.yaml new file mode 100644 index 0000000000000..64e01c6836e32 --- /dev/null +++ b/llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.1.yaml @@ -0,0 +1,47 @@ +# RUN: yaml2obj %s | obj2yaml | FileCheck %s + +--- !dxcontainer +Header: + Hash: [ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, + 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 ] + Version: + Major: 1 + Minor: 0 + PartCount: 1 + PartOffsets: [ 60 ] +Parts: + - Name: RTS0 + Size: 89 + RootSignature: + Version: 2 + NumRootParameters: 1 + RootParametersOffset: 24 + NumStaticSamplers: 0 + StaticSamplersOffset: 60 + Parameters: + - ParameterType: 2 # SRV + ShaderVisibility: 3 # Domain + Descriptor: + ShaderRegister: 31 + RegisterSpace: 32 + DATA_STATIC_WHILE_SET_AT_EXECUTE: true + AllowInputAssemblerInputLayout: true + DenyGeometryShaderRootAccess: true + +# CHECK: - Name: RTS0 +# CHECK-NEXT: Size: 89 +# CHECK-NEXT: RootSignature: +# CHECK-NEXT: Version: 2 +# CHECK-NEXT: NumRootParameters: 1 +# CHECK-NEXT: RootParametersOffset: 24 +# CHECK-NEXT: NumStaticSamplers: 0 +# CHECK-NEXT: StaticSamplersOffset: 60 +# CHECK-NEXT: Parameters: +# CHECK-NEXT: - ParameterType: 2 +# CHECK-NEXT: ShaderVisibility: 3 +# CHECK-NEXT: Descriptor: +# CHECK-NEXT: RegisterSpace: 32 +# CHECK-NEXT: ShaderRegister: 31 +# CHECK-NEXT: DATA_STATIC_WHILE_SET_AT_EXECUTE: true +# CHECK-NEXT: AllowInputAssemblerInputLayout: true +# CHECK-NEXT: DenyGeometryShaderRootAccess: true diff --git a/llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml b/llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml index f366d71714359..debb459c3944e 100644 --- a/llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml +++ b/llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml @@ -11,10 +11,10 @@ Header: PartOffsets: [ 60 ] Parts: - Name: RTS0 - Size: 80 + Size: 96 RootSignature: Version: 2 - NumRootParameters: 2 + NumRootParameters: 3 RootParametersOffset: 24 NumStaticSamplers: 0 StaticSamplersOffset: 60 @@ -31,14 +31,20 @@ Parts: Num32BitValues: 21 ShaderRegister: 22 RegisterSpace: 23 + - ParameterType: 2 # SRV + ShaderVisibility: 3 # Domain + Descriptor: + ShaderRegister: 31 + RegisterSpace: 32 + DATA_STATIC_WHILE_SET_AT_EXECUTE: true AllowInputAssemblerInputLayout: true DenyGeometryShaderRootAccess: true # CHECK: - Name: RTS0 -# CHECK-NEXT: Size: 80 +# CHECK-NEXT: Size: 96 # CHECK-NEXT: RootSignature: # CHECK-NEXT: Version: 2 -# CHECK-NEXT: NumRootParameters: 2 +# CHECK-NEXT: NumRootParameters: 3 # CHECK-NEXT: RootParametersOffset: 24 # CHECK-NEXT: NumStaticSamplers: 0 # CHECK-NEXT: StaticSamplersOffset: 60 @@ -55,5 +61,11 @@ Parts: # CHECK-NEXT: Num32BitValues: 21 # CHECK-NEXT: RegisterSpace: 23 # CHECK-NEXT: ShaderRegister: 22 +# CHECK-NEXT: - ParameterType: 2 +# CHECK-NEXT: ShaderVisibility: 3 +# CHECK-NEXT: Descriptor: +# CHECK-NEXT: RegisterSpace: 32 +# CHECK-NEXT: ShaderRegister: 31 +# CHECK-NEXT: DATA_STATIC_WHILE_SET_AT_EXECUTE: true # CHECK-NEXT: AllowInputAssemblerInputLayout: true # CHECK-NEXT: DenyGeometryShaderRootAccess: true diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp index 62ef8e385373f..ed43f6deaa951 100644 --- a/llvm/unittests/Object/DXContainerTest.cpp +++ b/llvm/unittests/Object/DXContainerTest.cpp @@ -959,3 +959,94 @@ TEST(RootSignature, ParseRootConstant) { ASSERT_EQ(Constants->Num32BitValues, 16u); } } + +TEST(RootSignature, ParseRootDescriptor) { + { + uint8_t Buffer[] = { + 0x44, 0x58, 0x42, 0x43, 0x32, 0x9a, 0x53, 0xd8, 0xec, 0xbe, 0x35, 0x6f, + 0x05, 0x39, 0xe1, 0xfe, 0x31, 0x20, 0xf0, 0xc1, 0x01, 0x00, 0x00, 0x00, + 0x85, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, + 0x52, 0x54, 0x53, 0x30, 0x59, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x3c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, + 0x03, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x1f, 0x00, 0x00, 0x00, + 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00}; + DXContainer C = + llvm::cantFail(DXContainer::create(getMemoryBuffer<133>(Buffer))); + + auto MaybeRS = C.getRootSignature(); + ASSERT_TRUE(MaybeRS.has_value()); + const auto &RS = MaybeRS.value(); + ASSERT_EQ(RS.getVersion(), 1u); + ASSERT_EQ(RS.getNumParameters(), 1u); + ASSERT_EQ(RS.getRootParametersOffset(), 24u); + ASSERT_EQ(RS.getNumStaticSamplers(), 0u); + ASSERT_EQ(RS.getStaticSamplersOffset(), 60u); + ASSERT_EQ(RS.getFlags(), 17u); + + auto RootParam = *RS.param_headers().begin(); + ASSERT_EQ((unsigned)RootParam.ParameterType, 2u); + ASSERT_EQ((unsigned)RootParam.ShaderVisibility, 3u); + auto ParamView = RS.getParameter(RootParam); + ASSERT_THAT_ERROR(ParamView.takeError(), Succeeded()); + + DirectX::RootDescriptorView_V1_0 *RootDescriptorView = + dyn_cast(&*ParamView); + ASSERT_TRUE(RootDescriptorView != nullptr); + auto Descriptor = RootDescriptorView->read(); + + ASSERT_THAT_ERROR(Descriptor.takeError(), Succeeded()); + + ASSERT_EQ(Descriptor->ShaderRegister, 31u); + ASSERT_EQ(Descriptor->RegisterSpace, 32u); + } + + { + uint8_t Buffer[] = { + 0x44, 0x58, 0x42, 0x43, 0x32, 0x9a, 0x53, 0xd8, 0xec, 0xbe, 0x35, 0x6f, + 0x05, 0x39, 0xe1, 0xfe, 0x31, 0x20, 0xf0, 0xc1, 0x01, 0x00, 0x00, 0x00, + 0x85, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, + 0x52, 0x54, 0x53, 0x30, 0x59, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x3c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, + 0x03, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x1f, 0x00, 0x00, 0x00, + 0x20, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00}; + DXContainer C = + llvm::cantFail(DXContainer::create(getMemoryBuffer<133>(Buffer))); + + auto MaybeRS = C.getRootSignature(); + ASSERT_TRUE(MaybeRS.has_value()); + const auto &RS = MaybeRS.value(); + ASSERT_EQ(RS.getVersion(), 2u); + ASSERT_EQ(RS.getNumParameters(), 1u); + ASSERT_EQ(RS.getRootParametersOffset(), 24u); + ASSERT_EQ(RS.getNumStaticSamplers(), 0u); + ASSERT_EQ(RS.getStaticSamplersOffset(), 60u); + ASSERT_EQ(RS.getFlags(), 17u); + + auto RootParam = *RS.param_headers().begin(); + ASSERT_EQ((unsigned)RootParam.ParameterType, 2u); + ASSERT_EQ((unsigned)RootParam.ShaderVisibility, 3u); + auto ParamView = RS.getParameter(RootParam); + ASSERT_THAT_ERROR(ParamView.takeError(), Succeeded()); + + DirectX::RootDescriptorView_V1_1 *RootDescriptorView = + dyn_cast(&*ParamView); + ASSERT_TRUE(RootDescriptorView != nullptr); + auto Descriptor = RootDescriptorView->read(); + + ASSERT_THAT_ERROR(Descriptor.takeError(), Succeeded()); + + ASSERT_EQ(Descriptor->ShaderRegister, 31u); + ASSERT_EQ(Descriptor->RegisterSpace, 32u); + ASSERT_EQ(Descriptor->Flags, 4u); + } +} diff --git a/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp b/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp index 61390049bc0df..b6a5cee24b29d 100644 --- a/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp +++ b/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp @@ -251,3 +251,106 @@ TEST(RootSignature, ParseRootConstants) { EXPECT_EQ(Storage.size(), 133u); EXPECT_TRUE(memcmp(Buffer, Storage.data(), 133u) == 0); } + +TEST(RootSignature, ParseRootDescriptorsV10) { + SmallString<128> Storage; + + // First read a fully explicit yaml with all sizes and offsets provided + ASSERT_TRUE(convert(Storage, R"(--- !dxcontainer + Header: + Hash: [ 0x32, 0x9A, 0x53, 0xD8, 0xEC, 0xBE, 0x35, 0x6F, 0x5, + 0x39, 0xE1, 0xFE, 0x31, 0x20, 0xF0, 0xC1 ] + Version: + Major: 1 + Minor: 0 + FileSize: 133 + PartCount: 1 + PartOffsets: [ 36 ] + Parts: + - Name: RTS0 + Size: 89 + RootSignature: + Version: 1 + NumRootParameters: 1 + RootParametersOffset: 24 + NumStaticSamplers: 0 + StaticSamplersOffset: 60 + Parameters: + - ParameterType: 2 # SRV + ShaderVisibility: 3 # Domain + Descriptor: + ShaderRegister: 31 + RegisterSpace: 32 + AllowInputAssemblerInputLayout: true + DenyGeometryShaderRootAccess: true + )")); + + uint8_t Buffer[] = { + 0x44, 0x58, 0x42, 0x43, 0x32, 0x9a, 0x53, 0xd8, 0xec, 0xbe, 0x35, 0x6f, + 0x05, 0x39, 0xe1, 0xfe, 0x31, 0x20, 0xf0, 0xc1, 0x01, 0x00, 0x00, 0x00, + 0x85, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, + 0x52, 0x54, 0x53, 0x30, 0x59, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x3c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, + 0x03, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x1f, 0x00, 0x00, 0x00, + 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00}; + + EXPECT_EQ(Storage.size(), 133u); + EXPECT_TRUE(memcmp(Buffer, Storage.data(), 133u) == 0); +} + +TEST(RootSignature, ParseRootDescriptorsV11) { + SmallString<128> Storage; + + // First read a fully explicit yaml with all sizes and offsets provided + ASSERT_TRUE(convert(Storage, R"(--- !dxcontainer + Header: + Hash: [ 0x32, 0x9A, 0x53, 0xD8, 0xEC, 0xBE, 0x35, 0x6F, 0x5, + 0x39, 0xE1, 0xFE, 0x31, 0x20, 0xF0, 0xC1 ] + Version: + Major: 1 + Minor: 0 + FileSize: 133 + PartCount: 1 + PartOffsets: [ 36 ] + Parts: + - Name: RTS0 + Size: 89 + RootSignature: + Version: 2 + NumRootParameters: 1 + RootParametersOffset: 24 + NumStaticSamplers: 0 + StaticSamplersOffset: 60 + Parameters: + - ParameterType: 2 # SRV + ShaderVisibility: 3 # Domain + Descriptor: + ShaderRegister: 31 + RegisterSpace: 32 + DATA_STATIC_WHILE_SET_AT_EXECUTE: true + AllowInputAssemblerInputLayout: true + DenyGeometryShaderRootAccess: true + )")); + + uint8_t Buffer[] = { + 0x44, 0x58, 0x42, 0x43, 0x32, 0x9a, 0x53, 0xd8, 0xec, 0xbe, 0x35, 0x6f, + 0x05, 0x39, 0xe1, 0xfe, 0x31, 0x20, 0xf0, 0xc1, 0x01, 0x00, 0x00, 0x00, + 0x85, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, + 0x52, 0x54, 0x53, 0x30, 0x59, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x3c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, + 0x03, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x1f, 0x00, 0x00, 0x00, + 0x20, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00}; + + EXPECT_EQ(Storage.size(), 133u); + EXPECT_TRUE(memcmp(Buffer, Storage.data(), 133u) == 0); +} From 8b8c02a6107b2b01bbc9cc9d84504d71e2726523 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Thu, 24 Apr 2025 22:34:06 +0000 Subject: [PATCH 02/35] clean up --- llvm/include/llvm/BinaryFormat/DXContainer.h | 2 ++ llvm/include/llvm/MC/DXContainerRootSignature.h | 2 +- llvm/include/llvm/ObjectYAML/DXContainerYAML.h | 2 +- llvm/lib/ObjectYAML/DXContainerYAML.cpp | 1 - 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/llvm/include/llvm/BinaryFormat/DXContainer.h b/llvm/include/llvm/BinaryFormat/DXContainer.h index 439bf7b40f31b..3dbcfa82f3d7c 100644 --- a/llvm/include/llvm/BinaryFormat/DXContainer.h +++ b/llvm/include/llvm/BinaryFormat/DXContainer.h @@ -427,6 +427,7 @@ struct SignatureElement { static_assert(sizeof(SignatureElement) == 4 * sizeof(uint32_t), "PSV Signature elements must fit in 16 bytes."); + } // namespace v0 namespace v1 { @@ -467,6 +468,7 @@ struct RuntimeInfo : public v0::RuntimeInfo { sys::swapByteOrder(GeomData.MaxVertexCount); } }; + } // namespace v1 namespace v2 { diff --git a/llvm/include/llvm/MC/DXContainerRootSignature.h b/llvm/include/llvm/MC/DXContainerRootSignature.h index ac062a375818c..1f421d726bf38 100644 --- a/llvm/include/llvm/MC/DXContainerRootSignature.h +++ b/llvm/include/llvm/MC/DXContainerRootSignature.h @@ -27,7 +27,7 @@ struct RootSignatureDesc { uint32_t Version = 2U; uint32_t Flags = 0U; - uint32_t RootParameterOffset = 24U; + uint32_t RootParameterOffset = 0U; uint32_t StaticSamplersOffset = 0u; uint32_t NumStaticSamplers = 0u; SmallVector Parameters; diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h index e86a869da99bc..c54c995acd263 100644 --- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h +++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h @@ -95,7 +95,7 @@ struct RootParameterYamlDesc { uint32_t Type; uint32_t Visibility; uint32_t Offset; - RootParameterYamlDesc(){}; + RootParameterYamlDesc() {}; RootParameterYamlDesc(uint32_t T) : Type(T) { switch (T) { diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp index ef86da85989e6..e49712852d612 100644 --- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp +++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp @@ -15,7 +15,6 @@ #include "llvm/ADT/STLForwardCompat.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/BinaryFormat/DXContainer.h" -#include "llvm/Object/DXContainer.h" #include "llvm/Support/Error.h" #include "llvm/Support/ScopedPrinter.h" #include From 7ac964196fc9195165dc1128d0f889f6ff1a93b4 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Fri, 25 Apr 2025 22:28:48 +0000 Subject: [PATCH 03/35] addressing comments --- llvm/include/llvm/Object/DXContainer.h | 50 +++++++------------ .../include/llvm/ObjectYAML/DXContainerYAML.h | 6 +-- llvm/lib/ObjectYAML/DXContainerYAML.cpp | 17 ++----- llvm/unittests/Object/DXContainerTest.cpp | 12 ++--- 4 files changed, 32 insertions(+), 53 deletions(-) diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h index ba261a9e42aea..e359d85f08bec 100644 --- a/llvm/include/llvm/Object/DXContainer.h +++ b/llvm/include/llvm/Object/DXContainer.h @@ -120,18 +120,20 @@ template struct ViewArray { namespace DirectX { struct RootParameterView { const dxbc::RootParameterHeader &Header; - uint32_t Version; StringRef ParamData; RootParameterView(uint32_t V, const dxbc::RootParameterHeader &H, StringRef P) - : Header(H), Version(V), ParamData(P) {} + : Header(H), ParamData(P) {} - template Expected readParameter() { - T Struct; - if (sizeof(T) != ParamData.size()) + template Expected readParameter() { + assert(sizeof(VersionT) <= sizeof(T) && + "Parameter of higher version must inherit all previous version data " + "members"); + if (sizeof(VersionT) != ParamData.size()) return make_error( "Reading structure out of file bounds", object_error::parse_failed); - memcpy(&Struct, ParamData.data(), sizeof(T)); + T Struct; + memcpy(&Struct, ParamData.data(), sizeof(VersionT)); // DXContainer is always little endian if (sys::IsBigEndianHost) Struct.swapBytes(); @@ -150,34 +152,20 @@ struct RootConstantView : RootParameterView { } }; -struct RootDescriptorView_V1_0 : RootParameterView { - static bool classof(const RootParameterView *V) { - return (V->Version == 1 && - (V->Header.ParameterType == - llvm::to_underlying(dxbc::RootParameterType::CBV) || - V->Header.ParameterType == - llvm::to_underlying(dxbc::RootParameterType::SRV) || - V->Header.ParameterType == - llvm::to_underlying(dxbc::RootParameterType::UAV))); - } - - llvm::Expected read() { - return readParameter(); - } -}; - -struct RootDescriptorView_V1_1 : RootParameterView { +struct RootDescriptorView : RootParameterView { static bool classof(const RootParameterView *V) { - return (V->Version == 2 && - (V->Header.ParameterType == - llvm::to_underlying(dxbc::RootParameterType::CBV) || - V->Header.ParameterType == - llvm::to_underlying(dxbc::RootParameterType::SRV) || - V->Header.ParameterType == - llvm::to_underlying(dxbc::RootParameterType::UAV))); + return (V->Header.ParameterType == + llvm::to_underlying(dxbc::RootParameterType::CBV) || + V->Header.ParameterType == + llvm::to_underlying(dxbc::RootParameterType::SRV) || + V->Header.ParameterType == + llvm::to_underlying(dxbc::RootParameterType::UAV)); } - llvm::Expected read() { + llvm::Expected read(uint32_t Version) { + if (Version == 1) + return readParameter(); return readParameter(); } }; diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h index c54c995acd263..8bb9da7884bed 100644 --- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h +++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h @@ -79,7 +79,6 @@ struct RootConstantsYaml { uint32_t Num32BitValues; }; -#define ROOT_DESCRIPTOR_FLAG(Num, Val) bool Val = false; struct RootDescriptorYaml { RootDescriptorYaml() = default; @@ -88,6 +87,7 @@ struct RootDescriptorYaml { uint32_t getEncodedFlags() const; +#define ROOT_DESCRIPTOR_FLAG(Num, Val) bool Val = false; #include "llvm/BinaryFormat/DXContainerConstants.def" }; @@ -95,7 +95,7 @@ struct RootParameterYamlDesc { uint32_t Type; uint32_t Visibility; uint32_t Offset; - RootParameterYamlDesc() {}; + RootParameterYamlDesc(){}; RootParameterYamlDesc(uint32_t T) : Type(T) { switch (T) { @@ -116,7 +116,6 @@ struct RootParameterYamlDesc { }; }; -#define ROOT_ELEMENT_FLAG(Num, Val) bool Val = false; struct RootSignatureYamlDesc { RootSignatureYamlDesc() = default; @@ -137,6 +136,7 @@ struct RootSignatureYamlDesc { static llvm::Expected create(const object::DirectX::RootSignature &Data); +#define ROOT_ELEMENT_FLAG(Num, Val) bool Val = false; #include "llvm/BinaryFormat/DXContainerConstants.def" }; diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp index e49712852d612..c9d2084226b7a 100644 --- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp +++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp @@ -15,6 +15,7 @@ #include "llvm/ADT/STLForwardCompat.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/BinaryFormat/DXContainer.h" +#include "llvm/Object/DXContainer.h" #include "llvm/Support/Error.h" #include "llvm/Support/ScopedPrinter.h" #include @@ -78,20 +79,10 @@ DXContainerYAML::RootSignatureYamlDesc::create( NewP.Constants.Num32BitValues = Constants.Num32BitValues; NewP.Constants.ShaderRegister = Constants.ShaderRegister; NewP.Constants.RegisterSpace = Constants.RegisterSpace; - } else if (auto *RDV = dyn_cast( - &ParamView)) { - llvm::Expected DescriptorOrErr = - RDV->read(); - if (Error E = DescriptorOrErr.takeError()) - return std::move(E); - auto Descriptor = *DescriptorOrErr; - - NewP.Descriptor.ShaderRegister = Descriptor.ShaderRegister; - NewP.Descriptor.RegisterSpace = Descriptor.RegisterSpace; - } else if (auto *RDV = dyn_cast( - &ParamView)) { + } else if (auto *RDV = + dyn_cast(&ParamView)) { llvm::Expected DescriptorOrErr = - RDV->read(); + RDV->read(Data.getVersion()); if (Error E = DescriptorOrErr.takeError()) return std::move(E); auto Descriptor = *DescriptorOrErr; diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp index ed43f6deaa951..72f860a5039ff 100644 --- a/llvm/unittests/Object/DXContainerTest.cpp +++ b/llvm/unittests/Object/DXContainerTest.cpp @@ -994,10 +994,10 @@ TEST(RootSignature, ParseRootDescriptor) { auto ParamView = RS.getParameter(RootParam); ASSERT_THAT_ERROR(ParamView.takeError(), Succeeded()); - DirectX::RootDescriptorView_V1_0 *RootDescriptorView = - dyn_cast(&*ParamView); + DirectX::RootDescriptorView *RootDescriptorView = + dyn_cast(&*ParamView); ASSERT_TRUE(RootDescriptorView != nullptr); - auto Descriptor = RootDescriptorView->read(); + auto Descriptor = RootDescriptorView->read(RS.getVersion()); ASSERT_THAT_ERROR(Descriptor.takeError(), Succeeded()); @@ -1038,10 +1038,10 @@ TEST(RootSignature, ParseRootDescriptor) { auto ParamView = RS.getParameter(RootParam); ASSERT_THAT_ERROR(ParamView.takeError(), Succeeded()); - DirectX::RootDescriptorView_V1_1 *RootDescriptorView = - dyn_cast(&*ParamView); + DirectX::RootDescriptorView *RootDescriptorView = + dyn_cast(&*ParamView); ASSERT_TRUE(RootDescriptorView != nullptr); - auto Descriptor = RootDescriptorView->read(); + auto Descriptor = RootDescriptorView->read(RS.getVersion()); ASSERT_THAT_ERROR(Descriptor.takeError(), Succeeded()); From c1054581e1a9973408991e863a5e7eec51e74f04 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Sat, 26 Apr 2025 01:45:29 +0000 Subject: [PATCH 04/35] formating --- llvm/include/llvm/ObjectYAML/DXContainerYAML.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h index 8bb9da7884bed..d9d43b40db299 100644 --- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h +++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h @@ -95,7 +95,7 @@ struct RootParameterYamlDesc { uint32_t Type; uint32_t Visibility; uint32_t Offset; - RootParameterYamlDesc(){}; + RootParameterYamlDesc() {}; RootParameterYamlDesc(uint32_t T) : Type(T) { switch (T) { From efe76aafee2c07e5e1df69daa404d48682ed5434 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Sat, 26 Apr 2025 02:02:53 +0000 Subject: [PATCH 05/35] try fix test --- llvm/lib/ObjectYAML/DXContainerYAML.cpp | 7 +++++-- .../DXContainer/RootSignature-Descriptor1.0.yaml | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp index c9d2084226b7a..18c1299d4b867 100644 --- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp +++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp @@ -39,8 +39,9 @@ DXContainerYAML::RootSignatureYamlDesc::create( const object::DirectX::RootSignature &Data) { RootSignatureYamlDesc RootSigDesc; + uint32_t Version = Data.getVersion(); - RootSigDesc.Version = Data.getVersion(); + RootSigDesc.Version = Version; RootSigDesc.NumStaticSamplers = Data.getNumStaticSamplers(); RootSigDesc.StaticSamplersOffset = Data.getStaticSamplersOffset(); RootSigDesc.NumRootParameters = Data.getNumRootParameters(); @@ -82,17 +83,19 @@ DXContainerYAML::RootSignatureYamlDesc::create( } else if (auto *RDV = dyn_cast(&ParamView)) { llvm::Expected DescriptorOrErr = - RDV->read(Data.getVersion()); + RDV->read(Version); if (Error E = DescriptorOrErr.takeError()) return std::move(E); auto Descriptor = *DescriptorOrErr; NewP.Descriptor.ShaderRegister = Descriptor.ShaderRegister; NewP.Descriptor.RegisterSpace = Descriptor.RegisterSpace; + if (Version > 1) { #define ROOT_DESCRIPTOR_FLAG(Num, Val) \ NewP.Descriptor.Val = \ (Descriptor.Flags & \ llvm::to_underlying(dxbc::RootDescriptorFlag::Val)) > 0; #include "llvm/BinaryFormat/DXContainerConstants.def" + } } RootSigDesc.Parameters.push_back(NewP); diff --git a/llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.0.yaml b/llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.0.yaml index 46cdf416ffcae..889eccf74001f 100644 --- a/llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.0.yaml +++ b/llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.0.yaml @@ -27,7 +27,7 @@ Parts: AllowInputAssemblerInputLayout: true DenyGeometryShaderRootAccess: true -# CHECK: - Name: RTS0 +# CHECK: - Name: RTS0 # CHECK-NEXT: Size: 96 # CHECK-NEXT: RootSignature: # CHECK-NEXT: Version: 1 From a928e9d9a12fd1114e2c1732094ad1f32ebc196c Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Sat, 26 Apr 2025 06:33:13 +0000 Subject: [PATCH 06/35] addressing comments --- .../include/llvm/MC/DXContainerRootSignature.h | 3 +-- llvm/include/llvm/ObjectYAML/DXContainerYAML.h | 2 +- llvm/lib/MC/DXContainerRootSignature.cpp | 18 ++++++------------ llvm/lib/ObjectYAML/DXContainerEmitter.cpp | 17 ++++------------- 4 files changed, 12 insertions(+), 28 deletions(-) diff --git a/llvm/include/llvm/MC/DXContainerRootSignature.h b/llvm/include/llvm/MC/DXContainerRootSignature.h index 1f421d726bf38..44e26c81eedc1 100644 --- a/llvm/include/llvm/MC/DXContainerRootSignature.h +++ b/llvm/include/llvm/MC/DXContainerRootSignature.h @@ -19,8 +19,7 @@ struct RootParameter { dxbc::RootParameterHeader Header; union { dxbc::RootConstants Constants; - dxbc::RST0::v0::RootDescriptor Descriptor_V10; - dxbc::RST0::v1::RootDescriptor Descriptor_V11; + dxbc::RST0::v1::RootDescriptor Descriptor; }; }; struct RootSignatureDesc { diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h index d9d43b40db299..8bb9da7884bed 100644 --- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h +++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h @@ -95,7 +95,7 @@ struct RootParameterYamlDesc { uint32_t Type; uint32_t Visibility; uint32_t Offset; - RootParameterYamlDesc() {}; + RootParameterYamlDesc(){}; RootParameterYamlDesc(uint32_t T) : Type(T) { switch (T) { diff --git a/llvm/lib/MC/DXContainerRootSignature.cpp b/llvm/lib/MC/DXContainerRootSignature.cpp index a5210f4768f16..2693cb9943d5e 100644 --- a/llvm/lib/MC/DXContainerRootSignature.cpp +++ b/llvm/lib/MC/DXContainerRootSignature.cpp @@ -92,19 +92,13 @@ void RootSignatureDesc::write(raw_ostream &OS) const { case llvm::to_underlying(dxbc::RootParameterType::CBV): case llvm::to_underlying(dxbc::RootParameterType::SRV): case llvm::to_underlying(dxbc::RootParameterType::UAV): - if (Version == 1) { - support::endian::write(BOS, P.Descriptor_V10.ShaderRegister, - llvm::endianness::little); - support::endian::write(BOS, P.Descriptor_V10.RegisterSpace, - llvm::endianness::little); - } else { - support::endian::write(BOS, P.Descriptor_V11.ShaderRegister, - llvm::endianness::little); - support::endian::write(BOS, P.Descriptor_V11.RegisterSpace, - llvm::endianness::little); - support::endian::write(BOS, P.Descriptor_V11.Flags, + support::endian::write(BOS, P.Descriptor.ShaderRegister, + llvm::endianness::little); + support::endian::write(BOS, P.Descriptor.RegisterSpace, + llvm::endianness::little); + if (Version > 1) + support::endian::write(BOS, P.Descriptor.Flags, llvm::endianness::little); - } } } assert(Storage.size() == getSize()); diff --git a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp index be0e52fef04f5..239ee9e3de9b1 100644 --- a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp +++ b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp @@ -287,19 +287,10 @@ void DXContainerWriter::writeParts(raw_ostream &OS) { case llvm::to_underlying(dxbc::RootParameterType::SRV): case llvm::to_underlying(dxbc::RootParameterType::UAV): case llvm::to_underlying(dxbc::RootParameterType::CBV): - if (RS.Version == 1) { - NewParam.Descriptor_V10.RegisterSpace = - Param.Descriptor.RegisterSpace; - NewParam.Descriptor_V10.ShaderRegister = - Param.Descriptor.ShaderRegister; - } else { - NewParam.Descriptor_V11.RegisterSpace = - Param.Descriptor.RegisterSpace; - NewParam.Descriptor_V11.ShaderRegister = - Param.Descriptor.ShaderRegister; - NewParam.Descriptor_V11.Flags = Param.Descriptor.getEncodedFlags(); - } - + NewParam.Descriptor.RegisterSpace = Param.Descriptor.RegisterSpace; + NewParam.Descriptor.ShaderRegister = Param.Descriptor.ShaderRegister; + if (P.RootSignature->Version > 1) + NewParam.Descriptor.Flags = Param.Descriptor.getEncodedFlags(); break; } From a38f10b51ac930be4bb5a5718d204d9f2d0c0396 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Fri, 25 Apr 2025 05:09:08 +0000 Subject: [PATCH 07/35] refactoring mcdxbc struct to store root parameters out of order --- .../llvm/MC/DXContainerRootSignature.h | 137 +++++++++++++++++- llvm/lib/MC/DXContainerRootSignature.cpp | 68 ++++----- llvm/lib/ObjectYAML/DXContainerEmitter.cpp | 26 ++-- llvm/lib/Target/DirectX/DXILRootSignature.cpp | 45 +++--- 4 files changed, 201 insertions(+), 75 deletions(-) diff --git a/llvm/include/llvm/MC/DXContainerRootSignature.h b/llvm/include/llvm/MC/DXContainerRootSignature.h index 44e26c81eedc1..e1f4abbcebf8f 100644 --- a/llvm/include/llvm/MC/DXContainerRootSignature.h +++ b/llvm/include/llvm/MC/DXContainerRootSignature.h @@ -6,21 +6,146 @@ // //===----------------------------------------------------------------------===// +#include "llvm/ADT/STLForwardCompat.h" #include "llvm/BinaryFormat/DXContainer.h" +#include "llvm/Support/ErrorHandling.h" +#include #include -#include +#include namespace llvm { class raw_ostream; namespace mcdxbc { +struct RootParameterHeader : public dxbc::RootParameterHeader { + + size_t Location; + + RootParameterHeader() = default; + + RootParameterHeader(dxbc::RootParameterHeader H, size_t L) + : dxbc::RootParameterHeader(H), Location(L) {} +}; + +using RootDescriptor = std::variant; +using ParametersView = + std::variant; struct RootParameter { - dxbc::RootParameterHeader Header; - union { - dxbc::RootConstants Constants; - dxbc::RST0::v1::RootDescriptor Descriptor; + SmallVector Headers; + + SmallVector Constants; + SmallVector Descriptors; + + void addHeader(dxbc::RootParameterHeader H, size_t L) { + Headers.push_back(RootParameterHeader(H, L)); + } + + void addParameter(dxbc::RootParameterHeader H, dxbc::RootConstants C) { + addHeader(H, Constants.size()); + Constants.push_back(C); + } + + void addParameter(dxbc::RootParameterHeader H, + dxbc::RST0::v0::RootDescriptor D) { + addHeader(H, Descriptors.size()); + Descriptors.push_back(D); + } + + void addParameter(dxbc::RootParameterHeader H, + dxbc::RST0::v1::RootDescriptor D) { + addHeader(H, Descriptors.size()); + Descriptors.push_back(D); + } + + ParametersView get(const RootParameterHeader &H) const { + switch (H.ParameterType) { + case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): + return Constants[H.Location]; + case llvm::to_underlying(dxbc::RootParameterType::CBV): + case llvm::to_underlying(dxbc::RootParameterType::SRV): + case llvm::to_underlying(dxbc::RootParameterType::UAV): + RootDescriptor VersionedParam = Descriptors[H.Location]; + if (std::holds_alternative( + VersionedParam)) + return std::get(VersionedParam); + return std::get(VersionedParam); + } + + llvm_unreachable("Unimplemented parameter type"); + } + + struct iterator { + const RootParameter &Parameters; + SmallVector::const_iterator Current; + + // Changed parameter type to match member variable (removed const) + iterator(const RootParameter &P, + SmallVector::const_iterator C) + : Parameters(P), Current(C) {} + iterator(const iterator &) = default; + + ParametersView operator*() { + ParametersView Val; + switch (Current->ParameterType) { + case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): + Val = Parameters.Constants[Current->Location]; + break; + + case llvm::to_underlying(dxbc::RootParameterType::CBV): + case llvm::to_underlying(dxbc::RootParameterType::SRV): + case llvm::to_underlying(dxbc::RootParameterType::UAV): + RootDescriptor VersionedParam = + Parameters.Descriptors[Current->Location]; + if (std::holds_alternative( + VersionedParam)) + Val = std::get(VersionedParam); + else + Val = std::get(VersionedParam); + break; + } + return Val; + } + + iterator operator++() { + Current++; + return *this; + } + + iterator operator++(int) { + iterator Tmp = *this; + ++*this; + return Tmp; + } + + iterator operator--() { + Current--; + return *this; + } + + iterator operator--(int) { + iterator Tmp = *this; + --*this; + return Tmp; + } + + bool operator==(const iterator I) { return I.Current == Current; } + bool operator!=(const iterator I) { return !(*this == I); } }; + + iterator begin() const { return iterator(*this, Headers.begin()); } + + iterator end() const { return iterator(*this, Headers.end()); } + + size_t size() const { return Headers.size(); } + + bool isEmpty() const { return Headers.empty(); } + + llvm::iterator_range getAll() const { + return llvm::make_range(begin(), end()); + } }; struct RootSignatureDesc { @@ -29,7 +154,7 @@ struct RootSignatureDesc { uint32_t RootParameterOffset = 0U; uint32_t StaticSamplersOffset = 0u; uint32_t NumStaticSamplers = 0u; - SmallVector Parameters; + mcdxbc::RootParameter Parameters; void write(raw_ostream &OS) const; diff --git a/llvm/lib/MC/DXContainerRootSignature.cpp b/llvm/lib/MC/DXContainerRootSignature.cpp index 2693cb9943d5e..18242ccc1e935 100644 --- a/llvm/lib/MC/DXContainerRootSignature.cpp +++ b/llvm/lib/MC/DXContainerRootSignature.cpp @@ -8,7 +8,9 @@ #include "llvm/MC/DXContainerRootSignature.h" #include "llvm/ADT/SmallString.h" +#include "llvm/BinaryFormat/DXContainer.h" #include "llvm/Support/EndianStream.h" +#include using namespace llvm; using namespace llvm::mcdxbc; @@ -32,22 +34,15 @@ size_t RootSignatureDesc::getSize() const { size_t Size = sizeof(dxbc::RootSignatureHeader) + Parameters.size() * sizeof(dxbc::RootParameterHeader); - for (const mcdxbc::RootParameter &P : Parameters) { - switch (P.Header.ParameterType) { - case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): - Size += sizeof(dxbc::RootConstants); - break; - case llvm::to_underlying(dxbc::RootParameterType::CBV): - case llvm::to_underlying(dxbc::RootParameterType::SRV): - case llvm::to_underlying(dxbc::RootParameterType::UAV): - if (Version == 1) - Size += sizeof(dxbc::RST0::v0::RootDescriptor); - else - Size += sizeof(dxbc::RST0::v1::RootDescriptor); - - break; - } + for (const auto &P : Parameters) { + std::visit( + [&Size](auto &Value) -> void { + using T = std::decay_t; + Size += sizeof(T); + }, + P); } + return Size; } @@ -66,39 +61,40 @@ void RootSignatureDesc::write(raw_ostream &OS) const { support::endian::write(BOS, Flags, llvm::endianness::little); SmallVector ParamsOffsets; - for (const mcdxbc::RootParameter &P : Parameters) { - support::endian::write(BOS, P.Header.ParameterType, - llvm::endianness::little); - support::endian::write(BOS, P.Header.ShaderVisibility, - llvm::endianness::little); + for (const auto &P : Parameters.Headers) { + support::endian::write(BOS, P.ParameterType, llvm::endianness::little); + support::endian::write(BOS, P.ShaderVisibility, llvm::endianness::little); ParamsOffsets.push_back(writePlaceholder(BOS)); } assert(NumParameters == ParamsOffsets.size()); - for (size_t I = 0; I < NumParameters; ++I) { + auto P = Parameters.begin(); + for (size_t I = 0; I < NumParameters; ++I, P++) { rewriteOffsetToCurrentByte(BOS, ParamsOffsets[I]); - const mcdxbc::RootParameter &P = Parameters[I]; - switch (P.Header.ParameterType) { - case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): - support::endian::write(BOS, P.Constants.ShaderRegister, + if (std::holds_alternative(*P)) { + auto Constants = std::get(*P); + support::endian::write(BOS, Constants.ShaderRegister, llvm::endianness::little); - support::endian::write(BOS, P.Constants.RegisterSpace, + support::endian::write(BOS, Constants.RegisterSpace, llvm::endianness::little); - support::endian::write(BOS, P.Constants.Num32BitValues, + support::endian::write(BOS, Constants.Num32BitValues, llvm::endianness::little); - break; - case llvm::to_underlying(dxbc::RootParameterType::CBV): - case llvm::to_underlying(dxbc::RootParameterType::SRV): - case llvm::to_underlying(dxbc::RootParameterType::UAV): - support::endian::write(BOS, P.Descriptor.ShaderRegister, + } else if (std::holds_alternative(*P)) { + auto Descriptor = std::get(*P); + support::endian::write(BOS, Descriptor.ShaderRegister, + llvm::endianness::little); + support::endian::write(BOS, Descriptor.RegisterSpace, + llvm::endianness::little); + } else if (std::holds_alternative(*P)) { + auto Descriptor = std::get(*P); + + support::endian::write(BOS, Descriptor.ShaderRegister, llvm::endianness::little); - support::endian::write(BOS, P.Descriptor.RegisterSpace, + support::endian::write(BOS, Descriptor.RegisterSpace, llvm::endianness::little); - if (Version > 1) - support::endian::write(BOS, P.Descriptor.Flags, - llvm::endianness::little); + support::endian::write(BOS, Descriptor.Flags, llvm::endianness::little); } } assert(Storage.size() == getSize()); diff --git a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp index 239ee9e3de9b1..3e58c2cd7497b 100644 --- a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp +++ b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp @@ -274,27 +274,31 @@ void DXContainerWriter::writeParts(raw_ostream &OS) { RS.StaticSamplersOffset = P.RootSignature->StaticSamplersOffset; for (const auto &Param : P.RootSignature->Parameters) { - mcdxbc::RootParameter NewParam; - NewParam.Header = dxbc::RootParameterHeader{ - Param.Type, Param.Visibility, Param.Offset}; + auto Header = dxbc::RootParameterHeader{Param.Type, Param.Visibility, + Param.Offset}; switch (Param.Type) { case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): - NewParam.Constants.Num32BitValues = Param.Constants.Num32BitValues; - NewParam.Constants.RegisterSpace = Param.Constants.RegisterSpace; - NewParam.Constants.ShaderRegister = Param.Constants.ShaderRegister; + dxbc::RootConstants Constants; + Constants.Num32BitValues = Param.Constants.Num32BitValues; + Constants.RegisterSpace = Param.Constants.RegisterSpace; + Constants.ShaderRegister = Param.Constants.ShaderRegister; + RS.Parameters.addParameter(Header, Constants); break; case llvm::to_underlying(dxbc::RootParameterType::SRV): case llvm::to_underlying(dxbc::RootParameterType::UAV): case llvm::to_underlying(dxbc::RootParameterType::CBV): - NewParam.Descriptor.RegisterSpace = Param.Descriptor.RegisterSpace; - NewParam.Descriptor.ShaderRegister = Param.Descriptor.ShaderRegister; + dxbc::RST0::v1::RootDescriptor Descriptor; + Descriptor.RegisterSpace = Param.Descriptor.RegisterSpace; + Descriptor.ShaderRegister = Param.Descriptor.ShaderRegister; if (P.RootSignature->Version > 1) - NewParam.Descriptor.Flags = Param.Descriptor.getEncodedFlags(); + Descriptor.Flags = Param.Descriptor.getEncodedFlags(); + RS.Parameters.addParameter(Header, Descriptor); break; + default: + // Handling invalid parameter type edge case + RS.Parameters.addHeader(Header, -1); } - - RS.Parameters.push_back(NewParam); } RS.write(OS); diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index ef299c17baf76..a2141aa1364ad 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -30,6 +30,7 @@ #include #include #include +#include using namespace llvm; using namespace llvm::dxil; @@ -75,31 +76,32 @@ static bool parseRootConstants(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD, if (RootConstantNode->getNumOperands() != 5) return reportError(Ctx, "Invalid format for RootConstants Element"); - mcdxbc::RootParameter NewParameter; - NewParameter.Header.ParameterType = + dxbc::RootParameterHeader Header; + Header.ParameterType = llvm::to_underlying(dxbc::RootParameterType::Constants32Bit); if (std::optional Val = extractMdIntValue(RootConstantNode, 1)) - NewParameter.Header.ShaderVisibility = *Val; + Header.ShaderVisibility = *Val; else return reportError(Ctx, "Invalid value for ShaderVisibility"); + dxbc::RootConstants Constants; if (std::optional Val = extractMdIntValue(RootConstantNode, 2)) - NewParameter.Constants.ShaderRegister = *Val; + Constants.ShaderRegister = *Val; else return reportError(Ctx, "Invalid value for ShaderRegister"); if (std::optional Val = extractMdIntValue(RootConstantNode, 3)) - NewParameter.Constants.RegisterSpace = *Val; + Constants.RegisterSpace = *Val; else return reportError(Ctx, "Invalid value for RegisterSpace"); if (std::optional Val = extractMdIntValue(RootConstantNode, 4)) - NewParameter.Constants.Num32BitValues = *Val; + Constants.Num32BitValues = *Val; else return reportError(Ctx, "Invalid value for Num32BitValues"); - RSD.Parameters.push_back(NewParameter); + RSD.Parameters.addParameter(Header, Constants); return false; } @@ -164,12 +166,11 @@ static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) { return reportValueError(Ctx, "RootFlags", RSD.Flags); } - for (const mcdxbc::RootParameter &P : RSD.Parameters) { - if (!dxbc::isValidShaderVisibility(P.Header.ShaderVisibility)) - return reportValueError(Ctx, "ShaderVisibility", - P.Header.ShaderVisibility); + for (const mcdxbc::RootParameterHeader &Header : RSD.Parameters.Headers) { + if (!dxbc::isValidShaderVisibility(Header.ShaderVisibility)) + return reportValueError(Ctx, "ShaderVisibility", Header.ShaderVisibility); - assert(dxbc::isValidParameterType(P.Header.ParameterType) && + assert(dxbc::isValidParameterType(Header.ParameterType) && "Invalid value for ParameterType"); } @@ -289,20 +290,20 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M, << "\n"; OS << indent(Space) << "NumParameters: " << RS.Parameters.size() << "\n"; Space++; - for (auto const &P : RS.Parameters) { - OS << indent(Space) << "- Parameter Type: " << P.Header.ParameterType + for (auto const &Header : RS.Parameters.Headers) { + OS << indent(Space) << "- Parameter Type: " << Header.ParameterType << "\n"; OS << indent(Space + 2) - << "Shader Visibility: " << P.Header.ShaderVisibility << "\n"; - switch (P.Header.ParameterType) { - case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): + << "Shader Visibility: " << Header.ShaderVisibility << "\n"; + mcdxbc::ParametersView P = RS.Parameters.get(Header); + if (std::holds_alternative(P)) { + auto Constants = std::get(P); + OS << indent(Space + 2) << "Register Space: " << Constants.RegisterSpace + << "\n"; OS << indent(Space + 2) - << "Register Space: " << P.Constants.RegisterSpace << "\n"; + << "Shader Register: " << Constants.ShaderRegister << "\n"; OS << indent(Space + 2) - << "Shader Register: " << P.Constants.ShaderRegister << "\n"; - OS << indent(Space + 2) - << "Num 32 Bit Values: " << P.Constants.Num32BitValues << "\n"; - break; + << "Num 32 Bit Values: " << Constants.Num32BitValues << "\n"; } } Space--; From 9a7c359fd5ce3621647e35f4656b0ae5e46abf2e Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Mon, 28 Apr 2025 18:12:28 +0000 Subject: [PATCH 08/35] changing name --- .../llvm/MC/DXContainerRootSignature.h | 111 +++++------------- llvm/lib/MC/DXContainerRootSignature.cpp | 27 +++-- llvm/lib/ObjectYAML/DXContainerEmitter.cpp | 19 +-- llvm/lib/Target/DirectX/DXILRootSignature.cpp | 29 +++-- 4 files changed, 74 insertions(+), 112 deletions(-) diff --git a/llvm/include/llvm/MC/DXContainerRootSignature.h b/llvm/include/llvm/MC/DXContainerRootSignature.h index e1f4abbcebf8f..2ac79f4c454d8 100644 --- a/llvm/include/llvm/MC/DXContainerRootSignature.h +++ b/llvm/include/llvm/MC/DXContainerRootSignature.h @@ -11,6 +11,7 @@ #include "llvm/Support/ErrorHandling.h" #include #include +#include #include namespace llvm { @@ -18,14 +19,14 @@ namespace llvm { class raw_ostream; namespace mcdxbc { -struct RootParameterHeader : public dxbc::RootParameterHeader { - +struct RootParameterInfo { + dxbc::RootParameterHeader Header; size_t Location; - RootParameterHeader() = default; + RootParameterInfo() = default; - RootParameterHeader(dxbc::RootParameterHeader H, size_t L) - : dxbc::RootParameterHeader(H), Location(L) {} + RootParameterInfo(dxbc::RootParameterHeader H, size_t L) + : Header(H), Location(L) {} }; using RootDescriptor = std::variant; -struct RootParameter { - SmallVector Headers; +struct RootParametersContainer { + SmallVector ParametersInfo; SmallVector Constants; SmallVector Descriptors; - void addHeader(dxbc::RootParameterHeader H, size_t L) { - Headers.push_back(RootParameterHeader(H, L)); + void addInfo(dxbc::RootParameterHeader H, size_t L) { + ParametersInfo.push_back(RootParameterInfo(H, L)); } void addParameter(dxbc::RootParameterHeader H, dxbc::RootConstants C) { - addHeader(H, Constants.size()); + addInfo(H, Constants.size()); Constants.push_back(C); } void addParameter(dxbc::RootParameterHeader H, dxbc::RST0::v0::RootDescriptor D) { - addHeader(H, Descriptors.size()); + addInfo(H, Descriptors.size()); Descriptors.push_back(D); } void addParameter(dxbc::RootParameterHeader H, dxbc::RST0::v1::RootDescriptor D) { - addHeader(H, Descriptors.size()); + addInfo(H, Descriptors.size()); Descriptors.push_back(D); } - ParametersView get(const RootParameterHeader &H) const { - switch (H.ParameterType) { + std::optional getParameter(const RootParameterInfo *H) const { + switch (H->Header.ParameterType) { case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): - return Constants[H.Location]; + return Constants[H->Location]; case llvm::to_underlying(dxbc::RootParameterType::CBV): case llvm::to_underlying(dxbc::RootParameterType::SRV): case llvm::to_underlying(dxbc::RootParameterType::UAV): - RootDescriptor VersionedParam = Descriptors[H.Location]; + RootDescriptor VersionedParam = Descriptors[H->Location]; if (std::holds_alternative( VersionedParam)) return std::get(VersionedParam); return std::get(VersionedParam); } - llvm_unreachable("Unimplemented parameter type"); + return std::nullopt; } - struct iterator { - const RootParameter &Parameters; - SmallVector::const_iterator Current; - - // Changed parameter type to match member variable (removed const) - iterator(const RootParameter &P, - SmallVector::const_iterator C) - : Parameters(P), Current(C) {} - iterator(const iterator &) = default; - - ParametersView operator*() { - ParametersView Val; - switch (Current->ParameterType) { - case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): - Val = Parameters.Constants[Current->Location]; - break; - - case llvm::to_underlying(dxbc::RootParameterType::CBV): - case llvm::to_underlying(dxbc::RootParameterType::SRV): - case llvm::to_underlying(dxbc::RootParameterType::UAV): - RootDescriptor VersionedParam = - Parameters.Descriptors[Current->Location]; - if (std::holds_alternative( - VersionedParam)) - Val = std::get(VersionedParam); - else - Val = std::get(VersionedParam); - break; - } - return Val; - } - - iterator operator++() { - Current++; - return *this; - } - - iterator operator++(int) { - iterator Tmp = *this; - ++*this; - return Tmp; - } - - iterator operator--() { - Current--; - return *this; - } - - iterator operator--(int) { - iterator Tmp = *this; - --*this; - return Tmp; - } - - bool operator==(const iterator I) { return I.Current == Current; } - bool operator!=(const iterator I) { return !(*this == I); } - }; - - iterator begin() const { return iterator(*this, Headers.begin()); } + size_t size() const { return ParametersInfo.size(); } - iterator end() const { return iterator(*this, Headers.end()); } - - size_t size() const { return Headers.size(); } - - bool isEmpty() const { return Headers.empty(); } + SmallVector::const_iterator begin() const { + return ParametersInfo.begin(); + } + SmallVector::const_iterator end() const { + return ParametersInfo.end(); + } - llvm::iterator_range getAll() const { + llvm::iterator_range::const_iterator> + getInfo() const { return llvm::make_range(begin(), end()); } }; @@ -154,7 +99,7 @@ struct RootSignatureDesc { uint32_t RootParameterOffset = 0U; uint32_t StaticSamplersOffset = 0u; uint32_t NumStaticSamplers = 0u; - mcdxbc::RootParameter Parameters; + mcdxbc::RootParametersContainer ParametersContainer; void write(raw_ostream &OS) const; diff --git a/llvm/lib/MC/DXContainerRootSignature.cpp b/llvm/lib/MC/DXContainerRootSignature.cpp index 18242ccc1e935..6cc8a9167bf40 100644 --- a/llvm/lib/MC/DXContainerRootSignature.cpp +++ b/llvm/lib/MC/DXContainerRootSignature.cpp @@ -32,15 +32,18 @@ static void rewriteOffsetToCurrentByte(raw_svector_ostream &Stream, size_t RootSignatureDesc::getSize() const { size_t Size = sizeof(dxbc::RootSignatureHeader) + - Parameters.size() * sizeof(dxbc::RootParameterHeader); + ParametersContainer.size() * sizeof(dxbc::RootParameterHeader); - for (const auto &P : Parameters) { + for (const auto &I : ParametersContainer) { + std::optional P = ParametersContainer.getParameter(&I); + if (!P) + continue; std::visit( [&Size](auto &Value) -> void { using T = std::decay_t; Size += sizeof(T); }, - P); + *P); } return Size; @@ -51,7 +54,7 @@ void RootSignatureDesc::write(raw_ostream &OS) const { raw_svector_ostream BOS(Storage); BOS.reserveExtraSpace(getSize()); - const uint32_t NumParameters = Parameters.size(); + const uint32_t NumParameters = ParametersContainer.size(); support::endian::write(BOS, Version, llvm::endianness::little); support::endian::write(BOS, NumParameters, llvm::endianness::little); @@ -61,18 +64,22 @@ void RootSignatureDesc::write(raw_ostream &OS) const { support::endian::write(BOS, Flags, llvm::endianness::little); SmallVector ParamsOffsets; - for (const auto &P : Parameters.Headers) { - support::endian::write(BOS, P.ParameterType, llvm::endianness::little); - support::endian::write(BOS, P.ShaderVisibility, llvm::endianness::little); + for (const auto &P : ParametersContainer) { + support::endian::write(BOS, P.Header.ParameterType, + llvm::endianness::little); + support::endian::write(BOS, P.Header.ShaderVisibility, + llvm::endianness::little); ParamsOffsets.push_back(writePlaceholder(BOS)); } assert(NumParameters == ParamsOffsets.size()); - auto P = Parameters.begin(); - for (size_t I = 0; I < NumParameters; ++I, P++) { + const RootParameterInfo *H = ParametersContainer.begin(); + for (size_t I = 0; I < NumParameters; ++I, H++) { rewriteOffsetToCurrentByte(BOS, ParamsOffsets[I]); - + auto P = ParametersContainer.getParameter(H); + if (!P) + continue; if (std::holds_alternative(*P)) { auto Constants = std::get(*P); support::endian::write(BOS, Constants.ShaderRegister, diff --git a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp index 3e58c2cd7497b..381fe41d8b01c 100644 --- a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp +++ b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp @@ -283,21 +283,26 @@ void DXContainerWriter::writeParts(raw_ostream &OS) { Constants.Num32BitValues = Param.Constants.Num32BitValues; Constants.RegisterSpace = Param.Constants.RegisterSpace; Constants.ShaderRegister = Param.Constants.ShaderRegister; - RS.Parameters.addParameter(Header, Constants); + RS.ParametersContainer.addParameter(Header, Constants); break; case llvm::to_underlying(dxbc::RootParameterType::SRV): case llvm::to_underlying(dxbc::RootParameterType::UAV): case llvm::to_underlying(dxbc::RootParameterType::CBV): - dxbc::RST0::v1::RootDescriptor Descriptor; - Descriptor.RegisterSpace = Param.Descriptor.RegisterSpace; - Descriptor.ShaderRegister = Param.Descriptor.ShaderRegister; - if (P.RootSignature->Version > 1) + if (RS.Version == 1) { + dxbc::RST0::v0::RootDescriptor Descriptor; + Descriptor.RegisterSpace = Param.Descriptor.RegisterSpace; + Descriptor.ShaderRegister = Param.Descriptor.ShaderRegister; + RS.ParametersContainer.addParameter(Header, Descriptor); + } else { + dxbc::RST0::v1::RootDescriptor Descriptor; + Descriptor.RegisterSpace = Param.Descriptor.RegisterSpace; + Descriptor.ShaderRegister = Param.Descriptor.ShaderRegister; Descriptor.Flags = Param.Descriptor.getEncodedFlags(); - RS.Parameters.addParameter(Header, Descriptor); + RS.ParametersContainer.addParameter(Header, Descriptor); break; default: // Handling invalid parameter type edge case - RS.Parameters.addHeader(Header, -1); + RS.ParametersContainer.addInfo(Header, -1); } } diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index a2141aa1364ad..40dcff3999b8f 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -101,7 +101,7 @@ static bool parseRootConstants(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD, else return reportError(Ctx, "Invalid value for Num32BitValues"); - RSD.Parameters.addParameter(Header, Constants); + RSD.ParametersContainer.addParameter(Header, Constants); return false; } @@ -166,11 +166,12 @@ static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) { return reportValueError(Ctx, "RootFlags", RSD.Flags); } - for (const mcdxbc::RootParameterHeader &Header : RSD.Parameters.Headers) { - if (!dxbc::isValidShaderVisibility(Header.ShaderVisibility)) - return reportValueError(Ctx, "ShaderVisibility", Header.ShaderVisibility); + for (const llvm::mcdxbc::RootParameterInfo &Info : RSD.ParametersContainer) { + if (!dxbc::isValidShaderVisibility(Info.Header.ShaderVisibility)) + return reportValueError(Ctx, "ShaderVisibility", + Info.Header.ShaderVisibility); - assert(dxbc::isValidParameterType(Header.ParameterType) && + assert(dxbc::isValidParameterType(Info.Header.ParameterType) && "Invalid value for ParameterType"); } @@ -288,16 +289,20 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M, OS << indent(Space) << "Version: " << RS.Version << "\n"; OS << indent(Space) << "RootParametersOffset: " << RS.RootParameterOffset << "\n"; - OS << indent(Space) << "NumParameters: " << RS.Parameters.size() << "\n"; + OS << indent(Space) << "NumParameters: " << RS.ParametersContainer.size() + << "\n"; Space++; - for (auto const &Header : RS.Parameters.Headers) { - OS << indent(Space) << "- Parameter Type: " << Header.ParameterType + for (auto const &Info : RS.ParametersContainer) { + OS << indent(Space) << "- Parameter Type: " << Info.Header.ParameterType << "\n"; OS << indent(Space + 2) - << "Shader Visibility: " << Header.ShaderVisibility << "\n"; - mcdxbc::ParametersView P = RS.Parameters.get(Header); - if (std::holds_alternative(P)) { - auto Constants = std::get(P); + << "Shader Visibility: " << Info.Header.ShaderVisibility << "\n"; + std::optional P = + RS.ParametersContainer.getParameter(&Info); + if (!P) + continue; + if (std::holds_alternative(*P)) { + auto Constants = std::get(*P); OS << indent(Space + 2) << "Register Space: " << Constants.RegisterSpace << "\n"; OS << indent(Space + 2) From d6c2b5583d3c13dcc96a5d5b36a8ac5741d8f6ce Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Mon, 28 Apr 2025 22:29:53 +0000 Subject: [PATCH 09/35] changing variant to host pointers --- .../llvm/MC/DXContainerRootSignature.h | 22 +++++++------- llvm/lib/MC/DXContainerRootSignature.cpp | 30 +++++++++---------- llvm/lib/Target/DirectX/DXILRootSignature.cpp | 10 +++---- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/llvm/include/llvm/MC/DXContainerRootSignature.h b/llvm/include/llvm/MC/DXContainerRootSignature.h index 2ac79f4c454d8..33680c90d595f 100644 --- a/llvm/include/llvm/MC/DXContainerRootSignature.h +++ b/llvm/include/llvm/MC/DXContainerRootSignature.h @@ -8,10 +8,10 @@ #include "llvm/ADT/STLForwardCompat.h" #include "llvm/BinaryFormat/DXContainer.h" -#include "llvm/Support/ErrorHandling.h" #include #include #include +#include #include namespace llvm { @@ -29,11 +29,11 @@ struct RootParameterInfo { : Header(H), Location(L) {} }; -using RootDescriptor = std::variant; +using RootDescriptor = std::variant; using ParametersView = - std::variant; + std::variant; struct RootParametersContainer { SmallVector ParametersInfo; @@ -52,27 +52,27 @@ struct RootParametersContainer { void addParameter(dxbc::RootParameterHeader H, dxbc::RST0::v0::RootDescriptor D) { addInfo(H, Descriptors.size()); - Descriptors.push_back(D); + Descriptors.push_back(std::move(&D)); } void addParameter(dxbc::RootParameterHeader H, dxbc::RST0::v1::RootDescriptor D) { addInfo(H, Descriptors.size()); - Descriptors.push_back(D); + Descriptors.push_back(std::move(&D)); } std::optional getParameter(const RootParameterInfo *H) const { switch (H->Header.ParameterType) { case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): - return Constants[H->Location]; + return &Constants[H->Location]; case llvm::to_underlying(dxbc::RootParameterType::CBV): case llvm::to_underlying(dxbc::RootParameterType::SRV): case llvm::to_underlying(dxbc::RootParameterType::UAV): RootDescriptor VersionedParam = Descriptors[H->Location]; - if (std::holds_alternative( + if (std::holds_alternative( VersionedParam)) - return std::get(VersionedParam); - return std::get(VersionedParam); + return std::get(VersionedParam); + return std::get(VersionedParam); } return std::nullopt; diff --git a/llvm/lib/MC/DXContainerRootSignature.cpp b/llvm/lib/MC/DXContainerRootSignature.cpp index 6cc8a9167bf40..c3a535592b78f 100644 --- a/llvm/lib/MC/DXContainerRootSignature.cpp +++ b/llvm/lib/MC/DXContainerRootSignature.cpp @@ -40,7 +40,7 @@ size_t RootSignatureDesc::getSize() const { continue; std::visit( [&Size](auto &Value) -> void { - using T = std::decay_t; + using T = std::decay_t; Size += sizeof(T); }, *P); @@ -80,28 +80,28 @@ void RootSignatureDesc::write(raw_ostream &OS) const { auto P = ParametersContainer.getParameter(H); if (!P) continue; - if (std::holds_alternative(*P)) { - auto Constants = std::get(*P); - support::endian::write(BOS, Constants.ShaderRegister, + if (std::holds_alternative(*P)) { + auto* Constants = std::get(*P); + support::endian::write(BOS, Constants->ShaderRegister, llvm::endianness::little); - support::endian::write(BOS, Constants.RegisterSpace, + support::endian::write(BOS, Constants->RegisterSpace, llvm::endianness::little); - support::endian::write(BOS, Constants.Num32BitValues, + support::endian::write(BOS, Constants->Num32BitValues, llvm::endianness::little); - } else if (std::holds_alternative(*P)) { - auto Descriptor = std::get(*P); - support::endian::write(BOS, Descriptor.ShaderRegister, + } else if (std::holds_alternative(*P)) { + auto* Descriptor = std::get(*P); + support::endian::write(BOS, Descriptor->ShaderRegister, llvm::endianness::little); - support::endian::write(BOS, Descriptor.RegisterSpace, + support::endian::write(BOS, Descriptor->RegisterSpace, llvm::endianness::little); - } else if (std::holds_alternative(*P)) { - auto Descriptor = std::get(*P); + } else if (std::holds_alternative(*P)) { + auto* Descriptor = std::get(*P); - support::endian::write(BOS, Descriptor.ShaderRegister, + support::endian::write(BOS, Descriptor->ShaderRegister, llvm::endianness::little); - support::endian::write(BOS, Descriptor.RegisterSpace, + support::endian::write(BOS, Descriptor->RegisterSpace, llvm::endianness::little); - support::endian::write(BOS, Descriptor.Flags, llvm::endianness::little); + support::endian::write(BOS, Descriptor->Flags, llvm::endianness::little); } } assert(Storage.size() == getSize()); diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index 40dcff3999b8f..4e5d44f30e908 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -301,14 +301,14 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M, RS.ParametersContainer.getParameter(&Info); if (!P) continue; - if (std::holds_alternative(*P)) { - auto Constants = std::get(*P); - OS << indent(Space + 2) << "Register Space: " << Constants.RegisterSpace + if (std::holds_alternative(*P)) { + auto* Constants = std::get(*P); + OS << indent(Space + 2) << "Register Space: " << Constants->RegisterSpace << "\n"; OS << indent(Space + 2) - << "Shader Register: " << Constants.ShaderRegister << "\n"; + << "Shader Register: " << Constants->ShaderRegister << "\n"; OS << indent(Space + 2) - << "Num 32 Bit Values: " << Constants.Num32BitValues << "\n"; + << "Num 32 Bit Values: " << Constants->Num32BitValues << "\n"; } } Space--; From 93e4cf299ba6898be9178f1c039eba052a8c9255 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Mon, 28 Apr 2025 22:39:36 +0000 Subject: [PATCH 10/35] clean up --- .../llvm/MC/DXContainerRootSignature.h | 21 +++++++------------ llvm/lib/MC/DXContainerRootSignature.cpp | 14 +++++++------ llvm/lib/Target/DirectX/DXILRootSignature.cpp | 8 +++---- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/llvm/include/llvm/MC/DXContainerRootSignature.h b/llvm/include/llvm/MC/DXContainerRootSignature.h index 33680c90d595f..507204d5b2ba4 100644 --- a/llvm/include/llvm/MC/DXContainerRootSignature.h +++ b/llvm/include/llvm/MC/DXContainerRootSignature.h @@ -29,11 +29,11 @@ struct RootParameterInfo { : Header(H), Location(L) {} }; -using RootDescriptor = std::variant; -using ParametersView = - std::variant; +using RootDescriptor = std::variant; +using ParametersView = std::variant; struct RootParametersContainer { SmallVector ParametersInfo; @@ -69,10 +69,10 @@ struct RootParametersContainer { case llvm::to_underlying(dxbc::RootParameterType::SRV): case llvm::to_underlying(dxbc::RootParameterType::UAV): RootDescriptor VersionedParam = Descriptors[H->Location]; - if (std::holds_alternative( + if (std::holds_alternative( VersionedParam)) - return std::get(VersionedParam); - return std::get(VersionedParam); + return std::get(VersionedParam); + return std::get(VersionedParam); } return std::nullopt; @@ -86,11 +86,6 @@ struct RootParametersContainer { SmallVector::const_iterator end() const { return ParametersInfo.end(); } - - llvm::iterator_range::const_iterator> - getInfo() const { - return llvm::make_range(begin(), end()); - } }; struct RootSignatureDesc { diff --git a/llvm/lib/MC/DXContainerRootSignature.cpp b/llvm/lib/MC/DXContainerRootSignature.cpp index c3a535592b78f..252f864f6f4bd 100644 --- a/llvm/lib/MC/DXContainerRootSignature.cpp +++ b/llvm/lib/MC/DXContainerRootSignature.cpp @@ -80,22 +80,24 @@ void RootSignatureDesc::write(raw_ostream &OS) const { auto P = ParametersContainer.getParameter(H); if (!P) continue; - if (std::holds_alternative(*P)) { - auto* Constants = std::get(*P); + if (std::holds_alternative(*P)) { + auto *Constants = std::get(*P); support::endian::write(BOS, Constants->ShaderRegister, llvm::endianness::little); support::endian::write(BOS, Constants->RegisterSpace, llvm::endianness::little); support::endian::write(BOS, Constants->Num32BitValues, llvm::endianness::little); - } else if (std::holds_alternative(*P)) { - auto* Descriptor = std::get(*P); + } else if (std::holds_alternative( + *P)) { + auto *Descriptor = std::get(*P); support::endian::write(BOS, Descriptor->ShaderRegister, llvm::endianness::little); support::endian::write(BOS, Descriptor->RegisterSpace, llvm::endianness::little); - } else if (std::holds_alternative(*P)) { - auto* Descriptor = std::get(*P); + } else if (std::holds_alternative( + *P)) { + auto *Descriptor = std::get(*P); support::endian::write(BOS, Descriptor->ShaderRegister, llvm::endianness::little); diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index 4e5d44f30e908..30ca4d8f7c8ed 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -301,10 +301,10 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M, RS.ParametersContainer.getParameter(&Info); if (!P) continue; - if (std::holds_alternative(*P)) { - auto* Constants = std::get(*P); - OS << indent(Space + 2) << "Register Space: " << Constants->RegisterSpace - << "\n"; + if (std::holds_alternative(*P)) { + auto *Constants = std::get(*P); + OS << indent(Space + 2) + << "Register Space: " << Constants->RegisterSpace << "\n"; OS << indent(Space + 2) << "Shader Register: " << Constants->ShaderRegister << "\n"; OS << indent(Space + 2) From b45b1b611730589a9a378ea5a6b9e411fb959c84 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Mon, 28 Apr 2025 22:49:22 +0000 Subject: [PATCH 11/35] fix --- llvm/lib/ObjectYAML/DXContainerEmitter.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp index 381fe41d8b01c..5f2e71ac2495e 100644 --- a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp +++ b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp @@ -305,6 +305,7 @@ void DXContainerWriter::writeParts(raw_ostream &OS) { RS.ParametersContainer.addInfo(Header, -1); } } + } RS.write(OS); break; From f804a23d7a3f29149b7b8c88c68fa24bbb1b23a0 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Mon, 28 Apr 2025 23:49:45 +0000 Subject: [PATCH 12/35] fix --- .../llvm/MC/DXContainerRootSignature.h | 19 ++++++++++--------- llvm/lib/MC/DXContainerRootSignature.cpp | 10 ++++++---- llvm/lib/ObjectYAML/DXContainerEmitter.cpp | 2 +- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/llvm/include/llvm/MC/DXContainerRootSignature.h b/llvm/include/llvm/MC/DXContainerRootSignature.h index 507204d5b2ba4..c8af613a57094 100644 --- a/llvm/include/llvm/MC/DXContainerRootSignature.h +++ b/llvm/include/llvm/MC/DXContainerRootSignature.h @@ -29,8 +29,8 @@ struct RootParameterInfo { : Header(H), Location(L) {} }; -using RootDescriptor = std::variant; +using RootDescriptor = std::variant; using ParametersView = std::variant; @@ -52,13 +52,13 @@ struct RootParametersContainer { void addParameter(dxbc::RootParameterHeader H, dxbc::RST0::v0::RootDescriptor D) { addInfo(H, Descriptors.size()); - Descriptors.push_back(std::move(&D)); + Descriptors.push_back(D); } void addParameter(dxbc::RootParameterHeader H, dxbc::RST0::v1::RootDescriptor D) { addInfo(H, Descriptors.size()); - Descriptors.push_back(std::move(&D)); + Descriptors.push_back(D); } std::optional getParameter(const RootParameterInfo *H) const { @@ -68,11 +68,12 @@ struct RootParametersContainer { case llvm::to_underlying(dxbc::RootParameterType::CBV): case llvm::to_underlying(dxbc::RootParameterType::SRV): case llvm::to_underlying(dxbc::RootParameterType::UAV): - RootDescriptor VersionedParam = Descriptors[H->Location]; - if (std::holds_alternative( - VersionedParam)) - return std::get(VersionedParam); - return std::get(VersionedParam); + const RootDescriptor &VersionedParam = Descriptors[H->Location]; + if (std::holds_alternative( + VersionedParam)) { + return &std::get(VersionedParam); + } + return &std::get(VersionedParam); } return std::nullopt; diff --git a/llvm/lib/MC/DXContainerRootSignature.cpp b/llvm/lib/MC/DXContainerRootSignature.cpp index 252f864f6f4bd..641c2f5fa1b1b 100644 --- a/llvm/lib/MC/DXContainerRootSignature.cpp +++ b/llvm/lib/MC/DXContainerRootSignature.cpp @@ -80,8 +80,8 @@ void RootSignatureDesc::write(raw_ostream &OS) const { auto P = ParametersContainer.getParameter(H); if (!P) continue; - if (std::holds_alternative(*P)) { - auto *Constants = std::get(*P); + if (std::holds_alternative(P.value())) { + auto *Constants = std::get(P.value()); support::endian::write(BOS, Constants->ShaderRegister, llvm::endianness::little); support::endian::write(BOS, Constants->RegisterSpace, @@ -90,14 +90,16 @@ void RootSignatureDesc::write(raw_ostream &OS) const { llvm::endianness::little); } else if (std::holds_alternative( *P)) { - auto *Descriptor = std::get(*P); + auto *Descriptor = + std::get(P.value()); support::endian::write(BOS, Descriptor->ShaderRegister, llvm::endianness::little); support::endian::write(BOS, Descriptor->RegisterSpace, llvm::endianness::little); } else if (std::holds_alternative( *P)) { - auto *Descriptor = std::get(*P); + auto *Descriptor = + std::get(P.value()); support::endian::write(BOS, Descriptor->ShaderRegister, llvm::endianness::little); diff --git a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp index 5f2e71ac2495e..b8ea1b048edfe 100644 --- a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp +++ b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp @@ -299,13 +299,13 @@ void DXContainerWriter::writeParts(raw_ostream &OS) { Descriptor.ShaderRegister = Param.Descriptor.ShaderRegister; Descriptor.Flags = Param.Descriptor.getEncodedFlags(); RS.ParametersContainer.addParameter(Header, Descriptor); + } break; default: // Handling invalid parameter type edge case RS.ParametersContainer.addInfo(Header, -1); } } - } RS.write(OS); break; From 15eb6f50b947f59b265fd092d6a97c51e5742e6b Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Mon, 5 May 2025 18:26:14 +0000 Subject: [PATCH 13/35] fix naming --- llvm/include/llvm/BinaryFormat/DXContainerConstants.def | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def index bd9bd760547dc..db0379b90ef6b 100644 --- a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def +++ b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def @@ -73,7 +73,7 @@ ROOT_ELEMENT_FLAG(11, SamplerHeapDirectlyIndexed) #endif // ROOT_ELEMENT_FLAG -// ROOT_ELEMENT_FLAG(bit offset for the flag, name). +// ROOT_DESCRIPTOR_FLAG(bit offset for the flag, name). #ifdef ROOT_DESCRIPTOR_FLAG ROOT_DESCRIPTOR_FLAG(0, NONE) From b9d7f076e9c1b3a1c1027b4ed9e8012a58b7cd2b Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Mon, 5 May 2025 18:26:14 +0000 Subject: [PATCH 14/35] fix naming --- llvm/include/llvm/BinaryFormat/DXContainerConstants.def | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def index bd9bd760547dc..db0379b90ef6b 100644 --- a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def +++ b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def @@ -73,7 +73,7 @@ ROOT_ELEMENT_FLAG(11, SamplerHeapDirectlyIndexed) #endif // ROOT_ELEMENT_FLAG -// ROOT_ELEMENT_FLAG(bit offset for the flag, name). +// ROOT_DESCRIPTOR_FLAG(bit offset for the flag, name). #ifdef ROOT_DESCRIPTOR_FLAG ROOT_DESCRIPTOR_FLAG(0, NONE) From 46cc8c1a557c2783159411b72543e1d350e2b55b Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Thu, 8 May 2025 17:37:32 +0000 Subject: [PATCH 15/35] addressing comments --- llvm/include/llvm/MC/DXContainerRootSignature.h | 2 -- llvm/lib/MC/DXContainerRootSignature.cpp | 2 -- llvm/lib/ObjectYAML/DXContainerEmitter.cpp | 6 ++++-- llvm/lib/Target/DirectX/DXILRootSignature.cpp | 1 - 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/llvm/include/llvm/MC/DXContainerRootSignature.h b/llvm/include/llvm/MC/DXContainerRootSignature.h index c8af613a57094..b290f4c96c20f 100644 --- a/llvm/include/llvm/MC/DXContainerRootSignature.h +++ b/llvm/include/llvm/MC/DXContainerRootSignature.h @@ -6,9 +6,7 @@ // //===----------------------------------------------------------------------===// -#include "llvm/ADT/STLForwardCompat.h" #include "llvm/BinaryFormat/DXContainer.h" -#include #include #include #include diff --git a/llvm/lib/MC/DXContainerRootSignature.cpp b/llvm/lib/MC/DXContainerRootSignature.cpp index 641c2f5fa1b1b..54a8bd3e90c69 100644 --- a/llvm/lib/MC/DXContainerRootSignature.cpp +++ b/llvm/lib/MC/DXContainerRootSignature.cpp @@ -8,9 +8,7 @@ #include "llvm/MC/DXContainerRootSignature.h" #include "llvm/ADT/SmallString.h" -#include "llvm/BinaryFormat/DXContainer.h" #include "llvm/Support/EndianStream.h" -#include using namespace llvm; using namespace llvm::mcdxbc; diff --git a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp index b8ea1b048edfe..1c163587d295c 100644 --- a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp +++ b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp @@ -298,11 +298,13 @@ void DXContainerWriter::writeParts(raw_ostream &OS) { Descriptor.RegisterSpace = Param.Descriptor.RegisterSpace; Descriptor.ShaderRegister = Param.Descriptor.ShaderRegister; Descriptor.Flags = Param.Descriptor.getEncodedFlags(); - RS.ParametersContainer.addParameter(Header, Descriptor); + RS.ParametersContainer.addParameter(Header, Descriptor); } break; default: - // Handling invalid parameter type edge case + // Handling invalid parameter type edge case. We intentionally let + // obj2yaml/yaml2obj parse and emit invalid dxcontainer data, in order + // for that to be used as a testing tool more effectively. RS.ParametersContainer.addInfo(Header, -1); } } diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index 30ca4d8f7c8ed..1bd816b026fec 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -30,7 +30,6 @@ #include #include #include -#include using namespace llvm; using namespace llvm::dxil; From 1b3e10ab85716e194ce16c273a6091e0136bf890 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Thu, 8 May 2025 21:44:55 +0000 Subject: [PATCH 16/35] addressing comments --- llvm/include/llvm/BinaryFormat/DXContainer.h | 21 ++++++++----- .../llvm/MC/DXContainerRootSignature.h | 2 +- llvm/include/llvm/Object/DXContainer.h | 31 +++++++++++-------- .../include/llvm/ObjectYAML/DXContainerYAML.h | 2 +- llvm/lib/MC/DXContainerRootSignature.cpp | 4 +-- llvm/lib/ObjectYAML/DXContainerYAML.cpp | 2 +- 6 files changed, 36 insertions(+), 26 deletions(-) diff --git a/llvm/include/llvm/BinaryFormat/DXContainer.h b/llvm/include/llvm/BinaryFormat/DXContainer.h index 3dbcfa82f3d7c..4fbc0cf1e5954 100644 --- a/llvm/include/llvm/BinaryFormat/DXContainer.h +++ b/llvm/include/llvm/BinaryFormat/DXContainer.h @@ -585,8 +585,8 @@ struct ProgramSignatureElement { static_assert(sizeof(ProgramSignatureElement) == 32, "ProgramSignatureElement is misaligned"); -namespace RST0 { -namespace v0 { +namespace RTS0 { +namespace v1 { struct RootDescriptor { uint32_t ShaderRegister; uint32_t RegisterSpace; @@ -595,18 +595,23 @@ struct RootDescriptor { sys::swapByteOrder(RegisterSpace); } }; -} // namespace v0 +} // namespace v1 -namespace v1 { -struct RootDescriptor : public v0::RootDescriptor { +namespace v2 { +struct RootDescriptor : public v1::RootDescriptor { uint32_t Flags; + + RootDescriptor() = default; + RootDescriptor(v1::RootDescriptor &Base) + : v1::RootDescriptor(Base), Flags(0u) {} + void swapBytes() { - v0::RootDescriptor::swapBytes(); + v1::RootDescriptor::swapBytes(); sys::swapByteOrder(Flags); } }; -} // namespace v1 -} // namespace RST0 +} // namespace v2 +} // namespace RTS0 // following dx12 naming // https://learn.microsoft.com/en-us/windows/win32/api/d3d12/ns-d3d12-d3d12_root_constants struct RootConstants { diff --git a/llvm/include/llvm/MC/DXContainerRootSignature.h b/llvm/include/llvm/MC/DXContainerRootSignature.h index 44e26c81eedc1..e3c4900ba175c 100644 --- a/llvm/include/llvm/MC/DXContainerRootSignature.h +++ b/llvm/include/llvm/MC/DXContainerRootSignature.h @@ -19,7 +19,7 @@ struct RootParameter { dxbc::RootParameterHeader Header; union { dxbc::RootConstants Constants; - dxbc::RST0::v1::RootDescriptor Descriptor; + dxbc::RTS0::v2::RootDescriptor Descriptor; }; }; struct RootSignatureDesc { diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h index e359d85f08bec..f23797149d377 100644 --- a/llvm/include/llvm/Object/DXContainer.h +++ b/llvm/include/llvm/Object/DXContainer.h @@ -17,6 +17,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" +#include "llvm/ADT/Twine.h" #include "llvm/BinaryFormat/DXContainer.h" #include "llvm/Object/Error.h" #include "llvm/Support/Error.h" @@ -124,16 +125,13 @@ struct RootParameterView { RootParameterView(uint32_t V, const dxbc::RootParameterHeader &H, StringRef P) : Header(H), ParamData(P) {} - template Expected readParameter() { - assert(sizeof(VersionT) <= sizeof(T) && - "Parameter of higher version must inherit all previous version data " - "members"); - if (sizeof(VersionT) != ParamData.size()) + template Expected readParameter() { + if (sizeof(T) != ParamData.size()) return make_error( "Reading structure out of file bounds", object_error::parse_failed); T Struct; - memcpy(&Struct, ParamData.data(), sizeof(VersionT)); + memcpy(&Struct, ParamData.data(), sizeof(T)); // DXContainer is always little endian if (sys::IsBigEndianHost) Struct.swapBytes(); @@ -162,11 +160,18 @@ struct RootDescriptorView : RootParameterView { llvm::to_underlying(dxbc::RootParameterType::UAV)); } - llvm::Expected read(uint32_t Version) { - if (Version == 1) - return readParameter(); - return readParameter(); + llvm::Expected read(uint32_t Version) { + if (Version == 1) { + auto Descriptor = readParameter(); + if (Error E = Descriptor.takeError()) + return E; + return dxbc::RTS0::v2::RootDescriptor(*Descriptor); + } + if (Version != 2) + return make_error("Invalid Root Signature version: " + + Twine(Version), + object_error::parse_failed); + return readParameter(); } }; @@ -217,9 +222,9 @@ class RootSignature { case dxbc::RootParameterType::SRV: case dxbc::RootParameterType::UAV: if (Version == 1) - DataSize = sizeof(dxbc::RST0::v0::RootDescriptor); + DataSize = sizeof(dxbc::RTS0::v1::RootDescriptor); else - DataSize = sizeof(dxbc::RST0::v1::RootDescriptor); + DataSize = sizeof(dxbc::RTS0::v2::RootDescriptor); break; } size_t EndOfSectionByte = getNumStaticSamplers() == 0 diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h index 8bb9da7884bed..d9d43b40db299 100644 --- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h +++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h @@ -95,7 +95,7 @@ struct RootParameterYamlDesc { uint32_t Type; uint32_t Visibility; uint32_t Offset; - RootParameterYamlDesc(){}; + RootParameterYamlDesc() {}; RootParameterYamlDesc(uint32_t T) : Type(T) { switch (T) { diff --git a/llvm/lib/MC/DXContainerRootSignature.cpp b/llvm/lib/MC/DXContainerRootSignature.cpp index 2693cb9943d5e..161711a79e467 100644 --- a/llvm/lib/MC/DXContainerRootSignature.cpp +++ b/llvm/lib/MC/DXContainerRootSignature.cpp @@ -41,9 +41,9 @@ size_t RootSignatureDesc::getSize() const { case llvm::to_underlying(dxbc::RootParameterType::SRV): case llvm::to_underlying(dxbc::RootParameterType::UAV): if (Version == 1) - Size += sizeof(dxbc::RST0::v0::RootDescriptor); + Size += sizeof(dxbc::RTS0::v1::RootDescriptor); else - Size += sizeof(dxbc::RST0::v1::RootDescriptor); + Size += sizeof(dxbc::RTS0::v2::RootDescriptor); break; } diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp index 18c1299d4b867..4c681d80b3358 100644 --- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp +++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp @@ -82,7 +82,7 @@ DXContainerYAML::RootSignatureYamlDesc::create( NewP.Constants.RegisterSpace = Constants.RegisterSpace; } else if (auto *RDV = dyn_cast(&ParamView)) { - llvm::Expected DescriptorOrErr = + llvm::Expected DescriptorOrErr = RDV->read(Version); if (Error E = DescriptorOrErr.takeError()) return std::move(E); From 1f3195784775b3244fd6c0d7447026d290605e85 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Thu, 8 May 2025 21:48:35 +0000 Subject: [PATCH 17/35] addressing comments --- llvm/include/llvm/Object/DXContainer.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h index f23797149d377..c93c85e638f7b 100644 --- a/llvm/include/llvm/Object/DXContainer.h +++ b/llvm/include/llvm/Object/DXContainer.h @@ -25,6 +25,7 @@ #include "llvm/TargetParser/Triple.h" #include #include +#include #include namespace llvm { @@ -122,8 +123,10 @@ namespace DirectX { struct RootParameterView { const dxbc::RootParameterHeader &Header; StringRef ParamData; + uint32_t Version; + RootParameterView(uint32_t V, const dxbc::RootParameterHeader &H, StringRef P) - : Header(H), ParamData(P) {} + : Header(H), ParamData(P), Version(V) {} template Expected readParameter() { if (sizeof(T) != ParamData.size()) From e8fbfce6d9c761a640534e8d9ed731a5a4ac986a Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Thu, 8 May 2025 22:03:45 +0000 Subject: [PATCH 18/35] clean up --- llvm/include/llvm/Object/DXContainer.h | 2 +- llvm/include/llvm/ObjectYAML/DXContainerYAML.h | 1 - llvm/lib/ObjectYAML/DXContainerYAML.cpp | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h index c93c85e638f7b..985a4b0c49e5a 100644 --- a/llvm/include/llvm/Object/DXContainer.h +++ b/llvm/include/llvm/Object/DXContainer.h @@ -129,11 +129,11 @@ struct RootParameterView { : Header(H), ParamData(P), Version(V) {} template Expected readParameter() { + T Struct; if (sizeof(T) != ParamData.size()) return make_error( "Reading structure out of file bounds", object_error::parse_failed); - T Struct; memcpy(&Struct, ParamData.data(), sizeof(T)); // DXContainer is always little endian if (sys::IsBigEndianHost) diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h index d9d43b40db299..73532fa392520 100644 --- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h +++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h @@ -21,7 +21,6 @@ #include "llvm/ObjectYAML/YAML.h" #include "llvm/Support/YAMLTraits.h" #include -#include #include #include #include diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp index 4c681d80b3358..8964c913946f2 100644 --- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp +++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp @@ -15,7 +15,6 @@ #include "llvm/ADT/STLForwardCompat.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/BinaryFormat/DXContainer.h" -#include "llvm/Object/DXContainer.h" #include "llvm/Support/Error.h" #include "llvm/Support/ScopedPrinter.h" #include From a31e5a5d14a2e81a411e30e2b50150a0ba85d409 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Fri, 9 May 2025 00:10:39 +0000 Subject: [PATCH 19/35] removing v parameter --- llvm/include/llvm/Object/DXContainer.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h index 985a4b0c49e5a..61bfa9411cc57 100644 --- a/llvm/include/llvm/Object/DXContainer.h +++ b/llvm/include/llvm/Object/DXContainer.h @@ -123,10 +123,9 @@ namespace DirectX { struct RootParameterView { const dxbc::RootParameterHeader &Header; StringRef ParamData; - uint32_t Version; - RootParameterView(uint32_t V, const dxbc::RootParameterHeader &H, StringRef P) - : Header(H), ParamData(P), Version(V) {} + RootParameterView(const dxbc::RootParameterHeader &H, StringRef P) + : Header(H), ParamData(P) {} template Expected readParameter() { T Struct; @@ -238,7 +237,7 @@ class RootSignature { return parseFailed("Reading structure out of file bounds"); StringRef Buff = PartData.substr(Header.ParameterOffset, DataSize); - RootParameterView View = RootParameterView(Version, Header, Buff); + RootParameterView View = RootParameterView(Header, Buff); return View; } }; From ad415a7a5a4ac5be41445f2d0527895b60e05d7b Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Fri, 9 May 2025 00:28:07 +0000 Subject: [PATCH 20/35] clean up --- llvm/include/llvm/MC/DXContainerRootSignature.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/llvm/include/llvm/MC/DXContainerRootSignature.h b/llvm/include/llvm/MC/DXContainerRootSignature.h index 7489b7437ff69..b07d0f66e02a4 100644 --- a/llvm/include/llvm/MC/DXContainerRootSignature.h +++ b/llvm/include/llvm/MC/DXContainerRootSignature.h @@ -8,8 +8,7 @@ #include "llvm/BinaryFormat/DXContainer.h" #include -#include -#include +#include #include namespace llvm { From f8755551e2049418d6f09bedaa685e72d3a76f15 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Tue, 13 May 2025 02:07:31 +0000 Subject: [PATCH 21/35] adding support for root descriptors --- llvm/lib/Target/DirectX/DXILRootSignature.cpp | 64 ++++++++++++++++++- llvm/lib/Target/DirectX/DXILRootSignature.h | 3 +- .../RootSignature-RootDescriptor.ll | 34 ++++++++++ 3 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index 1bd816b026fec..77bdb2c2f588f 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -55,6 +55,14 @@ static std::optional extractMdIntValue(MDNode *Node, return std::nullopt; } +static std::optional extractMdStringValue(MDNode *Node, + unsigned int OpId) { + MDString *NodeText = cast(Node->getOperand(OpId)); + if (NodeText == nullptr) + return std::nullopt; + return NodeText->getString(); +} + static bool parseRootFlags(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD, MDNode *RootFlagNode) { @@ -105,6 +113,56 @@ static bool parseRootConstants(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD, return false; } +static bool parseRootDescriptors(LLVMContext *Ctx, + mcdxbc::RootSignatureDesc &RSD, + MDNode *RootDescriptorNode) { + + if (RootDescriptorNode->getNumOperands() != 5) + return reportError(Ctx, "Invalid format for RootConstants Element"); + + std::optional ElementText = + extractMdStringValue(RootDescriptorNode, 0); + assert(!ElementText->empty()); + + dxbc::RootParameterHeader Header; + Header.ParameterType = + StringSwitch(*ElementText) + .Case("RootCBV", llvm::to_underlying(dxbc::RootParameterType::CBV)) + .Case("RootSRV", llvm::to_underlying(dxbc::RootParameterType::SRV)) + .Case("RootUAV", llvm::to_underlying(dxbc::RootParameterType::UAV)); + + if (std::optional Val = extractMdIntValue(RootDescriptorNode, 1)) + Header.ShaderVisibility = *Val; + else + return reportError(Ctx, "Invalid value for ShaderVisibility"); + + dxbc::RTS0::v1::RootDescriptor Descriptor; + if (std::optional Val = extractMdIntValue(RootDescriptorNode, 2)) + Descriptor.ShaderRegister = *Val; + else + return reportError(Ctx, "Invalid value for ShaderRegister"); + + if (std::optional Val = extractMdIntValue(RootDescriptorNode, 3)) + Descriptor.RegisterSpace = *Val; + else + return reportError(Ctx, "Invalid value for RegisterSpace"); + + if (RSD.Version == 1) { + RSD.ParametersContainer.addParameter(Header, Descriptor); + return false; + } + assert(RSD.Version > 1); + dxbc::RTS0::v2::RootDescriptor DescriptorV2(Descriptor); + + if (std::optional Val = extractMdIntValue(RootDescriptorNode, 4)) + DescriptorV2.Flags = *Val; + else + return reportError(Ctx, "Invalid value for Root Descriptor Flags"); + + RSD.ParametersContainer.addParameter(Header, DescriptorV2); + return false; +} + static bool parseRootSignatureElement(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD, MDNode *Element) { @@ -116,6 +174,9 @@ static bool parseRootSignatureElement(LLVMContext *Ctx, StringSwitch(ElementText->getString()) .Case("RootFlags", RootSignatureElementKind::RootFlags) .Case("RootConstants", RootSignatureElementKind::RootConstants) + .Case("RootCBV", RootSignatureElementKind::RootDescriptors) + .Case("RootSRV", RootSignatureElementKind::RootDescriptors) + .Case("RootUAV", RootSignatureElementKind::RootDescriptors) .Default(RootSignatureElementKind::Error); switch (ElementKind) { @@ -124,7 +185,8 @@ static bool parseRootSignatureElement(LLVMContext *Ctx, return parseRootFlags(Ctx, RSD, Element); case RootSignatureElementKind::RootConstants: return parseRootConstants(Ctx, RSD, Element); - break; + case RootSignatureElementKind::RootDescriptors: + return parseRootDescriptors(Ctx, RSD, Element); case RootSignatureElementKind::Error: return reportError(Ctx, "Invalid Root Signature Element: " + ElementText->getString()); diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.h b/llvm/lib/Target/DirectX/DXILRootSignature.h index 93ec614f1ab85..b8742d1b1fdfd 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.h +++ b/llvm/lib/Target/DirectX/DXILRootSignature.h @@ -27,7 +27,8 @@ namespace dxil { enum class RootSignatureElementKind { Error = 0, RootFlags = 1, - RootConstants = 2 + RootConstants = 2, + RootDescriptors = 3 }; class RootSignatureAnalysis : public AnalysisInfoMixin { friend AnalysisInfoMixin; diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll new file mode 100644 index 0000000000000..4143e56b609b9 --- /dev/null +++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll @@ -0,0 +1,34 @@ +; RUN: opt %s -dxil-embed -dxil-globals -S -o - | FileCheck %s +; RUN: llc %s --filetype=obj -o - | obj2yaml | FileCheck %s --check-prefix=DXC + +target triple = "dxil-unknown-shadermodel6.0-compute" + +; CHECK: @dx.rts0 = private constant [48 x i8] c"{{.*}}", section "RTS0", align 4 + +define void @main() #0 { +entry: + ret void +} +attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } + + +!dx.rootsignatures = !{!2} ; list of function/root signature pairs +!2 = !{ ptr @main, !3 } ; function, root signature +!3 = !{ !5 } ; list of root signature elements +!5 = !{ !"RootCBV", i32 0, i32 1, i32 2, i32 3 } + +; DXC: - Name: RTS0 +; DXC-NEXT: Size: 48 +; DXC-NEXT: RootSignature: +; DXC-NEXT: Version: 2 +; DXC-NEXT: NumRootParameters: 1 +; DXC-NEXT: RootParametersOffset: 24 +; DXC-NEXT: NumStaticSamplers: 0 +; DXC-NEXT: StaticSamplersOffset: 0 +; DXC-NEXT: Parameters: +; DXC-NEXT: - ParameterType: 2 +; DXC-NEXT: ShaderVisibility: 0 +; DXC-NEXT: Descriptor: +; DXC-NEXT: RegisterSpace: 2 +; DXC-NEXT: ShaderRegister: 1 +; DXC: DATA_VOLATILE: true From 4f7f9986503dca40f01b1a8bc7a54a440a620cdf Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Tue, 13 May 2025 16:34:57 +0000 Subject: [PATCH 22/35] removing none as a flag option --- llvm/include/llvm/BinaryFormat/DXContainerConstants.def | 1 - .../DirectX/ContainerData/RootSignature-RootDescriptor.ll | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def index 1645018aebedb..eb67c82628716 100644 --- a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def +++ b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def @@ -78,7 +78,6 @@ ROOT_ELEMENT_FLAG(11, SamplerHeapDirectlyIndexed) // ROOT_DESCRIPTOR_FLAG(bit offset for the flag, name). #ifdef ROOT_DESCRIPTOR_FLAG -ROOT_DESCRIPTOR_FLAG(0, NONE) ROOT_DESCRIPTOR_FLAG(1, DATA_VOLATILE) ROOT_DESCRIPTOR_FLAG(2, DATA_STATIC_WHILE_SET_AT_EXECUTE) ROOT_DESCRIPTOR_FLAG(3, DATA_STATIC) diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll index 4143e56b609b9..5f2ce37afd893 100644 --- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll +++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll @@ -31,4 +31,4 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } ; DXC-NEXT: Descriptor: ; DXC-NEXT: RegisterSpace: 2 ; DXC-NEXT: ShaderRegister: 1 -; DXC: DATA_VOLATILE: true +; DXC-NEXT: DATA_VOLATILE: true From 3eb5e101b0087fcb29f7ecb6655c82fddf34428a Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Tue, 13 May 2025 18:06:42 +0000 Subject: [PATCH 23/35] adding tests --- .../BinaryFormat/DXContainerConstants.def | 1 + llvm/lib/Target/DirectX/DXILRootSignature.cpp | 61 +++++++++++++++++++ ...tSignature-RootDescriptor-Invalid-Flags.ll | 18 ++++++ ...re-RootDescriptor-Invalid-RegisterSpace.ll | 18 ++++++ ...re-RootDescriptor-Invalid-RegisterValue.ll | 18 ++++++ .../RootSignature-RootDescriptor.ll | 4 +- 6 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-Flags.ll create mode 100644 llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterSpace.ll create mode 100644 llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterValue.ll diff --git a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def index eb67c82628716..1645018aebedb 100644 --- a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def +++ b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def @@ -78,6 +78,7 @@ ROOT_ELEMENT_FLAG(11, SamplerHeapDirectlyIndexed) // ROOT_DESCRIPTOR_FLAG(bit offset for the flag, name). #ifdef ROOT_DESCRIPTOR_FLAG +ROOT_DESCRIPTOR_FLAG(0, NONE) ROOT_DESCRIPTOR_FLAG(1, DATA_VOLATILE) ROOT_DESCRIPTOR_FLAG(2, DATA_STATIC_WHILE_SET_AT_EXECUTE) ROOT_DESCRIPTOR_FLAG(3, DATA_STATIC) diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index 77bdb2c2f588f..a2242dff2c2c1 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -12,6 +12,7 @@ //===----------------------------------------------------------------------===// #include "DXILRootSignature.h" #include "DirectX.h" +#include "llvm/ADT/STLForwardCompat.h" #include "llvm/ADT/StringSwitch.h" #include "llvm/ADT/Twine.h" #include "llvm/Analysis/DXILMetadataAnalysis.h" @@ -30,6 +31,7 @@ #include #include #include +#include using namespace llvm; using namespace llvm::dxil; @@ -217,6 +219,19 @@ static bool verifyVersion(uint32_t Version) { return (Version == 1 || Version == 2); } +static bool verifyRegisterValue(uint32_t RegisterValue) { + return !(RegisterValue == 0xFFFFFFFF); +} + +static bool verifyRegisterSpace(uint32_t RegisterSpace) { + return !(RegisterSpace >= 0xFFFFFFF0 && RegisterSpace <= 0xFFFFFFFF); +} + +static bool verifyDescriptorFlag(uint32_t Flags) { + return (Flags & ~0xE) == 0; +} + + static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) { if (!verifyVersion(RSD.Version)) { @@ -234,6 +249,38 @@ static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) { assert(dxbc::isValidParameterType(Info.Header.ParameterType) && "Invalid value for ParameterType"); + + + auto P = RSD.ParametersContainer.getParameter(&Info); + if(!P) + return reportError(Ctx, "Cannot locate parameter from Header Info"); + + if( std::holds_alternative(*P)){ + auto *Descriptor = std::get(P.value()); + + if(!verifyRegisterValue(Descriptor->ShaderRegister)) + return reportValueError(Ctx, "ShaderRegister", + Descriptor->ShaderRegister); + + if(!verifyRegisterSpace(Descriptor->RegisterSpace)) + return reportValueError(Ctx, "RegisterSpace", + Descriptor->RegisterSpace); + + } else if( std::holds_alternative(*P)){ + auto *Descriptor = std::get(P.value()); + + if(!verifyRegisterValue(Descriptor->ShaderRegister)) + return reportValueError(Ctx, "ShaderRegister", + Descriptor->ShaderRegister); + + if(!verifyRegisterSpace(Descriptor->RegisterSpace)) + return reportValueError(Ctx, "RegisterSpace", + Descriptor->RegisterSpace); + + if(!verifyDescriptorFlag(Descriptor->Flags)) + return reportValueError(Ctx, "DescriptorFlag", + Descriptor->Flags); + } } return false; @@ -370,6 +417,20 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M, << "Shader Register: " << Constants->ShaderRegister << "\n"; OS << indent(Space + 2) << "Num 32 Bit Values: " << Constants->Num32BitValues << "\n"; + } else if (std::holds_alternative(*P)) { + auto *Constants = std::get(*P); + OS << indent(Space + 2) + << "Register Space: " << Constants->RegisterSpace << "\n"; + OS << indent(Space + 2) + << "Shader Register: " << Constants->ShaderRegister << "\n"; + } else if (std::holds_alternative(*P)) { + auto *Constants = std::get(*P); + OS << indent(Space + 2) + << "Register Space: " << Constants->RegisterSpace << "\n"; + OS << indent(Space + 2) + << "Shader Register: " << Constants->ShaderRegister << "\n"; + OS << indent(Space + 2) + << "Flags: " << Constants->Flags << "\n"; } } Space--; diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-Flags.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-Flags.ll new file mode 100644 index 0000000000000..4229981240918 --- /dev/null +++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-Flags.ll @@ -0,0 +1,18 @@ +; RUN: not opt -passes='print' %s -S -o - 2>&1 | FileCheck %s + +target triple = "dxil-unknown-shadermodel6.0-compute" + + +; CHECK: error: Invalid value for DescriptorFlag: 3 +; CHECK-NOT: Root Signature Definitions +define void @main() #0 { +entry: + ret void +} +attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } + + +!dx.rootsignatures = !{!2} ; list of function/root signature pairs +!2 = !{ ptr @main, !3 } ; function, root signature +!3 = !{ !5 } ; list of root signature elements +!5 = !{ !"RootCBV", i32 0, i32 1, i32 2, i32 3 } diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterSpace.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterSpace.ll new file mode 100644 index 0000000000000..020d117ba45dc --- /dev/null +++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterSpace.ll @@ -0,0 +1,18 @@ +; RUN: not opt -passes='print' %s -S -o - 2>&1 | FileCheck %s + +target triple = "dxil-unknown-shadermodel6.0-compute" + + +; CHECK: error: Invalid value for RegisterSpace: 4294967280 +; CHECK-NOT: Root Signature Definitions +define void @main() #0 { +entry: + ret void +} +attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } + + +!dx.rootsignatures = !{!2} ; list of function/root signature pairs +!2 = !{ ptr @main, !3 } ; function, root signature +!3 = !{ !5 } ; list of root signature elements +!5 = !{ !"RootCBV", i32 0, i32 1, i32 4294967280, i32 0 } diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterValue.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterValue.ll new file mode 100644 index 0000000000000..edb8b943c6e35 --- /dev/null +++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterValue.ll @@ -0,0 +1,18 @@ +; RUN: not opt -passes='print' %s -S -o - 2>&1 | FileCheck %s + +target triple = "dxil-unknown-shadermodel6.0-compute" + + +; CHECK: error: Invalid value for ShaderRegister: 4294967295 +; CHECK-NOT: Root Signature Definitions +define void @main() #0 { +entry: + ret void +} +attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } + + +!dx.rootsignatures = !{!2} ; list of function/root signature pairs +!2 = !{ ptr @main, !3 } ; function, root signature +!3 = !{ !5 } ; list of root signature elements +!5 = !{ !"RootCBV", i32 0, i32 4294967295, i32 2, i32 3 } diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll index 5f2ce37afd893..9217945855cd9 100644 --- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll +++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll @@ -15,7 +15,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } !dx.rootsignatures = !{!2} ; list of function/root signature pairs !2 = !{ ptr @main, !3 } ; function, root signature !3 = !{ !5 } ; list of root signature elements -!5 = !{ !"RootCBV", i32 0, i32 1, i32 2, i32 3 } +!5 = !{ !"RootCBV", i32 0, i32 1, i32 2, i32 8 } ; DXC: - Name: RTS0 ; DXC-NEXT: Size: 48 @@ -31,4 +31,4 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } ; DXC-NEXT: Descriptor: ; DXC-NEXT: RegisterSpace: 2 ; DXC-NEXT: ShaderRegister: 1 -; DXC-NEXT: DATA_VOLATILE: true +; DXC-NEXT: DATA_STATIC: true From 58e17890afd06822d4fd663e8bd9195b4bc206a8 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Tue, 13 May 2025 19:30:07 +0000 Subject: [PATCH 24/35] clean up and add more tests --- llvm/lib/Target/DirectX/DXILRootSignature.cpp | 53 +++++++++---------- ...ure-RootDescriptor-Invalid-RegisterKind.ll | 18 +++++++ 2 files changed, 43 insertions(+), 28 deletions(-) create mode 100644 llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterKind.ll diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index a2242dff2c2c1..3ee52d32eaf2d 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -12,7 +12,6 @@ //===----------------------------------------------------------------------===// #include "DXILRootSignature.h" #include "DirectX.h" -#include "llvm/ADT/STLForwardCompat.h" #include "llvm/ADT/StringSwitch.h" #include "llvm/ADT/Twine.h" #include "llvm/Analysis/DXILMetadataAnalysis.h" @@ -31,7 +30,6 @@ #include #include #include -#include using namespace llvm; using namespace llvm::dxil; @@ -227,10 +225,7 @@ static bool verifyRegisterSpace(uint32_t RegisterSpace) { return !(RegisterSpace >= 0xFFFFFFF0 && RegisterSpace <= 0xFFFFFFFF); } -static bool verifyDescriptorFlag(uint32_t Flags) { - return (Flags & ~0xE) == 0; -} - +static bool verifyDescriptorFlag(uint32_t Flags) { return (Flags & ~0xE) == 0; } static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) { @@ -249,37 +244,38 @@ static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) { assert(dxbc::isValidParameterType(Info.Header.ParameterType) && "Invalid value for ParameterType"); - - + auto P = RSD.ParametersContainer.getParameter(&Info); - if(!P) - return reportError(Ctx, "Cannot locate parameter from Header Info"); - - if( std::holds_alternative(*P)){ - auto *Descriptor = std::get(P.value()); + if (!P) + return reportError(Ctx, "Cannot locate parameter from Header Info"); + + if (std::holds_alternative(*P)) { + auto *Descriptor = + std::get(P.value()); - if(!verifyRegisterValue(Descriptor->ShaderRegister)) + if (!verifyRegisterValue(Descriptor->ShaderRegister)) return reportValueError(Ctx, "ShaderRegister", Descriptor->ShaderRegister); - - if(!verifyRegisterSpace(Descriptor->RegisterSpace)) + + if (!verifyRegisterSpace(Descriptor->RegisterSpace)) return reportValueError(Ctx, "RegisterSpace", Descriptor->RegisterSpace); - } else if( std::holds_alternative(*P)){ - auto *Descriptor = std::get(P.value()); + } else if (std::holds_alternative( + *P)) { + auto *Descriptor = + std::get(P.value()); - if(!verifyRegisterValue(Descriptor->ShaderRegister)) + if (!verifyRegisterValue(Descriptor->ShaderRegister)) return reportValueError(Ctx, "ShaderRegister", Descriptor->ShaderRegister); - - if(!verifyRegisterSpace(Descriptor->RegisterSpace)) + + if (!verifyRegisterSpace(Descriptor->RegisterSpace)) return reportValueError(Ctx, "RegisterSpace", Descriptor->RegisterSpace); - if(!verifyDescriptorFlag(Descriptor->Flags)) - return reportValueError(Ctx, "DescriptorFlag", - Descriptor->Flags); + if (!verifyDescriptorFlag(Descriptor->Flags)) + return reportValueError(Ctx, "DescriptorFlag", Descriptor->Flags); } } @@ -417,20 +413,21 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M, << "Shader Register: " << Constants->ShaderRegister << "\n"; OS << indent(Space + 2) << "Num 32 Bit Values: " << Constants->Num32BitValues << "\n"; - } else if (std::holds_alternative(*P)) { + } else if (std::holds_alternative( + *P)) { auto *Constants = std::get(*P); OS << indent(Space + 2) << "Register Space: " << Constants->RegisterSpace << "\n"; OS << indent(Space + 2) << "Shader Register: " << Constants->ShaderRegister << "\n"; - } else if (std::holds_alternative(*P)) { + } else if (std::holds_alternative( + *P)) { auto *Constants = std::get(*P); OS << indent(Space + 2) << "Register Space: " << Constants->RegisterSpace << "\n"; OS << indent(Space + 2) << "Shader Register: " << Constants->ShaderRegister << "\n"; - OS << indent(Space + 2) - << "Flags: " << Constants->Flags << "\n"; + OS << indent(Space + 2) << "Flags: " << Constants->Flags << "\n"; } } Space--; diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterKind.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterKind.ll new file mode 100644 index 0000000000000..4aed84efbe2bc --- /dev/null +++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterKind.ll @@ -0,0 +1,18 @@ +; RUN: not opt -passes='print' %s -S -o - 2>&1 | FileCheck %s + +target triple = "dxil-unknown-shadermodel6.0-compute" + + +; CHECK: error: Invalid Root Signature Element: Invalid +; CHECK-NOT: Root Signature Definitions +define void @main() #0 { +entry: + ret void +} +attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } + + +!dx.rootsignatures = !{!2} ; list of function/root signature pairs +!2 = !{ ptr @main, !3 } ; function, root signature +!3 = !{ !5 } ; list of root signature elements +!5 = !{ !"Invalid", i32 0, i32 1, i32 2, i32 3 } From 81915ad5d9cb44a65cbdcd7945233519d395232f Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Fri, 30 May 2025 21:43:56 +0000 Subject: [PATCH 25/35] addressing comments --- llvm/lib/Target/DirectX/DXILRootSignature.cpp | 12 +++++++----- .../ContainerData/RootSignature-Parameters.ll | 10 ++++++++-- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index 3ee52d32eaf2d..e3e1734bf67d2 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -118,11 +118,13 @@ static bool parseRootDescriptors(LLVMContext *Ctx, MDNode *RootDescriptorNode) { if (RootDescriptorNode->getNumOperands() != 5) - return reportError(Ctx, "Invalid format for RootConstants Element"); + return reportError(Ctx, "Invalid format for Root Descriptor Element"); std::optional ElementText = extractMdStringValue(RootDescriptorNode, 0); - assert(!ElementText->empty()); + + if (!ElementText.has_value()) + return reportError(Ctx, "Root Descriptor, first element is not a string."); dxbc::RootParameterHeader Header; Header.ParameterType = @@ -273,9 +275,9 @@ static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) { if (!verifyRegisterSpace(Descriptor->RegisterSpace)) return reportValueError(Ctx, "RegisterSpace", Descriptor->RegisterSpace); - - if (!verifyDescriptorFlag(Descriptor->Flags)) - return reportValueError(Ctx, "DescriptorFlag", Descriptor->Flags); + if (RSD.Version > 1) + if (!verifyDescriptorFlag(Descriptor->Flags)) + return reportValueError(Ctx, "DescriptorFlag", Descriptor->Flags); } } diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters.ll index b55d1283df0c9..714c76213e1b5 100644 --- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters.ll +++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters.ll @@ -12,19 +12,25 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } !dx.rootsignatures = !{!2} ; list of function/root signature pairs !2 = !{ ptr @main, !3 } ; function, root signature -!3 = !{ !4, !5 } ; list of root signature elements +!3 = !{ !4, !5, !6 } ; list of root signature elements !4 = !{ !"RootFlags", i32 1 } ; 1 = allow_input_assembler_input_layout !5 = !{ !"RootConstants", i32 0, i32 1, i32 2, i32 3 } +!6 = !{ !"RootSRV", i32 1, i32 4, i32 5, i32 6 } ;CHECK-LABEL: Definition for 'main': ;CHECK-NEXT: Flags: 0x000001 ;CHECK-NEXT: Version: 2 ;CHECK-NEXT: RootParametersOffset: 24 -;CHECK-NEXT: NumParameters: 1 +;CHECK-NEXT: NumParameters: 2 ;CHECK-NEXT: - Parameter Type: 1 ;CHECK-NEXT: Shader Visibility: 0 ;CHECK-NEXT: Register Space: 2 ;CHECK-NEXT: Shader Register: 1 ;CHECK-NEXT: Num 32 Bit Values: 3 +;CHECK-NEXT: - Parameter Type: 3 +;CHECK-NEXT: Shader Visibility: 1 +;CHECK-NEXT: Register Space: 5 +;CHECK-NEXT: Shader Register: 4 +;CHECK-NEXT: Flags: 6 ;CHECK-NEXT: NumStaticSamplers: 0 ;CHECK-NEXT: StaticSamplersOffset: 0 From 0d541624e447c053ab68aa5d53e91da01eef7f1b Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Mon, 2 Jun 2025 18:12:25 +0000 Subject: [PATCH 26/35] clean --- llvm/include/llvm/MC/DXContainerRootSignature.h | 1 - llvm/lib/MC/DXContainerRootSignature.cpp | 10 +++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/llvm/include/llvm/MC/DXContainerRootSignature.h b/llvm/include/llvm/MC/DXContainerRootSignature.h index c49a13fb0012d..4c2ca063158c4 100644 --- a/llvm/include/llvm/MC/DXContainerRootSignature.h +++ b/llvm/include/llvm/MC/DXContainerRootSignature.h @@ -9,7 +9,6 @@ #include "llvm/BinaryFormat/DXContainer.h" #include #include -#include namespace llvm { diff --git a/llvm/lib/MC/DXContainerRootSignature.cpp b/llvm/lib/MC/DXContainerRootSignature.cpp index ebbb9cbda4111..6c71823a51f85 100644 --- a/llvm/lib/MC/DXContainerRootSignature.cpp +++ b/llvm/lib/MC/DXContainerRootSignature.cpp @@ -90,8 +90,7 @@ void RootSignatureDesc::write(raw_ostream &OS) const { } assert(NumParameters == ParamsOffsets.size()); - const RootParameterInfo *H = ParametersContainer.begin(); - for (size_t I = 0; I < NumParameters; ++I, H++) { + for (size_t I = 0; I < NumParameters; ++I) { rewriteOffsetToCurrentByte(BOS, ParamsOffsets[I]); const auto &[Type, Loc] = ParametersContainer.getTypeAndLocForParameter(I); switch (Type) { @@ -104,12 +103,13 @@ void RootSignatureDesc::write(raw_ostream &OS) const { llvm::endianness::little); support::endian::write(BOS, Constants.Num32BitValues, llvm::endianness::little); - } break; + break; + } case llvm::to_underlying(dxbc::RootParameterType::CBV): case llvm::to_underlying(dxbc::RootParameterType::SRV): - case llvm::to_underlying(dxbc::RootParameterType::UAV):{ + case llvm::to_underlying(dxbc::RootParameterType::UAV): { const dxbc::RTS0::v2::RootDescriptor &Descriptor = - ParametersContainer.getRootDescriptor(Loc); + ParametersContainer.getRootDescriptor(Loc); support::endian::write(BOS, Descriptor.ShaderRegister, llvm::endianness::little); From 7f70dc55ddbfee28c50b4f2e10f64557875433b3 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Mon, 2 Jun 2025 18:22:50 +0000 Subject: [PATCH 27/35] cleanup --- llvm/lib/Target/DirectX/DXILRootSignature.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index 1064a53679583..878272537d366 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -12,7 +12,6 @@ //===----------------------------------------------------------------------===// #include "DXILRootSignature.h" #include "DirectX.h" -#include "llvm/ADT/STLForwardCompat.h" #include "llvm/ADT/StringSwitch.h" #include "llvm/ADT/Twine.h" #include "llvm/Analysis/DXILMetadataAnalysis.h" From 0c570c8bb77ff4091af4ce66bbb2f2fcdf00b963 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Mon, 2 Jun 2025 21:56:05 +0000 Subject: [PATCH 28/35] adding requested comment --- llvm/lib/Target/DirectX/DXILRootSignature.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index 878272537d366..12f7066f5d349 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -129,6 +129,8 @@ static bool parseRootDescriptors(LLVMContext *Ctx, return reportError(Ctx, "Root Descriptor, first element is not a string."); dxbc::RTS0::v1::RootParameterHeader Header; + // a default scenario is not needed here. Scenarios where ElementText is + // invalid is previously checked and error handled when this method is called. Header.ParameterType = StringSwitch(*ElementText) .Case("RootCBV", llvm::to_underlying(dxbc::RootParameterType::CBV)) From d1ca37ddd4eed677b6b86842b2f8d96b64595a38 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Tue, 3 Jun 2025 17:36:39 +0000 Subject: [PATCH 29/35] addressing PR comments --- llvm/lib/Target/DirectX/DXILRootSignature.cpp | 62 +++++++++++-------- llvm/lib/Target/DirectX/DXILRootSignature.h | 4 +- 2 files changed, 38 insertions(+), 28 deletions(-) diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index 12f7066f5d349..5d3855b22058c 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -12,6 +12,7 @@ //===----------------------------------------------------------------------===// #include "DXILRootSignature.h" #include "DirectX.h" +#include "llvm/ADT/STLForwardCompat.h" #include "llvm/ADT/StringSwitch.h" #include "llvm/ADT/Twine.h" #include "llvm/Analysis/DXILMetadataAnalysis.h" @@ -57,7 +58,7 @@ static std::optional extractMdIntValue(MDNode *Node, static std::optional extractMdStringValue(MDNode *Node, unsigned int OpId) { - MDString *NodeText = cast(Node->getOperand(OpId)); + MDString *NodeText = dyn_cast(Node->getOperand(OpId)); if (NodeText == nullptr) return std::nullopt; return NodeText->getString(); @@ -117,25 +118,31 @@ static bool parseRootConstants(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD, static bool parseRootDescriptors(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD, - MDNode *RootDescriptorNode) { - + MDNode *RootDescriptorNode, + RootSignatureElementKind ElementKind) { + assert(ElementKind == RootSignatureElementKind::SRV || + ElementKind == RootSignatureElementKind::UAV || + ElementKind == RootSignatureElementKind::CBV && + "parseRootDescriptors should only be called with RootDescriptor " + "element kind."); if (RootDescriptorNode->getNumOperands() != 5) return reportError(Ctx, "Invalid format for Root Descriptor Element"); - std::optional ElementText = - extractMdStringValue(RootDescriptorNode, 0); - - if (!ElementText.has_value()) - return reportError(Ctx, "Root Descriptor, first element is not a string."); - dxbc::RTS0::v1::RootParameterHeader Header; - // a default scenario is not needed here. Scenarios where ElementText is - // invalid is previously checked and error handled when this method is called. - Header.ParameterType = - StringSwitch(*ElementText) - .Case("RootCBV", llvm::to_underlying(dxbc::RootParameterType::CBV)) - .Case("RootSRV", llvm::to_underlying(dxbc::RootParameterType::SRV)) - .Case("RootUAV", llvm::to_underlying(dxbc::RootParameterType::UAV)); + switch (ElementKind) { + case RootSignatureElementKind::SRV: + Header.ParameterType = llvm::to_underlying(dxbc::RootParameterType::SRV); + break; + case RootSignatureElementKind::UAV: + Header.ParameterType = llvm::to_underlying(dxbc::RootParameterType::UAV); + break; + case RootSignatureElementKind::CBV: + Header.ParameterType = llvm::to_underlying(dxbc::RootParameterType::CBV); + break; + default: + llvm_unreachable("invalid Root Descriptor kind"); + break; + } if (std::optional Val = extractMdIntValue(RootDescriptorNode, 1)) Header.ShaderVisibility = *Val; @@ -171,17 +178,17 @@ static bool parseRootDescriptors(LLVMContext *Ctx, static bool parseRootSignatureElement(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD, MDNode *Element) { - MDString *ElementText = cast(Element->getOperand(0)); - if (ElementText == nullptr) + std::optional ElementText = extractMdStringValue(Element, 0); + if (!ElementText.has_value()) return reportError(Ctx, "Invalid format for Root Element"); RootSignatureElementKind ElementKind = - StringSwitch(ElementText->getString()) + StringSwitch(*ElementText) .Case("RootFlags", RootSignatureElementKind::RootFlags) .Case("RootConstants", RootSignatureElementKind::RootConstants) - .Case("RootCBV", RootSignatureElementKind::RootDescriptors) - .Case("RootSRV", RootSignatureElementKind::RootDescriptors) - .Case("RootUAV", RootSignatureElementKind::RootDescriptors) + .Case("RootCBV", RootSignatureElementKind::CBV) + .Case("RootSRV", RootSignatureElementKind::SRV) + .Case("RootUAV", RootSignatureElementKind::UAV) .Default(RootSignatureElementKind::Error); switch (ElementKind) { @@ -190,11 +197,12 @@ static bool parseRootSignatureElement(LLVMContext *Ctx, return parseRootFlags(Ctx, RSD, Element); case RootSignatureElementKind::RootConstants: return parseRootConstants(Ctx, RSD, Element); - case RootSignatureElementKind::RootDescriptors: - return parseRootDescriptors(Ctx, RSD, Element); + case RootSignatureElementKind::CBV: + case RootSignatureElementKind::SRV: + case RootSignatureElementKind::UAV: + return parseRootDescriptors(Ctx, RSD, Element, ElementKind); case RootSignatureElementKind::Error: - return reportError(Ctx, "Invalid Root Signature Element: " + - ElementText->getString()); + return reportError(Ctx, "Invalid Root Signature Element: " + *ElementText); } llvm_unreachable("Unhandled RootSignatureElementKind enum."); @@ -223,7 +231,7 @@ static bool verifyVersion(uint32_t Version) { } static bool verifyRegisterValue(uint32_t RegisterValue) { - return !(RegisterValue == 0xFFFFFFFF); + return RegisterValue != ~0U; } static bool verifyRegisterSpace(uint32_t RegisterSpace) { diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.h b/llvm/lib/Target/DirectX/DXILRootSignature.h index b8742d1b1fdfd..3f25551b2b5ef 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.h +++ b/llvm/lib/Target/DirectX/DXILRootSignature.h @@ -28,7 +28,9 @@ enum class RootSignatureElementKind { Error = 0, RootFlags = 1, RootConstants = 2, - RootDescriptors = 3 + SRV = 3, + UAV = 4, + CBV = 5, }; class RootSignatureAnalysis : public AnalysisInfoMixin { friend AnalysisInfoMixin; From cb0780bf2d1b18c070622bbb761295d2a00c2b40 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Tue, 3 Jun 2025 17:37:06 +0000 Subject: [PATCH 30/35] formating --- llvm/lib/Target/DirectX/DXILRootSignature.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index 5d3855b22058c..d27d9d40c5ffb 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -420,7 +420,7 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M, << "Num 32 Bit Values: " << Constants.Num32BitValues << "\n"; break; } - case llvm::to_underlying(dxbc::RootParameterType::CBV): + case llvm::to_underlying(dxbc::RootParameterType::CBV): case llvm::to_underlying(dxbc::RootParameterType::UAV): case llvm::to_underlying(dxbc::RootParameterType::SRV): { const dxbc::RTS0::v2::RootDescriptor &Descriptor = From eeffded2ee6bab110124bd5065f8b6cf407e6cf7 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Tue, 3 Jun 2025 17:39:27 +0000 Subject: [PATCH 31/35] addressing PR comments --- llvm/lib/Target/DirectX/DXILRootSignature.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index d27d9d40c5ffb..eb9235f4b61d0 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -234,6 +234,8 @@ static bool verifyRegisterValue(uint32_t RegisterValue) { return RegisterValue != ~0U; } +// This Range is reserverved, therefore invalid, according to the spec +// https://github.com/llvm/wg-hlsl/blob/main/proposals/0002-root-signature-in-clang.md#all-the-values-should-be-legal static bool verifyRegisterSpace(uint32_t RegisterSpace) { return !(RegisterSpace >= 0xFFFFFFF0 && RegisterSpace <= 0xFFFFFFFF); } From 3cbe0cf278387ab63104650a575cde5dc9d8ef23 Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Tue, 3 Jun 2025 17:46:29 +0000 Subject: [PATCH 32/35] formating --- llvm/lib/Target/DirectX/DXILRootSignature.cpp | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index eb9235f4b61d0..ecd9c546025a6 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -260,26 +260,26 @@ static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) { assert(dxbc::isValidParameterType(Info.Header.ParameterType) && "Invalid value for ParameterType"); - switch(Info.Header.ParameterType) { - - case llvm::to_underlying(dxbc::RootParameterType::CBV): - case llvm::to_underlying(dxbc::RootParameterType::UAV): - case llvm::to_underlying(dxbc::RootParameterType::SRV): { - const dxbc::RTS0::v2::RootDescriptor &Descriptor = RSD.ParametersContainer.getRootDescriptor(Info.Location); - if (!verifyRegisterValue(Descriptor.ShaderRegister)) - return reportValueError(Ctx, "ShaderRegister", - Descriptor.ShaderRegister); - - if (!verifyRegisterSpace(Descriptor.RegisterSpace)) - return reportValueError(Ctx, "RegisterSpace", - Descriptor.RegisterSpace); - - if(RSD.Version > 1) { - if (!verifyDescriptorFlag(Descriptor.Flags)) - return reportValueError(Ctx, "DescriptorFlag", Descriptor.Flags); - } - break; + switch (Info.Header.ParameterType) { + + case llvm::to_underlying(dxbc::RootParameterType::CBV): + case llvm::to_underlying(dxbc::RootParameterType::UAV): + case llvm::to_underlying(dxbc::RootParameterType::SRV): { + const dxbc::RTS0::v2::RootDescriptor &Descriptor = + RSD.ParametersContainer.getRootDescriptor(Info.Location); + if (!verifyRegisterValue(Descriptor.ShaderRegister)) + return reportValueError(Ctx, "ShaderRegister", + Descriptor.ShaderRegister); + + if (!verifyRegisterSpace(Descriptor.RegisterSpace)) + return reportValueError(Ctx, "RegisterSpace", Descriptor.RegisterSpace); + + if (RSD.Version > 1) { + if (!verifyDescriptorFlag(Descriptor.Flags)) + return reportValueError(Ctx, "DescriptorFlag", Descriptor.Flags); } + break; + } } } @@ -425,14 +425,14 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M, case llvm::to_underlying(dxbc::RootParameterType::CBV): case llvm::to_underlying(dxbc::RootParameterType::UAV): case llvm::to_underlying(dxbc::RootParameterType::SRV): { - const dxbc::RTS0::v2::RootDescriptor &Descriptor = + const dxbc::RTS0::v2::RootDescriptor &Descriptor = RS.ParametersContainer.getRootDescriptor(Loc); OS << indent(Space + 2) << "Register Space: " << Descriptor.RegisterSpace << "\n"; OS << indent(Space + 2) << "Shader Register: " << Descriptor.ShaderRegister << "\n"; if(RS.Version > 1) - OS << indent(Space + 2) << "Flags: " << Descriptor.Flags << "\n"; + OS << indent(Space + 2) << "Flags: " << Descriptor.Flags << "\n"; break; } } From 92b766b9d52a83636017ce0759b2463c5c2c565c Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Tue, 3 Jun 2025 17:51:50 +0000 Subject: [PATCH 33/35] formating --- llvm/lib/Target/DirectX/DXILRootSignature.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index ecd9c546025a6..a361a5e95a7de 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -431,7 +431,7 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M, << "Register Space: " << Descriptor.RegisterSpace << "\n"; OS << indent(Space + 2) << "Shader Register: " << Descriptor.ShaderRegister << "\n"; - if(RS.Version > 1) + if (RS.Version > 1) OS << indent(Space + 2) << "Flags: " << Descriptor.Flags << "\n"; break; } From 8732594e4011182dc4a3dd0631bd49bd8831863b Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Tue, 3 Jun 2025 18:51:35 +0000 Subject: [PATCH 34/35] adding test --- ...Parameters-Invalid-ParameterIsNotString.ll | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters-Invalid-ParameterIsNotString.ll diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters-Invalid-ParameterIsNotString.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters-Invalid-ParameterIsNotString.ll new file mode 100644 index 0000000000000..04edd00eee643 --- /dev/null +++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters-Invalid-ParameterIsNotString.ll @@ -0,0 +1,19 @@ +; RUN: not opt -passes='print' %s -S -o - 2>&1 | FileCheck %s + +; CHECK: error: Invalid format for Root Element +; CHECK-NOT: Root Signature Definitions + +target triple = "dxil-unknown-shadermodel6.0-compute" + + +define void @main() #0 { +entry: + ret void +} + +attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } + +!dx.rootsignatures = !{!0} +!0 = !{ ptr @main, !1 } +!1 = !{ !2 } +!2 = !{ i32 0 } From fdb8b98105ba7830c22cef2bcbba5123b1521a9b Mon Sep 17 00:00:00 2001 From: joaosaffran Date: Tue, 3 Jun 2025 18:52:17 +0000 Subject: [PATCH 35/35] clean up --- llvm/lib/Target/DirectX/DXILRootSignature.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index a361a5e95a7de..3d195acb19e18 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -12,7 +12,6 @@ //===----------------------------------------------------------------------===// #include "DXILRootSignature.h" #include "DirectX.h" -#include "llvm/ADT/STLForwardCompat.h" #include "llvm/ADT/StringSwitch.h" #include "llvm/ADT/Twine.h" #include "llvm/Analysis/DXILMetadataAnalysis.h"