Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 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
f3ecd8a
removing dependency of Object
joaosaffran Aug 20, 2025
8c143ba
removing binary format descriptor range dependency
joaosaffran Aug 20, 2025
a4d77d7
Revert "removing binary format descriptor range dependency"
joaosaffran Aug 20, 2025
19ec1c3
Merge branch 'main' into refactoring/updating-descriptor-range
joaosaffran Aug 29, 2025
182c817
removing binary format descriptor range dependency
joaosaffran Aug 20, 2025
f9d16d2
creating toDescriptorRange and change verifyDescriptorRangeFlag signa…
joaosaffran Aug 29, 2025
42f8f11
adding test and removing string switch
joaosaffran Aug 29, 2025
7ada31b
removing copy
joaosaffran Aug 29, 2025
fa60959
clean up
joaosaffran Aug 30, 2025
e065a82
removing function I thought I needed
joaosaffran Sep 6, 2025
17dbe9f
refactoring to use dxil::ResourceClass for range type
joaosaffran Sep 8, 2025
5c23b7e
adding assert
joaosaffran Sep 8, 2025
1c539f0
changing casting to use proper llvm-casting
joaosaffran Sep 8, 2025
4074ceb
removing unused code
joaosaffran Sep 9, 2025
36afa22
removing assert and adding lastEntry
joaosaffran Sep 10, 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
5 changes: 2 additions & 3 deletions clang/lib/Sema/SemaHLSL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1283,9 +1283,8 @@ bool SemaHLSL::handleRootSignatureElements(
ReportError(Loc, 1, 0xfffffffe);
}

if (!llvm::hlsl::rootsig::verifyDescriptorRangeFlag(
Version, llvm::to_underlying(Clause->Type),
llvm::to_underlying(Clause->Flags)))
if (!llvm::hlsl::rootsig::verifyDescriptorRangeFlag(Version, Clause->Type,
Clause->Flags))
ReportFlagError(Loc);
}
}
Expand Down
11 changes: 4 additions & 7 deletions llvm/include/llvm/BinaryFormat/DXContainer.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,6 @@ enum class RootParameterType : uint32_t {

LLVM_ABI ArrayRef<EnumEntry<RootParameterType>> getRootParameterTypes();

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

LLVM_ABI ArrayRef<EnumEntry<DescriptorRangeType>> getDescriptorRangeTypes();

#define ROOT_PARAMETER(Val, Enum) \
case Val: \
return true;
Expand All @@ -209,6 +202,10 @@ inline bool isValidParameterType(uint32_t V) {
return false;
}

inline bool isValidRangeType(uint32_t V) {
return V <= llvm::to_underlying(dxil::ResourceClass::LastEntry);
}

#define SHADER_VISIBILITY(Val, Enum) Enum = Val,
enum class ShaderVisibility : uint32_t {
#include "DXContainerConstants.def"
Expand Down
10 changes: 0 additions & 10 deletions llvm/include/llvm/BinaryFormat/DXContainerConstants.def
Original file line number Diff line number Diff line change
Expand Up @@ -104,16 +104,6 @@ DESCRIPTOR_RANGE_FLAG(0x10000, DescriptorsStaticKeepingBufferBoundsChecks, DESCR
#undef DESCRIPTOR_RANGE_FLAG
#endif // DESCRIPTOR_RANGE_FLAG

// DESCRIPTOR_RANGE(value, name).
#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

Comment on lines -107 to -116
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those are no longer needed, they were replaced by dxil::ResourceClass

#ifdef ROOT_PARAMETER

ROOT_PARAMETER(0, DescriptorTable)
Expand Down
5 changes: 3 additions & 2 deletions llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ LLVM_ABI bool verifyRegisterValue(uint32_t RegisterValue);
LLVM_ABI bool verifyRegisterSpace(uint32_t RegisterSpace);
LLVM_ABI bool verifyRootDescriptorFlag(uint32_t Version, uint32_t FlagsVal);
LLVM_ABI bool verifyRangeType(uint32_t Type);
LLVM_ABI bool verifyDescriptorRangeFlag(uint32_t Version, uint32_t Type,
uint32_t FlagsVal);
LLVM_ABI bool verifyDescriptorRangeFlag(uint32_t Version,
dxil::ResourceClass Type,
dxbc::DescriptorRangeFlags FlagsVal);
LLVM_ABI bool verifyNumDescriptors(uint32_t NumDescriptors);
LLVM_ABI bool verifySamplerFilter(uint32_t Value);
LLVM_ABI bool verifyAddress(uint32_t Address);
Expand Down
15 changes: 12 additions & 3 deletions llvm/include/llvm/MC/DXContainerRootSignature.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@ struct RootDescriptor {
uint32_t Flags;
};

struct DescriptorRange {
dxil::ResourceClass RangeType;
uint32_t NumDescriptors;
uint32_t BaseShaderRegister;
uint32_t RegisterSpace;
uint32_t Flags;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to keep the flags as uint32_t, since the flags are not mutually exclusive.

uint32_t OffsetInDescriptorsFromTableStart;
};

struct RootParameterInfo {
dxbc::RootParameterType Type;
dxbc::ShaderVisibility Visibility;
Expand All @@ -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();
}
};
Expand Down
1 change: 1 addition & 0 deletions llvm/include/llvm/Support/DXILABI.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ enum class ResourceClass : uint8_t {
UAV,
CBuffer,
Sampler,
LastEntry = Sampler,
};

/// The kind of resource for an SRV or UAV resource. Sometimes referred to as
Expand Down
31 changes: 13 additions & 18 deletions llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 = dxil::ResourceClass::CBuffer;
else if (*ElementText == "SRV")
Range.RangeType = dxil::ResourceClass::SRV;
else if (*ElementText == "UAV")
Range.RangeType = dxil::ResourceClass::UAV;
else if (*ElementText == "Sampler")
Range.RangeType = dxil::ResourceClass::Sampler;
else
return make_error<GenericRSMetadataError>("Invalid Descriptor Range type.",
RangeDescriptorNode);

Expand Down Expand Up @@ -568,13 +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
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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) {
Copy link
Contributor

Choose a reason for hiding this comment

The 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 =
joinErrors(std::move(DeferredErrs),
Expand All @@ -588,7 +582,8 @@ Error MetadataParser::validateRootSignature(
"NumDescriptors", Range.NumDescriptors));

if (!hlsl::rootsig::verifyDescriptorRangeFlag(
RSD.Version, Range.RangeType, Range.Flags))
RSD.Version, Range.RangeType,
dxbc::DescriptorRangeFlags(Range.Flags)))
DeferredErrs =
joinErrors(std::move(DeferredErrs),
make_error<RootSignatureValidationError<uint32_t>>(
Expand Down
20 changes: 3 additions & 17 deletions llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,25 +51,11 @@ bool verifyRootDescriptorFlag(uint32_t Version, uint32_t FlagsVal) {
return (Flags | DataFlags) == DataFlags;
}

bool verifyRangeType(uint32_t Type) {
switch (Type) {
case llvm::to_underlying(dxbc::DescriptorRangeType::CBV):
case llvm::to_underlying(dxbc::DescriptorRangeType::SRV):
case llvm::to_underlying(dxbc::DescriptorRangeType::UAV):
case llvm::to_underlying(dxbc::DescriptorRangeType::Sampler):
return true;
};

return false;
}
Comment on lines -54 to -64
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This check now can be done by the function isValidRangeType, and should be called whenever reading root signature binary data.


bool verifyDescriptorRangeFlag(uint32_t Version, uint32_t Type,
uint32_t FlagsVal) {
bool verifyDescriptorRangeFlag(uint32_t Version, dxil::ResourceClass 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 == dxil::ResourceClass::Sampler);

if (Version == 1) {
// Since the metadata is unversioned, we expect to explicitly see the values
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/MC/DXContainerRootSignature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,8 @@ void RootSignatureDesc::write(raw_ostream &OS) const {
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, static_cast<uint32_t>(Range.RangeType),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

dxil::ResourceClasses are uint8_t by default, so changing its type or casting is required. I opted to cast, since that seems to be the less disruptive option.

llvm::endianness::little);
support::endian::write(BOS, Range.NumDescriptors,
llvm::endianness::little);
support::endian::write(BOS, Range.BaseShaderRegister,
Expand Down
7 changes: 4 additions & 3 deletions llvm/lib/ObjectYAML/DXContainerEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 = dxil::ResourceClass(R.RangeType);
Range.NumDescriptors = R.NumDescriptors;
Range.BaseShaderRegister = R.BaseShaderRegister;
Range.RegisterSpace = R.RegisterSpace;
Expand Down
24 changes: 3 additions & 21 deletions llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,6 @@
using namespace llvm;
using namespace llvm::dxil;

static ResourceClass toResourceClass(dxbc::DescriptorRangeType RangeType) {
using namespace dxbc;
switch (RangeType) {
case DescriptorRangeType::SRV:
return ResourceClass::SRV;
case DescriptorRangeType::UAV:
return ResourceClass::UAV;
case DescriptorRangeType::CBV:
return ResourceClass::CBuffer;
case DescriptorRangeType::Sampler:
return ResourceClass::Sampler;
}
llvm_unreachable("Unknown DescriptorRangeType");
}

static ResourceClass toResourceClass(dxbc::RootParameterType Type) {
using namespace dxbc;
switch (Type) {
Expand Down Expand Up @@ -205,16 +190,13 @@ static void validateRootSignature(Module &M,
const mcdxbc::DescriptorTable &Table =
RSD.ParametersContainer.getDescriptorTable(ParamInfo.Location);

for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) {
for (const mcdxbc::DescriptorRange &Range : Table.Ranges) {
uint32_t UpperBound =
Range.NumDescriptors == ~0U
? Range.BaseShaderRegister
: Range.BaseShaderRegister + Range.NumDescriptors - 1;
Builder.trackBinding(
toResourceClass(
static_cast<dxbc::DescriptorRangeType>(Range.RangeType)),
Range.RegisterSpace, Range.BaseShaderRegister, UpperBound,
&ParamInfo);
Builder.trackBinding(Range.RangeType, Range.RegisterSpace,
Range.BaseShaderRegister, UpperBound, &ParamInfo);
}
break;
}
Expand Down
5 changes: 3 additions & 2 deletions llvm/lib/Target/DirectX/DXILRootSignature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,9 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M,
RS.ParametersContainer.getDescriptorTable(Info.Location);
OS << " NumRanges: " << Table.Ranges.size() << "\n";

for (const dxbc::RTS0::v2::DescriptorRange Range : Table) {
OS << " - Range Type: " << Range.RangeType << "\n"
for (const mcdxbc::DescriptorRange &Range : Table) {
OS << " - Range Type: "
<< dxil::getResourceClassName(Range.RangeType) << "\n"
<< " Register Space: " << Range.RegisterSpace << "\n"
<< " Base Shader Register: " << Range.BaseShaderRegister << "\n"
<< " Num Descriptors: " << Range.NumDescriptors << "\n"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
;CHECK-NEXT: - Parameter Type: DescriptorTable
;CHECK-NEXT: Shader Visibility: All
;CHECK-NEXT: NumRanges: 2
;CHECK-NEXT: - Range Type: 0
;CHECK-NEXT: - Range Type: SRV
;CHECK-NEXT: Register Space: 0
;CHECK-NEXT: Base Shader Register: 1
;CHECK-NEXT: Num Descriptors: 1
;CHECK-NEXT: Offset In Descriptors From Table Start: 4294967295
;CHECK-NEXT: Flags: 4
;CHECK-NEXT: - Range Type: 1
;CHECK-NEXT: - Range Type: UAV
;CHECK-NEXT: Register Space: 10
;CHECK-NEXT: Base Shader Register: 1
;CHECK-NEXT: Num Descriptors: 5
Expand Down