-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[DirectX] Removing dxbc DescriptorRange from mcbxdc #154629
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 21 commits
31ec5e5
1690a9c
f6f2e61
6539364
1d29111
8eb82fd
d38c00d
3b25b34
567a3d4
fb248da
dc436d5
8353fe0
f3ecd8a
8c143ba
a4d77d7
19ec1c3
182c817
f9d16d2
42f8f11
7ada31b
fa60959
e065a82
17dbe9f
5c23b7e
1c539f0
4074ceb
36afa22
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,15 @@ struct RootDescriptor { | |
uint32_t Flags; | ||
}; | ||
|
||
struct DescriptorRange { | ||
dxbc::DescriptorRangeType RangeType; | ||
uint32_t NumDescriptors; | ||
uint32_t BaseShaderRegister; | ||
uint32_t RegisterSpace; | ||
uint32_t Flags; | ||
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. I decided to keep the flags as |
||
uint32_t OffsetInDescriptorsFromTableStart; | ||
}; | ||
|
||
struct RootParameterInfo { | ||
dxbc::RootParameterType Type; | ||
dxbc::ShaderVisibility Visibility; | ||
|
@@ -42,11 +51,11 @@ struct RootParameterInfo { | |
}; | ||
|
||
struct DescriptorTable { | ||
SmallVector<dxbc::RTS0::v2::DescriptorRange> Ranges; | ||
SmallVector<dxbc::RTS0::v2::DescriptorRange>::const_iterator begin() const { | ||
SmallVector<DescriptorRange> Ranges; | ||
SmallVector<DescriptorRange>::const_iterator begin() const { | ||
return Ranges.begin(); | ||
} | ||
SmallVector<dxbc::RTS0::v2::DescriptorRange>::const_iterator end() const { | ||
SmallVector<DescriptorRange>::const_iterator end() const { | ||
return Ranges.end(); | ||
} | ||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -149,6 +149,16 @@ ArrayRef<EnumEntry<RootParameterType>> dxbc::getRootParameterTypes() { | |
return ArrayRef(RootParameterTypes); | ||
} | ||
|
||
#define DESCRIPTOR_RANGE(Val, Enum) {#Enum, DescriptorRangeType::Enum}, | ||
|
||
static const EnumEntry<DescriptorRangeType> DescriptorRangeTypes[] = { | ||
#include "llvm/BinaryFormat/DXContainerConstants.def" | ||
}; | ||
|
||
ArrayRef<EnumEntry<DescriptorRangeType>> dxbc::getDescriptorRangeTypes() { | ||
|
||
return ArrayRef(DescriptorRangeTypes); | ||
} | ||
|
||
#define SEMANTIC_KIND(Val, Enum) {#Enum, PSV::SemanticKind::Enum}, | ||
|
||
static const EnumEntry<PSV::SemanticKind> SemanticKindNames[] = { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -322,23 +322,23 @@ Error MetadataParser::parseDescriptorRange(mcdxbc::DescriptorTable &Table, | |
if (RangeDescriptorNode->getNumOperands() != 6) | ||
return make_error<InvalidRSMetadataFormat>("Descriptor Range"); | ||
|
||
dxbc::RTS0::v2::DescriptorRange Range; | ||
mcdxbc::DescriptorRange Range; | ||
|
||
std::optional<StringRef> ElementText = | ||
extractMdStringValue(RangeDescriptorNode, 0); | ||
|
||
if (!ElementText.has_value()) | ||
return make_error<InvalidRSMetadataFormat>("Descriptor Range"); | ||
|
||
Range.RangeType = | ||
StringSwitch<uint32_t>(*ElementText) | ||
.Case("CBV", to_underlying(dxbc::DescriptorRangeType::CBV)) | ||
.Case("SRV", to_underlying(dxbc::DescriptorRangeType::SRV)) | ||
.Case("UAV", to_underlying(dxbc::DescriptorRangeType::UAV)) | ||
.Case("Sampler", to_underlying(dxbc::DescriptorRangeType::Sampler)) | ||
.Default(~0U); | ||
|
||
if (Range.RangeType == ~0U) | ||
if (*ElementText == "CBV") | ||
Range.RangeType = dxbc::DescriptorRangeType::CBV; | ||
else if (*ElementText == "SRV") | ||
Range.RangeType = dxbc::DescriptorRangeType::SRV; | ||
else if (*ElementText == "UAV") | ||
Range.RangeType = dxbc::DescriptorRangeType::UAV; | ||
else if (*ElementText == "Sampler") | ||
Range.RangeType = dxbc::DescriptorRangeType::Sampler; | ||
else | ||
return make_error<GenericRSMetadataError>("Invalid Descriptor Range type.", | ||
RangeDescriptorNode); | ||
|
||
|
@@ -568,12 +568,7 @@ Error MetadataParser::validateRootSignature( | |
case dxbc::RootParameterType::DescriptorTable: { | ||
const mcdxbc::DescriptorTable &Table = | ||
RSD.ParametersContainer.getDescriptorTable(Info.Location); | ||
for (const dxbc::RTS0::v2::DescriptorRange &Range : Table) { | ||
if (!hlsl::rootsig::verifyRangeType(Range.RangeType)) | ||
DeferredErrs = | ||
joinErrors(std::move(DeferredErrs), | ||
make_error<RootSignatureValidationError<uint32_t>>( | ||
"RangeType", Range.RangeType)); | ||
Comment on lines
-571
to
-576
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. Range type should've been verified and converted to an enum by this point, so this check don't make sense anymore. |
||
for (const mcdxbc::DescriptorRange &Range : Table) { | ||
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. nit: remove whitespace below this line |
||
|
||
if (!hlsl::rootsig::verifyRegisterSpace(Range.RegisterSpace)) | ||
DeferredErrs = | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,13 +63,11 @@ bool verifyRangeType(uint32_t Type) { | |
return false; | ||
} | ||
|
||
bool verifyDescriptorRangeFlag(uint32_t Version, uint32_t Type, | ||
uint32_t FlagsVal) { | ||
bool verifyDescriptorRangeFlag(uint32_t Version, dxbc::DescriptorRangeType Type, | ||
dxbc::DescriptorRangeFlags Flags) { | ||
using FlagT = dxbc::DescriptorRangeFlags; | ||
FlagT Flags = FlagT(FlagsVal); | ||
|
||
const bool IsSampler = | ||
(Type == llvm::to_underlying(dxbc::DescriptorRangeType::Sampler)); | ||
const bool IsSampler = (Type == dxbc::DescriptorRangeType::Sampler); | ||
|
||
if (Version == 1) { | ||
// Since the metadata is unversioned, we expect to explicitly see the values | ||
|
@@ -124,6 +122,11 @@ bool verifyDescriptorRangeFlag(uint32_t Version, uint32_t Type, | |
} | ||
return (Flags & ~Mask) == FlagT::None; | ||
} | ||
bool verifyDescriptorRangeFlag(uint32_t Version, dxbc::DescriptorRangeType Type, | ||
uint32_t Flags) { | ||
return verifyDescriptorRangeFlag(Version, Type, | ||
dxbc::DescriptorRangeFlags(Flags)); | ||
} | ||
|
||
|
||
bool verifyNumDescriptors(uint32_t NumDescriptors) { | ||
return NumDescriptors > 0; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -313,9 +313,10 @@ Error DXContainerWriter::writeParts(raw_ostream &OS) { | |
P.RootSignature->Parameters.getOrInsertTable(L); | ||
mcdxbc::DescriptorTable Table; | ||
for (const auto &R : TableYaml.Ranges) { | ||
|
||
dxbc::RTS0::v2::DescriptorRange Range; | ||
Range.RangeType = R.RangeType; | ||
assert(dxbc::isValidRangeType(R.RangeType) && | ||
"Invalid Descriptor Range Type"); | ||
Comment on lines
+316
to
+317
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. Once we use enums in the yaml represention this should be impossible to trigger. But I am making it clear here, by adding this assert. |
||
mcdxbc::DescriptorRange Range; | ||
Range.RangeType = dxbc::DescriptorRangeType(R.RangeType); | ||
Range.NumDescriptors = R.NumDescriptors; | ||
Range.BaseShaderRegister = R.BaseShaderRegister; | ||
Range.RegisterSpace = R.RegisterSpace; | ||
|
Uh oh!
There was an error while loading. Please reload this page.