Skip to content

Commit 770d0b0

Browse files
joaosaffranjoaosaffran
joaosaffran
and
joaosaffran
authored
[DirectX] Fix order of v2::DescriptorRange (#145555)
As pointed in #145438, the order of elements in `v2::DescriptorRange` is wrong according to the root signature file format. This changes the order and updates the code and test to continue to pass. Closes: #145438 --------- Co-authored-by: joaosaffran <[email protected]>
1 parent fe8a262 commit 770d0b0

File tree

6 files changed

+78
-58
lines changed

6 files changed

+78
-58
lines changed

‎llvm/include/llvm/BinaryFormat/DXContainer.h

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -740,10 +740,19 @@ struct RootDescriptor : public v1::RootDescriptor {
740740
}
741741
};
742742

743-
struct DescriptorRange : public v1::DescriptorRange {
743+
struct DescriptorRange {
744+
uint32_t RangeType;
745+
uint32_t NumDescriptors;
746+
uint32_t BaseShaderRegister;
747+
uint32_t RegisterSpace;
744748
uint32_t Flags;
749+
uint32_t OffsetInDescriptorsFromTableStart;
745750
void swapBytes() {
746-
v1::DescriptorRange::swapBytes();
751+
sys::swapByteOrder(RangeType);
752+
sys::swapByteOrder(NumDescriptors);
753+
sys::swapByteOrder(BaseShaderRegister);
754+
sys::swapByteOrder(RegisterSpace);
755+
sys::swapByteOrder(OffsetInDescriptorsFromTableStart);
747756
sys::swapByteOrder(Flags);
748757
}
749758
};

‎llvm/include/llvm/Object/DXContainer.h

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -178,19 +178,14 @@ struct RootDescriptorView : RootParameterView {
178178
return readParameter<dxbc::RTS0::v2::RootDescriptor>();
179179
}
180180
};
181-
182-
struct DescriptorTable {
181+
template <typename T> struct DescriptorTable {
183182
uint32_t NumRanges;
184183
uint32_t RangesOffset;
185-
ViewArray<dxbc::RTS0::v2::DescriptorRange> Ranges;
184+
ViewArray<T> Ranges;
186185

187-
typename ViewArray<dxbc::RTS0::v2::DescriptorRange>::iterator begin() const {
188-
return Ranges.begin();
189-
}
186+
typename ViewArray<T>::iterator begin() const { return Ranges.begin(); }
190187

191-
typename ViewArray<dxbc::RTS0::v2::DescriptorRange>::iterator end() const {
192-
return Ranges.end();
193-
}
188+
typename ViewArray<T>::iterator end() const { return Ranges.end(); }
194189
};
195190

196191
struct DescriptorTableView : RootParameterView {
@@ -200,9 +195,9 @@ struct DescriptorTableView : RootParameterView {
200195
}
201196

202197
// Define a type alias to access the template parameter from inside classof
203-
llvm::Expected<DescriptorTable> read(uint32_t Version) {
198+
template <typename T> llvm::Expected<DescriptorTable<T>> read() {
204199
const char *Current = ParamData.begin();
205-
DescriptorTable Table;
200+
DescriptorTable<T> Table;
206201

207202
Table.NumRanges =
208203
support::endian::read<uint32_t, llvm::endianness::little>(Current);
@@ -212,13 +207,8 @@ struct DescriptorTableView : RootParameterView {
212207
support::endian::read<uint32_t, llvm::endianness::little>(Current);
213208
Current += sizeof(uint32_t);
214209

215-
size_t RangeSize = sizeof(dxbc::RTS0::v1::DescriptorRange);
216-
if (Version > 1)
217-
RangeSize = sizeof(dxbc::RTS0::v2::DescriptorRange);
218-
219-
Table.Ranges.Stride = RangeSize;
220-
Table.Ranges.Data =
221-
ParamData.substr(2 * sizeof(uint32_t), Table.NumRanges * RangeSize);
210+
Table.Ranges.Data = ParamData.substr(2 * sizeof(uint32_t),
211+
Table.NumRanges * Table.Ranges.Stride);
222212
return Table;
223213
}
224214
};

‎llvm/lib/MC/DXContainerRootSignature.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,10 @@ void RootSignatureDesc::write(raw_ostream &OS) const {
133133
llvm::endianness::little);
134134
support::endian::write(BOS, Range.RegisterSpace,
135135
llvm::endianness::little);
136-
support::endian::write(BOS, Range.OffsetInDescriptorsFromTableStart,
137-
llvm::endianness::little);
138136
if (Version > 1)
139137
support::endian::write(BOS, Range.Flags, llvm::endianness::little);
138+
support::endian::write(BOS, Range.OffsetInDescriptorsFromTableStart,
139+
llvm::endianness::little);
140140
}
141141
break;
142142
}

‎llvm/lib/ObjectYAML/DXContainerYAML.cpp

Lines changed: 51 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,47 @@ DXContainerYAML::ShaderFeatureFlags::ShaderFeatureFlags(uint64_t FlagData) {
3333
#include "llvm/BinaryFormat/DXContainerConstants.def"
3434
}
3535

36+
template <typename T>
37+
static llvm::Error
38+
readDescriptorRanges(DXContainerYAML::RootParameterHeaderYaml &Header,
39+
DXContainerYAML::RootSignatureYamlDesc &RootSigDesc,
40+
object::DirectX::DescriptorTableView *DTV) {
41+
42+
llvm::Expected<object::DirectX::DescriptorTable<T>> TableOrErr =
43+
DTV->read<T>();
44+
if (Error E = TableOrErr.takeError())
45+
return E;
46+
auto Table = *TableOrErr;
47+
48+
DXContainerYAML::RootParameterLocationYaml Location(Header);
49+
DXContainerYAML::DescriptorTableYaml &TableYaml =
50+
RootSigDesc.Parameters.getOrInsertTable(Location);
51+
RootSigDesc.Parameters.insertLocation(Location);
52+
53+
TableYaml.NumRanges = Table.NumRanges;
54+
TableYaml.RangesOffset = Table.RangesOffset;
55+
56+
for (const auto &R : Table.Ranges) {
57+
DXContainerYAML::DescriptorRangeYaml NewR;
58+
NewR.OffsetInDescriptorsFromTableStart =
59+
R.OffsetInDescriptorsFromTableStart;
60+
NewR.NumDescriptors = R.NumDescriptors;
61+
NewR.BaseShaderRegister = R.BaseShaderRegister;
62+
NewR.RegisterSpace = R.RegisterSpace;
63+
NewR.RangeType = R.RangeType;
64+
if constexpr (std::is_same_v<T, dxbc::RTS0::v2::DescriptorRange>) {
65+
// Set all flag fields for v2
66+
#define DESCRIPTOR_RANGE_FLAG(Num, Val) \
67+
NewR.Val = \
68+
(R.Flags & llvm::to_underlying(dxbc::DescriptorRangeFlag::Val)) != 0;
69+
#include "llvm/BinaryFormat/DXContainerConstants.def"
70+
}
71+
TableYaml.Ranges.push_back(NewR);
72+
}
73+
74+
return Error::success();
75+
}
76+
3677
llvm::Expected<DXContainerYAML::RootSignatureYamlDesc>
3778
DXContainerYAML::RootSignatureYamlDesc::create(
3879
const object::DirectX::RootSignature &Data) {
@@ -107,36 +148,16 @@ DXContainerYAML::RootSignatureYamlDesc::create(
107148
}
108149
} else if (auto *DTV =
109150
dyn_cast<object::DirectX::DescriptorTableView>(&ParamView)) {
110-
llvm::Expected<object::DirectX::DescriptorTable> TableOrErr =
111-
DTV->read(Version);
112-
if (Error E = TableOrErr.takeError())
113-
return std::move(E);
114-
auto Table = *TableOrErr;
115-
RootParameterLocationYaml Location(Header);
116-
DescriptorTableYaml &TableYaml =
117-
RootSigDesc.Parameters.getOrInsertTable(Location);
118-
RootSigDesc.Parameters.insertLocation(Location);
119-
120-
TableYaml.NumRanges = Table.NumRanges;
121-
TableYaml.RangesOffset = Table.RangesOffset;
122-
123-
for (const auto &R : Table) {
124-
DescriptorRangeYaml NewR;
125-
126-
NewR.OffsetInDescriptorsFromTableStart =
127-
R.OffsetInDescriptorsFromTableStart;
128-
NewR.NumDescriptors = R.NumDescriptors;
129-
NewR.BaseShaderRegister = R.BaseShaderRegister;
130-
NewR.RegisterSpace = R.RegisterSpace;
131-
NewR.RangeType = R.RangeType;
132-
if (Version > 1) {
133-
#define DESCRIPTOR_RANGE_FLAG(Num, Val) \
134-
NewR.Val = \
135-
(R.Flags & llvm::to_underlying(dxbc::DescriptorRangeFlag::Val)) > 0;
136-
#include "llvm/BinaryFormat/DXContainerConstants.def"
137-
}
138-
TableYaml.Ranges.push_back(NewR);
139-
}
151+
if (Version == 1) {
152+
if (Error E = readDescriptorRanges<dxbc::RTS0::v1::DescriptorRange>(
153+
Header, RootSigDesc, DTV))
154+
return std::move(E);
155+
} else if (Version == 2) {
156+
if (Error E = readDescriptorRanges<dxbc::RTS0::v2::DescriptorRange>(
157+
Header, RootSigDesc, DTV))
158+
return std::move(E);
159+
} else
160+
llvm_unreachable("Unknown version for DescriptorRanges");
140161
}
141162
}
142163

