https://github.com/inbelic updated https://github.com/llvm/llvm-project/pull/133800
>From 9ff87eb37437dc92a554d1d89b236e9a13249694 Mon Sep 17 00:00:00 2001 From: Finn Plummer <finnplum...@microsoft.com> Date: Mon, 31 Mar 2025 18:29:26 +0000 Subject: [PATCH 1/8] [HLSL][RootSignature] Add infastructure to parse parameters - defines `ParamType` as a way to represent a reference to some parameter in a root signature - defines `ParseParam` and `ParseParams` as an infastructure to define how the parameters of a given struct should be parsed in an orderless manner --- .../clang/Basic/DiagnosticParseKinds.td | 4 +- .../clang/Parse/ParseHLSLRootSignature.h | 32 ++++++++++++ clang/lib/Parse/ParseHLSLRootSignature.cpp | 51 +++++++++++++++++++ .../llvm/Frontend/HLSL/HLSLRootSignature.h | 6 +++ 4 files changed, 92 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td index 954f538e15026..2a0bf81c68e44 100644 --- a/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/clang/include/clang/Basic/DiagnosticParseKinds.td @@ -1830,8 +1830,10 @@ def err_hlsl_virtual_function def err_hlsl_virtual_inheritance : Error<"virtual inheritance is unsupported in HLSL">; -// HLSL Root Siganture diagnostic messages +// HLSL Root Signature Parser Diagnostics def err_hlsl_unexpected_end_of_params : Error<"expected %0 to denote end of parameters, or, another valid parameter of %1">; +def err_hlsl_rootsig_repeat_param : Error<"specified the same parameter '%0' multiple times">; +def err_hlsl_rootsig_missing_param : Error<"did not specify mandatory parameter '%0'">; } // end of Parser diagnostics diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h index 18cc2c6692551..55d84f91b8834 100644 --- a/clang/include/clang/Parse/ParseHLSLRootSignature.h +++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h @@ -69,6 +69,38 @@ class RootSignatureParser { bool parseDescriptorTable(); bool parseDescriptorTableClause(); + /// Each unique ParamType will have a custom parse method defined that we can + /// use to invoke the parameters. + /// + /// This function will switch on the ParamType using std::visit and dispatch + /// onto the corresponding parse method + bool parseParam(llvm::hlsl::rootsig::ParamType Ref); + + /// Parameter arguments (eg. `bReg`, `space`, ...) can be specified in any + /// order, exactly once, and only a subset are mandatory. This function acts + /// as the infastructure to do so in a declarative way. + /// + /// For the example: + /// SmallDenseMap<TokenKind, ParamType> Params = { + /// TokenKind::bReg, &Clause.Register, + /// TokenKind::kw_space, &Clause.Space + /// }; + /// SmallDenseSet<TokenKind> Mandatory = { + /// TokenKind::kw_numDescriptors + /// }; + /// + /// We can read it is as: + /// + /// when 'b0' is encountered, invoke the parse method for the type + /// of &Clause.Register (Register *) and update the parameter + /// when 'space' is encountered, invoke a parse method for the type + /// of &Clause.Space (uint32_t *) and update the parameter + /// + /// and 'bReg' must be specified + bool parseParams( + llvm::SmallDenseMap<TokenKind, llvm::hlsl::rootsig::ParamType> &Params, + llvm::SmallDenseSet<TokenKind> &Mandatory); + /// Invoke the Lexer to consume a token and update CurToken with the result void consumeNextToken() { CurToken = Lexer.ConsumeToken(); } diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp index 93a9689ebdf72..54c51e56b84dd 100644 --- a/clang/lib/Parse/ParseHLSLRootSignature.cpp +++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp @@ -120,6 +120,57 @@ bool RootSignatureParser::parseDescriptorTableClause() { return false; } +// Helper struct so that we can use the overloaded notation of std::visit +template <class... Ts> struct ParseMethods : Ts... { using Ts::operator()...; }; +template <class... Ts> ParseMethods(Ts...) -> ParseMethods<Ts...>; + +bool RootSignatureParser::parseParam(ParamType Ref) { + bool Error = true; + std::visit(ParseMethods{}, Ref); + + return Error; +} + +bool RootSignatureParser::parseParams( + llvm::SmallDenseMap<TokenKind, ParamType> &Params, + llvm::SmallDenseSet<TokenKind> &Mandatory) { + + // Initialize a vector of possible keywords + SmallVector<TokenKind> Keywords; + for (auto Pair : Params) + Keywords.push_back(Pair.first); + + // Keep track of which keywords have been seen to report duplicates + llvm::SmallDenseSet<TokenKind> Seen; + + while (tryConsumeExpectedToken(Keywords)) { + if (Seen.contains(CurToken.Kind)) { + getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param) + << CurToken.Kind; + return true; + } + Seen.insert(CurToken.Kind); + + if (parseParam(Params[CurToken.Kind])) + return true; + + if (!tryConsumeExpectedToken(TokenKind::pu_comma)) + break; + } + + bool AllMandatoryDefined = true; + for (auto Kind : Mandatory) { + bool SeenParam = Seen.contains(Kind); + if (!SeenParam) { + getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_missing_param) + << Kind; + } + AllMandatoryDefined &= SeenParam; + } + + return !AllMandatoryDefined; +} + bool RootSignatureParser::peekExpectedToken(TokenKind Expected) { return peekExpectedToken(ArrayRef{Expected}); } diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h index c1b67844c747f..825beeea961cd 100644 --- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h +++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h @@ -37,6 +37,12 @@ struct DescriptorTableClause { // Models RootElement : DescriptorTable | DescriptorTableClause using RootElement = std::variant<DescriptorTable, DescriptorTableClause>; +// ParamType is used as an 'any' type that will reference to a parameter in +// RootElement. Each variant of ParamType is expected to have a Parse method +// defined that will be dispatched on when we are attempting to parse a +// parameter +using ParamType = std::variant<std::monostate>; + } // namespace rootsig } // namespace hlsl } // namespace llvm >From 180c33c33e9ee6e2f38d7b092d564d295591bc81 Mon Sep 17 00:00:00 2001 From: Finn Plummer <finnplum...@microsoft.com> Date: Thu, 27 Mar 2025 18:34:02 +0000 Subject: [PATCH 2/8] define ParseUInt and ParseRegister to plug into parameters --- .../clang/Basic/DiagnosticParseKinds.td | 1 + .../clang/Parse/ParseHLSLRootSignature.h | 8 + clang/lib/Parse/ParseHLSLRootSignature.cpp | 105 +++++++++++-- .../Parse/ParseHLSLRootSignatureTest.cpp | 146 +++++++++++++++++- .../llvm/Frontend/HLSL/HLSLRootSignature.h | 11 +- 5 files changed, 257 insertions(+), 14 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td index 2a0bf81c68e44..e801dfb3b85f4 100644 --- a/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/clang/include/clang/Basic/DiagnosticParseKinds.td @@ -1835,5 +1835,6 @@ def err_hlsl_unexpected_end_of_params : Error<"expected %0 to denote end of parameters, or, another valid parameter of %1">; def err_hlsl_rootsig_repeat_param : Error<"specified the same parameter '%0' multiple times">; def err_hlsl_rootsig_missing_param : Error<"did not specify mandatory parameter '%0'">; +def err_hlsl_number_literal_overflow : Error<"integer literal is too large to be represented as a 32-bit %select{signed |}0 integer type">; } // end of Parser diagnostics diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h index 55d84f91b8834..f015abe3549aa 100644 --- a/clang/include/clang/Parse/ParseHLSLRootSignature.h +++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h @@ -101,6 +101,14 @@ class RootSignatureParser { llvm::SmallDenseMap<TokenKind, llvm::hlsl::rootsig::ParamType> &Params, llvm::SmallDenseSet<TokenKind> &Mandatory); + /// Parameter parse methods corresponding to a ParamType + bool parseUIntParam(uint32_t *X); + bool parseRegister(llvm::hlsl::rootsig::Register *Reg); + + /// Use NumericLiteralParser to convert CurToken.NumSpelling into a unsigned + /// 32-bit integer + bool handleUIntLiteral(uint32_t *X); + /// Invoke the Lexer to consume a token and update CurToken with the result void consumeNextToken() { CurToken = Lexer.ConsumeToken(); } diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp index 54c51e56b84dd..b599b9714a949 100644 --- a/clang/lib/Parse/ParseHLSLRootSignature.cpp +++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp @@ -8,6 +8,8 @@ #include "clang/Parse/ParseHLSLRootSignature.h" +#include "clang/Lex/LiteralSupport.h" + #include "llvm/Support/raw_ostream.h" using namespace llvm::hlsl::rootsig; @@ -89,46 +91,70 @@ bool RootSignatureParser::parseDescriptorTableClause() { CurToken.TokKind == TokenKind::kw_UAV || CurToken.TokKind == TokenKind::kw_Sampler) && "Expects to only be invoked starting at given keyword"); + TokenKind ParamKind = CurToken.TokKind; // retain for diagnostics DescriptorTableClause Clause; - switch (CurToken.TokKind) { + TokenKind ExpectedRegister; + switch (ParamKind) { default: llvm_unreachable("Switch for consumed token was not provided"); case TokenKind::kw_CBV: Clause.Type = ClauseType::CBuffer; + ExpectedRegister = TokenKind::bReg; break; case TokenKind::kw_SRV: Clause.Type = ClauseType::SRV; + ExpectedRegister = TokenKind::tReg; break; case TokenKind::kw_UAV: Clause.Type = ClauseType::UAV; + ExpectedRegister = TokenKind::uReg; break; case TokenKind::kw_Sampler: Clause.Type = ClauseType::Sampler; + ExpectedRegister = TokenKind::sReg; break; } if (consumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after, - CurToken.TokKind)) + ParamKind)) return true; - if (consumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after, - CurToken.TokKind)) + llvm::SmallDenseMap<TokenKind, ParamType> Params = { + {ExpectedRegister, &Clause.Register}, + {TokenKind::kw_space, &Clause.Space}, + }; + llvm::SmallDenseSet<TokenKind> Mandatory = { + ExpectedRegister, + }; + + if (parseParams(Params, Mandatory)) + return true; + + if (consumeExpectedToken(TokenKind::pu_r_paren, diag::err_hlsl_unexpected_end_of_params, + ParamKind)) return true; Elements.push_back(Clause); return false; } -// Helper struct so that we can use the overloaded notation of std::visit +// Helper struct defined to use the overloaded notation of std::visit. template <class... Ts> struct ParseMethods : Ts... { using Ts::operator()...; }; template <class... Ts> ParseMethods(Ts...) -> ParseMethods<Ts...>; bool RootSignatureParser::parseParam(ParamType Ref) { - bool Error = true; - std::visit(ParseMethods{}, Ref); - - return Error; + return std::visit( + ParseMethods{ + [this](Register *X) -> bool { return parseRegister(X); }, + [this](uint32_t *X) -> bool { + return consumeExpectedToken(TokenKind::pu_equal, + diag::err_expected_after, + CurToken.Kind) || + parseUIntParam(X); + }, + }, + Ref); } bool RootSignatureParser::parseParams( @@ -171,6 +197,67 @@ bool RootSignatureParser::parseParams( return !AllMandatoryDefined; } +bool RootSignatureParser::parseUIntParam(uint32_t *X) { + assert(CurToken.Kind == TokenKind::pu_equal && + "Expects to only be invoked starting at given keyword"); + tryConsumeExpectedToken(TokenKind::pu_plus); + return consumeExpectedToken(TokenKind::int_literal, diag::err_expected_after, + CurToken.Kind) || + handleUIntLiteral(X); +} + +bool RootSignatureParser::parseRegister(Register *Register) { + assert( + (CurToken.Kind == TokenKind::bReg || CurToken.Kind == TokenKind::tReg || + CurToken.Kind == TokenKind::uReg || CurToken.Kind == TokenKind::sReg) && + "Expects to only be invoked starting at given keyword"); + + switch (CurToken.Kind) { + case TokenKind::bReg: + Register->ViewType = RegisterType::BReg; + break; + case TokenKind::tReg: + Register->ViewType = RegisterType::TReg; + break; + case TokenKind::uReg: + Register->ViewType = RegisterType::UReg; + break; + case TokenKind::sReg: + Register->ViewType = RegisterType::SReg; + break; + default: + break; // Unreachable given Try + assert pattern + } + + if (handleUIntLiteral(&Register->Number)) + return true; // propogate NumericLiteralParser error + + return false; +} + +bool RootSignatureParser::handleUIntLiteral(uint32_t *X) { + // Parse the numeric value and do semantic checks on its specification + clang::NumericLiteralParser Literal(CurToken.NumSpelling, CurToken.TokLoc, + PP.getSourceManager(), PP.getLangOpts(), + PP.getTargetInfo(), PP.getDiagnostics()); + if (Literal.hadError) + return true; // Error has already been reported so just return + + assert(Literal.isIntegerLiteral() && "IsNumberChar will only support digits"); + + llvm::APSInt Val = llvm::APSInt(32, false); + if (Literal.GetIntegerValue(Val)) { + // Report that the value has overflowed + PP.getDiagnostics().Report(CurToken.TokLoc, + diag::err_hlsl_number_literal_overflow) + << 0 << CurToken.NumSpelling; + return true; + } + + *X = Val.getExtValue(); + return false; +} + bool RootSignatureParser::peekExpectedToken(TokenKind Expected) { return peekExpectedToken(ArrayRef{Expected}); } diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp index acdf455a5d6aa..5b162e9a1b4cd 100644 --- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp +++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp @@ -130,10 +130,10 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseEmptyTest) { TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) { const llvm::StringLiteral Source = R"cc( DescriptorTable( - CBV(), - SRV(), - Sampler(), - UAV() + CBV(b0), + SRV(space = 3, t42), + Sampler(s987, space = +2), + UAV(u4294967294) ), DescriptorTable() )cc"; @@ -155,18 +155,34 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) { RootElement Elem = Elements[0]; ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::CBuffer); + ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.ViewType, + RegisterType::BReg); + ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.Number, 0u); + ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, 0u); Elem = Elements[1]; ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::SRV); + ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.ViewType, + RegisterType::TReg); + ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.Number, 42u); + ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, 3u); Elem = Elements[2]; ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler); + ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.ViewType, + RegisterType::SReg); + ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.Number, 987u); + ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, 2u); Elem = Elements[3]; ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::UAV); + ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.ViewType, + RegisterType::UReg); + ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.Number, 4294967294u); + ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, 0u); Elem = Elements[4]; ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem)); @@ -176,6 +192,32 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) { Elem = Elements[5]; ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem)); ASSERT_EQ(std::get<DescriptorTable>(Elem).NumClauses, 0u); + + ASSERT_TRUE(Consumer->isSatisfied()); +} + +TEST_F(ParseHLSLRootSignatureTest, ValidTrailingCommaTest) { + // This test will checks we can handling trailing commas ',' + const llvm::StringLiteral Source = R"cc( + DescriptorTable( + CBV(b0, ), + SRV(t42), + ) + )cc"; + + TrivialModuleLoader ModLoader; + auto PP = createPP(Source, ModLoader); + auto TokLoc = SourceLocation(); + + hlsl::RootSignatureLexer Lexer(Source, TokLoc); + SmallVector<RootElement> Elements; + hlsl::RootSignatureParser Parser(Elements, Lexer, *PP); + + // Test no diagnostics produced + Consumer->setNoDiag(); + + ASSERT_FALSE(Parser.parse()); + ASSERT_TRUE(Consumer->isSatisfied()); } @@ -237,6 +279,102 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedEndOfStreamTest) { // Test correct diagnostic produced - end of stream Consumer->setExpected(diag::err_expected_after); + + ASSERT_TRUE(Parser.parse()); + + ASSERT_TRUE(Consumer->isSatisfied()); +} + +TEST_F(ParseHLSLRootSignatureTest, InvalidMissingParameterTest) { + // This test will check that the parsing fails due a mandatory + // parameter (register) not being specified + const llvm::StringLiteral Source = R"cc( + DescriptorTable( + CBV() + ) + )cc"; + + TrivialModuleLoader ModLoader; + auto PP = createPP(Source, ModLoader); + auto TokLoc = SourceLocation(); + + hlsl::RootSignatureLexer Lexer(Source, TokLoc); + SmallVector<RootElement> Elements; + hlsl::RootSignatureParser Parser(Elements, Lexer, *PP); + + // Test correct diagnostic produced + Consumer->setExpected(diag::err_hlsl_rootsig_missing_param); + ASSERT_TRUE(Parser.parse()); + + ASSERT_TRUE(Consumer->isSatisfied()); +} + +TEST_F(ParseHLSLRootSignatureTest, InvalidRepeatedMandatoryParameterTest) { + // This test will check that the parsing fails due the same mandatory + // parameter being specified multiple times + const llvm::StringLiteral Source = R"cc( + DescriptorTable( + CBV(b32, b84) + ) + )cc"; + + TrivialModuleLoader ModLoader; + auto PP = createPP(Source, ModLoader); + auto TokLoc = SourceLocation(); + + hlsl::RootSignatureLexer Lexer(Source, TokLoc); + SmallVector<RootElement> Elements; + hlsl::RootSignatureParser Parser(Elements, Lexer, *PP); + + // Test correct diagnostic produced + Consumer->setExpected(diag::err_hlsl_rootsig_repeat_param); + ASSERT_TRUE(Parser.parse()); + + ASSERT_TRUE(Consumer->isSatisfied()); +} + +TEST_F(ParseHLSLRootSignatureTest, InvalidRepeatedOptionalParameterTest) { + // This test will check that the parsing fails due the same optional + // parameter being specified multiple times + const llvm::StringLiteral Source = R"cc( + DescriptorTable( + CBV(space = 2, space = 0) + ) + )cc"; + + TrivialModuleLoader ModLoader; + auto PP = createPP(Source, ModLoader); + auto TokLoc = SourceLocation(); + + hlsl::RootSignatureLexer Lexer(Source, TokLoc); + SmallVector<RootElement> Elements; + hlsl::RootSignatureParser Parser(Elements, Lexer, *PP); + + // Test correct diagnostic produced + Consumer->setExpected(diag::err_hlsl_rootsig_repeat_param); + ASSERT_TRUE(Parser.parse()); + + ASSERT_TRUE(Consumer->isSatisfied()); +} + +TEST_F(ParseHLSLRootSignatureTest, InvalidLexOverflowedNumberTest) { + // This test will check that the lexing fails due to an integer overflow + const llvm::StringLiteral Source = R"cc( + DescriptorTable( + CBV(b4294967296) + ) + )cc"; + + TrivialModuleLoader ModLoader; + auto PP = createPP(Source, ModLoader); + auto TokLoc = SourceLocation(); + + hlsl::RootSignatureLexer Lexer(Source, TokLoc); + SmallVector<RootElement> Elements; + hlsl::RootSignatureParser Parser(Elements, Lexer, *PP); + + // Test correct diagnostic produced + Consumer->setExpected(diag::err_hlsl_number_literal_overflow); ASSERT_TRUE(Parser.parse()); ASSERT_TRUE(Consumer->isSatisfied()); diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h index 825beeea961cd..0ae8879b6c7d5 100644 --- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h +++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h @@ -23,6 +23,13 @@ namespace rootsig { // Definitions of the in-memory data layout structures +// Models the different registers: bReg | tReg | uReg | sReg +enum class RegisterType { BReg, TReg, UReg, SReg }; +struct Register { + RegisterType ViewType; + uint32_t Number; +}; + // Models the end of a descriptor table and stores its visibility struct DescriptorTable { uint32_t NumClauses = 0; // The number of clauses in the table @@ -32,6 +39,8 @@ struct DescriptorTable { using ClauseType = llvm::dxil::ResourceClass; struct DescriptorTableClause { ClauseType Type; + Register Register; + uint32_t Space = 0; }; // Models RootElement : DescriptorTable | DescriptorTableClause @@ -41,7 +50,7 @@ using RootElement = std::variant<DescriptorTable, DescriptorTableClause>; // RootElement. Each variant of ParamType is expected to have a Parse method // defined that will be dispatched on when we are attempting to parse a // parameter -using ParamType = std::variant<std::monostate>; +using ParamType = std::variant<uint32_t *, Register *>; } // namespace rootsig } // namespace hlsl >From ebab0ca6d5f344152fd5db9c665f82ceeb303b20 Mon Sep 17 00:00:00 2001 From: Finn Plummer <finnplum...@microsoft.com> Date: Mon, 31 Mar 2025 18:47:15 +0000 Subject: [PATCH 3/8] self-review: use consumeExpectedToken api to report unexpected_end_of_params diag --- clang/lib/Parse/ParseHLSLRootSignature.cpp | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp index b599b9714a949..65ff90092a4d7 100644 --- a/clang/lib/Parse/ParseHLSLRootSignature.cpp +++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp @@ -43,12 +43,11 @@ bool RootSignatureParser::parse() { break; } - if (!tryConsumeExpectedToken(TokenKind::end_of_stream)) { - getDiags().Report(CurToken.TokLoc, diag::err_hlsl_unexpected_end_of_params) - << /*expected=*/TokenKind::end_of_stream - << /*param of=*/TokenKind::kw_RootSignature; + if (consumeExpectedToken(TokenKind::end_of_stream, + diag::err_hlsl_unexpected_end_of_params, + /*param of=*/TokenKind::kw_RootSignature)) return true; - } + return false; } @@ -74,12 +73,10 @@ bool RootSignatureParser::parseDescriptorTable() { break; } - if (!tryConsumeExpectedToken(TokenKind::pu_r_paren)) { - getDiags().Report(CurToken.TokLoc, diag::err_hlsl_unexpected_end_of_params) - << /*expected=*/TokenKind::pu_r_paren - << /*param of=*/TokenKind::kw_DescriptorTable; + if (consumeExpectedToken(TokenKind::pu_r_paren, + diag::err_hlsl_unexpected_end_of_params, + /*param of=*/TokenKind::kw_DescriptorTable)) return true; - } Elements.push_back(Table); return false; @@ -132,7 +129,7 @@ bool RootSignatureParser::parseDescriptorTableClause() { return true; if (consumeExpectedToken(TokenKind::pu_r_paren, diag::err_hlsl_unexpected_end_of_params, - ParamKind)) + /*param of=*/ParamKind)) return true; Elements.push_back(Clause); @@ -279,6 +276,7 @@ bool RootSignatureParser::consumeExpectedToken(TokenKind Expected, case diag::err_expected: DB << Expected; break; + case diag::err_hlsl_unexpected_end_of_params: case diag::err_expected_either: case diag::err_expected_after: DB << Expected << Context; >From cd5c14228eea6eed0a7e551a846d29d4779484e9 Mon Sep 17 00:00:00 2001 From: Finn Plummer <finnplum...@microsoft.com> Date: Wed, 2 Apr 2025 17:51:15 +0000 Subject: [PATCH 4/8] self-review: fix-up comments --- clang/include/clang/Parse/ParseHLSLRootSignature.h | 6 +++--- clang/lib/Parse/ParseHLSLRootSignature.cpp | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h index f015abe3549aa..4c45af20464e5 100644 --- a/clang/include/clang/Parse/ParseHLSLRootSignature.h +++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h @@ -69,8 +69,8 @@ class RootSignatureParser { bool parseDescriptorTable(); bool parseDescriptorTableClause(); - /// Each unique ParamType will have a custom parse method defined that we can - /// use to invoke the parameters. + /// Each unique ParamType will have a custom parse method defined that can be + /// invoked to set a value to the referenced paramtype. /// /// This function will switch on the ParamType using std::visit and dispatch /// onto the corresponding parse method @@ -86,7 +86,7 @@ class RootSignatureParser { /// TokenKind::kw_space, &Clause.Space /// }; /// SmallDenseSet<TokenKind> Mandatory = { - /// TokenKind::kw_numDescriptors + /// TokenKind::bReg /// }; /// /// We can read it is as: diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp index 65ff90092a4d7..4b392a9924573 100644 --- a/clang/lib/Parse/ParseHLSLRootSignature.cpp +++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp @@ -210,6 +210,8 @@ bool RootSignatureParser::parseRegister(Register *Register) { "Expects to only be invoked starting at given keyword"); switch (CurToken.Kind) { + default: + llvm_unreachable("Switch for consumed token was not provided"); case TokenKind::bReg: Register->ViewType = RegisterType::BReg; break; @@ -222,8 +224,6 @@ bool RootSignatureParser::parseRegister(Register *Register) { case TokenKind::sReg: Register->ViewType = RegisterType::SReg; break; - default: - break; // Unreachable given Try + assert pattern } if (handleUIntLiteral(&Register->Number)) >From 490e6b99b4c99db5bf084c9151cacd91d607bbbc Mon Sep 17 00:00:00 2001 From: Finn Plummer <finnplum...@microsoft.com> Date: Wed, 2 Apr 2025 18:01:05 +0000 Subject: [PATCH 5/8] rebase changes: update to build issue fixes --- .../clang/Parse/ParseHLSLRootSignature.h | 14 ++++++------- clang/lib/Parse/ParseHLSLRootSignature.cpp | 20 +++++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h index 4c45af20464e5..ffa58e053b68b 100644 --- a/clang/include/clang/Parse/ParseHLSLRootSignature.h +++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h @@ -81,12 +81,12 @@ class RootSignatureParser { /// as the infastructure to do so in a declarative way. /// /// For the example: - /// SmallDenseMap<TokenKind, ParamType> Params = { - /// TokenKind::bReg, &Clause.Register, - /// TokenKind::kw_space, &Clause.Space + /// SmallDenseMap<RootSignatureToken::Kind, ParamType> Params = { + /// RootSignatureToken::Kind::bReg, &Clause.Register, + /// RootSignatureToken::Kind::kw_space, &Clause.Space /// }; - /// SmallDenseSet<TokenKind> Mandatory = { - /// TokenKind::bReg + /// SmallDenseSet<RootSignatureToken::Kind> Mandatory = { + /// RootSignatureToken::Kind::bReg /// }; /// /// We can read it is as: @@ -98,8 +98,8 @@ class RootSignatureParser { /// /// and 'bReg' must be specified bool parseParams( - llvm::SmallDenseMap<TokenKind, llvm::hlsl::rootsig::ParamType> &Params, - llvm::SmallDenseSet<TokenKind> &Mandatory); + llvm::SmallDenseMap<RootSignatureToken::Kind, llvm::hlsl::rootsig::ParamType> &Params, + llvm::SmallDenseSet<RootSignatureToken::Kind> &Mandatory); /// Parameter parse methods corresponding to a ParamType bool parseUIntParam(uint32_t *X); diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp index 4b392a9924573..4fcc4aca3788b 100644 --- a/clang/lib/Parse/ParseHLSLRootSignature.cpp +++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp @@ -147,7 +147,7 @@ bool RootSignatureParser::parseParam(ParamType Ref) { [this](uint32_t *X) -> bool { return consumeExpectedToken(TokenKind::pu_equal, diag::err_expected_after, - CurToken.Kind) || + CurToken.TokKind) || parseUIntParam(X); }, }, @@ -167,14 +167,14 @@ bool RootSignatureParser::parseParams( llvm::SmallDenseSet<TokenKind> Seen; while (tryConsumeExpectedToken(Keywords)) { - if (Seen.contains(CurToken.Kind)) { + if (Seen.contains(CurToken.TokKind)) { getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param) - << CurToken.Kind; + << CurToken.TokKind; return true; } - Seen.insert(CurToken.Kind); + Seen.insert(CurToken.TokKind); - if (parseParam(Params[CurToken.Kind])) + if (parseParam(Params[CurToken.TokKind])) return true; if (!tryConsumeExpectedToken(TokenKind::pu_comma)) @@ -195,21 +195,21 @@ bool RootSignatureParser::parseParams( } bool RootSignatureParser::parseUIntParam(uint32_t *X) { - assert(CurToken.Kind == TokenKind::pu_equal && + assert(CurToken.TokKind == TokenKind::pu_equal && "Expects to only be invoked starting at given keyword"); tryConsumeExpectedToken(TokenKind::pu_plus); return consumeExpectedToken(TokenKind::int_literal, diag::err_expected_after, - CurToken.Kind) || + CurToken.TokKind) || handleUIntLiteral(X); } bool RootSignatureParser::parseRegister(Register *Register) { assert( - (CurToken.Kind == TokenKind::bReg || CurToken.Kind == TokenKind::tReg || - CurToken.Kind == TokenKind::uReg || CurToken.Kind == TokenKind::sReg) && + (CurToken.TokKind == TokenKind::bReg || CurToken.TokKind == TokenKind::tReg || + CurToken.TokKind == TokenKind::uReg || CurToken.TokKind == TokenKind::sReg) && "Expects to only be invoked starting at given keyword"); - switch (CurToken.Kind) { + switch (CurToken.TokKind) { default: llvm_unreachable("Switch for consumed token was not provided"); case TokenKind::bReg: >From 8155c606700c351d047ee7fe72e1c5012190d8e3 Mon Sep 17 00:00:00 2001 From: Finn Plummer <finnplum...@microsoft.com> Date: Wed, 2 Apr 2025 18:03:38 +0000 Subject: [PATCH 6/8] clang-formatting --- clang/include/clang/Parse/ParseHLSLRootSignature.h | 6 +++--- clang/lib/Parse/ParseHLSLRootSignature.cpp | 12 +++++++----- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h index ffa58e053b68b..afd9f4d589c3a 100644 --- a/clang/include/clang/Parse/ParseHLSLRootSignature.h +++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h @@ -97,9 +97,9 @@ class RootSignatureParser { /// of &Clause.Space (uint32_t *) and update the parameter /// /// and 'bReg' must be specified - bool parseParams( - llvm::SmallDenseMap<RootSignatureToken::Kind, llvm::hlsl::rootsig::ParamType> &Params, - llvm::SmallDenseSet<RootSignatureToken::Kind> &Mandatory); + bool parseParams(llvm::SmallDenseMap<RootSignatureToken::Kind, + llvm::hlsl::rootsig::ParamType> &Params, + llvm::SmallDenseSet<RootSignatureToken::Kind> &Mandatory); /// Parameter parse methods corresponding to a ParamType bool parseUIntParam(uint32_t *X); diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp index 4fcc4aca3788b..eb8993fda9e7b 100644 --- a/clang/lib/Parse/ParseHLSLRootSignature.cpp +++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp @@ -128,7 +128,8 @@ bool RootSignatureParser::parseDescriptorTableClause() { if (parseParams(Params, Mandatory)) return true; - if (consumeExpectedToken(TokenKind::pu_r_paren, diag::err_hlsl_unexpected_end_of_params, + if (consumeExpectedToken(TokenKind::pu_r_paren, + diag::err_hlsl_unexpected_end_of_params, /*param of=*/ParamKind)) return true; @@ -204,10 +205,11 @@ bool RootSignatureParser::parseUIntParam(uint32_t *X) { } bool RootSignatureParser::parseRegister(Register *Register) { - assert( - (CurToken.TokKind == TokenKind::bReg || CurToken.TokKind == TokenKind::tReg || - CurToken.TokKind == TokenKind::uReg || CurToken.TokKind == TokenKind::sReg) && - "Expects to only be invoked starting at given keyword"); + assert((CurToken.TokKind == TokenKind::bReg || + CurToken.TokKind == TokenKind::tReg || + CurToken.TokKind == TokenKind::uReg || + CurToken.TokKind == TokenKind::sReg) && + "Expects to only be invoked starting at given keyword"); switch (CurToken.TokKind) { default: >From 3a598fdbba7bb81867a7aa623cb2f95c7ece881f Mon Sep 17 00:00:00 2001 From: Finn Plummer <finnplum...@microsoft.com> Date: Wed, 2 Apr 2025 17:46:59 +0000 Subject: [PATCH 7/8] self-review: clang-format fix up --- clang/lib/Parse/ParseHLSLRootSignature.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp index eb8993fda9e7b..f58566cf6de3c 100644 --- a/clang/lib/Parse/ParseHLSLRootSignature.cpp +++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp @@ -138,12 +138,15 @@ bool RootSignatureParser::parseDescriptorTableClause() { } // Helper struct defined to use the overloaded notation of std::visit. -template <class... Ts> struct ParseMethods : Ts... { using Ts::operator()...; }; -template <class... Ts> ParseMethods(Ts...) -> ParseMethods<Ts...>; +template <class... Ts> struct ParseParamTypeMethods : Ts... { + using Ts::operator()...; +}; +template <class... Ts> +ParseParamTypeMethods(Ts...) -> ParseParamTypeMethods<Ts...>; bool RootSignatureParser::parseParam(ParamType Ref) { return std::visit( - ParseMethods{ + ParseParamTypeMethods{ [this](Register *X) -> bool { return parseRegister(X); }, [this](uint32_t *X) -> bool { return consumeExpectedToken(TokenKind::pu_equal, >From 1d2bd91a01f31e544a6be16bd92a56bf8c9e8998 Mon Sep 17 00:00:00 2001 From: Finn Plummer <finnplum...@microsoft.com> Date: Fri, 11 Apr 2025 22:15:17 +0000 Subject: [PATCH 8/8] review: prototype using a stateful struct for parsed params pros: - more explicit mapping to what parse method should be called based on the current keyword/root element - removes complexity of using `std::visit` on the parameter types - allows for validations before the in-memory root element is constructed cons: - we will need to bind the parsed values to the in-memory representations for each element as opposed to just setting it directly. but it makes the validations performed more explicit notes: - this does not enforce that we should do all validations after parsing out, we do allow validations on the current token types (for instance with register types) --- .../clang/Parse/ParseHLSLRootSignature.h | 86 +++++++----- clang/lib/Parse/ParseHLSLRootSignature.cpp | 129 ++++++++++-------- 2 files changed, 125 insertions(+), 90 deletions(-) diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h index afd9f4d589c3a..899deb3ad2bd4 100644 --- a/clang/include/clang/Parse/ParseHLSLRootSignature.h +++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h @@ -69,45 +69,67 @@ class RootSignatureParser { bool parseDescriptorTable(); bool parseDescriptorTableClause(); - /// Each unique ParamType will have a custom parse method defined that can be - /// invoked to set a value to the referenced paramtype. - /// - /// This function will switch on the ParamType using std::visit and dispatch - /// onto the corresponding parse method - bool parseParam(llvm::hlsl::rootsig::ParamType Ref); - /// Parameter arguments (eg. `bReg`, `space`, ...) can be specified in any - /// order, exactly once, and only a subset are mandatory. This function acts - /// as the infastructure to do so in a declarative way. - /// - /// For the example: - /// SmallDenseMap<RootSignatureToken::Kind, ParamType> Params = { - /// RootSignatureToken::Kind::bReg, &Clause.Register, - /// RootSignatureToken::Kind::kw_space, &Clause.Space - /// }; - /// SmallDenseSet<RootSignatureToken::Kind> Mandatory = { - /// RootSignatureToken::Kind::bReg - /// }; + /// order and only exactly once. `ParsedParamState` provides a common + /// stateful structure to facilitate a common abstraction over collecting + /// parameters. By having a common return type we can follow clang's pattern + /// of first parsing out the values and then validating when we construct + /// the corresponding in-memory RootElements. + struct ParsedParamState { + // Parameter state to hold the parsed values. The value is only guarentted + // to be correct if one of its keyword bits is set in `Seen`. + // Eg) if any of the seen bits for `bReg, tReg, uReg, sReg` are set, then + // Register will have an initialized value + llvm::hlsl::rootsig::Register Register; + uint32_t Space; + + // Seen retains whether or not the corresponding keyword has been + // seen + uint32_t Seen = 0u; + + // Valid keywords for the different parameter types and its corresponding + // parsed value + ArrayRef<RootSignatureToken::Kind> Keywords; + + // Context of which RootElement is retained to dispatch on the correct + // parse method. + // Eg) If we encounter kw_flags, it will depend on Context + // we are parsing to determine which `parse*Flags` method to call + RootSignatureToken::Kind Context; + + // Retain start of params for reporting diagnostics + SourceLocation StartLoc; + + // Must provide the starting context of the parameters + ParsedParamState(ArrayRef<RootSignatureToken::Kind> Keywords, + RootSignatureToken::Kind Context, SourceLocation StartLoc) + : Keywords(Keywords), Context(Context), StartLoc(StartLoc) {} + + // Helper functions to interact with Seen + size_t getKeywordIdx(RootSignatureToken::Kind Keyword); + bool checkAndSetSeen(RootSignatureToken::Kind Keyword); + bool checkAndClearSeen(RootSignatureToken::Kind Keyword); + }; + + /// Root signature parameters follow the form of `keyword` `=` `value`, or, + /// are a register. Given a keyword and the context of which `RootElement` + /// type we are parsing, we can dispatch onto the correct parseMethod to + /// parse a value into the `ParsedParamState`. /// - /// We can read it is as: - /// - /// when 'b0' is encountered, invoke the parse method for the type - /// of &Clause.Register (Register *) and update the parameter - /// when 'space' is encountered, invoke a parse method for the type - /// of &Clause.Space (uint32_t *) and update the parameter - /// - /// and 'bReg' must be specified - bool parseParams(llvm::SmallDenseMap<RootSignatureToken::Kind, - llvm::hlsl::rootsig::ParamType> &Params, - llvm::SmallDenseSet<RootSignatureToken::Kind> &Mandatory); + /// This function implements the dispatch onto the correct parse method. + bool parseParam(ParsedParamState &Params); + + /// Parses out a `ParsedParamState` for the caller to use for construction + /// of the in-memory representation of a Root Element. + bool parseParams(ParsedParamState &Params); /// Parameter parse methods corresponding to a ParamType - bool parseUIntParam(uint32_t *X); - bool parseRegister(llvm::hlsl::rootsig::Register *Reg); + bool parseUIntParam(uint32_t &X); + bool parseRegister(llvm::hlsl::rootsig::Register &Reg); /// Use NumericLiteralParser to convert CurToken.NumSpelling into a unsigned /// 32-bit integer - bool handleUIntLiteral(uint32_t *X); + bool handleUIntLiteral(uint32_t &X); /// Invoke the Lexer to consume a token and update CurToken with the result void consumeNextToken() { CurToken = Lexer.ConsumeToken(); } diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp index f58566cf6de3c..34989a7af3e20 100644 --- a/clang/lib/Parse/ParseHLSLRootSignature.cpp +++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp @@ -117,16 +117,26 @@ bool RootSignatureParser::parseDescriptorTableClause() { ParamKind)) return true; - llvm::SmallDenseMap<TokenKind, ParamType> Params = { - {ExpectedRegister, &Clause.Register}, - {TokenKind::kw_space, &Clause.Space}, - }; - llvm::SmallDenseSet<TokenKind> Mandatory = { + TokenKind Keywords[2] = { ExpectedRegister, + TokenKind::kw_space, }; + ParsedParamState Params(Keywords, ParamKind, CurToken.TokLoc); + if (parseParams(Params)) + return true; - if (parseParams(Params, Mandatory)) + // Mandatory parameters: + if (!Params.checkAndClearSeen(ExpectedRegister)) { + getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_missing_param) + << ExpectedRegister; return true; + } + + Clause.Register = Params.Register; + + // Optional parameters: + if (Params.checkAndClearSeen(TokenKind::kw_space)) + Clause.Space = Params.Space; if (consumeExpectedToken(TokenKind::pu_r_paren, diag::err_hlsl_unexpected_end_of_params, @@ -137,68 +147,71 @@ bool RootSignatureParser::parseDescriptorTableClause() { return false; } -// Helper struct defined to use the overloaded notation of std::visit. -template <class... Ts> struct ParseParamTypeMethods : Ts... { - using Ts::operator()...; -}; -template <class... Ts> -ParseParamTypeMethods(Ts...) -> ParseParamTypeMethods<Ts...>; - -bool RootSignatureParser::parseParam(ParamType Ref) { - return std::visit( - ParseParamTypeMethods{ - [this](Register *X) -> bool { return parseRegister(X); }, - [this](uint32_t *X) -> bool { - return consumeExpectedToken(TokenKind::pu_equal, - diag::err_expected_after, - CurToken.TokKind) || - parseUIntParam(X); - }, - }, - Ref); +size_t RootSignatureParser::ParsedParamState::getKeywordIdx( + RootSignatureToken::Kind Keyword) { + ArrayRef KeywordRef = Keywords; + auto It = llvm::find(KeywordRef, Keyword); + assert(It != KeywordRef.end() && "Did not provide a valid param keyword"); + return std::distance(KeywordRef.begin(), It); } -bool RootSignatureParser::parseParams( - llvm::SmallDenseMap<TokenKind, ParamType> &Params, - llvm::SmallDenseSet<TokenKind> &Mandatory) { +bool RootSignatureParser::ParsedParamState::checkAndSetSeen( + RootSignatureToken::Kind Keyword) { + size_t Idx = getKeywordIdx(Keyword); + bool WasSeen = Seen & (1 << Idx); + Seen |= 1u << Idx; + return WasSeen; +} - // Initialize a vector of possible keywords - SmallVector<TokenKind> Keywords; - for (auto Pair : Params) - Keywords.push_back(Pair.first); +bool RootSignatureParser::ParsedParamState::checkAndClearSeen( + RootSignatureToken::Kind Keyword) { + size_t Idx = getKeywordIdx(Keyword); + bool WasSeen = Seen & (1 << Idx); + Seen &= ~(1u << Idx); + return WasSeen; +} - // Keep track of which keywords have been seen to report duplicates - llvm::SmallDenseSet<TokenKind> Seen; +bool RootSignatureParser::parseParam(ParsedParamState &Params) { + TokenKind Keyword = CurToken.TokKind; + if (Keyword == TokenKind::bReg || Keyword == TokenKind::tReg || + Keyword == TokenKind::uReg || Keyword == TokenKind::sReg) { + return parseRegister(Params.Register); + } + + if (consumeExpectedToken(TokenKind::pu_equal, diag::err_expected_after, + Keyword)) + return true; - while (tryConsumeExpectedToken(Keywords)) { - if (Seen.contains(CurToken.TokKind)) { + switch (Keyword) { + case RootSignatureToken::Kind::kw_space: + return parseUIntParam(Params.Space); + default: + llvm_unreachable("Switch for consumed keyword was not provided"); + } +} + +bool RootSignatureParser::parseParams(ParsedParamState &Params) { + assert(CurToken.TokKind == TokenKind::pu_l_paren && + "Expects to only be invoked starting at given token"); + + while (tryConsumeExpectedToken(Params.Keywords)) { + if (Params.checkAndSetSeen(CurToken.TokKind)) { getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param) << CurToken.TokKind; return true; } - Seen.insert(CurToken.TokKind); - if (parseParam(Params[CurToken.TokKind])) + if (parseParam(Params)) return true; if (!tryConsumeExpectedToken(TokenKind::pu_comma)) break; } - bool AllMandatoryDefined = true; - for (auto Kind : Mandatory) { - bool SeenParam = Seen.contains(Kind); - if (!SeenParam) { - getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_missing_param) - << Kind; - } - AllMandatoryDefined &= SeenParam; - } - - return !AllMandatoryDefined; + return false; } -bool RootSignatureParser::parseUIntParam(uint32_t *X) { +bool RootSignatureParser::parseUIntParam(uint32_t &X) { assert(CurToken.TokKind == TokenKind::pu_equal && "Expects to only be invoked starting at given keyword"); tryConsumeExpectedToken(TokenKind::pu_plus); @@ -207,7 +220,7 @@ bool RootSignatureParser::parseUIntParam(uint32_t *X) { handleUIntLiteral(X); } -bool RootSignatureParser::parseRegister(Register *Register) { +bool RootSignatureParser::parseRegister(Register &Register) { assert((CurToken.TokKind == TokenKind::bReg || CurToken.TokKind == TokenKind::tReg || CurToken.TokKind == TokenKind::uReg || @@ -218,26 +231,26 @@ bool RootSignatureParser::parseRegister(Register *Register) { default: llvm_unreachable("Switch for consumed token was not provided"); case TokenKind::bReg: - Register->ViewType = RegisterType::BReg; + Register.ViewType = RegisterType::BReg; break; case TokenKind::tReg: - Register->ViewType = RegisterType::TReg; + Register.ViewType = RegisterType::TReg; break; case TokenKind::uReg: - Register->ViewType = RegisterType::UReg; + Register.ViewType = RegisterType::UReg; break; case TokenKind::sReg: - Register->ViewType = RegisterType::SReg; + Register.ViewType = RegisterType::SReg; break; } - if (handleUIntLiteral(&Register->Number)) + if (handleUIntLiteral(Register.Number)) return true; // propogate NumericLiteralParser error return false; } -bool RootSignatureParser::handleUIntLiteral(uint32_t *X) { +bool RootSignatureParser::handleUIntLiteral(uint32_t &X) { // Parse the numeric value and do semantic checks on its specification clang::NumericLiteralParser Literal(CurToken.NumSpelling, CurToken.TokLoc, PP.getSourceManager(), PP.getLangOpts(), @@ -256,7 +269,7 @@ bool RootSignatureParser::handleUIntLiteral(uint32_t *X) { return true; } - *X = Val.getExtValue(); + X = Val.getExtValue(); return false; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits