Skip to content

Commit cc49f3b

Browse files
damyanpdamyanp
authored
[NFC][HLSL] Remove confusing enum aliases / duplicates (llvm#153909)
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 each other. Closes llvm#153890
1 parent 3ecfc03 commit cc49f3b

File tree

6 files changed

+59
-58
lines changed

6 files changed

+59
-58
lines changed

‎clang/lib/Parse/ParseHLSLRootSignature.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -234,15 +234,15 @@ std::optional<RootDescriptor> RootSignatureParser::parseRootDescriptor() {
234234
default:
235235
llvm_unreachable("Switch for consumed token was not provided");
236236
case TokenKind::kw_CBV:
237-
Descriptor.Type = DescriptorType::CBuffer;
237+
Descriptor.Type = ResourceClass::CBuffer;
238238
ExpectedReg = TokenKind::bReg;
239239
break;
240240
case TokenKind::kw_SRV:
241-
Descriptor.Type = DescriptorType::SRV;
241+
Descriptor.Type = ResourceClass::SRV;
242242
ExpectedReg = TokenKind::tReg;
243243
break;
244244
case TokenKind::kw_UAV:
245-
Descriptor.Type = DescriptorType::UAV;
245+
Descriptor.Type = ResourceClass::UAV;
246246
ExpectedReg = TokenKind::uReg;
247247
break;
248248
}
@@ -360,19 +360,19 @@ RootSignatureParser::parseDescriptorTableClause() {
360360
default:
361361
llvm_unreachable("Switch for consumed token was not provided");
362362
case TokenKind::kw_CBV:
363-
Clause.Type = ClauseType::CBuffer;
363+
Clause.Type = ResourceClass::CBuffer;
364364
ExpectedReg = TokenKind::bReg;
365365
break;
366366
case TokenKind::kw_SRV:
367-
Clause.Type = ClauseType::SRV;
367+
Clause.Type = ResourceClass::SRV;
368368
ExpectedReg = TokenKind::tReg;
369369
break;
370370
case TokenKind::kw_UAV:
371-
Clause.Type = ClauseType::UAV;
371+
Clause.Type = ResourceClass::UAV;
372372
ExpectedReg = TokenKind::uReg;
373373
break;
374374
case TokenKind::kw_Sampler:
375-
Clause.Type = ClauseType::Sampler;
375+
Clause.Type = ResourceClass::Sampler;
376376
ExpectedReg = TokenKind::sReg;
377377
break;
378378
}

‎clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp‎

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
180180
// First Descriptor Table with 4 elements
181181
RootElement Elem = Elements[0].getElement();
182182
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
183-
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::CBuffer);
183+
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::CBuffer);
184184
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType,
185185
RegisterType::BReg);
186186
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.Number, 0u);
@@ -193,7 +193,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
193193

194194
Elem = Elements[1].getElement();
195195
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
196-
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::SRV);
196+
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::SRV);
197197
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType,
198198
RegisterType::TReg);
199199
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.Number, 42u);
@@ -205,7 +205,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
205205

206206
Elem = Elements[2].getElement();
207207
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
208-
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler);
208+
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::Sampler);
209209
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType,
210210
RegisterType::SReg);
211211
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.Number, 987u);
@@ -218,7 +218,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
218218

219219
Elem = Elements[3].getElement();
220220
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
221-
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::UAV);
221+
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::UAV);
222222
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType,
223223
RegisterType::UReg);
224224
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.Number, 4294967294u);
@@ -445,7 +445,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidSamplerFlagsTest) {
445445
auto Elements = Parser.getElements();
446446
RootElement Elem = Elements[0].getElement();
447447
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
448-
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler);
448+
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::Sampler);
449449
auto ValidSamplerFlags =
450450
llvm::dxbc::DescriptorRangeFlags::DescriptorsVolatile;
451451
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidSamplerFlags);
@@ -591,7 +591,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
591591

592592
RootElement Elem = Elements[0].getElement();
593593
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
594-
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
594+
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::CBuffer);
595595
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::BReg);
596596
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 0u);
597597
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 0u);
@@ -602,7 +602,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
602602

603603
Elem = Elements[1].getElement();
604604
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
605-
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::SRV);
605+
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::SRV);
606606
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::TReg);
607607
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 42u);
608608
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 4u);
@@ -616,7 +616,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
616616

617617
Elem = Elements[2].getElement();
618618
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
619-
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::UAV);
619+
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::UAV);
620620
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::UReg);
621621
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 34893247u);
622622
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 0u);
@@ -628,7 +628,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
628628
RootDescriptorFlags::DataVolatile);
629629

