Skip to content

[DirectX] Refactor RootSignature Backend to remove to_underlying from Root Parameter Header #154249

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Aug 25, 2025
Merged

[DirectX] Refactor RootSignature Backend to remove to_underlying from Root Parameter Header #154249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
31ec5e5
making parameter type and shader visibility use enums
joaosaffran Aug 19, 2025
1690a9c
clean up
joaosaffran Aug 19, 2025
f6f2e61
removing root parameter header from MC
joaosaffran Aug 19, 2025
6539364
clean up
joaosaffran Aug 19, 2025
1d29111
fix whitespace in test
joaosaffran Aug 19, 2025
8eb82fd
adding missing import
joaosaffran Aug 19, 2025
d38c00d
remove unused
joaosaffran Aug 19, 2025
3b25b34
save a copy
joaosaffran Aug 19, 2025
567a3d4
remove default constructor
joaosaffran Aug 19, 2025
fb248da
rename visibility
joaosaffran Aug 19, 2025
dc436d5
remove cstdint
joaosaffran Aug 19, 2025
8353fe0
remove Loc
joaosaffran Aug 19, 2025
3a30581
addressing comments from bogner
joaosaffran Aug 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 19 additions & 26 deletions llvm/include/llvm/MC/DXContainerRootSignature.h
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, this pr is meant to remove any dependency from MC onto Object/DXContainer.h?

Shouldn't that mean that some include is removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It not from Object/DXContainer.h, it is from some values defined inside BinaryFormat/DXContainer.h mainly the RTS0 namespace

Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ class raw_ostream;
namespace mcdxbc {

struct RootParameterInfo {
dxbc::RTS0::v1::RootParameterHeader Header;
dxbc::RootParameterType Type;
dxbc::ShaderVisibility Visibility;
size_t Location;

RootParameterInfo() = default;

RootParameterInfo(dxbc::RTS0::v1::RootParameterHeader Header, size_t Location)
: Header(Header), Location(Location) {}
RootParameterInfo(dxbc::RootParameterType Type,
dxbc::ShaderVisibility Visibility, size_t Location)
: Type(Type), Visibility(Visibility), Location(Location) {}
};

struct DescriptorTable {
Expand All @@ -46,41 +46,34 @@ struct RootParametersContainer {
SmallVector<dxbc::RTS0::v2::RootDescriptor> Descriptors;
SmallVector<DescriptorTable> Tables;

void addInfo(dxbc::RTS0::v1::RootParameterHeader Header, size_t Location) {
ParametersInfo.push_back(RootParameterInfo(Header, Location));
void addInfo(dxbc::RootParameterType Type, dxbc::ShaderVisibility Visibility,
size_t Location) {
ParametersInfo.emplace_back(Type, Visibility, Location);
}

void addParameter(dxbc::RTS0::v1::RootParameterHeader Header,
void addParameter(dxbc::RootParameterType Type,
dxbc::ShaderVisibility Visibility,
dxbc::RTS0::v1::RootConstants Constant) {
addInfo(Header, Constants.size());
addInfo(Type, Visibility, Constants.size());
Constants.push_back(Constant);
}

void addInvalidParameter(dxbc::RTS0::v1::RootParameterHeader Header) {
addInfo(Header, -1);
}

void addParameter(dxbc::RTS0::v1::RootParameterHeader Header,
void addParameter(dxbc::RootParameterType Type,
dxbc::ShaderVisibility Visibility,
dxbc::RTS0::v2::RootDescriptor Descriptor) {
addInfo(Header, Descriptors.size());
addInfo(Type, Visibility, Descriptors.size());
Descriptors.push_back(Descriptor);
}

void addParameter(dxbc::RTS0::v1::RootParameterHeader Header,
DescriptorTable Table) {
addInfo(Header, Tables.size());
void addParameter(dxbc::RootParameterType Type,
dxbc::ShaderVisibility Visibility, DescriptorTable Table) {
addInfo(Type, Visibility, Tables.size());
Tables.push_back(Table);
}

std::pair<uint32_t, uint32_t>
getTypeAndLocForParameter(uint32_t Location) const {
const RootParameterInfo &Info = ParametersInfo[Location];
return {Info.Header.ParameterType, Info.Location};
}

const dxbc::RTS0::v1::RootParameterHeader &getHeader(size_t Location) const {
const RootParameterInfo &getInfo(uint32_t Location) const {
const RootParameterInfo &Info = ParametersInfo[Location];
return Info.Header;
return Info;
}

const dxbc::RTS0::v1::RootConstants &getConstant(size_t Index) const {
Expand Down
81 changes: 40 additions & 41 deletions llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,17 @@ static std::optional<StringRef> extractMdStringValue(MDNode *Node,
return NodeText->getString();
}

static Expected<dxbc::ShaderVisibility>
extractShaderVisibility(MDNode *Node, unsigned int OpId) {
if (std::optional<uint32_t> Val = extractMdIntValue(Node, OpId)) {
if (!dxbc::isValidShaderVisibility(*Val))
return make_error<RootSignatureValidationError<uint32_t>>(
"ShaderVisibility", *Val);
return dxbc::ShaderVisibility(*Val);
}
return make_error<InvalidRSMetadataValue>("ShaderVisibility");
}

namespace {

// We use the OverloadVisit with std::visit to ensure the compiler catches if a
Expand Down Expand Up @@ -224,15 +235,10 @@ Error MetadataParser::parseRootConstants(mcdxbc::RootSignatureDesc &RSD,
if (RootConstantNode->getNumOperands() != 5)
return make_error<InvalidRSMetadataFormat>("RootConstants Element");

dxbc::RTS0::v1::RootParameterHeader Header;
// The parameter offset doesn't matter here - we recalculate it during
// serialization Header.ParameterOffset = 0;
Header.ParameterType = to_underlying(dxbc::RootParameterType::Constants32Bit);

if (std::optional<uint32_t> Val = extractMdIntValue(RootConstantNode, 1))
Header.ShaderVisibility = *Val;
else
return make_error<InvalidRSMetadataValue>("ShaderVisibility");
Expected<dxbc::ShaderVisibility> Visibility =
extractShaderVisibility(RootConstantNode, 1);
if (auto E = Visibility.takeError())
return Error(std::move(E));

dxbc::RTS0::v1::RootConstants Constants;
if (std::optional<uint32_t> Val = extractMdIntValue(RootConstantNode, 2))
Expand All @@ -250,7 +256,8 @@ Error MetadataParser::parseRootConstants(mcdxbc::RootSignatureDesc &RSD,
else
return make_error<InvalidRSMetadataValue>("Num32BitValues");

RSD.ParametersContainer.addParameter(Header, Constants);
RSD.ParametersContainer.addParameter(dxbc::RootParameterType::Constants32Bit,
*Visibility, Constants);

return Error::success();
}
Expand All @@ -266,26 +273,26 @@ Error MetadataParser::parseRootDescriptors(
if (RootDescriptorNode->getNumOperands() != 5)
return make_error<InvalidRSMetadataFormat>("Root Descriptor Element");

dxbc::RTS0::v1::RootParameterHeader Header;
dxbc::RootParameterType Type;
switch (ElementKind) {
case RootSignatureElementKind::SRV:
Header.ParameterType = to_underlying(dxbc::RootParameterType::SRV);
Type = dxbc::RootParameterType::SRV;
break;
case RootSignatureElementKind::UAV:
Header.ParameterType = to_underlying(dxbc::RootParameterType::UAV);
Type = dxbc::RootParameterType::UAV;
break;
case RootSignatureElementKind::CBV:
Header.ParameterType = to_underlying(dxbc::RootParameterType::CBV);
Type = dxbc::RootParameterType::CBV;
break;
default:
llvm_unreachable("invalid Root Descriptor kind");
break;
}

if (std::optional<uint32_t> Val = extractMdIntValue(RootDescriptorNode, 1))
Header.ShaderVisibility = *Val;
else
return make_error<InvalidRSMetadataValue>("ShaderVisibility");
Expected<dxbc::ShaderVisibility> Visibility =
extractShaderVisibility(RootDescriptorNode, 1);
if (auto E = Visibility.takeError())
return Error(std::move(E));

dxbc::RTS0::v2::RootDescriptor Descriptor;
if (std::optional<uint32_t> Val = extractMdIntValue(RootDescriptorNode, 2))
Expand All @@ -299,7 +306,7 @@ Error MetadataParser::parseRootDescriptors(
return make_error<InvalidRSMetadataValue>("RegisterSpace");

if (RSD.Version == 1) {
RSD.ParametersContainer.addParameter(Header, Descriptor);
RSD.ParametersContainer.addParameter(Type, *Visibility, Descriptor);
return Error::success();
}
assert(RSD.Version > 1);
Expand All @@ -309,7 +316,7 @@ Error MetadataParser::parseRootDescriptors(
else
return make_error<InvalidRSMetadataValue>("Root Descriptor Flags");

RSD.ParametersContainer.addParameter(Header, Descriptor);
RSD.ParametersContainer.addParameter(Type, *Visibility, Descriptor);
return Error::success();
}

Expand Down Expand Up @@ -375,15 +382,12 @@ Error MetadataParser::parseDescriptorTable(mcdxbc::RootSignatureDesc &RSD,
if (NumOperands < 2)
return make_error<InvalidRSMetadataFormat>("Descriptor Table");

dxbc::RTS0::v1::RootParameterHeader Header;
if (std::optional<uint32_t> Val = extractMdIntValue(DescriptorTableNode, 1))
Header.ShaderVisibility = *Val;
else
return make_error<InvalidRSMetadataValue>("ShaderVisibility");
Expected<dxbc::ShaderVisibility> Visibility =
extractShaderVisibility(DescriptorTableNode, 1);
if (auto E = Visibility.takeError())
return Error(std::move(E));

mcdxbc::DescriptorTable Table;
Header.ParameterType =
to_underlying(dxbc::RootParameterType::DescriptorTable);

for (unsigned int I = 2; I < NumOperands; I++) {
MDNode *Element = dyn_cast<MDNode>(DescriptorTableNode->getOperand(I));
Expand All @@ -395,7 +399,8 @@ Error MetadataParser::parseDescriptorTable(mcdxbc::RootSignatureDesc &RSD,
return Err;
}

RSD.ParametersContainer.addParameter(Header, Table);
RSD.ParametersContainer.addParameter(dxbc::RootParameterType::DescriptorTable,
*Visibility, Table);
return Error::success();
}

Expand Down Expand Up @@ -531,20 +536,14 @@ Error MetadataParser::validateRootSignature(
}

for (const mcdxbc::RootParameterInfo &Info : RSD.ParametersContainer) {
if (!dxbc::isValidShaderVisibility(Info.Header.ShaderVisibility))
DeferredErrs =
joinErrors(std::move(DeferredErrs),
make_error<RootSignatureValidationError<uint32_t>>(
"ShaderVisibility", Info.Header.ShaderVisibility));

assert(dxbc::isValidParameterType(Info.Header.ParameterType) &&
"Invalid value for ParameterType");

switch (Info.Header.ParameterType) {
switch (Info.Type) {
case dxbc::RootParameterType::Constants32Bit:
break;

case to_underlying(dxbc::RootParameterType::CBV):
case to_underlying(dxbc::RootParameterType::UAV):
case to_underlying(dxbc::RootParameterType::SRV): {
case dxbc::RootParameterType::CBV:
case dxbc::RootParameterType::UAV:
case dxbc::RootParameterType::SRV: {
const dxbc::RTS0::v2::RootDescriptor &Descriptor =
RSD.ParametersContainer.getRootDescriptor(Info.Location);
if (!hlsl::rootsig::verifyRegisterValue(Descriptor.ShaderRegister))
Expand All @@ -569,7 +568,7 @@ Error MetadataParser::validateRootSignature(
}
break;
}
case to_underlying(dxbc::RootParameterType::DescriptorTable): {
case dxbc::RootParameterType::DescriptorTable: {
const mcdxbc::DescriptorTable &Table =
RSD.ParametersContainer.getDescriptorTable(Info.Location);
for (const dxbc::RTS0::v2::DescriptorRange &Range : Table) {
Expand Down
40 changes: 19 additions & 21 deletions llvm/lib/MC/DXContainerRootSignature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,20 @@ size_t RootSignatureDesc::getSize() const {
StaticSamplers.size() * sizeof(dxbc::RTS0::v1::StaticSampler);

for (const RootParameterInfo &I : ParametersContainer) {
switch (I.Header.ParameterType) {
case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit):
switch (I.Type) {
case dxbc::RootParameterType::Constants32Bit:
Size += sizeof(dxbc::RTS0::v1::RootConstants);
break;
case llvm::to_underlying(dxbc::RootParameterType::CBV):
case llvm::to_underlying(dxbc::RootParameterType::SRV):
case llvm::to_underlying(dxbc::RootParameterType::UAV):
case dxbc::RootParameterType::CBV:
case dxbc::RootParameterType::SRV:
case dxbc::RootParameterType::UAV:
if (Version == 1)
Size += sizeof(dxbc::RTS0::v1::RootDescriptor);
else
Size += sizeof(dxbc::RTS0::v2::RootDescriptor);

break;
case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable):
case dxbc::RootParameterType::DescriptorTable:
const DescriptorTable &Table =
ParametersContainer.getDescriptorTable(I.Location);

Expand Down Expand Up @@ -84,23 +84,21 @@ void RootSignatureDesc::write(raw_ostream &OS) const {
support::endian::write(BOS, Flags, llvm::endianness::little);

SmallVector<uint32_t> ParamsOffsets;
for (const RootParameterInfo &P : ParametersContainer) {
support::endian::write(BOS, P.Header.ParameterType,
llvm::endianness::little);
support::endian::write(BOS, P.Header.ShaderVisibility,
llvm::endianness::little);
for (const RootParameterInfo &I : ParametersContainer) {
support::endian::write(BOS, I.Type, llvm::endianness::little);
support::endian::write(BOS, I.Visibility, llvm::endianness::little);

ParamsOffsets.push_back(writePlaceholder(BOS));
}

assert(NumParameters == ParamsOffsets.size());
for (size_t I = 0; I < NumParameters; ++I) {
rewriteOffsetToCurrentByte(BOS, ParamsOffsets[I]);
const auto &[Type, Loc] = ParametersContainer.getTypeAndLocForParameter(I);
switch (Type) {
case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): {
const RootParameterInfo &Info = ParametersContainer.getInfo(I);
switch (Info.Type) {
case dxbc::RootParameterType::Constants32Bit: {
const dxbc::RTS0::v1::RootConstants &Constants =
ParametersContainer.getConstant(Loc);
ParametersContainer.getConstant(Info.Location);
support::endian::write(BOS, Constants.ShaderRegister,
llvm::endianness::little);
support::endian::write(BOS, Constants.RegisterSpace,
Expand All @@ -109,11 +107,11 @@ void RootSignatureDesc::write(raw_ostream &OS) const {
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): {
case dxbc::RootParameterType::CBV:
case dxbc::RootParameterType::SRV:
case dxbc::RootParameterType::UAV: {
const dxbc::RTS0::v2::RootDescriptor &Descriptor =
ParametersContainer.getRootDescriptor(Loc);
ParametersContainer.getRootDescriptor(Info.Location);

support::endian::write(BOS, Descriptor.ShaderRegister,
llvm::endianness::little);
Expand All @@ -123,9 +121,9 @@ void RootSignatureDesc::write(raw_ostream &OS) const {
support::endian::write(BOS, Descriptor.Flags, llvm::endianness::little);
break;
}
case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable): {
case dxbc::RootParameterType::DescriptorTable: {
const DescriptorTable &Table =
ParametersContainer.getDescriptorTable(Loc);
ParametersContainer.getDescriptorTable(Info.Location);
support::endian::write(BOS, (uint32_t)Table.Ranges.size(),
llvm::endianness::little);
rewriteOffsetToCurrentByte(BOS, writePlaceholder(BOS));
Expand Down
34 changes: 18 additions & 16 deletions llvm/lib/ObjectYAML/DXContainerEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,23 +275,30 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {

for (DXContainerYAML::RootParameterLocationYaml &L :
P.RootSignature->Parameters.Locations) {
dxbc::RTS0::v1::RootParameterHeader Header{L.Header.Type, L.Header.Visibility,
L.Header.Offset};

switch (L.Header.Type) {
case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): {
assert(dxbc::isValidParameterType(L.Header.Type) &&
"invalid DXContainer YAML");
assert(dxbc::isValidShaderVisibility(L.Header.Visibility) &&
"invalid DXContainer YAML");
dxbc::RootParameterType Type = dxbc::RootParameterType(L.Header.Type);
dxbc::ShaderVisibility Visibility =
dxbc::ShaderVisibility(L.Header.Visibility);

switch (Type) {
case dxbc::RootParameterType::Constants32Bit: {
const DXContainerYAML::RootConstantsYaml &ConstantYaml =
P.RootSignature->Parameters.getOrInsertConstants(L);
dxbc::RTS0::v1::RootConstants Constants;

Constants.Num32BitValues = ConstantYaml.Num32BitValues;
Constants.RegisterSpace = ConstantYaml.RegisterSpace;
Constants.ShaderRegister = ConstantYaml.ShaderRegister;
RS.ParametersContainer.addParameter(Header, Constants);
RS.ParametersContainer.addParameter(Type, Visibility, Constants);
break;
}
case llvm::to_underlying(dxbc::RootParameterType::CBV):
case llvm::to_underlying(dxbc::RootParameterType::SRV):
case llvm::to_underlying(dxbc::RootParameterType::UAV): {
case dxbc::RootParameterType::CBV:
case dxbc::RootParameterType::SRV:
case dxbc::RootParameterType::UAV: {
const DXContainerYAML::RootDescriptorYaml &DescriptorYaml =
P.RootSignature->Parameters.getOrInsertDescriptor(L);

Expand All @@ -300,10 +307,10 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
Descriptor.ShaderRegister = DescriptorYaml.ShaderRegister;
if (RS.Version > 1)
Descriptor.Flags = DescriptorYaml.getEncodedFlags();
RS.ParametersContainer.addParameter(Header, Descriptor);
RS.ParametersContainer.addParameter(Type, Visibility, Descriptor);
break;
}
case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable): {
case dxbc::RootParameterType::DescriptorTable: {
const DXContainerYAML::DescriptorTableYaml &TableYaml =
P.RootSignature->Parameters.getOrInsertTable(L);
mcdxbc::DescriptorTable Table;
Expand All @@ -320,14 +327,9 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
Range.Flags = R.getEncodedFlags();
Table.Ranges.push_back(Range);
}
RS.ParametersContainer.addParameter(Header, Table);
RS.ParametersContainer.addParameter(Type, Visibility, Table);
break;
}
default:
// Handling invalid parameter type edge case. We intentionally let
// obj2yaml/yaml2obj parse and emit invalid dxcontainer data, in order
// for that to be used as a testing tool more effectively.
RS.ParametersContainer.addInvalidParameter(Header);
}
}

Expand Down
Loading