Skip to content

Commit 64de852

Browse files
inbelicFinn Plummer
andauthored
[HLSL][RootSignature] Implement initial parsing of the descriptor table clause params (#133800)
- Defines `ParseDescriptorTableClauseParams` to establish the pattern of how we will parse parameters in root signatures. Namely, to use recursive descent parsing in a way that follows closely to the EBNF notation definition in the root signature spec. - Implements parsing of two param types: `UInt32` and `Register` to demonstrate the parsing implementation and allow for unit testing - Changes the calling convention to use `std::optional` return values instead of boolean error returns and parameters by reference Part two of implementing: #126569 --------- Co-authored-by: Finn Plummer <[email protected]>
1 parent a3f38f2 commit 64de852

File tree

5 files changed

+336
-30
lines changed

5 files changed

+336
-30
lines changed

clang/include/clang/Basic/DiagnosticParseKinds.td

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1836,8 +1836,11 @@ def err_hlsl_virtual_function
18361836
def err_hlsl_virtual_inheritance
18371837
: Error<"virtual inheritance is unsupported in HLSL">;
18381838

1839-
// HLSL Root Siganture diagnostic messages
1839+
// HLSL Root Signature Parser Diagnostics
18401840
def err_hlsl_unexpected_end_of_params
18411841
: Error<"expected %0 to denote end of parameters, or, another valid parameter of %1">;
1842+
def err_hlsl_rootsig_repeat_param : Error<"specified the same parameter '%0' multiple times">;
1843+
def err_hlsl_rootsig_missing_param : Error<"did not specify mandatory parameter '%0'">;
1844+
def err_hlsl_number_literal_overflow : Error<"integer literal is too large to be represented as a 32-bit %select{signed |}0 integer type">;
18421845

18431846
} // end of Parser diagnostics

clang/include/clang/Parse/ParseHLSLRootSignature.h

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,26 +40,31 @@ class RootSignatureParser {
4040
private:
4141
DiagnosticsEngine &getDiags() { return PP.getDiagnostics(); }
4242

43-
// All private Parse.* methods follow a similar pattern:
43+
// All private parse.* methods follow a similar pattern:
4444
// - Each method will start with an assert to denote what the CurToken is
4545
// expected to be and will parse from that token forward
4646
//
4747
// - Therefore, it is the callers responsibility to ensure that you are
4848
// at the correct CurToken. This should be done with the pattern of:
4949
//
50-
// if (TryConsumeExpectedToken(RootSignatureToken::Kind))
51-
// if (Parse.*())
52-
// return true;
50+
// if (tryConsumeExpectedToken(RootSignatureToken::Kind)) {
51+
// auto ParsedObject = parse.*();
52+
// if (!ParsedObject.has_value())
53+
// return std::nullopt;
54+
// ...
55+
// }
5356
//
5457
// or,
5558
//
56-
// if (ConsumeExpectedToken(RootSignatureToken::Kind, ...))
57-
// return true;
58-
// if (Parse.*())
59-
// return true;
59+
// if (consumeExpectedToken(RootSignatureToken::Kind, ...))
60+
// return std::nullopt;
61+
// auto ParsedObject = parse.*();
62+
// if (!ParsedObject.has_value())
63+
// return std::nullopt;
64+
// ...
6065
//
61-
// - All methods return true if a parsing error is encountered. It is the
62-
// callers responsibility to propogate this error up, or deal with it
66+
// - All methods return std::nullopt if a parsing error is encountered. It
67+
// is the callers responsibility to propogate this error up, or deal with it
6368
// otherwise
6469
//
6570
// - An error will be raised if the proceeding tokens are not what is
@@ -69,6 +74,23 @@ class RootSignatureParser {
6974
bool parseDescriptorTable();
7075
bool parseDescriptorTableClause();
7176

77+
/// Parameter arguments (eg. `bReg`, `space`, ...) can be specified in any
78+
/// order and only exactly once. `ParsedClauseParams` denotes the current
79+
/// state of parsed params
80+
struct ParsedClauseParams {
81+
std::optional<llvm::hlsl::rootsig::Register> Register;
82+
std::optional<uint32_t> Space;
83+
};
84+
std::optional<ParsedClauseParams>
85+
parseDescriptorTableClauseParams(RootSignatureToken::Kind RegType);
86+
87+
std::optional<uint32_t> parseUIntParam();
88+
std::optional<llvm::hlsl::rootsig::Register> parseRegister();
89+
90+
/// Use NumericLiteralParser to convert CurToken.NumSpelling into a unsigned
91+
/// 32-bit integer
92+
std::optional<uint32_t> handleUIntLiteral();
93+
7294
/// Invoke the Lexer to consume a token and update CurToken with the result
7395
void consumeNextToken() { CurToken = Lexer.consumeToken(); }
7496

clang/lib/Parse/ParseHLSLRootSignature.cpp

Lines changed: 149 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
#include "clang/Parse/ParseHLSLRootSignature.h"
1010

11+
#include "clang/Lex/LiteralSupport.h"
12+
1113
#include "llvm/Support/raw_ostream.h"
1214

1315
using namespace llvm::hlsl::rootsig;
@@ -41,12 +43,11 @@ bool RootSignatureParser::parse() {
4143
break;
4244
}
4345

44-
if (!tryConsumeExpectedToken(TokenKind::end_of_stream)) {
45-
getDiags().Report(CurToken.TokLoc, diag::err_hlsl_unexpected_end_of_params)
46-
<< /*expected=*/TokenKind::end_of_stream
47-
<< /*param of=*/TokenKind::kw_RootSignature;
46+
if (consumeExpectedToken(TokenKind::end_of_stream,
47+
diag::err_hlsl_unexpected_end_of_params,
48+
/*param of=*/TokenKind::kw_RootSignature))
4849
return true;
49-
}
50+
5051
return false;
5152
}
5253

@@ -72,12 +73,10 @@ bool RootSignatureParser::parseDescriptorTable() {
7273
break;
7374
}
7475

75-
if (!tryConsumeExpectedToken(TokenKind::pu_r_paren)) {
76-
getDiags().Report(CurToken.TokLoc, diag::err_hlsl_unexpected_end_of_params)
77-
<< /*expected=*/TokenKind::pu_r_paren
78-
<< /*param of=*/TokenKind::kw_DescriptorTable;
76+
if (consumeExpectedToken(TokenKind::pu_r_paren,
77+
diag::err_hlsl_unexpected_end_of_params,
78+
/*param of=*/TokenKind::kw_DescriptorTable))
7979
return true;
80-
}
8180

8281
Elements.push_back(Table);
8382
return false;
@@ -90,36 +89,170 @@ bool RootSignatureParser::parseDescriptorTableClause() {
9089
CurToken.TokKind == TokenKind::kw_Sampler) &&
9190
"Expects to only be invoked starting at given keyword");
9291