630630
Elem = Elements[3].getElement();
631-
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
631+
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::CBuffer);
632632
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::BReg);
633633
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 0u);
634634
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 0u);
@@ -696,40 +696,40 @@ TEST_F(ParseHLSLRootSignatureTest, ValidVersion10Test) {
696696
auto DefRootDescriptorFlag = llvm::dxbc::RootDescriptorFlags::DataVolatile;
697697
RootElement Elem = Elements[0].getElement();
698698
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
699-
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
699+
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::CBuffer);
700700
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, DefRootDescriptorFlag);
701701

702702
Elem = Elements[1].getElement();
703703
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
704-
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::SRV);
704+
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::SRV);
705705
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, DefRootDescriptorFlag);
706706

707707
Elem = Elements[2].getElement();
708708
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
709-
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::UAV);
709+
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::UAV);
710710
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, DefRootDescriptorFlag);
711711

712712
auto ValidNonSamplerFlags =
713713
llvm::dxbc::DescriptorRangeFlags::DescriptorsVolatile |
714714
llvm::dxbc::DescriptorRangeFlags::DataVolatile;
715715
Elem = Elements[3].getElement();
716716
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
717-
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::CBuffer);
717+
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::CBuffer);
718718
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidNonSamplerFlags);
719719

720720
Elem = Elements[4].getElement();
721721
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
722-
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::SRV);
722+
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::SRV);
723723
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidNonSamplerFlags);
724724

725725
Elem = Elements[5].getElement();
726726
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
727-
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::UAV);
727+
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::UAV);
728728
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidNonSamplerFlags);
729729

730730
Elem = Elements[6].getElement();
731731
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
732-
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler);
732+
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::Sampler);
733733
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
734734
llvm::dxbc::DescriptorRangeFlags::DescriptorsVolatile);
735735

@@ -767,43 +767,43 @@ TEST_F(ParseHLSLRootSignatureTest, ValidVersion11Test) {
767767
auto Elements = Parser.getElements();
768768
RootElement Elem = Elements[0].getElement();
769769
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
770-
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
770+
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::CBuffer);
771771
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags,
772772
llvm::dxbc::RootDescriptorFlags::DataStaticWhileSetAtExecute);
773773

774774
Elem = Elements[1].getElement();
775775
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
776-
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::SRV);
776+
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::SRV);
777777
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags,
778778
llvm::dxbc::RootDescriptorFlags::DataStaticWhileSetAtExecute);
779779

780780
Elem = Elements[2].getElement();
781781
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
782-
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::UAV);
782+
ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::UAV);
783783
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags,
784784
llvm::dxbc::RootDescriptorFlags::DataVolatile);
785785

786786
Elem = Elements[3].getElement();
787787
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
788-
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::CBuffer);
788+
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::CBuffer);
789789
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
790790
llvm::dxbc::DescriptorRangeFlags::DataStaticWhileSetAtExecute);
791791

792792
Elem = Elements[4].getElement();
793793
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
794-
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::SRV);
794+
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::SRV);
795795
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
796796
llvm::dxbc::DescriptorRangeFlags::DataStaticWhileSetAtExecute);
797797

798798
Elem = Elements[5].getElement();
799799
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
800-
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::UAV);
800+
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::UAV);
801801
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
802802
llvm::dxbc::DescriptorRangeFlags::DataVolatile);
803803

804804
Elem = Elements[6].getElement();
805805
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
806-
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler);
806+
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::Sampler);
807807
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
808808
llvm::dxbc::DescriptorRangeFlags::None);
809809

‎llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,9 @@ struct RootConstants {
4242
dxbc::ShaderVisibility Visibility = dxbc::ShaderVisibility::All;
4343
};
4444

45-
enum class DescriptorType : uint8_t { SRV = 0, UAV, CBuffer };
4645
// Models RootDescriptor : CBV | SRV | UAV, by collecting like parameters
4746
struct RootDescriptor {
48-
DescriptorType Type;
47+
dxil::ResourceClass Type;
4948
Register Reg;
5049
uint32_t Space = 0;
5150
dxbc::ShaderVisibility Visibility = dxbc::ShaderVisibility::All;
@@ -60,13 +59,16 @@ struct RootDescriptor {
6059
assert(Version == llvm::dxbc::RootSignatureVersion::V1_1 &&
6160
"Specified an invalid root signature version");
6261
switch (Type) {
63-
case DescriptorType::CBuffer:
64-
case DescriptorType::SRV:
62+
case dxil::ResourceClass::CBuffer:
63+
case dxil::ResourceClass::SRV:
6564
Flags = dxbc::RootDescriptorFlags::DataStaticWhileSetAtExecute;
6665
break;
67-
case DescriptorType::UAV:
66+
case dxil::ResourceClass::UAV:
6867
Flags = dxbc::RootDescriptorFlags::DataVolatile;
6968
break;
69+
case dxil::ResourceClass::Sampler:
70+
llvm_unreachable(
71+
"ResourceClass::Sampler is not valid for RootDescriptors");
7072
}
7173
}
7274
};
@@ -82,9 +84,8 @@ struct DescriptorTable {
8284
static const uint32_t NumDescriptorsUnbounded = 0xffffffff;
8385
static const uint32_t DescriptorTableOffsetAppend = 0xffffffff;
8486
// Models DTClause : CBV | SRV | UAV | Sampler, by collecting like parameters
85-
using ClauseType = llvm::dxil::ResourceClass;
8687
struct DescriptorTableClause {
87-
ClauseType Type;
88+
dxil::ResourceClass Type;
8889
Register Reg;
8990
uint32_t NumDescriptors = 1;
9091
uint32_t Space = 0;
@@ -94,22 +95,22 @@ struct DescriptorTableClause {
9495
void setDefaultFlags(dxbc::RootSignatureVersion Version) {
9596
if (Version == dxbc::RootSignatureVersion::V1_0) {
9697
Flags = dxbc::DescriptorRangeFlags::DescriptorsVolatile;
97-
if (Type != ClauseType::Sampler)
98+
if (Type != dxil::ResourceClass::Sampler)
9899
Flags |= dxbc::DescriptorRangeFlags::DataVolatile;
99100
return;
100101
}
101102

102103
assert(Version == dxbc::RootSignatureVersion::V1_1 &&
103104
"Specified an invalid root signature version");
104105
switch (Type) {
105-
case ClauseType::CBuffer:
106-
case ClauseType::SRV:
106+
case dxil::ResourceClass::CBuffer:
107+
case dxil::ResourceClass::SRV:
107108
Flags = dxbc::DescriptorRangeFlags::DataStaticWhileSetAtExecute;
108109
break;
109-
case ClauseType::UAV:
110+
case dxil::ResourceClass::UAV:
110111
Flags = dxbc::DescriptorRangeFlags::DataVolatile;
111112
break;
112-
case ClauseType::Sampler:
113+
case dxil::ResourceClass::Sampler:
113114
Flags = dxbc::DescriptorRangeFlags::None;
114115
break;
115116
}

‎llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,8 @@ static raw_ostream &operator<<(raw_ostream &OS,
9393
return OS;
9494
}
9595

96-
static raw_ostream &operator<<(raw_ostream &OS, const ClauseType &Type) {
96+
static raw_ostream &operator<<(raw_ostream &OS,
97+
const dxil::ResourceClass &Type) {
9798
OS << dxil::getResourceClassName(Type);
9899
return OS;
99100
}
@@ -152,8 +153,7 @@ raw_ostream &operator<<(raw_ostream &OS, const DescriptorTableClause &Clause) {
152153
}
153154

154155
raw_ostream &operator<<(raw_ostream &OS, const RootDescriptor &Descriptor) {
155-
ClauseType Type = ClauseType(llvm::to_underlying(Descriptor.Type));
156-
OS << "Root" << Type << "(" << Descriptor.Reg
156+
OS << "Root" << Descriptor.Type << "(" << Descriptor.Reg
157157
<< ", space = " << Descriptor.Space
158158
<< ", visibility = " << Descriptor.Visibility
159159
<< ", flags = " << Descriptor.Flags << ")";

‎llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,7 @@ MDNode *MetadataBuilder::BuildRootConstants(const RootConstants &Constants) {
120120

121121
MDNode *MetadataBuilder::BuildRootDescriptor(const RootDescriptor &Descriptor) {
122122
IRBuilder<> Builder(Ctx);
123-
StringRef ResName =
124-
dxil::getResourceClassName(dxil::ResourceClass(Descriptor.Type));
123+
StringRef ResName = dxil::getResourceClassName(Descriptor.Type);
125124
assert(!ResName.empty() && "Provided an invalid Resource Class");
126125
SmallString<7> Name({"Root", ResName});
127126
Metadata *Operands[] = {

0 commit comments

Comments
 (0)