-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[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
Changes from 1 commit
0abacfc
8b8c02a
7ac9641
c105458
efe76aa
a928e9d
a38f10b
9a7c359
d6c2b55
93e4cf2
b45b1b6
f804a23
44bd13a
ac51bf6
97fb003
93e04bd
2f6d579
76b1b75
b2bfb02
9ee2964
2527580
c3a46da
15eb6f5
3e26364
b9d7f07
46cc8c1
1b3e10a
1f31957
e8fbfce
a31e5a5
a394ad0
ad415a7
8ff4845
98c6a5f
d67f7d3
5453ad0
836a8a8
5bd57a6
960cb9c
a60c7a3
2a4c2cb
c29d3f2
1513dab
95f3e99
eb97f1b
a9b87c2
28be2f8
76a2b07
b891126
2a93252
d616b65
6eac7c4
df194b0
0136cfc
b589d10
c7042b2
70a9b7f
e3489a4
f1dd0ce
6aa895e
f8080c4
aabd424
e655315
3094a75
984baf6
3979151
e65f850
f5bffca
a585134
08c5207
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,7 +73,7 @@ ROOT_ELEMENT_FLAG(11, SamplerHeapDirectlyIndexed) | |
#endif // ROOT_ELEMENT_FLAG | ||
|
||
|
||
// ROOT_ELEMENT_FLAG(bit offset for the flag, name). | ||
// ROOT_DESCRIPTOR_FLAG(bit offset for the flag, name). | ||
#ifdef ROOT_DESCRIPTOR_FLAG | ||
|
||
ROOT_DESCRIPTOR_FLAG(0, NONE) | ||
|
@@ -84,15 +84,38 @@ 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True, will fix that There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
ROOT_PARAMETER(4, UAV) | ||
#undef ROOT_PARAMETER | ||
#endif // ROOT_PARAMETER | ||
|
||
|
||
#ifdef DESCRIPTOR_RANGE | ||
|
||
DESCRIPTOR_RANGE(0, SRV) | ||
DESCRIPTOR_RANGE(1, UAV) | ||
DESCRIPTOR_RANGE(2, CBV) | ||
DESCRIPTOR_RANGE(3, Sampler) | ||
#undef DESCRIPTOR_RANGE | ||
#endif // DESCRIPTOR_RANGE | ||
|
||
#ifdef SHADER_VISIBILITY | ||
|
||
SHADER_VISIBILITY(0, All) | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -7,6 +7,7 @@ | |||
//===----------------------------------------------------------------------===// | ||||
|
||||
#include "llvm/ADT/STLForwardCompat.h" | ||||
#include "llvm/ADT/SmallVector.h" | ||||
#include "llvm/BinaryFormat/DXContainer.h" | ||||
#include <cstddef> | ||||
#include <cstdint> | ||||
|
@@ -28,17 +29,30 @@ struct RootParameterInfo { | |||
RootParameterInfo(dxbc::RootParameterHeader H, size_t L) | ||||
: Header(H), Location(L) {} | ||||
}; | ||||
using DescriptorRanges = std::variant<dxbc::RST0::v0::DescriptorRange, | ||||
dxbc::RST0::v1::DescriptorRange>; | ||||
struct DescriptorTable { | ||||
SmallVector<DescriptorRanges> Ranges; | ||||
|
||||
SmallVector<DescriptorRanges>::const_iterator begin() const { | ||||
return Ranges.begin(); | ||||
} | ||||
SmallVector<DescriptorRanges>::const_iterator end() const { | ||||
return Ranges.end(); | ||||
} | ||||
}; | ||||
|
||||
using RootDescriptor = std::variant<dxbc::RST0::v0::RootDescriptor, | ||||
dxbc::RST0::v1::RootDescriptor>; | ||||
using ParametersView = std::variant<const dxbc::RootConstants *, | ||||
const dxbc::RST0::v0::RootDescriptor *, | ||||
const dxbc::RST0::v1::RootDescriptor *>; | ||||
|
||||
using ParametersView = std::variant< | ||||
const dxbc::RootConstants *, const dxbc::RST0::v0::RootDescriptor *, | ||||
const dxbc::RST0::v1::RootDescriptor *, const DescriptorTable *>; | ||||
struct RootParametersContainer { | ||||
SmallVector<RootParameterInfo> ParametersInfo; | ||||
|
||||
SmallVector<dxbc::RootConstants> Constants; | ||||
SmallVector<RootDescriptor> Descriptors; | ||||
SmallVector<DescriptorTable> Tables; | ||||
|
||||
void addInfo(dxbc::RootParameterHeader H, size_t L) { | ||||
ParametersInfo.push_back(RootParameterInfo(H, L)); | ||||
|
@@ -61,20 +75,28 @@ struct RootParametersContainer { | |||
Descriptors.push_back(D); | ||||
} | ||||
|
||||
void addParameter(dxbc::RootParameterHeader H, DescriptorTable D) { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
addInfo(H, Tables.size()); | ||||
Tables.push_back(D); | ||||
} | ||||
|
||||
std::optional<ParametersView> getParameter(const RootParameterInfo *H) const { | ||||
switch (H->Header.ParameterType) { | ||||
case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): | ||||
return &Constants[H->Location]; | ||||
case llvm::to_underlying(dxbc::RootParameterType::CBV): | ||||
case llvm::to_underlying(dxbc::RootParameterType::SRV): | ||||
case llvm::to_underlying(dxbc::RootParameterType::UAV): | ||||
case llvm::to_underlying(dxbc::RootParameterType::UAV): { | ||||
const RootDescriptor &VersionedParam = Descriptors[H->Location]; | ||||
if (std::holds_alternative<dxbc::RST0::v0::RootDescriptor>( | ||||
VersionedParam)) { | ||||
return &std::get<dxbc::RST0::v0::RootDescriptor>(VersionedParam); | ||||
} | ||||
return &std::get<dxbc::RST0::v1::RootDescriptor>(VersionedParam); | ||||
} | ||||
case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable): | ||||
return &Tables[H->Location]; | ||||
} | ||||
|
||||
return std::nullopt; | ||||
} | ||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have expected this to also be
uint32_t
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OffsetInDescriptorsFromTableStart
, supports -1 to represent unboundedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are other negative numbers (that is, unsigned values between 2147483648 and 4294967294) impossible / invalid, or do we need to treat them as large numbers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just checked the spec, I made a mistake
NumDescriptors
allows -1 to describe unbounded UINT_MAX .According to the spec: "Use -1 or UINT_MAX to specify an unbounded size.", my understanding is that only -1 is a valid here.
Since folks are annoyed with this I will make it
uint32_t
.