https://github.com/joaosaffran updated https://github.com/llvm/llvm-project/pull/137284
>From 7ac964196fc9195165dc1128d0f889f6ff1a93b4 Mon Sep 17 00:00:00 2001 From: joaosaffran <joao.saff...@microsoft.com> Date: Fri, 25 Apr 2025 22:28:48 +0000 Subject: [PATCH 1/9] 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 <typename T> 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 <typename T> Expected<T> readParameter() { - T Struct; - if (sizeof(T) != ParamData.size()) + template <typename T, typename VersionT = T> Expected<T> 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<GenericBinaryError>( "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<dxbc::RST0::v0::RootDescriptor> read() { - return readParameter<dxbc::RST0::v0::RootDescriptor>(); - } -}; - -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<dxbc::RST0::v1::RootDescriptor> read() { + llvm::Expected<dxbc::RST0::v1::RootDescriptor> read(uint32_t Version) { + if (Version == 1) + return readParameter<dxbc::RST0::v1::RootDescriptor, + dxbc::RST0::v0::RootDescriptor>(); return readParameter<dxbc::RST0::v1::RootDescriptor>(); } }; 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<DXContainerYAML::RootSignatureYamlDesc> 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 <cstdint> @@ -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<object::DirectX::RootDescriptorView_V1_0>( - &ParamView)) { - llvm::Expected<dxbc::RST0::v0::RootDescriptor> 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<object::DirectX::RootDescriptorView_V1_1>( - &ParamView)) { + } else if (auto *RDV = + dyn_cast<object::DirectX::RootDescriptorView>(&ParamView)) { llvm::Expected<dxbc::RST0::v1::RootDescriptor> 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<DirectX::RootDescriptorView_V1_0>(&*ParamView); + DirectX::RootDescriptorView *RootDescriptorView = + dyn_cast<DirectX::RootDescriptorView>(&*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<DirectX::RootDescriptorView_V1_1>(&*ParamView); + DirectX::RootDescriptorView *RootDescriptorView = + dyn_cast<DirectX::RootDescriptorView>(&*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 <joao.saff...@microsoft.com> Date: Sat, 26 Apr 2025 01:45:29 +0000 Subject: [PATCH 2/9] 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 <joao.saff...@microsoft.com> Date: Sat, 26 Apr 2025 02:02:53 +0000 Subject: [PATCH 3/9] 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<object::DirectX::RootDescriptorView>(&ParamView)) { llvm::Expected<dxbc::RST0::v1::RootDescriptor> 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 <joao.saff...@microsoft.com> Date: Sat, 26 Apr 2025 06:33:13 +0000 Subject: [PATCH 4/9] 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 <joao.saff...@microsoft.com> Date: Fri, 25 Apr 2025 05:09:08 +0000 Subject: [PATCH 5/9] 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 <cstddef> #include <cstdint> -#include <limits> +#include <variant> 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<dxbc::RST0::v0::RootDescriptor, + dxbc::RST0::v1::RootDescriptor>; +using ParametersView = + std::variant<dxbc::RootConstants, dxbc::RST0::v0::RootDescriptor, + dxbc::RST0::v1::RootDescriptor>; struct RootParameter { - dxbc::RootParameterHeader Header; - union { - dxbc::RootConstants Constants; - dxbc::RST0::v1::RootDescriptor Descriptor; + SmallVector<RootParameterHeader> Headers; + + SmallVector<dxbc::RootConstants> Constants; + SmallVector<RootDescriptor> 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<dxbc::RST0::v0::RootDescriptor>( + VersionedParam)) + return std::get<dxbc::RST0::v0::RootDescriptor>(VersionedParam); + return std::get<dxbc::RST0::v1::RootDescriptor>(VersionedParam); + } + + llvm_unreachable("Unimplemented parameter type"); + } + + struct iterator { + const RootParameter &Parameters; + SmallVector<RootParameterHeader>::const_iterator Current; + + // Changed parameter type to match member variable (removed const) + iterator(const RootParameter &P, + SmallVector<RootParameterHeader>::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<dxbc::RST0::v0::RootDescriptor>( + VersionedParam)) + Val = std::get<dxbc::RST0::v0::RootDescriptor>(VersionedParam); + else + Val = std::get<dxbc::RST0::v1::RootDescriptor>(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<RootParameter::iterator> 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<mcdxbc::RootParameter> 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 <variant> 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<decltype(Value)>; + 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<uint32_t> 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<dxbc::RootConstants>(*P)) { + auto Constants = std::get<dxbc::RootConstants>(*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<dxbc::RST0::v0::RootDescriptor>(*P)) { + auto Descriptor = std::get<dxbc::RST0::v0::RootDescriptor>(*P); + support::endian::write(BOS, Descriptor.ShaderRegister, + llvm::endianness::little); + support::endian::write(BOS, Descriptor.RegisterSpace, + llvm::endianness::little); + } else if (std::holds_alternative<dxbc::RST0::v1::RootDescriptor>(*P)) { + auto Descriptor = std::get<dxbc::RST0::v1::RootDescriptor>(*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 <cstdint> #include <optional> #include <utility> +#include <variant> 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<uint32_t> 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<uint32_t> Val = extractMdIntValue(RootConstantNode, 2)) - NewParameter.Constants.ShaderRegister = *Val; + Constants.ShaderRegister = *Val; else return reportError(Ctx, "Invalid value for ShaderRegister"); if (std::optional<uint32_t> Val = extractMdIntValue(RootConstantNode, 3)) - NewParameter.Constants.RegisterSpace = *Val; + Constants.RegisterSpace = *Val; else return reportError(Ctx, "Invalid value for RegisterSpace"); if (std::optional<uint32_t> 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<dxbc::RootConstants>(P)) { + auto Constants = std::get<dxbc::RootConstants>(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 <joao.saff...@microsoft.com> Date: Mon, 28 Apr 2025 18:12:28 +0000 Subject: [PATCH 6/9] 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 <cstddef> #include <cstdint> +#include <optional> #include <variant> 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<dxbc::RST0::v0::RootDescriptor, @@ -33,117 +34,61 @@ using RootDescriptor = std::variant<dxbc::RST0::v0::RootDescriptor, using ParametersView = std::variant<dxbc::RootConstants, dxbc::RST0::v0::RootDescriptor, dxbc::RST0::v1::RootDescriptor>; -struct RootParameter { - SmallVector<RootParameterHeader> Headers; +struct RootParametersContainer { + SmallVector<RootParameterInfo> ParametersInfo; SmallVector<dxbc::RootConstants> Constants; SmallVector<RootDescriptor> 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<ParametersView> 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<dxbc::RST0::v0::RootDescriptor>( VersionedParam)) return std::get<dxbc::RST0::v0::RootDescriptor>(VersionedParam); return std::get<dxbc::RST0::v1::RootDescriptor>(VersionedParam); } - llvm_unreachable("Unimplemented parameter type"); + return std::nullopt; } - struct iterator { - const RootParameter &Parameters; - SmallVector<RootParameterHeader>::const_iterator Current; - - // Changed parameter type to match member variable (removed const) - iterator(const RootParameter &P, - SmallVector<RootParameterHeader>::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<dxbc::RST0::v0::RootDescriptor>( - VersionedParam)) - Val = std::get<dxbc::RST0::v0::RootDescriptor>(VersionedParam); - else - Val = std::get<dxbc::RST0::v1::RootDescriptor>(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<RootParameterInfo>::const_iterator begin() const { + return ParametersInfo.begin(); + } + SmallVector<RootParameterInfo>::const_iterator end() const { + return ParametersInfo.end(); + } - llvm::iterator_range<RootParameter::iterator> getAll() const { + llvm::iterator_range<SmallVector<RootParameterInfo>::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<ParametersView> P = ParametersContainer.getParameter(&I); + if (!P) + continue; std::visit( [&Size](auto &Value) -> void { using T = std::decay_t<decltype(Value)>; 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<uint32_t> 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<dxbc::RootConstants>(*P)) { auto Constants = std::get<dxbc::RootConstants>(*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<dxbc::RootConstants>(P)) { - auto Constants = std::get<dxbc::RootConstants>(P); + << "Shader Visibility: " << Info.Header.ShaderVisibility << "\n"; + std::optional<mcdxbc::ParametersView> P = + RS.ParametersContainer.getParameter(&Info); + if (!P) + continue; + if (std::holds_alternative<dxbc::RootConstants>(*P)) { + auto Constants = std::get<dxbc::RootConstants>(*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 <joao.saff...@microsoft.com> Date: Mon, 28 Apr 2025 22:29:53 +0000 Subject: [PATCH 7/9] 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 <cstddef> #include <cstdint> #include <optional> +#include <utility> #include <variant> namespace llvm { @@ -29,11 +29,11 @@ struct RootParameterInfo { : Header(H), Location(L) {} }; -using RootDescriptor = std::variant<dxbc::RST0::v0::RootDescriptor, - dxbc::RST0::v1::RootDescriptor>; +using RootDescriptor = std::variant<dxbc::RST0::v0::RootDescriptor*, + dxbc::RST0::v1::RootDescriptor*>; using ParametersView = - std::variant<dxbc::RootConstants, dxbc::RST0::v0::RootDescriptor, - dxbc::RST0::v1::RootDescriptor>; + std::variant<const dxbc::RootConstants*, const dxbc::RST0::v0::RootDescriptor*, + const dxbc::RST0::v1::RootDescriptor*>; struct RootParametersContainer { SmallVector<RootParameterInfo> 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<ParametersView> 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<dxbc::RST0::v0::RootDescriptor>( + if (std::holds_alternative<dxbc::RST0::v0::RootDescriptor*>( VersionedParam)) - return std::get<dxbc::RST0::v0::RootDescriptor>(VersionedParam); - return std::get<dxbc::RST0::v1::RootDescriptor>(VersionedParam); + return std::get<dxbc::RST0::v0::RootDescriptor*>(VersionedParam); + return std::get<dxbc::RST0::v1::RootDescriptor*>(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<decltype(Value)>; + using T = std::decay_t<decltype(*Value)>; 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<dxbc::RootConstants>(*P)) { - auto Constants = std::get<dxbc::RootConstants>(*P); - support::endian::write(BOS, Constants.ShaderRegister, + if (std::holds_alternative<const dxbc::RootConstants*>(*P)) { + auto* Constants = std::get<const dxbc::RootConstants*>(*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<dxbc::RST0::v0::RootDescriptor>(*P)) { - auto Descriptor = std::get<dxbc::RST0::v0::RootDescriptor>(*P); - support::endian::write(BOS, Descriptor.ShaderRegister, + } else if (std::holds_alternative<const dxbc::RST0::v0::RootDescriptor*>(*P)) { + auto* Descriptor = std::get<const dxbc::RST0::v0::RootDescriptor*>(*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<dxbc::RST0::v1::RootDescriptor>(*P)) { - auto Descriptor = std::get<dxbc::RST0::v1::RootDescriptor>(*P); + } else if (std::holds_alternative<const dxbc::RST0::v1::RootDescriptor*>(*P)) { + auto* Descriptor = std::get<const dxbc::RST0::v1::RootDescriptor*>(*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<dxbc::RootConstants>(*P)) { - auto Constants = std::get<dxbc::RootConstants>(*P); - OS << indent(Space + 2) << "Register Space: " << Constants.RegisterSpace + if (std::holds_alternative<const dxbc::RootConstants*>(*P)) { + auto* Constants = std::get<const dxbc::RootConstants*>(*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 <joao.saff...@microsoft.com> Date: Mon, 28 Apr 2025 22:39:36 +0000 Subject: [PATCH 8/9] 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<dxbc::RST0::v0::RootDescriptor*, - dxbc::RST0::v1::RootDescriptor*>; -using ParametersView = - std::variant<const dxbc::RootConstants*, const dxbc::RST0::v0::RootDescriptor*, - const dxbc::RST0::v1::RootDescriptor*>; +using RootDescriptor = std::variant<dxbc::RST0::v0::RootDescriptor *, + dxbc::RST0::v1::RootDescriptor *>; +using ParametersView = std::variant<const dxbc::RootConstants *, + const dxbc::RST0::v0::RootDescriptor *, + const dxbc::RST0::v1::RootDescriptor *>; struct RootParametersContainer { SmallVector<RootParameterInfo> 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<dxbc::RST0::v0::RootDescriptor*>( + if (std::holds_alternative<dxbc::RST0::v0::RootDescriptor *>( VersionedParam)) - return std::get<dxbc::RST0::v0::RootDescriptor*>(VersionedParam); - return std::get<dxbc::RST0::v1::RootDescriptor*>(VersionedParam); + return std::get<dxbc::RST0::v0::RootDescriptor *>(VersionedParam); + return std::get<dxbc::RST0::v1::RootDescriptor *>(VersionedParam); } return std::nullopt; @@ -86,11 +86,6 @@ struct RootParametersContainer { SmallVector<RootParameterInfo>::const_iterator end() const { return ParametersInfo.end(); } - - llvm::iterator_range<SmallVector<RootParameterInfo>::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<const dxbc::RootConstants*>(*P)) { - auto* Constants = std::get<const dxbc::RootConstants*>(*P); + if (std::holds_alternative<const dxbc::RootConstants *>(*P)) { + auto *Constants = std::get<const dxbc::RootConstants *>(*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<const dxbc::RST0::v0::RootDescriptor*>(*P)) { - auto* Descriptor = std::get<const dxbc::RST0::v0::RootDescriptor*>(*P); + } else if (std::holds_alternative<const dxbc::RST0::v0::RootDescriptor *>( + *P)) { + auto *Descriptor = std::get<const dxbc::RST0::v0::RootDescriptor *>(*P); support::endian::write(BOS, Descriptor->ShaderRegister, llvm::endianness::little); support::endian::write(BOS, Descriptor->RegisterSpace, llvm::endianness::little); - } else if (std::holds_alternative<const dxbc::RST0::v1::RootDescriptor*>(*P)) { - auto* Descriptor = std::get<const dxbc::RST0::v1::RootDescriptor*>(*P); + } else if (std::holds_alternative<const dxbc::RST0::v1::RootDescriptor *>( + *P)) { + auto *Descriptor = std::get<const dxbc::RST0::v1::RootDescriptor *>(*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<const dxbc::RootConstants*>(*P)) { - auto* Constants = std::get<const dxbc::RootConstants*>(*P); - OS << indent(Space + 2) << "Register Space: " << Constants->RegisterSpace - << "\n"; + if (std::holds_alternative<const dxbc::RootConstants *>(*P)) { + auto *Constants = std::get<const dxbc::RootConstants *>(*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 <joao.saff...@microsoft.com> Date: Mon, 28 Apr 2025 22:49:22 +0000 Subject: [PATCH 9/9] 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; _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits