[clang] [llvm] [NFC][HLSL] Remove confusing enum aliases / duplicates (PR #153909)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 15 17:53:02 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Damyan Pepper (damyanp)
<details>
<summary>Changes</summary>
Remove:
* DescriptorType enum - this almost exactly shadowed the ResourceClass enum
* ClauseType aliased ResourceClass
Although these were introduced to make the HLSL
root signature handling code a bit cleaner, they
were ultimately causing confusion as they appeared to be unique enums that needed to be converted
between.
Closes #<!-- -->153890
---
Full diff: https://github.com/llvm/llvm-project/pull/153909.diff
5 Files Affected:
- (modified) clang/lib/Parse/ParseHLSLRootSignature.cpp (+7-7)
- (modified) clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp (+23-23)
- (modified) llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h (+13-12)
- (modified) llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp (+4-5)
- (modified) llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp (+2-5)
``````````diff
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 98dc458f7adc5..5490c61f52356 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -234,15 +234,15 @@ std::optional<RootDescriptor> RootSignatureParser::parseRootDescriptor() {
default:
llvm_unreachable("Switch for consumed token was not provided");
case TokenKind::kw_CBV:
- Descriptor.Type = DescriptorType::CBuffer;
+ Descriptor.Type = ResourceClass::CBuffer;
ExpectedReg = TokenKind::bReg;
break;
case TokenKind::kw_SRV:
- Descriptor.Type = DescriptorType::SRV;
+ Descriptor.Type = ResourceClass::SRV;
ExpectedReg = TokenKind::tReg;
break;
case TokenKind::kw_UAV:
- Descriptor.Type = DescriptorType::UAV;
+ Descriptor.Type = ResourceClass::UAV;
ExpectedReg = TokenKind::uReg;
break;
}
@@ -360,19 +360,19 @@ RootSignatureParser::parseDescriptorTableClause() {
default:
llvm_unreachable("Switch for consumed token was not provided");
case TokenKind::kw_CBV:
- Clause.Type = ClauseType::CBuffer;
+ Clause.Type = ResourceClass::CBuffer;
ExpectedReg = TokenKind::bReg;
break;
case TokenKind::kw_SRV:
- Clause.Type = ClauseType::SRV;
+ Clause.Type = ResourceClass::SRV;
ExpectedReg = TokenKind::tReg;
break;
case TokenKind::kw_UAV:
- Clause.Type = ClauseType::UAV;
+ Clause.Type = ResourceClass::UAV;
ExpectedReg = TokenKind::uReg;
break;
case TokenKind::kw_Sampler:
- Clause.Type = ClauseType::Sampler;
+ Clause.Type = ResourceClass::Sampler;
ExpectedReg = TokenKind::sReg;
break;
}
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 44f6b0469f38e..44c0978a243bc 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -180,7 +180,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
// First Descriptor Table with 4 elements
RootElement Elem = Elements[0].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::CBuffer);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType,
RegisterType::BReg);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.Number, 0u);
@@ -193,7 +193,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
Elem = Elements[1].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::SRV);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::SRV);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType,
RegisterType::TReg);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.Number, 42u);
@@ -205,7 +205,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
Elem = Elements[2].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::Sampler);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType,
RegisterType::SReg);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.Number, 987u);
@@ -218,7 +218,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
Elem = Elements[3].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::UAV);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::UAV);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType,
RegisterType::UReg);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.Number, 4294967294u);
@@ -445,7 +445,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidSamplerFlagsTest) {
auto Elements = Parser.getElements();
RootElement Elem = Elements[0].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::Sampler);
auto ValidSamplerFlags =
llvm::dxbc::DescriptorRangeFlags::DescriptorsVolatile;
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidSamplerFlags);
@@ -591,7 +591,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
RootElement Elem = Elements[0].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::BReg);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 0u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 0u);
@@ -602,7 +602,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
Elem = Elements[1].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::SRV);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::SRV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::TReg);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 42u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 4u);
@@ -616,7 +616,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
Elem = Elements[2].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::UAV);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::UAV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::UReg);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 34893247u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 0u);
@@ -628,7 +628,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
RootDescriptorFlags::DataVolatile);
Elem = Elements[3].getElement();
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::BReg);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 0u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 0u);
@@ -696,17 +696,17 @@ TEST_F(ParseHLSLRootSignatureTest, ValidVersion10Test) {
auto DefRootDescriptorFlag = llvm::dxbc::RootDescriptorFlags::DataVolatile;
RootElement Elem = Elements[0].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, DefRootDescriptorFlag);
Elem = Elements[1].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::SRV);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::SRV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, DefRootDescriptorFlag);
Elem = Elements[2].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::UAV);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::UAV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, DefRootDescriptorFlag);
auto ValidNonSamplerFlags =
@@ -714,22 +714,22 @@ TEST_F(ParseHLSLRootSignatureTest, ValidVersion10Test) {
llvm::dxbc::DescriptorRangeFlags::DataVolatile;
Elem = Elements[3].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::CBuffer);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidNonSamplerFlags);
Elem = Elements[4].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::SRV);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::SRV);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidNonSamplerFlags);
Elem = Elements[5].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::UAV);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::UAV);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidNonSamplerFlags);
Elem = Elements[6].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::Sampler);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
llvm::dxbc::DescriptorRangeFlags::DescriptorsVolatile);
@@ -767,43 +767,43 @@ TEST_F(ParseHLSLRootSignatureTest, ValidVersion11Test) {
auto Elements = Parser.getElements();
RootElement Elem = Elements[0].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags,
llvm::dxbc::RootDescriptorFlags::DataStaticWhileSetAtExecute);
Elem = Elements[1].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::SRV);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::SRV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags,
llvm::dxbc::RootDescriptorFlags::DataStaticWhileSetAtExecute);
Elem = Elements[2].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::UAV);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::UAV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags,
llvm::dxbc::RootDescriptorFlags::DataVolatile);
Elem = Elements[3].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::CBuffer);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
llvm::dxbc::DescriptorRangeFlags::DataStaticWhileSetAtExecute);
Elem = Elements[4].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::SRV);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::SRV);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
llvm::dxbc::DescriptorRangeFlags::DataStaticWhileSetAtExecute);
Elem = Elements[5].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::UAV);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::UAV);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
llvm::dxbc::DescriptorRangeFlags::DataVolatile);
Elem = Elements[6].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::Sampler);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
llvm::dxbc::DescriptorRangeFlags::None);
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index e44612af071bc..87777fddc9157 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -42,10 +42,9 @@ struct RootConstants {
dxbc::ShaderVisibility Visibility = dxbc::ShaderVisibility::All;
};
-enum class DescriptorType : uint8_t { SRV = 0, UAV, CBuffer };
// Models RootDescriptor : CBV | SRV | UAV, by collecting like parameters
struct RootDescriptor {
- DescriptorType Type;
+ dxil::ResourceClass Type;
Register Reg;
uint32_t Space = 0;
dxbc::ShaderVisibility Visibility = dxbc::ShaderVisibility::All;
@@ -60,13 +59,16 @@ struct RootDescriptor {
assert(Version == llvm::dxbc::RootSignatureVersion::V1_1 &&
"Specified an invalid root signature version");
switch (Type) {
- case DescriptorType::CBuffer:
- case DescriptorType::SRV:
+ case dxil::ResourceClass::CBuffer:
+ case dxil::ResourceClass::SRV:
Flags = dxbc::RootDescriptorFlags::DataStaticWhileSetAtExecute;
break;
- case DescriptorType::UAV:
+ case dxil::ResourceClass::UAV:
Flags = dxbc::RootDescriptorFlags::DataVolatile;
break;
+ case dxil::ResourceClass::Sampler:
+ llvm_unreachable(
+ "ResourceClass::Sampler is not valid for RootDescriptors");
}
}
};
@@ -82,9 +84,8 @@ struct DescriptorTable {
static const uint32_t NumDescriptorsUnbounded = 0xffffffff;
static const uint32_t DescriptorTableOffsetAppend = 0xffffffff;
// Models DTClause : CBV | SRV | UAV | Sampler, by collecting like parameters
-using ClauseType = llvm::dxil::ResourceClass;
struct DescriptorTableClause {
- ClauseType Type;
+ dxil::ResourceClass Type;
Register Reg;
uint32_t NumDescriptors = 1;
uint32_t Space = 0;
@@ -94,7 +95,7 @@ struct DescriptorTableClause {
void setDefaultFlags(dxbc::RootSignatureVersion Version) {
if (Version == dxbc::RootSignatureVersion::V1_0) {
Flags = dxbc::DescriptorRangeFlags::DescriptorsVolatile;
- if (Type != ClauseType::Sampler)
+ if (Type != dxil::ResourceClass::Sampler)
Flags |= dxbc::DescriptorRangeFlags::DataVolatile;
return;
}
@@ -102,14 +103,14 @@ struct DescriptorTableClause {
assert(Version == dxbc::RootSignatureVersion::V1_1 &&
"Specified an invalid root signature version");
switch (Type) {
- case ClauseType::CBuffer:
- case ClauseType::SRV:
+ case dxil::ResourceClass::CBuffer:
+ case dxil::ResourceClass::SRV:
Flags = dxbc::DescriptorRangeFlags::DataStaticWhileSetAtExecute;
break;
- case ClauseType::UAV:
+ case dxil::ResourceClass::UAV:
Flags = dxbc::DescriptorRangeFlags::DataVolatile;
break;
- case ClauseType::Sampler:
+ case dxil::ResourceClass::Sampler:
Flags = dxbc::DescriptorRangeFlags::None;
break;
}
diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
index 050cc46e8c9b0..14c729b1ad97b 100644
--- a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
+++ b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
@@ -92,9 +92,9 @@ static raw_ostream &operator<<(raw_ostream &OS,
return OS;
}
-static raw_ostream &operator<<(raw_ostream &OS, const ClauseType &Type) {
- OS << enumToStringRef(dxil::ResourceClass(llvm::to_underlying(Type)),
- dxil::getResourceClasses());
+static raw_ostream &operator<<(raw_ostream &OS,
+ const dxil::ResourceClass &Type) {
+ OS << enumToStringRef(Type, dxil::getResourceClasses());
return OS;
}
@@ -153,8 +153,7 @@ raw_ostream &operator<<(raw_ostream &OS, const DescriptorTableClause &Clause) {
}
raw_ostream &operator<<(raw_ostream &OS, const RootDescriptor &Descriptor) {
- ClauseType Type = ClauseType(llvm::to_underlying(Descriptor.Type));
- OS << "Root" << Type << "(" << Descriptor.Reg
+ OS << "Root" << Descriptor.Type << "(" << Descriptor.Reg
<< ", space = " << Descriptor.Space
<< ", visibility = " << Descriptor.Visibility
<< ", flags = " << Descriptor.Flags << ")";
diff --git a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp
index 157bfc665b207..ae58d80e12d5f 100644
--- a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp
+++ b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp
@@ -120,8 +120,7 @@ MDNode *MetadataBuilder::BuildRootConstants(const RootConstants &Constants) {
MDNode *MetadataBuilder::BuildRootDescriptor(const RootDescriptor &Descriptor) {
IRBuilder<> Builder(Ctx);
StringRef ResName =
- enumToStringRef(dxil::ResourceClass(to_underlying(Descriptor.Type)),
- dxil::getResourceClasses());
+ enumToStringRef(Descriptor.Type, dxil::getResourceClasses());
assert(!ResName.empty() && "Provided an invalid Resource Class");
SmallString<7> Name({"Root", ResName});
Metadata *Operands[] = {
@@ -161,9 +160,7 @@ MDNode *MetadataBuilder::BuildDescriptorTable(const DescriptorTable &Table) {
MDNode *MetadataBuilder::BuildDescriptorTableClause(
const DescriptorTableClause &Clause) {
IRBuilder<> Builder(Ctx);
- StringRef ResName =
- enumToStringRef(dxil::ResourceClass(to_underlying(Clause.Type)),
- dxil::getResourceClasses());
+ StringRef ResName = enumToStringRef(Clause.Type, dxil::getResourceClasses());
assert(!ResName.empty() && "Provided an invalid Resource Class");
Metadata *Operands[] = {
MDString::get(Ctx, ResName),
``````````
</details>
https://github.com/llvm/llvm-project/pull/153909
More information about the cfe-commits
mailing list