92+
TokenKind ParamKind = CurToken.TokKind;
93+
94+
if (consumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
95+
CurToken.TokKind))
96+
return true;
97+
9398
DescriptorTableClause Clause;
94-
switch (CurToken.TokKind) {
99+
TokenKind ExpectedReg;
100+
switch (ParamKind) {
95101
default:
96102
llvm_unreachable("Switch for consumed token was not provided");
97103
case TokenKind::kw_CBV:
98104
Clause.Type = ClauseType::CBuffer;
105+
ExpectedReg = TokenKind::bReg;
99106
break;
100107
case TokenKind::kw_SRV:
101108
Clause.Type = ClauseType::SRV;
109+
ExpectedReg = TokenKind::tReg;
102110
break;
103111
case TokenKind::kw_UAV:
104112
Clause.Type = ClauseType::UAV;
113+
ExpectedReg = TokenKind::uReg;
105114
break;
106115
case TokenKind::kw_Sampler:
107116
Clause.Type = ClauseType::Sampler;
117+
ExpectedReg = TokenKind::sReg;
108118
break;
109119
}
110120

111-
if (consumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
112-
CurToken.TokKind))
121+
auto Params = parseDescriptorTableClauseParams(ExpectedReg);
122+
if (!Params.has_value())
113123
return true;
114124

115-
if (consumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after,
116-
CurToken.TokKind))
125+
// Check mandatory parameters were provided
126+
if (!Params->Register.has_value()) {
127+
getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_missing_param)
128+
<< ExpectedReg;
129+
return true;
130+
}
131+
132+
Clause.Register = Params->Register.value();
133+
134+
// Fill in optional values
135+
if (Params->Space.has_value())
136+
Clause.Space = Params->Space.value();
137+
138+
if (consumeExpectedToken(TokenKind::pu_r_paren,
139+
diag::err_hlsl_unexpected_end_of_params,
140+
/*param of=*/ParamKind))
117141
return true;
118142

119143
Elements.push_back(Clause);
120144
return false;
121145
}
122146

