Skip to content

[DirectX] adding support to read/write descriptor table data using obj2yaml/yaml2obj #138315

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.

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 70 commits into from
May 29, 2025
Merged
Show file tree
Hide file tree
Changes from 69 commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
0abacfc
adding support for Root Descriptors
Apr 24, 2025
8b8c02a
clean up
Apr 24, 2025
7ac9641
addressing comments
Apr 25, 2025
c105458
formating
Apr 26, 2025
efe76aa
try fix test
Apr 26, 2025
a928e9d
addressing comments
Apr 26, 2025
a38f10b
refactoring mcdxbc struct to store root parameters out of order
Apr 25, 2025
9a7c359
changing name
Apr 28, 2025
d6c2b55
changing variant to host pointers
Apr 28, 2025
93e4cf2
clean up
Apr 28, 2025
b45b1b6
fix
Apr 28, 2025
f804a23
fix
Apr 28, 2025
44bd13a
making read work
Apr 29, 2025
ac51bf6
adding reading logic
May 2, 2025
97fb003
test pass locally
May 2, 2025
93e04bd
adding tests
May 2, 2025
2f6d579
adding more tests
May 2, 2025
76b1b75
clean up
May 2, 2025
b2bfb02
refactoring root signature dxcontainer yaml representation
May 2, 2025
9ee2964
clean up
May 2, 2025
2527580
fix test
May 2, 2025
c3a46da
copy test
May 2, 2025
15eb6f5
fix naming
May 5, 2025
3e26364
moving root signature binary representation to RTS0 namespace
May 5, 2025
b9d7f07
fix naming
May 5, 2025
46cc8c1
addressing comments
May 8, 2025
1b3e10a
addressing comments
May 8, 2025
1f31957
addressing comments
May 8, 2025
e8fbfce
clean up
May 8, 2025
a31e5a5
removing v parameter
May 9, 2025
a394ad0
Merge branch 'obj2yaml/root-descriptors' into refactoring/remove-union
May 9, 2025
ad415a7
clean up
May 9, 2025
8ff4845
Merge branch 'main' into refactoring/remove-union
May 9, 2025
98c6a5f
updating namespace naming
May 14, 2025
d67f7d3
addressing comment
May 14, 2025
5453ad0
clean up
May 14, 2025
836a8a8
format
May 14, 2025
5bd57a6
adding comment
May 14, 2025
960cb9c
adrresing comments
May 14, 2025
a60c7a3
clean up
May 14, 2025
2a4c2cb
formatting
May 15, 2025
c29d3f2
addressing comments
May 15, 2025
1513dab
Merge branch 'refactoring/remove-union' into obj2yaml/descriptor-table
May 16, 2025
95f3e99
Merge branch 'main' into obj2yaml/descriptor-table
May 16, 2025
eb97f1b
fixing test issues
May 16, 2025
a9b87c2
format
May 16, 2025
28be2f8
format
May 20, 2025
76a2b07
clean up
May 21, 2025
b891126
clean up
May 21, 2025
2a93252
fix formating issue
May 21, 2025
d616b65
fix formating issue
May 21, 2025
6eac7c4
making NumDescriptors uint32_t
May 22, 2025
df194b0
Merge branch 'main' into refactoring/dxcontainer-yaml
May 22, 2025
0136cfc
fix testing issues
May 22, 2025
b589d10
clean up
May 22, 2025
c7042b2
clean up
May 22, 2025
70a9b7f
addressing PR Comments
May 22, 2025
e3489a4
Merge branch 'refactoring/dxcontainer-yaml' into users/joaosaffran/13…
May 22, 2025
f1dd0ce
formating
May 22, 2025
6aa895e
Merge branch 'users/joaosaffran/138318' into obj2yaml/descriptor-table
May 22, 2025
f8080c4
fix tests
May 22, 2025
aabd424
addressing comments
May 23, 2025
e655315
move to namespace
May 23, 2025
3094a75
cleanup
May 23, 2025
984baf6
movin all to RTS0
May 23, 2025
3979151
clean up
May 23, 2025
e65f850
Merge branch 'main' into obj2yaml/descriptor-table
May 28, 2025
f5bffca
fix
May 28, 2025
a585134
adding check
May 28, 2025
08c5207
fix format?
May 29, 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
35 changes: 35 additions & 0 deletions llvm/include/llvm/BinaryFormat/DXContainer.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,13 +163,25 @@ enum class RootDescriptorFlag : uint32_t {
#include "DXContainerConstants.def"
};

