Skip to content

Commit 502d04e

Browse files
joaosaffranjoaosaffran
authored and
sivan-shani
committed
[NFC] Refactoring DXContainerYaml Root Parameter representation (llvm#138318)
This PR removes the union usage from `DXContainerYaml` Root Parameters representation, it uses variant instead. closes: [llvm#139585](llvm#139585) --------- Co-authored-by: joaosaffran <joao.saffran@microsoft.com>
1 parent 4fcfc94 commit 502d04e

File tree

3 files changed

+116
-69
lines changed

3 files changed

+116
-69
lines changed

‎llvm/include/llvm/ObjectYAML/DXContainerYAML.h

Lines changed: 49 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -90,29 +90,53 @@ struct RootDescriptorYaml {
9090
#include "llvm/BinaryFormat/DXContainerConstants.def"
9191
};
9292

93-
struct RootParameterYamlDesc {
93+
struct RootParameterHeaderYaml {
9494
uint32_t Type;
9595
uint32_t Visibility;
9696
uint32_t Offset;
97-
RootParameterYamlDesc() {};
98-
RootParameterYamlDesc(uint32_t T) : Type(T) {
99-
switch (T) {
100-
101-
case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit):
102-
Constants = RootConstantsYaml();
103-
break;
104-
case llvm::to_underlying(dxbc::RootParameterType::CBV):
105-
case llvm::to_underlying(dxbc::RootParameterType::SRV):
106-
case llvm::to_underlying(dxbc::RootParameterType::UAV):
107-
Descriptor = RootDescriptorYaml();
108-
break;
97+
98+
RootParameterHeaderYaml(){};
99+
RootParameterHeaderYaml(uint32_t T) : Type(T) {}
100+
};
101+
102+
struct RootParameterLocationYaml {
103+
RootParameterHeaderYaml Header;
104+
std::optional<size_t> IndexInSignature;
105+
106+
RootParameterLocationYaml(){};
107+
explicit RootParameterLocationYaml(RootParameterHeaderYaml Header)
108+
: Header(Header) {}
109+
};
110+
111+
struct RootParameterYamlDesc {
112+
SmallVector<RootParameterLocationYaml> Locations;
113+
114+
SmallVector<RootConstantsYaml> Constants;
115+
SmallVector<RootDescriptorYaml> Descriptors;
116+
117+
template <typename T>
118+
T &getOrInsertImpl(RootParameterLocationYaml &ParamDesc,
119+
SmallVectorImpl<T> &Container) {
120+
if (!ParamDesc.IndexInSignature) {
121+
ParamDesc.IndexInSignature = Container.size();
122+
Container.emplace_back();
109123
}
124+
return Container[*ParamDesc.IndexInSignature];
125+
}
126+
127+
RootConstantsYaml &
128+
getOrInsertConstants(RootParameterLocationYaml &ParamDesc) {
129+
return getOrInsertImpl(ParamDesc, Constants);
130+
}
131+
132+
RootDescriptorYaml &
133+
getOrInsertDescriptor(RootParameterLocationYaml &ParamDesc) {
134+
return getOrInsertImpl(ParamDesc, Descriptors);
110135
}
111136

112-
union {
113-
RootConstantsYaml Constants;
114-
RootDescriptorYaml Descriptor;
115-
};
137+
void insertLocation(RootParameterLocationYaml &Location) {
138+
Locations.push_back(Location);
139+
}
116140
};
117141

118142
struct RootSignatureYamlDesc {
@@ -124,14 +148,10 @@ struct RootSignatureYamlDesc {
124148
uint32_t NumStaticSamplers;
125149
uint32_t StaticSamplersOffset;
126150

127-
SmallVector<RootParameterYamlDesc> Parameters;
151+
RootParameterYamlDesc Parameters;
128152

129153
uint32_t getEncodedFlags();
130154

131-
iterator_range<RootParameterYamlDesc *> params() {
132-
return make_range(Parameters.begin(), Parameters.end());
133-
}
134-
135155
static llvm::Expected<DXContainerYAML::RootSignatureYamlDesc>
136156
create(const object::DirectX::RootSignature &Data);
137157

@@ -242,7 +262,7 @@ LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DXContainerYAML::ResourceBindInfo)
242262
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DXContainerYAML::SignatureElement)
243263
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DXContainerYAML::PSVInfo::MaskVector)
244264
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DXContainerYAML::SignatureParameter)
245-
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DXContainerYAML::RootParameterYamlDesc)
265+
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DXContainerYAML::RootParameterLocationYaml)
246266
LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::PSV::SemanticKind)
247267
LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::PSV::ComponentType)
248268
LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::PSV::InterpolationMode)
@@ -315,8 +335,12 @@ template <> struct MappingTraits<DXContainerYAML::RootSignatureYamlDesc> {
315335
DXContainerYAML::RootSignatureYamlDesc &RootSignature);
316336
};
317337

318-
template <> struct MappingTraits<llvm::DXContainerYAML::RootParameterYamlDesc> {
319-
static void mapping(IO &IO, llvm::DXContainerYAML::RootParameterYamlDesc &P);
338+
template <>
339+
struct MappingContextTraits<DXContainerYAML::RootParameterLocationYaml,
340+
DXContainerYAML::RootSignatureYamlDesc> {
341+
static void mapping(IO &IO,
342+
llvm::DXContainerYAML::RootParameterLocationYaml &L,
343+
DXContainerYAML::RootSignatureYamlDesc &S);
320344
};
321345

322346
template <> struct MappingTraits<llvm::DXContainerYAML::RootConstantsYaml> {

‎llvm/lib/ObjectYAML/DXContainerEmitter.cpp

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -273,28 +273,36 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
273273
RS.NumStaticSamplers = P.RootSignature->NumStaticSamplers;
274274
RS.StaticSamplersOffset = P.RootSignature->StaticSamplersOffset;
275275

276-
for (const auto &Param : P.RootSignature->Parameters) {
277-
dxbc::RootParameterHeader Header{Param.Type, Param.Visibility,
278-
Param.Offset};
279-
280-
switch (Param.Type) {
281-
case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit):
276+
for (DXContainerYAML::RootParameterLocationYaml &L :
277+
P.RootSignature->Parameters.Locations) {
278+
dxbc::RootParameterHeader Header{L.Header.Type, L.Header.Visibility,
279+
L.Header.Offset};
280+
281+
switch (L.Header.Type) {
282+
case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): {
283+
const DXContainerYAML::RootConstantsYaml &ConstantYaml =
284+
P.RootSignature->Parameters.getOrInsertConstants(L);
282285
dxbc::RootConstants Constants;
283-
Constants.Num32BitValues = Param.Constants.Num32BitValues;
284-
Constants.RegisterSpace = Param.Constants.RegisterSpace;
285-
Constants.ShaderRegister = Param.Constants.ShaderRegister;
286+
Constants.Num32BitValues = ConstantYaml.Num32BitValues;
287+
Constants.RegisterSpace = ConstantYaml.RegisterSpace;
288+
Constants.ShaderRegister = ConstantYaml.ShaderRegister;
286289
RS.ParametersContainer.addParameter(Header, Constants);
287290
break;
288-
case llvm::to_underlying(dxbc::RootParameterType::SRV):
289-
case llvm::to_underlying(dxbc::RootParameterType::UAV):
291+
}
290292
case llvm::to_underlying(dxbc::RootParameterType::CBV):
293+
case llvm::to_underlying(dxbc::RootParameterType::SRV):
294+
case llvm::to_underlying(dxbc::RootParameterType::UAV): {
295+
const DXContainerYAML::RootDescriptorYaml &DescriptorYaml =
296+
P.RootSignature->Parameters.getOrInsertDescriptor(L);
297+
291298
dxbc::RTS0::v2::RootDescriptor Descriptor;
292-
Descriptor.RegisterSpace = Param.Descriptor.RegisterSpace;
293-
Descriptor.ShaderRegister = Param.Descriptor.ShaderRegister;
299+
Descriptor.RegisterSpace = DescriptorYaml.RegisterSpace;
300+
Descriptor.ShaderRegister = DescriptorYaml.ShaderRegister;
294301
if (RS.Version > 1)
295-
Descriptor.Flags = Param.Descriptor.getEncodedFlags();
302+
Descriptor.Flags = DescriptorYaml.getEncodedFlags();
296303
RS.ParametersContainer.addParameter(Header, Descriptor);
297304
break;
305+
}
298306
default:
299307
// Handling invalid parameter type edge case. We intentionally let
300308
// obj2yaml/yaml2obj parse and emit invalid dxcontainer data, in order

‎llvm/lib/ObjectYAML/DXContainerYAML.cpp

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,15 @@ DXContainerYAML::RootSignatureYamlDesc::create(
5353
return createStringError(std::errc::invalid_argument,
5454
"Invalid value for parameter type");
5555

56-
RootParameterYamlDesc NewP(PH.ParameterType);
57-
NewP.Offset = PH.ParameterOffset;
58-
NewP.Type = PH.ParameterType;
56+
RootParameterHeaderYaml Header(PH.ParameterType);
57+
Header.Offset = PH.ParameterOffset;
58+
Header.Type = PH.ParameterType;
5959

6060
if (!dxbc::isValidShaderVisibility(PH.ShaderVisibility))
6161
return createStringError(std::errc::invalid_argument,
6262
"Invalid value for shader visibility");
6363

64-
NewP.Visibility = PH.ShaderVisibility;
64+
Header.Visibility = PH.ShaderVisibility;
6565

6666
llvm::Expected<object::DirectX::RootParameterView> ParamViewOrErr =
6767
Data.getParameter(PH);
@@ -75,29 +75,36 @@ DXContainerYAML::RootSignatureYamlDesc::create(
7575
return std::move(E);
7676

7777
auto Constants = *ConstantsOrErr;
78+
RootParameterLocationYaml Location(Header);
79+
RootConstantsYaml &ConstantYaml =
80+
RootSigDesc.Parameters.getOrInsertConstants(Location);
81+
RootSigDesc.Parameters.insertLocation(Location);
82+
ConstantYaml.Num32BitValues = Constants.Num32BitValues;
83+
ConstantYaml.ShaderRegister = Constants.ShaderRegister;
84+
ConstantYaml.RegisterSpace = Constants.RegisterSpace;
7885

79-
NewP.Constants.Num32BitValues = Constants.Num32BitValues;
80-
NewP.Constants.ShaderRegister = Constants.ShaderRegister;
81-
NewP.Constants.RegisterSpace = Constants.RegisterSpace;
8286
} else if (auto *RDV =
8387
dyn_cast<object::DirectX::RootDescriptorView>(&ParamView)) {
8488
llvm::Expected<dxbc::RTS0::v2::RootDescriptor> DescriptorOrErr =
8589
RDV->read(Version);
8690
if (Error E = DescriptorOrErr.takeError())
8791
return std::move(E);
8892
auto Descriptor = *DescriptorOrErr;
89-
NewP.Descriptor.ShaderRegister = Descriptor.ShaderRegister;
90-
NewP.Descriptor.RegisterSpace = Descriptor.RegisterSpace;
93+
RootParameterLocationYaml Location(Header);
94+
RootDescriptorYaml &YamlDescriptor =
95+
RootSigDesc.Parameters.getOrInsertDescriptor(Location);
96+
RootSigDesc.Parameters.insertLocation(Location);
97+
98+
YamlDescriptor.ShaderRegister = Descriptor.ShaderRegister;
99+
YamlDescriptor.RegisterSpace = Descriptor.RegisterSpace;
91100
if (Version > 1) {
92101
#define ROOT_DESCRIPTOR_FLAG(Num, Val) \
93-
NewP.Descriptor.Val = \
102+
YamlDescriptor.Val = \
94103
(Descriptor.Flags & \
95104
llvm::to_underlying(dxbc::RootDescriptorFlag::Val)) > 0;
96105
#include "llvm/BinaryFormat/DXContainerConstants.def"
97106
}
98107
}
99-
100-
RootSigDesc.Parameters.push_back(NewP);
101108
}
102109
#define ROOT_ELEMENT_FLAG(Num, Val) \
103110
RootSigDesc.Val = \
@@ -290,11 +297,36 @@ void MappingTraits<DXContainerYAML::RootSignatureYamlDesc>::mapping(
290297
IO.mapRequired("RootParametersOffset", S.RootParametersOffset);
291298
IO.mapRequired("NumStaticSamplers", S.NumStaticSamplers);
292299
IO.mapRequired("StaticSamplersOffset", S.StaticSamplersOffset);
293-
IO.mapRequired("Parameters", S.Parameters);
300+
IO.mapRequired("Parameters", S.Parameters.Locations, S);
294301
#define ROOT_ELEMENT_FLAG(Num, Val) IO.mapOptional(#Val, S.Val, false);
295302
#include "llvm/BinaryFormat/DXContainerConstants.def"
296303
}
297304

305+
void MappingContextTraits<DXContainerYAML::RootParameterLocationYaml,
306+
DXContainerYAML::RootSignatureYamlDesc>::
307+
mapping(IO &IO, DXContainerYAML::RootParameterLocationYaml &L,
308+
DXContainerYAML::RootSignatureYamlDesc &S) {
309+
IO.mapRequired("ParameterType", L.Header.Type);
310+
IO.mapRequired("ShaderVisibility", L.Header.Visibility);
311+
312+
switch (L.Header.Type) {
313+
case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): {
314+
DXContainerYAML::RootConstantsYaml &Constants =
315+
S.Parameters.getOrInsertConstants(L);
316+
IO.mapRequired("Constants", Constants);
317+
break;
318+
}
319+
case llvm::to_underlying(dxbc::RootParameterType::CBV):
320+
case llvm::to_underlying(dxbc::RootParameterType::SRV):
321+
case llvm::to_underlying(dxbc::RootParameterType::UAV): {
322+
DXContainerYAML::RootDescriptorYaml &Descriptor =
323+
S.Parameters.getOrInsertDescriptor(L);
324+
IO.mapRequired("Descriptor", Descriptor);
325+
break;
326+
}
327+
}
328+
}
329+
298330
void MappingTraits<llvm::DXContainerYAML::RootConstantsYaml>::mapping(
299331
IO &IO, llvm::DXContainerYAML::RootConstantsYaml &C) {
300332
IO.mapRequired("Num32BitValues", C.Num32BitValues);
@@ -310,23 +342,6 @@ void MappingTraits<llvm::DXContainerYAML::RootDescriptorYaml>::mapping(
310342
#include "llvm/BinaryFormat/DXContainerConstants.def"
311343
}
312344

313-
void MappingTraits<llvm::DXContainerYAML::RootParameterYamlDesc>::mapping(
314-
IO &IO, llvm::DXContainerYAML::RootParameterYamlDesc &P) {
315-
IO.mapRequired("ParameterType", P.Type);
316-
IO.mapRequired("ShaderVisibility", P.Visibility);
317-
318-
switch (P.Type) {
319-
case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit):
320-
IO.mapRequired("Constants", P.Constants);
321-
break;
322-
case llvm::to_underlying(dxbc::RootParameterType::CBV):
323-
case llvm::to_underlying(dxbc::RootParameterType::SRV):
324-
case llvm::to_underlying(dxbc::RootParameterType::UAV):
325-
IO.mapRequired("Descriptor", P.Descriptor);
326-
break;
327-
}
328-
}
329-
330345
void MappingTraits<DXContainerYAML::Part>::mapping(IO &IO,
331346
DXContainerYAML::Part &P) {
332347
IO.mapRequired("Name", P.Name);

0 commit comments

Comments
 (0)