147+
std::optional<RootSignatureParser::ParsedClauseParams>
148+
RootSignatureParser::parseDescriptorTableClauseParams(TokenKind RegType) {
149+
assert(CurToken.TokKind == TokenKind::pu_l_paren &&
150+
"Expects to only be invoked starting at given token");
151+
152+
// Parameter arguments (eg. `bReg`, `space`, ...) can be specified in any
153+
// order and only exactly once. Parse through as many arguments as possible
154+
// reporting an error if a duplicate is seen.
155+
ParsedClauseParams Params;
156+
do {
157+
// ( `b` | `t` | `u` | `s`) POS_INT
158+
if (tryConsumeExpectedToken(RegType)) {
159+
if (Params.Register.has_value()) {
160+
getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param)
161+
<< CurToken.TokKind;
162+
return std::nullopt;
163+
}
164+
auto Reg = parseRegister();
165+
if (!Reg.has_value())
166+
return std::nullopt;
167+
Params.Register = Reg;
168+
}
169+
170+
// `space` `=` POS_INT
171+
if (tryConsumeExpectedToken(TokenKind::kw_space)) {
172+
if (Params.Space.has_value()) {
173+
getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param)
174+
<< CurToken.TokKind;
175+
return std::nullopt;
176+
}
177+
178+
if (consumeExpectedToken(TokenKind::pu_equal))
179+
return std::nullopt;
180+
181+
auto Space = parseUIntParam();
182+
if (!Space.has_value())
183+
return std::nullopt;
184+
Params.Space = Space;
185+
}
186+
} while (tryConsumeExpectedToken(TokenKind::pu_comma));
187+
188+
return Params;
189+
}
190+
191+
std::optional<uint32_t> RootSignatureParser::parseUIntParam() {
192+
assert(CurToken.TokKind == TokenKind::pu_equal &&
193+
"Expects to only be invoked starting at given keyword");
194+
tryConsumeExpectedToken(TokenKind::pu_plus);
195+
if (consumeExpectedToken(TokenKind::int_literal, diag::err_expected_after,
196+
CurToken.TokKind))
197+
return std::nullopt;
198+
return handleUIntLiteral();
199+
}
200+
201+
std::optional<Register> RootSignatureParser::parseRegister() {
202+
assert((CurToken.TokKind == TokenKind::bReg ||
203+
CurToken.TokKind == TokenKind::tReg ||
204+
CurToken.TokKind == TokenKind::uReg ||
205+
CurToken.TokKind == TokenKind::sReg) &&
206+
"Expects to only be invoked starting at given keyword");
207+
208+
Register Register;
209+
switch (CurToken.TokKind) {
210+
default:
211+
llvm_unreachable("Switch for consumed token was not provided");
212+
case TokenKind::bReg:
213+
Register.ViewType = RegisterType::BReg;
214+
break;
215+
case TokenKind::tReg:
216+
Register.ViewType = RegisterType::TReg;
217+
break;
218+
case TokenKind::uReg:
219+
Register.ViewType = RegisterType::UReg;
220+
break;
221+
case TokenKind::sReg:
222+
Register.ViewType = RegisterType::SReg;
223+
break;
224+
}
225+
226+
auto Number = handleUIntLiteral();
227+
if (!Number.has_value())
228+
return std::nullopt; // propogate NumericLiteralParser error
229+
230+
Register.Number = *Number;
231+
return Register;
232+
}
233+
234+
std::optional<uint32_t> RootSignatureParser::handleUIntLiteral() {
235+
// Parse the numeric value and do semantic checks on its specification
236+
clang::NumericLiteralParser Literal(CurToken.NumSpelling, CurToken.TokLoc,
237+
PP.getSourceManager(), PP.getLangOpts(),
238+
PP.getTargetInfo(), PP.getDiagnostics());
239+
if (Literal.hadError)
240+
return true; // Error has already been reported so just return
241+
242+
assert(Literal.isIntegerLiteral() && "IsNumberChar will only support digits");
243+
244+
llvm::APSInt Val = llvm::APSInt(32, false);
245+
if (Literal.GetIntegerValue(Val)) {
246+
// Report that the value has overflowed
247+
PP.getDiagnostics().Report(CurToken.TokLoc,
248+
diag::err_hlsl_number_literal_overflow)
249+
<< 0 << CurToken.NumSpelling;
250+
return std::nullopt;
251+
}
252+
253+
return Val.getExtValue();
254+
}
255+
123256
bool RootSignatureParser::peekExpectedToken(TokenKind Expected) {
124257
return peekExpectedToken(ArrayRef{Expected});
125258
}
@@ -141,6 +274,7 @@ bool RootSignatureParser::consumeExpectedToken(TokenKind Expected,
141274
case diag::err_expected:
142275
DB << Expected;
143276
break;
277+
case diag::err_hlsl_unexpected_end_of_params:
144278
case diag::err_expected_either:
145279
case diag::err_expected_after:
146280
DB << Expected << Context;

0 commit comments

Comments
 (0)