‎llvm/unittests/Object/DXContainerTest.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1062,8 +1062,8 @@ TEST(RootSignature, ParseDescriptorTable) {
10621062
0x3c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
10631063
0x03, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
10641064
0x2c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff,
1065-
0x2a, 0x00, 0x00, 0x00, 0x2b, 0x00, 0x00, 0x00, 0x29, 0x00, 0x00, 0x00,
1066-
0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
1065+
0x2a, 0x00, 0x00, 0x00, 0x2b, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00,
1066+
0x29, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
10671067
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
10681068
0x00};
10691069
DXContainer C =
@@ -1088,7 +1088,7 @@ TEST(RootSignature, ParseDescriptorTable) {
10881088
auto *DescriptorTableView =
10891089
dyn_cast<DirectX::DescriptorTableView>(&*ParamView);
10901090
ASSERT_TRUE(DescriptorTableView != nullptr);
1091-
auto Table = DescriptorTableView->read(2);
1091+
auto Table = DescriptorTableView->read<dxbc::RTS0::v2::DescriptorRange>();
10921092

10931093
ASSERT_THAT_ERROR(Table.takeError(), Succeeded());
10941094

@@ -1140,7 +1140,7 @@ TEST(RootSignature, ParseDescriptorTable) {
11401140
auto *DescriptorTableView =
11411141
dyn_cast<DirectX::DescriptorTableView>(&*ParamView);
11421142
ASSERT_TRUE(DescriptorTableView != nullptr);
1143-
auto Table = DescriptorTableView->read(1);
1143+
auto Table = DescriptorTableView->read<dxbc::RTS0::v1::DescriptorRange>();
11441144

11451145
ASSERT_THAT_ERROR(Table.takeError(), Succeeded());
11461146

‎llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -459,8 +459,8 @@ TEST(RootSignature, ParseDescriptorTableV11) {
459459
0x3c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
460460
0x03, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
461461
0x2c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff,
462-
0x2a, 0x00, 0x00, 0x00, 0x2b, 0x00, 0x00, 0x00, 0x29, 0x00, 0x00, 0x00,
463-
0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
462+
0x2a, 0x00, 0x00, 0x00, 0x2b, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00,
463+
0x29, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
464464
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
465465
0x00};
466466

0 commit comments

Comments
 (0)