#define DESCRIPTOR_RANGE_FLAG(Num, Val) Val = 1ull << Num,
enum class DescriptorRangeFlag : uint32_t {
#include "DXContainerConstants.def"
};

#define ROOT_PARAMETER(Val, Enum) Enum = Val,
enum class RootParameterType : uint32_t {
#include "DXContainerConstants.def"
};

ArrayRef<EnumEntry<RootParameterType>> getRootParameterTypes();

#define DESCRIPTOR_RANGE(Val, Enum) Enum = Val,
enum class DescriptorRangeType : uint32_t {
#include "DXContainerConstants.def"
};

ArrayRef<EnumEntry<DescriptorRangeType>> getDescriptorRangeTypes();

#define ROOT_PARAMETER(Val, Enum) \
case Val: \
return true;
Expand Down Expand Up @@ -588,6 +600,21 @@ static_assert(sizeof(ProgramSignatureElement) == 32,

namespace RTS0 {
namespace v1 {
struct DescriptorRange {
uint32_t RangeType;
uint32_t NumDescriptors;
uint32_t BaseShaderRegister;
uint32_t RegisterSpace;
uint32_t OffsetInDescriptorsFromTableStart;
void swapBytes() {
sys::swapByteOrder(RangeType);
sys::swapByteOrder(NumDescriptors);
sys::swapByteOrder(BaseShaderRegister);
sys::swapByteOrder(RegisterSpace);
sys::swapByteOrder(OffsetInDescriptorsFromTableStart);
}
};

struct RootDescriptor {
uint32_t ShaderRegister;
uint32_t RegisterSpace;
Expand Down Expand Up @@ -655,6 +682,14 @@ struct RootDescriptor : public v1::RootDescriptor {
sys::swapByteOrder(Flags);
}
};

struct DescriptorRange : public v1::DescriptorRange {
uint32_t Flags;
void swapBytes() {
v1::DescriptorRange::swapBytes();
sys::swapByteOrder(Flags);
}
};
} // namespace v2
} // namespace RTS0

Expand Down
13 changes: 13 additions & 0 deletions llvm/include/llvm/BinaryFormat/DXContainerConstants.def
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,21 @@ ROOT_DESCRIPTOR_FLAG(3, DATA_STATIC)
#endif // ROOT_DESCRIPTOR_FLAG


// DESCRIPTOR_RANGE_FLAG(bit offset for the flag, name).
#ifdef DESCRIPTOR_RANGE_FLAG

DESCRIPTOR_RANGE_FLAG(0, NONE)
DESCRIPTOR_RANGE_FLAG(1, DESCRIPTORS_VOLATILE)
DESCRIPTOR_RANGE_FLAG(2, DATA_VOLATILE)
DESCRIPTOR_RANGE_FLAG(3, DATA_STATIC_WHILE_SET_AT_EXECUTE)
DESCRIPTOR_RANGE_FLAG(4, DATA_STATIC)
DESCRIPTOR_RANGE_FLAG(16, DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS)
Comment on lines +92 to +97
Copy link
Contributor

Choose a reason for hiding this comment

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

These values are wrong - when we turn these into an enum in DXContainer.h, we do Val = 1ull << Num, so we get NONE equal to 1 and DESCRIPTORS_VOLATILE equal to 2, even though DESCRIPTORS_VOLATILE should be 1 AFAICT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, will fix that

Copy link
Contributor

Choose a reason for hiding this comment

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

As noted here: #143041 (review).

Maybe reusing the values will address this as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix PR: #143201

#undef DESCRIPTOR_RANGE_FLAG
#endif // DESCRIPTOR_RANGE_FLAG

#ifdef ROOT_PARAMETER

ROOT_PARAMETER(0, DescriptorTable)
ROOT_PARAMETER(1, Constants32Bit)
ROOT_PARAMETER(2, CBV)
ROOT_PARAMETER(3, SRV)
Expand Down
21 changes: 21 additions & 0 deletions llvm/include/llvm/MC/DXContainerRootSignature.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,22 @@ struct RootParameterInfo {
: Header(Header), Location(Location) {}
};

struct DescriptorTable {
SmallVector<dxbc::RTS0::v2::DescriptorRange> Ranges;
SmallVector<dxbc::RTS0::v2::DescriptorRange>::const_iterator begin() const {
return Ranges.begin();
}
SmallVector<dxbc::RTS0::v2::DescriptorRange>::const_iterator end() const {
return Ranges.end();
}
};

struct RootParametersContainer {
SmallVector<RootParameterInfo> ParametersInfo;

SmallVector<dxbc::RTS0::v1::RootConstants> Constants;
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));
Expand All @@ -51,6 +62,12 @@ struct RootParametersContainer {
Descriptors.push_back(Descriptor);
}

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

