https://github.com/inbelic updated https://github.com/llvm/llvm-project/pull/140147
>From fc2fc60145e65a1b32542cbaf9b4bd6ce3b1509d Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Thu, 15 May 2025 17:48:07 +0000 Subject: [PATCH 1/2] [HLSL][RootSignature] Add parsing for empty RootParams - define the RootParam in-memory struct containing its type - add test harness for testing --- .../clang/Parse/ParseHLSLRootSignature.h | 1 + clang/lib/Parse/ParseHLSLRootSignature.cpp | 43 +++++++++++++++++++ .../Parse/ParseHLSLRootSignatureTest.cpp | 37 ++++++++++++++++ .../llvm/Frontend/HLSL/HLSLRootSignature.h | 14 +++--- 4 files changed, 90 insertions(+), 5 deletions(-) diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h index 915266f8a36ae..b96416ab6c6c4 100644 --- a/clang/include/clang/Parse/ParseHLSLRootSignature.h +++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h @@ -73,6 +73,7 @@ class RootSignatureParser { /// Root Element parse methods: std::optional<llvm::hlsl::rootsig::RootFlags> parseRootFlags(); std::optional<llvm::hlsl::rootsig::RootConstants> parseRootConstants(); + std::optional<llvm::hlsl::rootsig::RootParam> parseRootParam(); std::optional<llvm::hlsl::rootsig::DescriptorTable> parseDescriptorTable(); std::optional<llvm::hlsl::rootsig::DescriptorTableClause> parseDescriptorTableClause(); diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp index 5603900429844..3ed60442eaa14 100644 --- a/clang/lib/Parse/ParseHLSLRootSignature.cpp +++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp @@ -47,6 +47,14 @@ bool RootSignatureParser::parse() { return true; Elements.push_back(*Table); } + + if (tryConsumeExpectedToken( + {TokenKind::kw_CBV, TokenKind::kw_SRV, TokenKind::kw_UAV})) { + auto RootParam = parseRootParam(); + if (!RootParam.has_value()) + return true; + Elements.push_back(*RootParam); + } } while (tryConsumeExpectedToken(TokenKind::pu_comma)); return consumeExpectedToken(TokenKind::end_of_stream, @@ -155,6 +163,41 @@ std::optional<RootConstants> RootSignatureParser::parseRootConstants() { return Constants; } +std::optional<RootParam> RootSignatureParser::parseRootParam() { + assert((CurToken.TokKind == TokenKind::kw_CBV || + CurToken.TokKind == TokenKind::kw_SRV || + CurToken.TokKind == TokenKind::kw_UAV) && + "Expects to only be invoked starting at given keyword"); + + TokenKind ParamKind = CurToken.TokKind; + + if (consumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after, + CurToken.TokKind)) + return std::nullopt; + + RootParam Param; + switch (ParamKind) { + default: + llvm_unreachable("Switch for consumed token was not provided"); + case TokenKind::kw_CBV: + Param.Type = ParamType::CBuffer; + break; + case TokenKind::kw_SRV: + Param.Type = ParamType::SRV; + break; + case TokenKind::kw_UAV: + Param.Type = ParamType::UAV; + break; + } + + if (consumeExpectedToken(TokenKind::pu_r_paren, + diag::err_hlsl_unexpected_end_of_params, + /*param of=*/TokenKind::kw_RootConstants)) + return std::nullopt; + + return Param; +} + std::optional<DescriptorTable> RootSignatureParser::parseDescriptorTable() { assert(CurToken.TokKind == TokenKind::kw_DescriptorTable && "Expects to only be invoked starting at given keyword"); diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp index c97f8d0b392d1..876dcd17f0389 100644 --- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp +++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp @@ -344,6 +344,43 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootFlagsTest) { ASSERT_TRUE(Consumer->isSatisfied()); } +TEST_F(ParseHLSLRootSignatureTest, ValidParseRootParamsTest) { + const llvm::StringLiteral Source = R"cc( + CBV(), + SRV(), + UAV() + )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_EQ(Elements.size(), 3u); + + RootElement Elem = Elements[0]; + ASSERT_TRUE(std::holds_alternative<RootParam>(Elem)); + ASSERT_EQ(std::get<RootParam>(Elem).Type, ParamType::CBuffer); + + Elem = Elements[1]; + ASSERT_TRUE(std::holds_alternative<RootParam>(Elem)); + ASSERT_EQ(std::get<RootParam>(Elem).Type, ParamType::SRV); + + Elem = Elements[2]; + ASSERT_TRUE(std::holds_alternative<RootParam>(Elem)); + ASSERT_EQ(std::get<RootParam>(Elem).Type, ParamType::UAV); + + ASSERT_TRUE(Consumer->isSatisfied()); +} + TEST_F(ParseHLSLRootSignatureTest, ValidTrailingCommaTest) { // This test will checks we can handling trailing commas ',' const llvm::StringLiteral Source = R"cc( diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h index 9fdb40db9c23d..2bcae22c6cfb2 100644 --- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h +++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h @@ -85,6 +85,11 @@ struct RootConstants { ShaderVisibility Visibility = ShaderVisibility::All; }; +using ParamType = llvm::dxil::ResourceClass; +struct RootParam { + ParamType Type; +}; + // Models the end of a descriptor table and stores its visibility struct DescriptorTable { ShaderVisibility Visibility = ShaderVisibility::All; @@ -125,8 +130,8 @@ struct DescriptorTableClause { void dump(raw_ostream &OS) const; }; -/// Models RootElement : RootFlags | RootConstants | DescriptorTable -/// | DescriptorTableClause +/// Models RootElement : RootFlags | RootConstants | RootParam +/// | DescriptorTable | DescriptorTableClause /// /// A Root Signature is modeled in-memory by an array of RootElements. These /// aim to map closely to their DSL grammar reprsentation defined in the spec. @@ -140,9 +145,8 @@ struct DescriptorTableClause { /// The DescriptorTable is modelled by having its Clauses as the previous /// RootElements in the array, and it holds a data member for the Visibility /// parameter. -using RootElement = std::variant<RootFlags, RootConstants, DescriptorTable, - DescriptorTableClause>; - +using RootElement = std::variant<RootFlags, RootConstants, RootParam, + DescriptorTable, DescriptorTableClause>; void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements); class MetadataBuilder { >From 182d1af916a8a122ae9b1af36f1b6f40de7db889 Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Wed, 21 May 2025 18:47:01 +0000 Subject: [PATCH 2/2] review: rename RootParam to RootDescriptor --- .../clang/Parse/ParseHLSLRootSignature.h | 2 +- clang/lib/Parse/ParseHLSLRootSignature.cpp | 22 +++++++++---------- .../Parse/ParseHLSLRootSignatureTest.cpp | 14 ++++++------ .../llvm/Frontend/HLSL/HLSLRootSignature.h | 12 +++++----- 4 files changed, 26 insertions(+), 24 deletions(-) diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h index b96416ab6c6c4..939d45e0c01bc 100644 --- a/clang/include/clang/Parse/ParseHLSLRootSignature.h +++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h @@ -73,7 +73,7 @@ class RootSignatureParser { /// Root Element parse methods: std::optional<llvm::hlsl::rootsig::RootFlags> parseRootFlags(); std::optional<llvm::hlsl::rootsig::RootConstants> parseRootConstants(); - std::optional<llvm::hlsl::rootsig::RootParam> parseRootParam(); + std::optional<llvm::hlsl::rootsig::RootDescriptor> parseRootDescriptor(); std::optional<llvm::hlsl::rootsig::DescriptorTable> parseDescriptorTable(); std::optional<llvm::hlsl::rootsig::DescriptorTableClause> parseDescriptorTableClause(); diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp index 3ed60442eaa14..d71f2c22d5d4b 100644 --- a/clang/lib/Parse/ParseHLSLRootSignature.cpp +++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp @@ -50,10 +50,10 @@ bool RootSignatureParser::parse() { if (tryConsumeExpectedToken( {TokenKind::kw_CBV, TokenKind::kw_SRV, TokenKind::kw_UAV})) { - auto RootParam = parseRootParam(); - if (!RootParam.has_value()) + auto Descriptor = parseRootDescriptor(); + if (!Descriptor.has_value()) return true; - Elements.push_back(*RootParam); + Elements.push_back(*Descriptor); } } while (tryConsumeExpectedToken(TokenKind::pu_comma)); @@ -163,30 +163,30 @@ std::optional<RootConstants> RootSignatureParser::parseRootConstants() { return Constants; } -std::optional<RootParam> RootSignatureParser::parseRootParam() { +std::optional<RootDescriptor> RootSignatureParser::parseRootDescriptor() { assert((CurToken.TokKind == TokenKind::kw_CBV || CurToken.TokKind == TokenKind::kw_SRV || CurToken.TokKind == TokenKind::kw_UAV) && "Expects to only be invoked starting at given keyword"); - TokenKind ParamKind = CurToken.TokKind; + TokenKind DescriptorKind = CurToken.TokKind; if (consumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after, CurToken.TokKind)) return std::nullopt; - RootParam Param; - switch (ParamKind) { + RootDescriptor Descriptor; + switch (DescriptorKind) { default: llvm_unreachable("Switch for consumed token was not provided"); case TokenKind::kw_CBV: - Param.Type = ParamType::CBuffer; + Descriptor.Type = DescriptorType::CBuffer; break; case TokenKind::kw_SRV: - Param.Type = ParamType::SRV; + Descriptor.Type = DescriptorType::SRV; break; case TokenKind::kw_UAV: - Param.Type = ParamType::UAV; + Descriptor.Type = DescriptorType::UAV; break; } @@ -195,7 +195,7 @@ std::optional<RootParam> RootSignatureParser::parseRootParam() { /*param of=*/TokenKind::kw_RootConstants)) return std::nullopt; - return Param; + return Descriptor; } std::optional<DescriptorTable> RootSignatureParser::parseDescriptorTable() { diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp index 876dcd17f0389..0ad73d9a08f92 100644 --- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp +++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp @@ -344,7 +344,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootFlagsTest) { ASSERT_TRUE(Consumer->isSatisfied()); } -TEST_F(ParseHLSLRootSignatureTest, ValidParseRootParamsTest) { +TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) { const llvm::StringLiteral Source = R"cc( CBV(), SRV(), @@ -367,16 +367,16 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootParamsTest) { ASSERT_EQ(Elements.size(), 3u); RootElement Elem = Elements[0]; - ASSERT_TRUE(std::holds_alternative<RootParam>(Elem)); - ASSERT_EQ(std::get<RootParam>(Elem).Type, ParamType::CBuffer); + ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem)); + ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ParamType::CBuffer); Elem = Elements[1]; - ASSERT_TRUE(std::holds_alternative<RootParam>(Elem)); - ASSERT_EQ(std::get<RootParam>(Elem).Type, ParamType::SRV); + ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem)); + ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ParamType::SRV); Elem = Elements[2]; - ASSERT_TRUE(std::holds_alternative<RootParam>(Elem)); - ASSERT_EQ(std::get<RootParam>(Elem).Type, ParamType::UAV); + ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem)); + ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ParamType::UAV); ASSERT_TRUE(Consumer->isSatisfied()); } diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h index 2bcae22c6cfb2..7829b842fa2be 100644 --- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h +++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h @@ -85,9 +85,10 @@ struct RootConstants { ShaderVisibility Visibility = ShaderVisibility::All; }; -using ParamType = llvm::dxil::ResourceClass; -struct RootParam { - ParamType Type; +using DescriptorType = llvm::dxil::ResourceClass; +// Models RootDescriptor : CBV | SRV | UAV, by collecting like parameters +struct RootDescriptor { + DescriptorType Type; }; // Models the end of a descriptor table and stores its visibility @@ -130,7 +131,7 @@ struct DescriptorTableClause { void dump(raw_ostream &OS) const; }; -/// Models RootElement : RootFlags | RootConstants | RootParam +/// Models RootElement : RootFlags | RootConstants | RootDescriptor /// | DescriptorTable | DescriptorTableClause /// /// A Root Signature is modeled in-memory by an array of RootElements. These @@ -145,8 +146,9 @@ struct DescriptorTableClause { /// The DescriptorTable is modelled by having its Clauses as the previous /// RootElements in the array, and it holds a data member for the Visibility /// parameter. -using RootElement = std::variant<RootFlags, RootConstants, RootParam, +using RootElement = std::variant<RootFlags, RootConstants, RootDescriptor, DescriptorTable, DescriptorTableClause>; + void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements); class MetadataBuilder { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits