https://github.com/inbelic created https://github.com/llvm/llvm-project/pull/133302
- defines the Parser class and an initial set of helper methods to support consuming tokens. functionality is demonstrated through a simple empty descriptor table test case - defines an initial in-memory representation of a DescriptorTable - implements a test harness that will be used to validate the correct diagnostics are generated. it will construct a dummy pre-processor with diagnostics consumer to do so >From a2d3cb70f683d61ce5478cd583af5fa4a91fddab Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Fri, 14 Feb 2025 19:44:36 +0000 Subject: [PATCH 1/6] [NFC][HLSL][RootSignature] add spelling of tokens to tokenkind.def --- .../clang/Lex/HLSLRootSignatureTokenKinds.def | 22 +++++++++---------- .../include/clang/Lex/LexHLSLRootSignature.h | 2 +- .../Lex/LexHLSLRootSignatureTest.cpp | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/clang/include/clang/Lex/HLSLRootSignatureTokenKinds.def b/clang/include/clang/Lex/HLSLRootSignatureTokenKinds.def index e6df763920430..697c6abc54efa 100644 --- a/clang/include/clang/Lex/HLSLRootSignatureTokenKinds.def +++ b/clang/include/clang/Lex/HLSLRootSignatureTokenKinds.def @@ -14,16 +14,16 @@ //===----------------------------------------------------------------------===// #ifndef TOK -#define TOK(X) +#define TOK(X, SPELLING) #endif #ifndef PUNCTUATOR -#define PUNCTUATOR(X,Y) TOK(pu_ ## X) +#define PUNCTUATOR(X,Y) TOK(pu_ ## X, Y) #endif #ifndef KEYWORD -#define KEYWORD(X) TOK(kw_ ## X) +#define KEYWORD(X) TOK(kw_ ## X, #X) #endif #ifndef ENUM -#define ENUM(NAME, LIT) TOK(en_ ## NAME) +#define ENUM(NAME, LIT) TOK(en_ ## NAME, LIT) #endif // Defines the various types of enum @@ -49,15 +49,15 @@ #endif // General Tokens: -TOK(invalid) -TOK(end_of_stream) -TOK(int_literal) +TOK(invalid, "invalid identifier") +TOK(end_of_stream, "end of stream") +TOK(int_literal, "integer literal") // Register Tokens: -TOK(bReg) -TOK(tReg) -TOK(uReg) -TOK(sReg) +TOK(bReg, "b register") +TOK(tReg, "t register") +TOK(uReg, "u register") +TOK(sReg, "s register") // Punctuators: PUNCTUATOR(l_paren, '(') diff --git a/clang/include/clang/Lex/LexHLSLRootSignature.h b/clang/include/clang/Lex/LexHLSLRootSignature.h index 21c44e0351d9e..b82237411b2ab 100644 --- a/clang/include/clang/Lex/LexHLSLRootSignature.h +++ b/clang/include/clang/Lex/LexHLSLRootSignature.h @@ -24,7 +24,7 @@ namespace hlsl { struct RootSignatureToken { enum Kind { -#define TOK(X) X, +#define TOK(X, SPELLING) X, #include "clang/Lex/HLSLRootSignatureTokenKinds.def" }; diff --git a/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp b/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp index 0576f08c4c276..1e014f6c41c57 100644 --- a/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp +++ b/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp @@ -113,7 +113,7 @@ TEST_F(LexHLSLRootSignatureTest, ValidLexAllTokensTest) { SmallVector<hlsl::RootSignatureToken> Tokens; SmallVector<hlsl::TokenKind> Expected = { -#define TOK(NAME) hlsl::TokenKind::NAME, +#define TOK(NAME, SPELLING) hlsl::TokenKind::NAME, #include "clang/Lex/HLSLRootSignatureTokenKinds.def" }; >From 0fdb4e7ddd3b7bc0e8b9ac35c87a154c01611b4a Mon Sep 17 00:00:00 2001 From: Finn Plummer <finnplum...@microsoft.com> Date: Wed, 26 Mar 2025 19:10:15 +0000 Subject: [PATCH 2/6] [HLSL][RootSignature] Implement parsing of an empty DescriptorTable - defines the Parser class and an initial set of helper methods to support consuming tokens. functionality is demonstrated through a simple empty descriptor table test case - defines an initial in-memory representation of a DescriptorTable - implements a test harness that will be used to validate the correct diagnostics are generated. it will construct a dummy pre-processor with diagnostics consumer to do so --- .../clang/Parse/ParseHLSLRootSignature.h | 84 +++++++ clang/lib/Parse/CMakeLists.txt | 1 + clang/lib/Parse/ParseHLSLRootSignature.cpp | 135 +++++++++++ clang/unittests/CMakeLists.txt | 1 + clang/unittests/Parse/CMakeLists.txt | 23 ++ .../Parse/ParseHLSLRootSignatureTest.cpp | 217 ++++++++++++++++++ .../llvm/Frontend/HLSL/HLSLRootSignature.h | 37 +++ 7 files changed, 498 insertions(+) create mode 100644 clang/include/clang/Parse/ParseHLSLRootSignature.h create mode 100644 clang/lib/Parse/ParseHLSLRootSignature.cpp create mode 100644 clang/unittests/Parse/CMakeLists.txt create mode 100644 clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp create mode 100644 llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h new file mode 100644 index 0000000000000..95243747fbd6b --- /dev/null +++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h @@ -0,0 +1,84 @@ +//===--- ParseHLSLRootSignature.h -------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file defines the ParseHLSLRootSignature interface. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_PARSE_PARSEHLSLROOTSIGNATURE_H +#define LLVM_CLANG_PARSE_PARSEHLSLROOTSIGNATURE_H + +#include "clang/Basic/DiagnosticParse.h" +#include "clang/Lex/Preprocessor.h" +#include "clang/Lex/LexHLSLRootSignature.h" + +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" + +#include "llvm/Frontend/HLSL/HLSLRootSignature.h" + +namespace clang { +namespace hlsl { + +class RootSignatureParser { +public: + RootSignatureParser(SmallVector<llvm::hlsl::rootsig::RootElement> &Elements, + RootSignatureLexer &Lexer, clang::Preprocessor &PP); + + /// Consumes tokens from the Lexer and constructs the in-memory + /// representations of the RootElements. Tokens are consumed until an + /// error is encountered or the end of the buffer. + /// + /// Returns true if a parsing error is encountered. + bool Parse(); + +private: + DiagnosticsEngine &Diags() { return PP.getDiagnostics(); } + + /// Root Element parse methods: + bool ParseDescriptorTable(); + + /// Invoke the Lexer to consume a token and update CurToken with the result + void ConsumeNextToken() { CurToken = Lexer.ConsumeToken(); } + + /// Return true if the next token one of the expected kinds + bool PeekExpectedToken(TokenKind Expected); + bool PeekExpectedToken(ArrayRef<TokenKind> AnyExpected); + + /// Consumes the next token and report an error if it is not of the expected + /// kind. + /// + /// Returns true if there was an error reported. + bool ConsumeExpectedToken(TokenKind Expected, + unsigned DiagID = diag::err_expected, + TokenKind Context = TokenKind::invalid); + bool ConsumeExpectedToken(ArrayRef<TokenKind> AnyExpected, + unsigned DiagID = diag::err_expected, + TokenKind Context = TokenKind::invalid); + + /// Peek if the next token is of the expected kind and if it is then consume + /// it. + /// + /// Returns true if it successfully matches the expected kind and the token + /// was consumed. + bool TryConsumeExpectedToken(TokenKind Expected); + bool TryConsumeExpectedToken(ArrayRef<TokenKind> Expected); + +private: + SmallVector<llvm::hlsl::rootsig::RootElement> &Elements; + RootSignatureLexer &Lexer; + + clang::Preprocessor &PP; + + RootSignatureToken CurToken; +}; + +} // namespace hlsl +} // namespace clang + +#endif // LLVM_CLANG_PARSE_PARSEHLSLROOTSIGNATURE_H diff --git a/clang/lib/Parse/CMakeLists.txt b/clang/lib/Parse/CMakeLists.txt index 22e902f7e1bc5..00fde537bb9c6 100644 --- a/clang/lib/Parse/CMakeLists.txt +++ b/clang/lib/Parse/CMakeLists.txt @@ -14,6 +14,7 @@ add_clang_library(clangParse ParseExpr.cpp ParseExprCXX.cpp ParseHLSL.cpp + ParseHLSLRootSignature.cpp ParseInit.cpp ParseObjc.cpp ParseOpenMP.cpp diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp new file mode 100644 index 0000000000000..86d587b10a8cb --- /dev/null +++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp @@ -0,0 +1,135 @@ +#include "clang/Parse/ParseHLSLRootSignature.h" + +#include "llvm/Support/raw_ostream.h" + +using namespace llvm::hlsl::rootsig; + +namespace clang { +namespace hlsl { + +static std::string FormatTokenKinds(ArrayRef<TokenKind> Kinds) { + std::string TokenString; + llvm::raw_string_ostream Out(TokenString); + bool First = true; + for (auto Kind : Kinds) { + if (!First) + Out << ", "; + switch (Kind) { +#define TOK(X, SPELLING) \ + case TokenKind::X: \ + Out << SPELLING; \ + break; +#include "clang/Lex/HLSLRootSignatureTokenKinds.def" + } + First = false; + } + + return TokenString; +} + +// Parser Definitions + +RootSignatureParser::RootSignatureParser(SmallVector<RootElement> &Elements, + RootSignatureLexer &Lexer, + Preprocessor &PP) + : Elements(Elements), Lexer(Lexer), PP(PP), CurToken(SourceLocation()) {} + +bool RootSignatureParser::Parse() { + // Iterate as many RootElements as possible + while (TryConsumeExpectedToken(TokenKind::kw_DescriptorTable)) { + bool Error = false; + // Dispatch onto parser method. + // We guard against the unreachable here as we just ensured that CurToken + // will be one of the kinds in the while condition + switch (CurToken.Kind) { + case TokenKind::kw_DescriptorTable: + Error = ParseDescriptorTable(); + break; + default: + llvm_unreachable("Switch for consumed token was not provided"); + } + + if (Error) return true; + + if (!TryConsumeExpectedToken(TokenKind::pu_comma)) + break; + } + + return ConsumeExpectedToken(TokenKind::end_of_stream, diag::err_expected); +} + +bool RootSignatureParser::ParseDescriptorTable() { + DescriptorTable Table; + + if (ConsumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after, + CurToken.Kind)) + return true; + + if (ConsumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after, + CurToken.Kind)) + return true; + + Elements.push_back(Table); + return false; +} + +// Returns true when given token is one of the expected kinds +static bool IsExpectedToken(TokenKind Kind, ArrayRef<TokenKind> AnyExpected) { + for (auto Expected : AnyExpected) + if (Kind == Expected) + return true; + return false; +} + +bool RootSignatureParser::PeekExpectedToken(TokenKind Expected) { + return PeekExpectedToken(ArrayRef{Expected}); +} + +bool RootSignatureParser::PeekExpectedToken(ArrayRef<TokenKind> AnyExpected) { + RootSignatureToken Result = Lexer.PeekNextToken(); + return IsExpectedToken(Result.Kind, AnyExpected); +} + +bool RootSignatureParser::ConsumeExpectedToken(TokenKind Expected, + unsigned DiagID, + TokenKind Context) { + return ConsumeExpectedToken(ArrayRef{Expected}, DiagID, Context); +} + +bool RootSignatureParser::ConsumeExpectedToken(ArrayRef<TokenKind> AnyExpected, + unsigned DiagID, + TokenKind Context) { + if (TryConsumeExpectedToken(AnyExpected)) + return false; + + // Report unexpected token kind error + DiagnosticBuilder DB = Diags().Report(CurToken.TokLoc, DiagID); + switch (DiagID) { + case diag::err_expected: + DB << FormatTokenKinds(AnyExpected); + break; + case diag::err_expected_either: + case diag::err_expected_after: + DB << FormatTokenKinds(AnyExpected) << FormatTokenKinds({Context}); + break; + default: + break; + } + return true; +} + +bool RootSignatureParser::TryConsumeExpectedToken(TokenKind Expected) { + return TryConsumeExpectedToken(ArrayRef{Expected}); +} + +bool RootSignatureParser::TryConsumeExpectedToken( + ArrayRef<TokenKind> AnyExpected) { + // If not the expected token just return + if (!PeekExpectedToken(AnyExpected)) + return false; + ConsumeNextToken(); + return true; +} + +} // namespace hlsl +} // namespace clang diff --git a/clang/unittests/CMakeLists.txt b/clang/unittests/CMakeLists.txt index 85d265426ec80..9b3ce8aa7de73 100644 --- a/clang/unittests/CMakeLists.txt +++ b/clang/unittests/CMakeLists.txt @@ -25,6 +25,7 @@ endfunction() add_subdirectory(Basic) add_subdirectory(Lex) +add_subdirectory(Parse) add_subdirectory(Driver) if(CLANG_ENABLE_STATIC_ANALYZER) add_subdirectory(Analysis) diff --git a/clang/unittests/Parse/CMakeLists.txt b/clang/unittests/Parse/CMakeLists.txt new file mode 100644 index 0000000000000..eeb58174568cd --- /dev/null +++ b/clang/unittests/Parse/CMakeLists.txt @@ -0,0 +1,23 @@ +set(LLVM_LINK_COMPONENTS + Support + ) +add_clang_unittest(ParseTests + ParseHLSLRootSignatureTest.cpp + ) +clang_target_link_libraries(ParseTests + PRIVATE + clangAST + clangASTMatchers + clangBasic + clangFrontend + clangParse + clangSema + clangSerialization + clangTooling + ) +target_link_libraries(ParseTests + PRIVATE + LLVMTestingAnnotations + LLVMTestingSupport + clangTesting + ) diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp new file mode 100644 index 0000000000000..35b44ebdcffda --- /dev/null +++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp @@ -0,0 +1,217 @@ +//=== ParseHLSLRootSignatureTest.cpp - Parse Root Signature tests ---------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/DiagnosticOptions.h" +#include "clang/Basic/FileManager.h" +#include "clang/Basic/LangOptions.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Basic/TargetInfo.h" +#include "clang/Lex/HeaderSearch.h" +#include "clang/Lex/HeaderSearchOptions.h" +#include "clang/Lex/Lexer.h" +#include "clang/Lex/ModuleLoader.h" +#include "clang/Lex/Preprocessor.h" +#include "clang/Lex/PreprocessorOptions.h" + +#include "clang/Lex/LexHLSLRootSignature.h" +#include "clang/Parse/ParseHLSLRootSignature.h" +#include "gtest/gtest.h" + +using namespace clang; +using namespace llvm::hlsl::rootsig; + +namespace { + +// Diagnostic helper for helper tests +class ExpectedDiagConsumer : public DiagnosticConsumer { + virtual void anchor() {} + + void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, + const Diagnostic &Info) override { + if (!FirstDiag || !ExpectedDiagID.has_value()) { + Satisfied = false; + return; + } + FirstDiag = false; + + Satisfied = ExpectedDiagID.value() == Info.getID(); + } + + bool FirstDiag = true; + bool Satisfied = false; + std::optional<unsigned> ExpectedDiagID; + +public: + void SetNoDiag() { + Satisfied = true; + ExpectedDiagID = std::nullopt; + } + + void SetExpected(unsigned DiagID) { + Satisfied = false; + ExpectedDiagID = DiagID; + } + + bool IsSatisfied() { return Satisfied; } +}; + +// The test fixture. +class ParseHLSLRootSignatureTest : public ::testing::Test { +protected: + ParseHLSLRootSignatureTest() + : FileMgr(FileMgrOpts), DiagID(new DiagnosticIDs()), + Consumer(new ExpectedDiagConsumer()), + Diags(DiagID, new DiagnosticOptions, Consumer), + SourceMgr(Diags, FileMgr), TargetOpts(new TargetOptions) { + // This is an arbitrarily chosen target triple to create the target info. + TargetOpts->Triple = "dxil"; + Target = TargetInfo::CreateTargetInfo(Diags, TargetOpts); + } + + std::unique_ptr<Preprocessor> CreatePP(StringRef Source, + TrivialModuleLoader &ModLoader) { + std::unique_ptr<llvm::MemoryBuffer> Buf = + llvm::MemoryBuffer::getMemBuffer(Source); + SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf))); + + HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr, + Diags, LangOpts, Target.get()); + std::unique_ptr<Preprocessor> PP = std::make_unique<Preprocessor>( + std::make_shared<PreprocessorOptions>(), Diags, LangOpts, SourceMgr, + HeaderInfo, ModLoader, + /*IILookup =*/nullptr, + /*OwnsHeaderSearch =*/false); + PP->Initialize(*Target); + PP->EnterMainSourceFile(); + return PP; + } + + FileSystemOptions FileMgrOpts; + FileManager FileMgr; + IntrusiveRefCntPtr<DiagnosticIDs> DiagID; + ExpectedDiagConsumer *Consumer; + DiagnosticsEngine Diags; + SourceManager SourceMgr; + LangOptions LangOpts; + std::shared_ptr<TargetOptions> TargetOpts; + IntrusiveRefCntPtr<TargetInfo> Target; +}; + +// Valid Parser Tests + +TEST_F(ParseHLSLRootSignatureTest, ValidParseEmptyTest) { + const llvm::StringLiteral Source = R"cc()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((int)Elements.size(), 0); + + ASSERT_TRUE(Consumer->IsSatisfied()); +} + +TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) { + const llvm::StringLiteral Source = R"cc( + DescriptorTable() + )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()); + RootElement Elem = Elements[0]; + + // Empty Descriptor Table + ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem)); + ASSERT_EQ(std::get<DescriptorTable>(Elem).NumClauses, 0u); + ASSERT_TRUE(Consumer->IsSatisfied()); +} + +// Invalid Parser Tests + +TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedTokenTest) { + const llvm::StringLiteral Source = R"cc( + DescriptorTable() + space + )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_expected); + ASSERT_TRUE(Parser.Parse()); + + ASSERT_TRUE(Consumer->IsSatisfied()); +} + +TEST_F(ParseHLSLRootSignatureTest, InvalidParseInvalidTokenTest) { + const llvm::StringLiteral Source = R"cc( + notAnIdentifier + )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 - invalid token + Consumer->SetExpected(diag::err_expected); + ASSERT_TRUE(Parser.Parse()); + + ASSERT_TRUE(Consumer->IsSatisfied()); +} + +TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedEndOfStreamTest) { + const llvm::StringLiteral Source = R"cc( + DescriptorTable + )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 - end of stream + Consumer->SetExpected(diag::err_expected_after); + ASSERT_TRUE(Parser.Parse()); + + ASSERT_TRUE(Consumer->IsSatisfied()); +} + +} // anonymous namespace diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h new file mode 100644 index 0000000000000..9bdd41e6dec7b --- /dev/null +++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h @@ -0,0 +1,37 @@ +//===- HLSLRootSignature.h - HLSL Root Signature helper objects -----------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +/// +/// \file This file contains helper objects for working with HLSL Root +/// Signatures. +/// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H +#define LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H + +#include <variant> + +namespace llvm { +namespace hlsl { +namespace rootsig { + +// Definitions of the in-memory data layout structures + +// Models the end of a descriptor table and stores its visibility +struct DescriptorTable { + uint32_t NumClauses = 0; // The number of clauses in the table +}; + +// Models RootElement : DescriptorTable +using RootElement = std::variant<DescriptorTable>; + +} // namespace rootsig +} // namespace hlsl +} // namespace llvm + +#endif // LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H >From e6b32c1bb30a48bd4b976e626cbc9476e799f4a9 Mon Sep 17 00:00:00 2001 From: Finn Plummer <finnplum...@microsoft.com> Date: Thu, 27 Mar 2025 16:32:15 +0000 Subject: [PATCH 3/6] implement parsing of empty DescriptorTableClause --- .../clang/Parse/ParseHLSLRootSignature.h | 1 + clang/lib/Parse/ParseHLSLRootSignature.cpp | 52 +++++++++++++++++++ .../Parse/ParseHLSLRootSignatureTest.cpp | 27 ++++++++++ .../llvm/Frontend/HLSL/HLSLRootSignature.h | 11 +++- 4 files changed, 89 insertions(+), 2 deletions(-) diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h index 95243747fbd6b..e7c4e0a5f0db8 100644 --- a/clang/include/clang/Parse/ParseHLSLRootSignature.h +++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h @@ -42,6 +42,7 @@ class RootSignatureParser { /// Root Element parse methods: bool ParseDescriptorTable(); + bool ParseDescriptorTableClause(); /// 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 86d587b10a8cb..1c45288bbd3d9 100644 --- a/clang/lib/Parse/ParseHLSLRootSignature.cpp +++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp @@ -59,12 +59,27 @@ bool RootSignatureParser::Parse() { } bool RootSignatureParser::ParseDescriptorTable() { + assert(CurToken.Kind == TokenKind::kw_DescriptorTable && + "Expects to only be invoked starting at given keyword"); + DescriptorTable Table; if (ConsumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after, CurToken.Kind)) return true; + // Iterate as many Clauses as possible + while (TryConsumeExpectedToken({TokenKind::kw_CBV, TokenKind::kw_SRV, + TokenKind::kw_UAV, TokenKind::kw_Sampler})) { + if (ParseDescriptorTableClause()) + return true; + + Table.NumClauses++; + + if (!TryConsumeExpectedToken(TokenKind::pu_comma)) + break; + } + if (ConsumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after, CurToken.Kind)) return true; @@ -73,6 +88,43 @@ bool RootSignatureParser::ParseDescriptorTable() { return false; } +bool RootSignatureParser::ParseDescriptorTableClause() { + assert((CurToken.Kind == TokenKind::kw_CBV || + CurToken.Kind == TokenKind::kw_SRV || + CurToken.Kind == TokenKind::kw_UAV || + CurToken.Kind == TokenKind::kw_Sampler) + && "Expects to only be invoked starting at given keyword"); + + DescriptorTableClause Clause; + switch (CurToken.Kind) { + default: break; // Unreachable given Try + assert pattern + case TokenKind::kw_CBV: + Clause.Type = ClauseType::CBuffer; + break; + case TokenKind::kw_SRV: + Clause.Type = ClauseType::SRV; + break; + case TokenKind::kw_UAV: + Clause.Type = ClauseType::UAV; + break; + case TokenKind::kw_Sampler: + Clause.Type = ClauseType::Sampler; + break; + } + + if (ConsumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after, + CurToken.Kind)) + return true; + + + if (ConsumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after, + CurToken.Kind)) + return true; + + Elements.push_back(Clause); + return false; +} + // Returns true when given token is one of the expected kinds static bool IsExpectedToken(TokenKind Kind, ArrayRef<TokenKind> AnyExpected) { for (auto Expected : AnyExpected) diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp index 35b44ebdcffda..d5c3d9778987f 100644 --- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp +++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp @@ -128,6 +128,12 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseEmptyTest) { TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) { const llvm::StringLiteral Source = R"cc( + DescriptorTable( + CBV(), + SRV(), + Sampler(), + UAV() + ), DescriptorTable() )cc"; @@ -143,9 +149,30 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) { Consumer->SetNoDiag(); ASSERT_FALSE(Parser.Parse()); + + // First Descriptor Table with 4 elements RootElement Elem = Elements[0]; + ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); + ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::CBuffer); + + Elem = Elements[1]; + ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); + ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::SRV); + + Elem = Elements[2]; + ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); + ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler); + + Elem = Elements[3]; + ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); + ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::UAV); + + Elem = Elements[4]; + ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem)); + ASSERT_EQ(std::get<DescriptorTable>(Elem).NumClauses, (uint32_t)4); // Empty Descriptor Table + Elem = Elements[5]; ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem)); ASSERT_EQ(std::get<DescriptorTable>(Elem).NumClauses, 0u); ASSERT_TRUE(Consumer->IsSatisfied()); diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h index 9bdd41e6dec7b..c1b67844c747f 100644 --- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h +++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h @@ -14,6 +14,7 @@ #ifndef LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H #define LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H +#include "llvm/Support/DXILABI.h" #include <variant> namespace llvm { @@ -27,8 +28,14 @@ struct DescriptorTable { uint32_t NumClauses = 0; // The number of clauses in the table }; -// Models RootElement : DescriptorTable -using RootElement = std::variant<DescriptorTable>; +// Models DTClause : CBV | SRV | UAV | Sampler, by collecting like parameters +using ClauseType = llvm::dxil::ResourceClass; +struct DescriptorTableClause { + ClauseType Type; +}; + +// Models RootElement : DescriptorTable | DescriptorTableClause +using RootElement = std::variant<DescriptorTable, DescriptorTableClause>; } // namespace rootsig } // namespace hlsl >From 4edfc09cd361d0933461a83794bcb16f0355e200 Mon Sep 17 00:00:00 2001 From: Finn Plummer <finnplum...@microsoft.com> Date: Thu, 27 Mar 2025 16:32:31 +0000 Subject: [PATCH 4/6] self-review: docs --- clang/include/clang/Parse/ParseHLSLRootSignature.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h index e7c4e0a5f0db8..09b520092fc9c 100644 --- a/clang/include/clang/Parse/ParseHLSLRootSignature.h +++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h @@ -40,6 +40,20 @@ class RootSignatureParser { private: DiagnosticsEngine &Diags() { return PP.getDiagnostics(); } + /// All private Parse.* methods follow a similar pattern: + /// - Each method will start with an assert to denote what the CurToken is + /// expected to be and will parse from that token forward + /// - Therefore, it is the callers responsibility to ensure that you are + /// at the correct CurToken. This should be done with the pattern of: + /// if (TryConsumeExpectedToken(TokenKind) + /// if (Parse.*()) + /// return true; + /// - All methods return true if a parsing error is encountered. It is the + /// callers responsibility to propogate this error up, or deal with it + /// otherwise + /// - An error will be raised if the proceeding tokens are not what is + /// expected, or, there is a lexing error + /// Root Element parse methods: bool ParseDescriptorTable(); bool ParseDescriptorTableClause(); >From decaace5853e1dc0959efe00523dab0338007a05 Mon Sep 17 00:00:00 2001 From: Finn Plummer <finnplum...@microsoft.com> Date: Thu, 27 Mar 2025 16:39:39 +0000 Subject: [PATCH 5/6] review: clang-format --- clang/include/clang/Parse/ParseHLSLRootSignature.h | 2 +- clang/lib/Parse/ParseHLSLRootSignature.cpp | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h index 09b520092fc9c..2d1ec3dd7f0c7 100644 --- a/clang/include/clang/Parse/ParseHLSLRootSignature.h +++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h @@ -14,8 +14,8 @@ #define LLVM_CLANG_PARSE_PARSEHLSLROOTSIGNATURE_H #include "clang/Basic/DiagnosticParse.h" -#include "clang/Lex/Preprocessor.h" #include "clang/Lex/LexHLSLRootSignature.h" +#include "clang/Lex/Preprocessor.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp index 1c45288bbd3d9..2dd01167eaf83 100644 --- a/clang/lib/Parse/ParseHLSLRootSignature.cpp +++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp @@ -49,7 +49,8 @@ bool RootSignatureParser::Parse() { llvm_unreachable("Switch for consumed token was not provided"); } - if (Error) return true; + if (Error) + return true; if (!TryConsumeExpectedToken(TokenKind::pu_comma)) break; @@ -92,12 +93,13 @@ bool RootSignatureParser::ParseDescriptorTableClause() { assert((CurToken.Kind == TokenKind::kw_CBV || CurToken.Kind == TokenKind::kw_SRV || CurToken.Kind == TokenKind::kw_UAV || - CurToken.Kind == TokenKind::kw_Sampler) - && "Expects to only be invoked starting at given keyword"); + CurToken.Kind == TokenKind::kw_Sampler) && + "Expects to only be invoked starting at given keyword"); DescriptorTableClause Clause; switch (CurToken.Kind) { - default: break; // Unreachable given Try + assert pattern + default: + break; // Unreachable given Try + assert pattern case TokenKind::kw_CBV: Clause.Type = ClauseType::CBuffer; break; @@ -116,7 +118,6 @@ bool RootSignatureParser::ParseDescriptorTableClause() { CurToken.Kind)) return true; - if (ConsumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after, CurToken.Kind)) return true; >From 13e5c91aadd1823b62cc8606b8d20640a5df6ca5 Mon Sep 17 00:00:00 2001 From: Finn Plummer <finnplum...@microsoft.com> Date: Thu, 27 Mar 2025 19:14:06 +0000 Subject: [PATCH 6/6] self-review: fix up comment --- .../clang/Parse/ParseHLSLRootSignature.h | 37 ++++++++++++------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h index 2d1ec3dd7f0c7..bd593fa3e8873 100644 --- a/clang/include/clang/Parse/ParseHLSLRootSignature.h +++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h @@ -40,19 +40,30 @@ class RootSignatureParser { private: DiagnosticsEngine &Diags() { return PP.getDiagnostics(); } - /// All private Parse.* methods follow a similar pattern: - /// - Each method will start with an assert to denote what the CurToken is - /// expected to be and will parse from that token forward - /// - Therefore, it is the callers responsibility to ensure that you are - /// at the correct CurToken. This should be done with the pattern of: - /// if (TryConsumeExpectedToken(TokenKind) - /// if (Parse.*()) - /// return true; - /// - All methods return true if a parsing error is encountered. It is the - /// callers responsibility to propogate this error up, or deal with it - /// otherwise - /// - An error will be raised if the proceeding tokens are not what is - /// expected, or, there is a lexing error + // All private Parse.* methods follow a similar pattern: + // - Each method will start with an assert to denote what the CurToken is + // expected to be and will parse from that token forward + // + // - Therefore, it is the callers responsibility to ensure that you are + // at the correct CurToken. This should be done with the pattern of: + // + // if (TryConsumeExpectedToken(TokenKind)) + // if (Parse.*()) + // return true; + // + // or, + // + // if (ConsumeExpectedToken(TokenKind, ...)) + // return true; + // if (Parse.*()) + // return true; + // + // - All methods return true if a parsing error is encountered. It is the + // callers responsibility to propogate this error up, or deal with it + // otherwise + // + // - An error will be raised if the proceeding tokens are not what is + // expected, or, there is a lexing error /// Root Element parse methods: bool ParseDescriptorTable(); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits