[DirectX][ObectYAML] Make RootParameterOffset and StaticSamplersOffset behaviour consistent
#155521
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
llvm:mc
|
@llvm/pr-subscribers-llvm-binary-utilities @llvm/pr-subscribers-backend-directx Author: Finn Plummer (inbelic) ChangesThis pr fixes some inconsistencies in behaviour of how we handle
For more context see: #155299. Description of existing test updates updates:
See newly added tests for testing of optional parameter functionality and Resolves: #155299 Patch is 40.86 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/155521.diff 27 Files Affected:
diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
index 7e0a4c6b07039..359b27761cea3 100644
--- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
+++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
@@ -187,9 +187,9 @@ struct RootSignatureYamlDesc {
uint32_t Version;
uint32_t NumRootParameters;
- uint32_t RootParametersOffset;
+ std::optional<uint32_t> RootParametersOffset;
uint32_t NumStaticSamplers;
- uint32_t StaticSamplersOffset;
+ std::optional<uint32_t> StaticSamplersOffset;
RootParameterYamlDesc Parameters;
SmallVector<StaticSamplerYamlDesc> StaticSamplers;
diff --git a/llvm/lib/MC/DXContainerRootSignature.cpp b/llvm/lib/MC/DXContainerRootSignature.cpp
index c04dc6bd1800a..a13b6e34df35a 100644
--- a/llvm/lib/MC/DXContainerRootSignature.cpp
+++ b/llvm/lib/MC/DXContainerRootSignature.cpp
@@ -76,11 +76,7 @@ void RootSignatureDesc::write(raw_ostream &OS) const {
support::endian::write(BOS, NumParameters, llvm::endianness::little);
support::endian::write(BOS, RootParameterOffset, llvm::endianness::little);
support::endian::write(BOS, NumSamplers, llvm::endianness::little);
- uint32_t SSO = StaticSamplersOffset;
- if (NumSamplers > 0)
- SSO = writePlaceholder(BOS);
- else
- support::endian::write(BOS, SSO, llvm::endianness::little);
+ uint32_t SSO = writePlaceholder(BOS);
support::endian::write(BOS, Flags, llvm::endianness::little);
SmallVector<uint32_t> ParamsOffsets;
@@ -144,23 +140,21 @@ void RootSignatureDesc::write(raw_ostream &OS) const {
}
}
}
- if (NumSamplers > 0) {
- rewriteOffsetToCurrentByte(BOS, SSO);
- for (const auto &S : StaticSamplers) {
- support::endian::write(BOS, S.Filter, llvm::endianness::little);
- support::endian::write(BOS, S.AddressU, llvm::endianness::little);
- support::endian::write(BOS, S.AddressV, llvm::endianness::little);
- support::endian::write(BOS, S.AddressW, llvm::endianness::little);
- support::endian::write(BOS, S.MipLODBias, llvm::endianness::little);
- support::endian::write(BOS, S.MaxAnisotropy, llvm::endianness::little);
- support::endian::write(BOS, S.ComparisonFunc, llvm::endianness::little);
- support::endian::write(BOS, S.BorderColor, llvm::endianness::little);
- support::endian::write(BOS, S.MinLOD, llvm::endianness::little);
- support::endian::write(BOS, S.MaxLOD, llvm::endianness::little);
- support::endian::write(BOS, S.ShaderRegister, llvm::endianness::little);
- support::endian::write(BOS, S.RegisterSpace, llvm::endianness::little);
- support::endian::write(BOS, S.ShaderVisibility, llvm::endianness::little);
- }
+ rewriteOffsetToCurrentByte(BOS, SSO);
+ for (const auto &S : StaticSamplers) {
+ support::endian::write(BOS, S.Filter, llvm::endianness::little);
+ support::endian::write(BOS, S.AddressU, llvm::endianness::little);
+ support::endian::write(BOS, S.AddressV, llvm::endianness::little);
+ support::endian::write(BOS, S.AddressW, llvm::endianness::little);
+ support::endian::write(BOS, S.MipLODBias, llvm::endianness::little);
+ support::endian::write(BOS, S.MaxAnisotropy, llvm::endianness::little);
+ support::endian::write(BOS, S.ComparisonFunc, llvm::endianness::little);
+ support::endian::write(BOS, S.BorderColor, llvm::endianness::little);
+ support::endian::write(BOS, S.MinLOD, llvm::endianness::little);
+ support::endian::write(BOS, S.MaxLOD, llvm::endianness::little);
+ support::endian::write(BOS, S.ShaderRegister, llvm::endianness::little);
+ support::endian::write(BOS, S.RegisterSpace, llvm::endianness::little);
+ support::endian::write(BOS, S.ShaderVisibility, llvm::endianness::little);
}
assert(Storage.size() == getSize());
OS.write(Storage.data(), Storage.size());
diff --git a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp
index b112c6f21ee5a..726a879075435 100644
--- a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp
@@ -38,7 +38,7 @@ class DXContainerWriter {
Error validateSize(uint32_t Computed);
void writeHeader(raw_ostream &OS);
- void writeParts(raw_ostream &OS);
+ Error writeParts(raw_ostream &OS);
};
} // namespace
@@ -107,7 +107,7 @@ void DXContainerWriter::writeHeader(raw_ostream &OS) {
Offsets.size() * sizeof(uint32_t));
}
-void DXContainerWriter::writeParts(raw_ostream &OS) {
+Error DXContainerWriter::writeParts(raw_ostream &OS) {
uint32_t RollingOffset =
sizeof(dxbc::Header) + (ObjectFile.Header.PartCount * sizeof(uint32_t));
for (auto I : llvm::zip(ObjectFile.Parts, *ObjectFile.Header.PartOffsets)) {
@@ -269,13 +269,27 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
mcdxbc::RootSignatureDesc RS;
RS.Flags = P.RootSignature->getEncodedFlags();
RS.Version = P.RootSignature->Version;
- RS.RootParameterOffset = P.RootSignature->RootParametersOffset;
- RS.NumStaticSamplers = P.RootSignature->NumStaticSamplers;
- RS.StaticSamplersOffset = P.RootSignature->StaticSamplersOffset;
+
+ // Handling of RootParameters
+ const uint32_t RootHeaderSize =
+ sizeof(dxbc::RTS0::v1::RootSignatureHeader);
+ if (P.RootSignature->RootParametersOffset &&
+ P.RootSignature->RootParametersOffset.value() != RootHeaderSize) {
+ return createStringError(
+ errc::invalid_argument,
+ "Specified RootParametersOffset does not match required value: %d.",
+ RootHeaderSize);
+ }
+
+ uint32_t Offset = RootHeaderSize;
+ RS.RootParameterOffset = Offset;
for (DXContainerYAML::RootParameterLocationYaml &L :
P.RootSignature->Parameters.Locations) {
+ // Offset RootParameterHeader
+ Offset += sizeof(dxbc::RTS0::v1::RootParameterHeader);
+
assert(dxbc::isValidParameterType(L.Header.Type) &&
"invalid DXContainer YAML");
assert(dxbc::isValidShaderVisibility(L.Header.Visibility) &&
@@ -294,6 +308,8 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
Constants.RegisterSpace = ConstantYaml.RegisterSpace;
Constants.ShaderRegister = ConstantYaml.ShaderRegister;
RS.ParametersContainer.addParameter(Type, Visibility, Constants);
+
+ Offset += sizeof(dxbc::RTS0::v1::RootConstants);
break;
}
case dxbc::RootParameterType::CBV:
@@ -305,8 +321,12 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
dxbc::RTS0::v2::RootDescriptor Descriptor;
Descriptor.RegisterSpace = DescriptorYaml.RegisterSpace;
Descriptor.ShaderRegister = DescriptorYaml.ShaderRegister;
- if (RS.Version > 1)
+ if (RS.Version > 1) {
Descriptor.Flags = DescriptorYaml.getEncodedFlags();
+ Offset += sizeof(dxbc::RTS0::v2::RootDescriptor);
+ } else
+ Offset += sizeof(dxbc::RTS0::v1::RootDescriptor);
+
RS.ParametersContainer.addParameter(Type, Visibility, Descriptor);
break;
}
@@ -314,6 +334,8 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
const DXContainerYAML::DescriptorTableYaml &TableYaml =
P.RootSignature->Parameters.getOrInsertTable(L);
mcdxbc::DescriptorTable Table;
+ Offset +=
+ 2 * sizeof(uint32_t); // DescriptorTable NumRanges and Offset
for (const auto &R : TableYaml.Ranges) {
dxbc::RTS0::v2::DescriptorRange Range;
@@ -323,8 +345,13 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
Range.RegisterSpace = R.RegisterSpace;
Range.OffsetInDescriptorsFromTableStart =
R.OffsetInDescriptorsFromTableStart;
- if (RS.Version > 1)
+
+ if (RS.Version > 1) {
+ Offset += sizeof(dxbc::RTS0::v2::DescriptorRange);
Range.Flags = R.getEncodedFlags();
+ } else
+ Offset += sizeof(dxbc::RTS0::v1::DescriptorRange);
+
Table.Ranges.push_back(Range);
}
RS.ParametersContainer.addParameter(Type, Visibility, Table);
@@ -333,6 +360,19 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
}
}
+ // Handling of StaticSamplers
+ RS.NumStaticSamplers = P.RootSignature->NumStaticSamplers;
+
+ if (P.RootSignature->StaticSamplersOffset &&
+ P.RootSignature->StaticSamplersOffset.value() != Offset) {
+ return createStringError(
+ errc::invalid_argument,
+ "Specified StaticSamplersOffset does not match computed value: %d.",
+ Offset);
+ }
+
+ RS.StaticSamplersOffset = Offset;
+
for (const auto &Param : P.RootSignature->samplers()) {
dxbc::RTS0::v1::StaticSampler NewSampler;
NewSampler.Filter = Param.Filter;
@@ -361,14 +401,15 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
OS.write_zeros(PartSize - BytesWritten);
RollingOffset += PartSize;
}
+
+ return Error::success();
}
Error DXContainerWriter::write(raw_ostream &OS) {
if (Error Err = computePartOffsets())
return Err;
writeHeader(OS);
- writeParts(OS);
- return Error::success();
+ return writeParts(OS);
}
namespace llvm {
diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
index 263f7bdf37bca..32b502ed4e21f 100644
--- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
@@ -376,9 +376,9 @@ void MappingTraits<DXContainerYAML::RootSignatureYamlDesc>::mapping(
IO &IO, DXContainerYAML::RootSignatureYamlDesc &S) {
IO.mapRequired("Version", S.Version);
IO.mapRequired("NumRootParameters", S.NumRootParameters);
- IO.mapRequired("RootParametersOffset", S.RootParametersOffset);
+ IO.mapOptional("RootParametersOffset", S.RootParametersOffset, std::nullopt);
IO.mapRequired("NumStaticSamplers", S.NumStaticSamplers);
- IO.mapRequired("StaticSamplersOffset", S.StaticSamplersOffset);
+ IO.mapOptional("StaticSamplersOffset", S.StaticSamplersOffset, std::nullopt);
IO.mapRequired("Parameters", S.Parameters.Locations, S);
IO.mapOptional("Samplers", S.StaticSamplers);
#define ROOT_SIGNATURE_FLAG(Num, Val) IO.mapOptional(#Val, S.Val, false);
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinations.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinations.ll
index 8eb7f90c6b757..1bc9b85935819 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinations.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinations.ll
@@ -59,7 +59,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
;DXC-NEXT: NumRootParameters: 1
;DXC-NEXT: RootParametersOffset: 24
;DXC-NEXT: NumStaticSamplers: 0
-;DXC-NEXT: StaticSamplersOffset: 0
+;DXC-NEXT: StaticSamplersOffset: 380
;DXC-NEXT: Parameters:
;DXC-NEXT: - ParameterType: 0
;DXC-NEXT: ShaderVisibility: 0
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll
index 053721de1eb1f..fec6c4c959642 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll
@@ -24,7 +24,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
; DXC-NEXT: NumRootParameters: 1
; DXC-NEXT: RootParametersOffset: 24
; DXC-NEXT: NumStaticSamplers: 0
-; DXC-NEXT: StaticSamplersOffset: 0
+; DXC-NEXT: StaticSamplersOffset: 84
; DXC-NEXT: Parameters:
; DXC-NEXT: - ParameterType: 0
; DXC-NEXT: ShaderVisibility: 0
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll
index 8e9b4b43b11a6..4f6f0d0bd6a14 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll
@@ -26,7 +26,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
; DXC-NEXT: NumRootParameters: 1
; DXC-NEXT: RootParametersOffset: 24
; DXC-NEXT: NumStaticSamplers: 0
-; DXC-NEXT: StaticSamplersOffset: 0
+; DXC-NEXT: StaticSamplersOffset: 92
; DXC-NEXT: Parameters:
; DXC-NEXT: - ParameterType: 0
; DXC-NEXT: ShaderVisibility: 0
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags.ll
index 10235b7d17960..165e4803f8702 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags.ll
@@ -25,6 +25,6 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
; DXC-NEXT: NumRootParameters: 0
; DXC-NEXT: RootParametersOffset: 24
; DXC-NEXT: NumStaticSamplers: 0
-; DXC-NEXT: StaticSamplersOffset: 0
+; DXC-NEXT: StaticSamplersOffset: 24
; DXC-NEXT: Parameters: []
; DXC-NEXT: AllowInputAssemblerInputLayout: true
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootConstants.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootConstants.ll
index 964554fe143ef..d217f396722bc 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootConstants.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootConstants.ll
@@ -24,7 +24,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
; DXC-NEXT: NumRootParameters: 1
; DXC-NEXT: RootParametersOffset: 24
; DXC-NEXT: NumStaticSamplers: 0
-; DXC-NEXT: StaticSamplersOffset: 0
+; DXC-NEXT: StaticSamplersOffset: 48
; DXC-NEXT: Parameters:
; DXC-NEXT: - ParameterType: 1
; DXC-NEXT: ShaderVisibility: 0
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll
index f77bb96840bea..54292bb651532 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll
@@ -24,7 +24,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
; DXC-NEXT: NumRootParameters: 1
; DXC-NEXT: RootParametersOffset: 24
; DXC-NEXT: NumStaticSamplers: 0
-; DXC-NEXT: StaticSamplersOffset: 0
+; DXC-NEXT: StaticSamplersOffset: 48
; DXC-NEXT: Parameters:
; DXC-NEXT: - ParameterType: 2
; DXC-NEXT: ShaderVisibility: 0
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor_V1.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor_V1.ll
index ddf556e7fe20a..891a03b688a82 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor_V1.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor_V1.ll
@@ -24,7 +24,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
; DXC-NEXT: NumRootParameters: 1
; DXC-NEXT: RootParametersOffset: 24
; DXC-NEXT: NumStaticSamplers: 0
-; DXC-NEXT: StaticSamplersOffset: 0
+; DXC-NEXT: StaticSamplersOffset: 44
; DXC-NEXT: Parameters:
; DXC-NEXT: - ParameterType: 2
; DXC-NEXT: ShaderVisibility: 0
diff --git a/llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.0.yaml b/llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.0.yaml
index 889eccf74001f..70dc35287ba91 100644
--- a/llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.0.yaml
+++ b/llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.0.yaml
@@ -17,7 +17,7 @@ Parts:
NumRootParameters: 1
RootParametersOffset: 24
NumStaticSamplers: 0
- StaticSamplersOffset: 60
+ StaticSamplersOffset: 44
Parameters:
- ParameterType: 2 # SRV
ShaderVisibility: 3 # Domain
@@ -34,7 +34,7 @@ Parts:
# CHECK-NEXT: NumRootParameters: 1
# CHECK-NEXT: RootParametersOffset: 24
# CHECK-NEXT: NumStaticSamplers: 0
-# CHECK-NEXT: StaticSamplersOffset: 60
+# CHECK-NEXT: StaticSamplersOffset: 44
# CHECK-NEXT: Parameters:
# CHECK-NEXT: - ParameterType: 2
# CHECK-NEXT: ShaderVisibility: 3
diff --git a/llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.1.yaml b/llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.1.yaml
index 64e01c6836e32..33a74dbf6a3f4 100644
--- a/llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.1.yaml
+++ b/llvm/test/ObjectYAML/DXContainer/RootSignature-Descriptor1.1.yaml
@@ -17,7 +17,7 @@ Parts:
NumRootParameters: 1
RootParametersOffset: 24
NumStaticSamplers: 0
- StaticSamplersOffset: 60
+ StaticSamplersOffset: 48
Parameters:
- ParameterType: 2 # SRV
ShaderVisibility: 3 # Domain
@@ -35,7 +35,7 @@ Parts:
# CHECK-NEXT: NumRootParameters: 1
# CHECK-NEXT: RootParametersOffset: 24
# CHECK-NEXT: NumStaticSamplers: 0
-# CHECK-NEXT: StaticSamplersOffset: 60
+# CHECK-NEXT: StaticSamplersOffset: 48
# CHECK-NEXT: Parameters:
# CHECK-NEXT: - ParameterType: 2
# CHECK-NEXT: ShaderVisibility: 3
diff --git a/llvm/test/ObjectYAML/DXContainer/RootSignature-DescriptorTable1.0.yaml b/llvm/test/ObjectYAML/DXContainer/RootSignature-DescriptorTable1.0.yaml
index 0441bb7a256b1..b04549fde88f7 100644
--- a/llvm/test/ObjectYAML/DXContainer/RootSignature-DescriptorTable1.0.yaml
+++ b/llvm/test/ObjectYAML/DXContainer/RootSignature-DescriptorTable1.0.yaml
@@ -18,7 +18,7 @@ Parts:
NumRootParameters: 1
RootParametersOffset: 24
NumStaticSamplers: 0
- StaticSamplersOffset: 60
+ StaticSamplersOffset: 64
Parameters:
- ParameterType: 0 # SRV
ShaderVisibility: 3 # Domain
@@ -40,7 +40,7 @@ Parts:
# CHECK-NEXT: NumRootParameters: 1
# CHECK-NEXT: RootParametersOffset: 24
# CHECK-NEXT: NumStaticSamplers: 0
-# CHECK-NEXT: StaticSamplersOffset: 60
+# CHECK-NEXT: StaticSamplersOffset: 64
# CHECK-NEXT: Parameters:
# CHECK-NEXT: - ParameterType: 0
# CHECK-NEXT: ShaderVisibility: 3
diff --git a/llvm/test/ObjectYAML/DXContainer/RootSignature-DescriptorTable1.1.yaml b/llvm/test/ObjectYAML/DXContainer/RootSignature-DescriptorTable1.1.yaml
index d06be5e181418..d8f399010053e 100644
--- a/llvm/test/ObjectYAML/DXContainer/RootSignature-DescriptorTable1.1.yaml
+++ b/llvm/test/ObjectYAML/DXContainer/RootSignature-DescriptorTable1.1.yaml
@@ -18,7 +18,7 @@ Parts:
NumRootParameters: 1
RootParametersOffset: 24
NumStaticSamplers: 0
- StaticSamplersOffset: 60
+ StaticSamplersOffset: 68
Parameters:
- ParameterType: 0 # SRV
ShaderVisibility: 3 # Domain
@@ -41,7 +41,7 @@ Parts:
# CHECK-NEXT: NumRootParameters: 1
# CHECK-NEXT: RootParametersOffset: 24
# CHECK-NEXT: NumStaticSamplers: 0
-# CHECK-NEXT: StaticSamplersOffset: 60
+# CHECK-NEXT: StaticSamplersOffset: 68
# CHECK-NEXT: Parameters:
# CHECK-NEXT: - ParameterType: 0
# CHECK-NEXT: ShaderVisibility: 3
diff --git a/llvm/test/ObjectYAML/DXContainer/RootSignature-Flags.yaml b/llvm/test/ObjectYAML/DXContainer/RootSignature-Flags.yaml
index 74816d403183a..c5855a249c31d 100644
--- a/llvm/test/ObjectYAML/DXContainer/RootSignature-Flags.yaml
+++ b/llvm/test/ObjectYAML/DXContainer/RootSignature-Flags.yaml
@@ -17,7 +17,7 @@ Parts:
NumRootParameters: 0
RootParametersOffset: 24
NumStaticSamplers: 0
- StaticSamplersOffset: 60
+ StaticSamplersOffset: 24
Parameters: []
AllowInputAssemblerInputLayout: true
DenyGeometryShaderRootAccess: true
@@ -29,7 +29,7 @@ Parts:
# CHECK-NEXT: NumRootParameters: 0
# C...
[truncated]
|
tex3d
approved these changes
Aug 27, 2025
joaosaffran
reviewed
Aug 27, 2025
Comment on lines
275
to
282
| sizeof(dxbc::RTS0::v1::RootSignatureHeader); | ||
| if (P.RootSignature->RootParametersOffset && | ||
| P.RootSignature->RootParametersOffset.value() != RootHeaderSize) { | ||
| return createStringError( | ||
| errc::invalid_argument, | ||
| "Specified RootParametersOffset does not match required value: %d.", | ||
| RootHeaderSize); | ||
| } |
There was a problem hiding this comment.
I feel like it is better to update mcdxbc::RootSignatureDesc write function to return a struct with such offsets. And then compare after the actual writing. Since those would be the actual offsets and also, prevent us from getting bitten by offset calculations in the future.
There was a problem hiding this comment.
True, good point to have the computation in a common place. I do think it is more inline with the other parts to do any of the parameter computation before writing though. (See the general use of finalize, eg PSV0).
So I split up the computation of getSize, so that any size compute logic is only writing once.
joaosaffran
reviewed
Aug 27, 2025
| support::endian::write(BOS, S.RegisterSpace, llvm::endianness::little); | ||
| support::endian::write(BOS, S.ShaderVisibility, llvm::endianness::little); | ||
| } | ||
| rewriteOffsetToCurrentByte(BOS, SSO); |
There was a problem hiding this comment.
I think adding some asserts like the one below, in the places we are manually calculating sizes and offset to be beneficial. The one in line 170 helped detect many bugs during development.
This would require making rewriteOffsetToCurrentByte return the new offset, but that should be a straight forward change.
Suggested change
| rewriteOffsetToCurrentByte(BOS, SSO); | |
| uint32_t SSORealOffset = rewriteOffsetToCurrentByte(BOS, SSO); | |
| assert(computeStaticSamplersOffset() == SSORealOffset); |
inbelic
force-pushed
the
inbelic/set-offset
branch
from
a10dfdf to
3736a63
Compare
bogner
approved these changes
Aug 27, 2025
|
This patch seems to have broken the Solaris/sparcv9 buildbot badly. |
Thanks for pointing it out. #155860 should address this. |
|
It did indeed once the bot caught up. Thanks for the fix. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pr fixes some inconsistencies in behaviour of how we handle
StaticSamplersOffsetwith respect to DXC andRootParameterOffset. Namely:RTS0always compute theStaticSamplersOffsetregardless if there are anyStaticSamplers. This is to be consistent and produce an identicalDXContaineras DXC.StaticSamplersOffsetandRootParametersOffsetoptional parameters in the yaml description. This means it will be used when it is specified (which was not necassarily the case before).StaticSamplersOffsetandRootParametersOffsetin a yaml description match the computed value.For more context see: #155299.
Description of existing test updates updates:
CodeGen/DirectX/ContainerData: Updated to codegen computed values (previously unspecified)llvm-objcopy/DXContainer: Updated toyaml2objcomputed values (previously unspecified)ObjectYAML/DXContainer: Updated toyaml2objcomputed values (previously incorrect)ObjectYAML/DXContainerYAMLTest: Updated toyaml2objcomputed values (previously incorrect)See newly added tests for testing of optional parameter functionality and
StaticSamplersOffsetcomputation.Resolves: #155299