Author: Finn Plummer Date: 2025-05-09T10:46:37-07:00 New Revision: 9be4d64ba0a241bfa36c7c81bca2e12b337ebccc
URL: https://github.com/llvm/llvm-project/commit/9be4d64ba0a241bfa36c7c81bca2e12b337ebccc DIFF: https://github.com/llvm/llvm-project/commit/9be4d64ba0a241bfa36c7c81bca2e12b337ebccc.diff LOG: [HLSL][RootSignature] Add optional parameters for RootConstants (#138007) - extends `parseRootConstantParams` and the struct to include the optional parameters of a RootConstant - adds corresponding unit tests Part three of and resolves https://github.com/llvm/llvm-project/issues/126576 Added: Modified: clang/include/clang/Parse/ParseHLSLRootSignature.h clang/lib/Parse/ParseHLSLRootSignature.cpp clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h Removed: ################################################################################ diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h index 0f05b05ed4df6..2ac2083983741 100644 --- a/clang/include/clang/Parse/ParseHLSLRootSignature.h +++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h @@ -82,6 +82,8 @@ class RootSignatureParser { struct ParsedConstantParams { std::optional<llvm::hlsl::rootsig::Register> Reg; std::optional<uint32_t> Num32BitConstants; + std::optional<uint32_t> Space; + std::optional<llvm::hlsl::rootsig::ShaderVisibility> Visibility; }; std::optional<ParsedConstantParams> parseRootConstantParams(); diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp index d392e9017ce85..86bf30668db46 100644 --- a/clang/lib/Parse/ParseHLSLRootSignature.cpp +++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp @@ -78,6 +78,13 @@ std::optional<RootConstants> RootSignatureParser::parseRootConstants() { Constants.Reg = Params->Reg.value(); + // Fill in optional parameters + if (Params->Visibility.has_value()) + Constants.Visibility = Params->Visibility.value(); + + if (Params->Space.has_value()) + Constants.Space = Params->Space.value(); + if (consumeExpectedToken(TokenKind::pu_r_paren, diag::err_hlsl_unexpected_end_of_params, /*param of=*/TokenKind::kw_RootConstants)) @@ -247,6 +254,40 @@ RootSignatureParser::parseRootConstantParams() { return std::nullopt; Params.Reg = Reg; } + + // `space` `=` POS_INT + if (tryConsumeExpectedToken(TokenKind::kw_space)) { + if (Params.Space.has_value()) { + getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param) + << CurToken.TokKind; + return std::nullopt; + } + + if (consumeExpectedToken(TokenKind::pu_equal)) + return std::nullopt; + + auto Space = parseUIntParam(); + if (!Space.has_value()) + return std::nullopt; + Params.Space = Space; + } + + // `visibility` `=` SHADER_VISIBILITY + if (tryConsumeExpectedToken(TokenKind::kw_visibility)) { + if (Params.Visibility.has_value()) { + getDiags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param) + << CurToken.TokKind; + return std::nullopt; + } + + if (consumeExpectedToken(TokenKind::pu_equal)) + return std::nullopt; + + auto Visibility = parseShaderVisibility(); + if (!Visibility.has_value()) + return std::nullopt; + Params.Visibility = Visibility; + } } while (tryConsumeExpectedToken(TokenKind::pu_comma)); return Params; diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp index 63b5a65fd3c38..eda68531de34f 100644 --- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp +++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp @@ -255,7 +255,9 @@ TEST_F(ParseHLSLRootSignatureTest, ValidSamplerFlagsTest) { TEST_F(ParseHLSLRootSignatureTest, ValidParseRootConsantsTest) { const llvm::StringLiteral Source = R"cc( RootConstants(num32BitConstants = 1, b0), - RootConstants(b42, num32BitConstants = 4294967295) + RootConstants(b42, space = 3, num32BitConstants = 4294967295, + visibility = SHADER_VISIBILITY_HULL + ) )cc"; TrivialModuleLoader ModLoader; @@ -278,12 +280,16 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootConsantsTest) { ASSERT_EQ(std::get<RootConstants>(Elem).Num32BitConstants, 1u); ASSERT_EQ(std::get<RootConstants>(Elem).Reg.ViewType, RegisterType::BReg); ASSERT_EQ(std::get<RootConstants>(Elem).Reg.Number, 0u); + ASSERT_EQ(std::get<RootConstants>(Elem).Space, 0u); + ASSERT_EQ(std::get<RootConstants>(Elem).Visibility, ShaderVisibility::All); Elem = Elements[1]; ASSERT_TRUE(std::holds_alternative<RootConstants>(Elem)); ASSERT_EQ(std::get<RootConstants>(Elem).Num32BitConstants, 4294967295u); ASSERT_EQ(std::get<RootConstants>(Elem).Reg.ViewType, RegisterType::BReg); ASSERT_EQ(std::get<RootConstants>(Elem).Reg.Number, 42u); + ASSERT_EQ(std::get<RootConstants>(Elem).Space, 3u); + ASSERT_EQ(std::get<RootConstants>(Elem).Visibility, ShaderVisibility::Hull); ASSERT_TRUE(Consumer->isSatisfied()); } @@ -469,7 +475,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidRepeatedMandatoryRCParameterTest) { ASSERT_TRUE(Consumer->isSatisfied()); } -TEST_F(ParseHLSLRootSignatureTest, InvalidRepeatedOptionalParameterTest) { +TEST_F(ParseHLSLRootSignatureTest, InvalidRepeatedOptionalDTParameterTest) { // This test will check that the parsing fails due the same optional // parameter being specified multiple times const llvm::StringLiteral Source = R"cc( @@ -493,6 +499,32 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidRepeatedOptionalParameterTest) { ASSERT_TRUE(Consumer->isSatisfied()); } +TEST_F(ParseHLSLRootSignatureTest, InvalidRepeatedOptionalRCParameterTest) { + // This test will check that the parsing fails due the same optional + // parameter being specified multiple times + const llvm::StringLiteral Source = R"cc( + RootConstants( + visibility = Shader_Visibility_All, + b0, num32BitConstants = 1, + visibility = Shader_Visibility_Pixel + ) + )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( diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h index a3f98a9f1944f..8b8324df18bb3 100644 --- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h +++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h @@ -58,6 +58,8 @@ struct Register { struct RootConstants { uint32_t Num32BitConstants; Register Reg; + uint32_t Space = 0; + ShaderVisibility Visibility = ShaderVisibility::All; }; // Models the end of a descriptor table and stores its visibility _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits