Skip to content

Commit 6f0253b

Browse files
inbelicinbelic
authored
[DirectX][ObectYAML] Make RootParameterOffset and StaticSamplersOffset behaviour consistent (#155521)
This pr fixes some inconsistencies in behaviour of how we handle `StaticSamplersOffset` with respect to DXC and `RootParameterOffset`. Namely: 1. Make codegen of `RTS0` always compute the `StaticSamplersOffset` regardless if there are any `StaticSampler`s. This is to be consistent and produce an identical `DXContainer` as DXC. 2. Make the `StaticSamplersOffset` and `RootParametersOffset` optional parameters in the yaml description. This means it will be used when it is specified (which was not necassarily the case before). 3. Enforce that the provided `StaticSamplersOffset` and `RootParametersOffset` in a yaml description match the computed value. For more context see: llvm/llvm-project#155299. Description of existing test updates updates: - `CodeGen/DirectX/ContainerData`: Updated to codegen computed values (previously unspecified) - `llvm-objcopy/DXContainer`: Updated to `yaml2obj` computed values (previously unspecified) - `ObjectYAML/DXContainer`: Updated to `yaml2obj` computed values (previously incorrect) - `ObjectYAML/DXContainerYAMLTest`: Updated to `yaml2obj` computed values (previously incorrect) See newly added tests for testing of optional parameter functionality and `StaticSamplersOffset` computation. Resolves: llvm/llvm-project#155299
1 parent a3eb311 commit 6f0253b

28 files changed

+349
-89
lines changed

‎llvm/include/llvm/MC/DXContainerRootSignature.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,8 @@ struct RootSignatureDesc {
110110
LLVM_ABI void write(raw_ostream &OS) const;
111111

112112
LLVM_ABI size_t getSize() const;
113+
LLVM_ABI uint32_t computeRootParametersOffset() const;
114+
LLVM_ABI uint32_t computeStaticSamplersOffset() const;
113115
};
114116
} // namespace mcdxbc
115117
} // namespace llvm

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,9 +187,9 @@ struct RootSignatureYamlDesc {
187187

188188
uint32_t Version;
189189
uint32_t NumRootParameters;
190-
uint32_t RootParametersOffset;
190+
std::optional<uint32_t> RootParametersOffset;
191191
uint32_t NumStaticSamplers;
192-
uint32_t StaticSamplersOffset;
192+
std::optional<uint32_t> StaticSamplersOffset;
193193

194194
RootParameterYamlDesc Parameters;
195195
SmallVector<StaticSamplerYamlDesc> StaticSamplers;

‎llvm/lib/MC/DXContainerRootSignature.cpp‎

Lines changed: 42 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -20,32 +20,43 @@ static uint32_t writePlaceholder(raw_svector_ostream &Stream) {
2020
return Offset;
2121
}
2222

23-
static void rewriteOffsetToCurrentByte(raw_svector_ostream &Stream,
24-
uint32_t Offset) {
23+
static uint32_t rewriteOffsetToCurrentByte(raw_svector_ostream &Stream,
24+
uint32_t Offset) {
2525
uint32_t Value =
2626
support::endian::byte_swap<uint32_t, llvm::endianness::little>(
2727
Stream.tell());
2828
Stream.pwrite(reinterpret_cast<const char *>(&Value), sizeof(Value), Offset);
29+
return Value;
2930
}
3031

3132
size_t RootSignatureDesc::getSize() const {
32-
size_t Size =
33-
sizeof(dxbc::RTS0::v1::RootSignatureHeader) +
34-
ParametersContainer.size() * sizeof(dxbc::RTS0::v1::RootParameterHeader) +
33+
uint32_t StaticSamplersOffset = computeStaticSamplersOffset();
34+
size_t StaticSamplersSize =
3535
StaticSamplers.size() * sizeof(dxbc::RTS0::v1::StaticSampler);
3636

37+
return size_t(StaticSamplersOffset) + StaticSamplersSize;
38+
}
39+
40+
uint32_t RootSignatureDesc::computeRootParametersOffset() const {
41+
return sizeof(dxbc::RTS0::v1::RootSignatureHeader);
42+
}
43+
44+
uint32_t RootSignatureDesc::computeStaticSamplersOffset() const {
45+
uint32_t Offset = computeRootParametersOffset();
46+
3747
for (const RootParameterInfo &I : ParametersContainer) {
48+
Offset += sizeof(dxbc::RTS0::v1::RootParameterHeader);
3849
switch (I.Type) {
3950
case dxbc::RootParameterType::Constants32Bit:
40-
Size += sizeof(dxbc::RTS0::v1::RootConstants);
51+
Offset += sizeof(dxbc::RTS0::v1::RootConstants);
4152
break;
4253
case dxbc::RootParameterType::CBV:
4354
case dxbc::RootParameterType::SRV:
4455
case dxbc::RootParameterType::UAV:
4556
if (Version == 1)
46-
Size += sizeof(dxbc::RTS0::v1::RootDescriptor);
57+
Offset += sizeof(dxbc::RTS0::v1::RootDescriptor);
4758
else
48-
Size += sizeof(dxbc::RTS0::v2::RootDescriptor);
59+
Offset += sizeof(dxbc::RTS0::v2::RootDescriptor);
4960

5061
break;
5162
case dxbc::RootParameterType::DescriptorTable:
@@ -54,15 +65,16 @@ size_t RootSignatureDesc::getSize() const {
5465

5566
// 4 bytes for the number of ranges in table and
5667
// 4 bytes for the ranges offset
57-
Size += 2 * sizeof(uint32_t);
68+
Offset += 2 * sizeof(uint32_t);
5869
if (Version == 1)
59-
Size += sizeof(dxbc::RTS0::v1::DescriptorRange) * Table.Ranges.size();
70+
Offset += sizeof(dxbc::RTS0::v1::DescriptorRange) * Table.Ranges.size();
6071
else
61-
Size += sizeof(dxbc::RTS0::v2::DescriptorRange) * Table.Ranges.size();
72+
Offset += sizeof(dxbc::RTS0::v2::DescriptorRange) * Table.Ranges.size();
6273
break;
6374
}
6475
}
65-
return Size;
76+
77+
return Offset;
6678
}
6779

6880
void RootSignatureDesc::write(raw_ostream &OS) const {
@@ -76,11 +88,7 @@ void RootSignatureDesc::write(raw_ostream &OS) const {
7688
support::endian::write(BOS, NumParameters, llvm::endianness::little);
7789
support::endian::write(BOS, RootParameterOffset, llvm::endianness::little);
7890
support::endian::write(BOS, NumSamplers, llvm::endianness::little);
79-
uint32_t SSO = StaticSamplersOffset;
80-
if (NumSamplers > 0)
81-
SSO = writePlaceholder(BOS);
82-
else
83-
support::endian::write(BOS, SSO, llvm::endianness::little);
91+
uint32_t SSO = writePlaceholder(BOS);
8492
support::endian::write(BOS, Flags, llvm::endianness::little);
8593

8694
SmallVector<uint32_t> ParamsOffsets;
@@ -144,23 +152,23 @@ void RootSignatureDesc::write(raw_ostream &OS) const {
144152
}
145153
}
146154
}
147-
if (NumSamplers > 0) {
148-
rewriteOffsetToCurrentByte(BOS, SSO);
149-
for (const auto &S : StaticSamplers) {
150-
support::endian::write(BOS, S.Filter, llvm::endianness::little);
151-
support::endian::write(BOS, S.AddressU, llvm::endianness::little);
152-
support::endian::write(BOS, S.AddressV, llvm::endianness::little);
153-
support::endian::write(BOS, S.AddressW, llvm::endianness::little);
154-
support::endian::write(BOS, S.MipLODBias, llvm::endianness::little);
155-
support::endian::write(BOS, S.MaxAnisotropy, llvm::endianness::little);
156-
support::endian::write(BOS, S.ComparisonFunc, llvm::endianness::little);
157-
support::endian::write(BOS, S.BorderColor, llvm::endianness::little);
158-
support::endian::write(BOS, S.MinLOD, llvm::endianness::little);
159-
support::endian::write(BOS, S.MaxLOD, llvm::endianness::little);
160-
support::endian::write(BOS, S.ShaderRegister, llvm::endianness::little);
161-
support::endian::write(BOS, S.RegisterSpace, llvm::endianness::little);
162-
support::endian::write(BOS, S.ShaderVisibility, llvm::endianness::little);
163-
}
155+
[[maybe_unused]] uint32_t Offset = rewriteOffsetToCurrentByte(BOS, SSO);
156+
assert(Offset == computeStaticSamplersOffset() &&
157+
"Computed offset does not match written offset");
158+
for (const auto &S : StaticSamplers) {
159+
support::endian::write(BOS, S.Filter, llvm::endianness::little);
160+
support::endian::write(BOS, S.AddressU, llvm::endianness::little);
161+
support::endian::write(BOS, S.AddressV, llvm::endianness::little);
162+
support::endian::write(BOS, S.AddressW, llvm::endianness::little);
163+
support::endian::write(BOS, S.MipLODBias, llvm::endianness::little);
164+
support::endian::write(BOS, S.MaxAnisotropy, llvm::endianness::little);
165+
support::endian::write(BOS, S.ComparisonFunc, llvm::endianness::little);
166+
support::endian::write(BOS, S.BorderColor, llvm::endianness::little);
167+
support::endian::write(BOS, S.MinLOD, llvm::endianness::little);
168+
support::endian::write(BOS, S.MaxLOD, llvm::endianness::little);
169+
support::endian::write(BOS, S.ShaderRegister, llvm::endianness::little);
170+
support::endian::write(BOS, S.RegisterSpace, llvm::endianness::little);
171+
support::endian::write(BOS, S.ShaderVisibility, llvm::endianness::little);
164172
}
165173
assert(Storage.size() == getSize());
166174
OS.write(Storage.data(), Storage.size());

‎llvm/lib/ObjectYAML/DXContainerEmitter.cpp‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ class DXContainerWriter {
3838
Error validateSize(uint32_t Computed);
3939

4040
void writeHeader(raw_ostream &OS);
41-
void writeParts(raw_ostream &OS);
41+
Error writeParts(raw_ostream &OS);
4242
};
4343
} // namespace
4444

@@ -107,7 +107,7 @@ void DXContainerWriter::writeHeader(raw_ostream &OS) {
107107
Offsets.size() * sizeof(uint32_t));
108108
}
109109

110-
void DXContainerWriter::writeParts(raw_ostream &OS) {
110+
Error DXContainerWriter::writeParts(raw_ostream &OS) {
111111
uint32_t RollingOffset =
112112
sizeof(dxbc::Header) + (ObjectFile.Header.PartCount * sizeof(uint32_t));
113113
for (auto I : llvm::zip(ObjectFile.Parts, *ObjectFile.Header.PartOffsets)) {
@@ -269,9 +269,7 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
269269
mcdxbc::RootSignatureDesc RS;
270270
RS.Flags = P.RootSignature->getEncodedFlags();
271271
RS.Version = P.RootSignature->Version;
272-
RS.RootParameterOffset = P.RootSignature->RootParametersOffset;
273272
RS.NumStaticSamplers = P.RootSignature->NumStaticSamplers;
274-
RS.StaticSamplersOffset = P.RootSignature->StaticSamplersOffset;
275273

276274
for (DXContainerYAML::RootParameterLocationYaml &L :
277275
P.RootSignature->Parameters.Locations) {
@@ -323,8 +321,10 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
323321
Range.RegisterSpace = R.RegisterSpace;
324322
Range.OffsetInDescriptorsFromTableStart =
325323
R.OffsetInDescriptorsFromTableStart;
324+
326325
if (RS.Version > 1)
327326
Range.Flags = R.getEncodedFlags();
327+
328328
Table.Ranges.push_back(Range);
329329
}
330330
RS.ParametersContainer.addParameter(Type, Visibility, Table);
@@ -352,6 +352,27 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
352352
RS.StaticSamplers.push_back(NewSampler);
353353
}
354354

355+
// Handling of offsets
356+
RS.RootParameterOffset = RS.computeRootParametersOffset();
357+
if (P.RootSignature->RootParametersOffset &&
358+
P.RootSignature->RootParametersOffset.value() !=
359+
RS.RootParameterOffset) {
360+
return createStringError(
361+
errc::invalid_argument,
362+
"Specified RootParametersOffset does not match required value: %d.",
363+
RS.RootParameterOffset);
364+
}
365+
366+
RS.StaticSamplersOffset = RS.computeStaticSamplersOffset();
367+
if (P.RootSignature->StaticSamplersOffset &&
368+
P.RootSignature->StaticSamplersOffset.value() !=
369+
RS.StaticSamplersOffset) {
370+
return createStringError(
371+
errc::invalid_argument,
372+
"Specified StaticSamplersOffset does not match computed value: %d.",
373+
RS.StaticSamplersOffset);
374+
}
375+
355376
RS.write(OS);
356377
break;
357378
}
@@ -361,14 +382,15 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
361382
OS.write_zeros(PartSize - BytesWritten);
362383
RollingOffset += PartSize;
363384
}
385+
386+
return Error::success();
364387
}
365388

366389
Error DXContainerWriter::write(raw_ostream &OS) {
367390
if (Error Err = computePartOffsets())
368391
return Err;
369392
writeHeader(OS);
370-
writeParts(OS);
371-
return Error::success();
393+
return writeParts(OS);
372394
}
373395

374396
namespace llvm {

‎llvm/lib/ObjectYAML/DXContainerYAML.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,9 +376,9 @@ void MappingTraits<DXContainerYAML::RootSignatureYamlDesc>::mapping(
376376
IO &IO, DXContainerYAML::RootSignatureYamlDesc &S) {
377377
IO.mapRequired("Version", S.Version);
378378
IO.mapRequired("NumRootParameters", S.NumRootParameters);
379-
IO.mapRequired("RootParametersOffset", S.RootParametersOffset);
379+
IO.mapOptional("RootParametersOffset", S.RootParametersOffset, std::nullopt);
380380
IO.mapRequired("NumStaticSamplers", S.NumStaticSamplers);
381-
IO.mapRequired("StaticSamplersOffset", S.StaticSamplersOffset);
381+
IO.mapOptional("StaticSamplersOffset", S.StaticSamplersOffset, std::nullopt);
382382
IO.mapRequired("Parameters", S.Parameters.Locations, S);
383383
IO.mapOptional("Samplers", S.StaticSamplers);
384384
#define ROOT_SIGNATURE_FLAG(Num, Val) IO.mapOptional(#Val, S.Val, false);

‎llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinations.ll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
5959
;DXC-NEXT: NumRootParameters: 1
6060
;DXC-NEXT: RootParametersOffset: 24
6161
;DXC-NEXT: NumStaticSamplers: 0
62-
;DXC-NEXT: StaticSamplersOffset: 0
62+
;DXC-NEXT: StaticSamplersOffset: 380
6363
;DXC-NEXT: Parameters:
6464
;DXC-NEXT: - ParameterType: 0
6565
;DXC-NEXT: ShaderVisibility: 0

‎llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
2424
; DXC-NEXT: NumRootParameters: 1
2525
; DXC-NEXT: RootParametersOffset: 24
2626
; DXC-NEXT: NumStaticSamplers: 0
27-
; DXC-NEXT: StaticSamplersOffset: 0
27+
; DXC-NEXT: StaticSamplersOffset: 84
2828
; DXC-NEXT: Parameters:
2929
; DXC-NEXT: - ParameterType: 0
3030
; DXC-NEXT: ShaderVisibility: 0

‎llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
2626
; DXC-NEXT: NumRootParameters: 1
2727
; DXC-NEXT: RootParametersOffset: 24
2828
; DXC-NEXT: NumStaticSamplers: 0
29-
; DXC-NEXT: StaticSamplersOffset: 0
29+
; DXC-NEXT: StaticSamplersOffset: 92
3030
; DXC-NEXT: Parameters:
3131
; DXC-NEXT: - ParameterType: 0
3232
; DXC-NEXT: ShaderVisibility: 0

‎llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags.ll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,6 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
2525
; DXC-NEXT: NumRootParameters: 0
2626
; DXC-NEXT: RootParametersOffset: 24
2727
; DXC-NEXT: NumStaticSamplers: 0
28-
; DXC-NEXT: StaticSamplersOffset: 0
28+
; DXC-NEXT: StaticSamplersOffset: 24
2929
; DXC-NEXT: Parameters: []
3030
; DXC-NEXT: AllowInputAssemblerInputLayout: true

‎llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootConstants.ll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
2424
; DXC-NEXT: NumRootParameters: 1
2525
; DXC-NEXT: RootParametersOffset: 24
2626
; DXC-NEXT: NumStaticSamplers: 0
27-
; DXC-NEXT: StaticSamplersOffset: 0
27+
; DXC-NEXT: StaticSamplersOffset: 48
2828
; DXC-NEXT: Parameters:
2929
; DXC-NEXT: - ParameterType: 1
3030
; DXC-NEXT: ShaderVisibility: 0

0 commit comments

Comments
 (0)