const std::pair<uint32_t, uint32_t>
getTypeAndLocForParameter(uint32_t Location) const {
const RootParameterInfo &Info = ParametersInfo[Location];
Expand All @@ -70,6 +87,10 @@ struct RootParametersContainer {
return Descriptors[Index];
}

const DescriptorTable &getDescriptorTable(size_t Index) const {
return Tables[Index];
}

size_t size() const { return ParametersInfo.size(); }

SmallVector<RootParameterInfo>::const_iterator begin() const {
Expand Down
68 changes: 65 additions & 3 deletions llvm/include/llvm/Object/DXContainer.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "llvm/ADT/Twine.h"
#include "llvm/BinaryFormat/DXContainer.h"
#include "llvm/Object/Error.h"
#include "llvm/Support/Endian.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/MemoryBufferRef.h"
#include "llvm/TargetParser/Triple.h"
Expand Down Expand Up @@ -177,6 +178,50 @@ struct RootDescriptorView : RootParameterView {
}
};

struct DescriptorTable {
uint32_t NumRanges;
uint32_t RangesOffset;
ViewArray<dxbc::RTS0::v2::DescriptorRange> Ranges;

typename ViewArray<dxbc::RTS0::v2::DescriptorRange>::iterator begin() const {
return Ranges.begin();
}

typename ViewArray<dxbc::RTS0::v2::DescriptorRange>::iterator end() const {
return Ranges.end();
}
};

struct DescriptorTableView : RootParameterView {
static bool classof(const RootParameterView *V) {
return (V->Header.ParameterType ==
llvm::to_underlying(dxbc::RootParameterType::DescriptorTable));
}

// Define a type alias to access the template parameter from inside classof
llvm::Expected<DescriptorTable> read(uint32_t Version) {
const char *Current = ParamData.begin();
DescriptorTable Table;

Table.NumRanges =
support::endian::read<uint32_t, llvm::endianness::little>(Current);
Current += sizeof(uint32_t);

Table.RangesOffset =
support::endian::read<uint32_t, llvm::endianness::little>(Current);
Current += sizeof(uint32_t);

size_t RangeSize = sizeof(dxbc::RTS0::v1::DescriptorRange);
if (Version > 1)
RangeSize = sizeof(dxbc::RTS0::v2::DescriptorRange);

Table.Ranges.Stride = RangeSize;
Table.Ranges.Data =
ParamData.substr(2 * sizeof(uint32_t), Table.NumRanges * RangeSize);
return Table;
}
};

static Error parseFailed(const Twine &Msg) {
return make_error<GenericBinaryError>(Msg.str(), object_error::parse_failed);
}
Expand Down Expand Up @@ -213,6 +258,9 @@ class RootSignature {
llvm::Expected<RootParameterView>
getParameter(const dxbc::RTS0::v1::RootParameterHeader &Header) const {
size_t DataSize;
size_t EndOfSectionByte = getNumStaticSamplers() == 0
? PartData.size()
: getStaticSamplersOffset();

if (!dxbc::isValidParameterType(Header.ParameterType))
return parseFailed("invalid parameter type");
Expand All @@ -229,10 +277,24 @@ class RootSignature {
else
DataSize = sizeof(dxbc::RTS0::v2::RootDescriptor);
break;
case dxbc::RootParameterType::DescriptorTable:
if (Header.ParameterOffset + sizeof(uint32_t) > EndOfSectionByte)
return parseFailed("Reading structure out of file bounds");

uint32_t NumRanges =
support::endian::read<uint32_t, llvm::endianness::little>(
PartData.begin() + Header.ParameterOffset);
Comment on lines +284 to +286
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a bit unfortunate that we need to read the data here in order to parse it. Do we do anything elsewhere to make sure that corrupted data doesn't bite us here? That is, does this do something reasonable for an absurdly large incorrect size or does it fall over?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't handle this case while reading the root signature part, not sure if there is validation process before reading the parts.

What will happen is: in case of absurdly larger numbers, invalid data would be read, since this would overflow the section containing descriptor ranges, and it would likely crash due to access out of bounds sections.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The DXContainer reader really needs to be hardened against crashing since this code may be used in drivers and/or runtime libraries.

We should at least validate that the offset is within the part's range and error appropriately.

if (Version == 1)
DataSize = sizeof(dxbc::RTS0::v1::DescriptorRange) * NumRanges;
else
DataSize = sizeof(dxbc::RTS0::v2::DescriptorRange) * NumRanges;

// 4 bytes for the number of ranges in table and
// 4 bytes for the ranges offset
DataSize += 2 * sizeof(uint32_t);
break;
}
size_t EndOfSectionByte = getNumStaticSamplers() == 0
? PartData.size()
: getStaticSamplersOffset();


if (Header.ParameterOffset + DataSize > EndOfSectionByte)
return parseFailed("Reading structure out of file bounds");
Expand Down
34 changes: 34 additions & 0 deletions llvm/include/llvm/ObjectYAML/DXContainerYAML.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,25 @@ struct RootDescriptorYaml {
#include "llvm/BinaryFormat/DXContainerConstants.def"
};

struct DescriptorRangeYaml {
uint32_t RangeType;
uint32_t NumDescriptors;
uint32_t BaseShaderRegister;
uint32_t RegisterSpace;
uint32_t OffsetInDescriptorsFromTableStart;

uint32_t getEncodedFlags() const;

#define DESCRIPTOR_RANGE_FLAG(Num, Val) bool Val = false;
#include "llvm/BinaryFormat/DXContainerConstants.def"
};

struct DescriptorTableYaml {
uint32_t NumRanges;
uint32_t RangesOffset;
SmallVector<DescriptorRangeYaml> Ranges;
};

struct RootParameterHeaderYaml {
uint32_t Type;
uint32_t Visibility;
Expand All @@ -113,6 +132,7 @@ struct RootParameterYamlDesc {

SmallVector<RootConstantsYaml> Constants;
SmallVector<RootDescriptorYaml> Descriptors;
SmallVector<DescriptorTableYaml> Tables;

template <typename T>
T &getOrInsertImpl(RootParameterLocationYaml &ParamDesc,
Expand All @@ -134,6 +154,11 @@ struct RootParameterYamlDesc {
return getOrInsertImpl(ParamDesc, Descriptors);
}

DescriptorTableYaml &
getOrInsertTable(RootParameterLocationYaml &ParamDesc) {
return getOrInsertImpl(ParamDesc, Tables);
}

void insertLocation(RootParameterLocationYaml &Location) {
Locations.push_back(Location);
}
Expand Down Expand Up @@ -263,6 +288,7 @@ LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DXContainerYAML::SignatureElement)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DXContainerYAML::PSVInfo::MaskVector)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DXContainerYAML::SignatureParameter)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DXContainerYAML::RootParameterLocationYaml)
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DXContainerYAML::DescriptorRangeYaml)
LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::PSV::SemanticKind)
LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::PSV::ComponentType)
LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::PSV::InterpolationMode)
Expand Down Expand Up @@ -351,6 +377,14 @@ template <> struct MappingTraits<llvm::DXContainerYAML::RootDescriptorYaml> {
static void mapping(IO &IO, llvm::DXContainerYAML::RootDescriptorYaml &D);
};

template <> struct MappingTraits<llvm::DXContainerYAML::DescriptorTableYaml> {
static void mapping(IO &IO, llvm::DXContainerYAML::DescriptorTableYaml &D);
};

template <> struct MappingTraits<llvm::DXContainerYAML::DescriptorRangeYaml> {
static void mapping(IO &IO, llvm::DXContainerYAML::DescriptorRangeYaml &D);
};

} // namespace yaml

} // namespace llvm
Expand Down
33 changes: 33 additions & 0 deletions llvm/lib/MC/DXContainerRootSignature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,18 @@ size_t RootSignatureDesc::getSize() const {
Size += sizeof(dxbc::RTS0::v2::RootDescriptor);

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

// 4 bytes for the number of ranges in table and
// 4 bytes for the ranges offset
Size += 2 * sizeof(uint32_t);
if (Version == 1)
Size += sizeof(dxbc::RTS0::v1::DescriptorRange) * Table.Ranges.size();
else
Size += sizeof(dxbc::RTS0::v2::DescriptorRange) * Table.Ranges.size();
break;
}
}
return Size;
Expand Down Expand Up @@ -106,6 +118,27 @@ void RootSignatureDesc::write(raw_ostream &OS) const {
support::endian::write(BOS, Descriptor.Flags, llvm::endianness::little);
break;
}
case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable): {
const DescriptorTable &Table =
ParametersContainer.getDescriptorTable(Loc);
support::endian::write(BOS, (uint32_t)Table.Ranges.size(),
llvm::endianness::little);
rewriteOffsetToCurrentByte(BOS, writePlaceholder(BOS));
for (const auto &Range : Table) {
support::endian::write(BOS, Range.RangeType, llvm::endianness::little);
support::endian::write(BOS, Range.NumDescriptors,
llvm::endianness::little);
support::endian::write(BOS, Range.BaseShaderRegister,
llvm::endianness::little);
support::endian::write(BOS, Range.RegisterSpace,
llvm::endianness::little);
support::endian::write(BOS, Range.OffsetInDescriptorsFromTableStart,
llvm::endianness::little);
if (Version > 1)
support::endian::write(BOS, Range.Flags, llvm::endianness::little);
}
break;
}
}
}
assert(Storage.size() == getSize());
Expand Down
20 changes: 20 additions & 0 deletions llvm/lib/ObjectYAML/DXContainerEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,26 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
RS.ParametersContainer.addParameter(Header, Descriptor);
break;
}
case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable): {
const DXContainerYAML::DescriptorTableYaml &TableYaml =
P.RootSignature->Parameters.getOrInsertTable(L);
mcdxbc::DescriptorTable Table;
for (const auto &R : TableYaml.Ranges) {

dxbc::RTS0::v2::DescriptorRange Range;
Range.RangeType = R.RangeType;
Range.NumDescriptors = R.NumDescriptors;
Range.BaseShaderRegister = R.BaseShaderRegister;
Range.RegisterSpace = R.RegisterSpace;
Range.OffsetInDescriptorsFromTableStart =
R.OffsetInDescriptorsFromTableStart;
if (RS.Version > 1)
Range.Flags = R.getEncodedFlags();
Table.Ranges.push_back(Range);
}
RS.ParametersContainer.addParameter(Header, Table);
break;
}
default:
// Handling invalid parameter type edge case. We intentionally let
// obj2yaml/yaml2obj parse and emit invalid dxcontainer data, in order
Expand Down
Loading
Loading