[clang] [llvm] [HLSL][RootSignature] Implement Parsing of Descriptor Tables (PR #122982)
Finn Plummer via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 5 18:56:34 PST 2025
https://github.com/inbelic updated https://github.com/llvm/llvm-project/pull/122982
>From 9fe6b897b161f7e40e4a5662ab2cad4dceed174e Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Thu, 13 Feb 2025 22:56:52 +0000
Subject: [PATCH 01/15] [HLSL][RootSignature] Parse empty Descriptor Table Root
Element
- 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/Basic/DiagnosticParseKinds.td | 2 +
.../clang/Parse/ParseHLSLRootSignature.h | 69 ++++++
clang/lib/Parse/CMakeLists.txt | 1 +
clang/lib/Parse/ParseHLSLRootSignature.cpp | 138 +++++++++++
clang/unittests/CMakeLists.txt | 1 +
clang/unittests/Parse/CMakeLists.txt | 23 ++
.../Parse/ParseHLSLRootSignatureTest.cpp | 217 ++++++++++++++++++
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 37 +++
8 files changed, 488 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/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index c513dab810d1f..4c6adab80e60e 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1818,4 +1818,6 @@ def ext_hlsl_access_specifiers : ExtWarn<
def err_hlsl_unsupported_component : Error<"invalid component '%0' used; expected 'x', 'y', 'z', or 'w'">;
def err_hlsl_packoffset_invalid_reg : Error<"invalid resource class specifier '%0' for packoffset, expected 'c'">;
+// HLSL Root Signature Parser Diagnostics
+def err_hlsl_rootsig_unexpected_token_kind : Error<"got %1 but expected %select{|one of}0 the following token kind%select{|s}0: %2">;
} // end of Parser diagnostics
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
new file mode 100644
index 0000000000000..fa46bdd663a2c
--- /dev/null
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -0,0 +1,69 @@
+//===--- 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();
+
+ DiagnosticsEngine &Diags() { return PP.getDiagnostics(); }
+
+private:
+ // Root Element helpers
+ bool ParseRootElement();
+ bool ParseDescriptorTable();
+
+ /// Invoke the Lexer to consume a token and update CurToken with the result
+ void ConsumeNextToken() { CurToken = Lexer.ConsumeToken(); }
+
+ /// 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);
+ bool ConsumeExpectedToken(ArrayRef<TokenKind> AnyExpected);
+
+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..aeb33ded4d815
--- /dev/null
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -0,0 +1,138 @@
+#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) {
+ case TokenKind::invalid:
+ Out << "invalid identifier";
+ break;
+ case TokenKind::end_of_stream:
+ Out << "end of stream";
+ break;
+ case TokenKind::int_literal:
+ Out << "integer literal";
+ break;
+ case TokenKind::bReg:
+ Out << "b register";
+ break;
+ case TokenKind::tReg:
+ Out << "t register";
+ break;
+ case TokenKind::uReg:
+ Out << "u register";
+ break;
+ case TokenKind::sReg:
+ Out << "s register";
+ break;
+#define PUNCTUATOR(X, Y) \
+ case TokenKind::pu_##X: \
+ Out << #Y; \
+ break;
+#define KEYWORD(NAME) \
+ case TokenKind::kw_##NAME: \
+ Out << #NAME; \
+ break;
+#define ENUM(NAME, LIT) \
+ case TokenKind::en_##NAME: \
+ Out << LIT; \
+ 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() {
+ // Handle edge-case of empty RootSignature()
+ if (Lexer.EndOfBuffer())
+ return false;
+
+ // Iterate as many RootElements as possible
+ while (!ParseRootElement()) {
+ if (Lexer.EndOfBuffer())
+ return false;
+ if (ConsumeExpectedToken(TokenKind::pu_comma))
+ return true;
+ }
+
+ return true;
+}
+
+bool RootSignatureParser::ParseRootElement() {
+ if (ConsumeExpectedToken(TokenKind::kw_DescriptorTable))
+ return true;
+
+ // Dispatch onto the correct parse method
+ switch (CurToken.Kind) {
+ case TokenKind::kw_DescriptorTable:
+ return ParseDescriptorTable();
+ default:
+ llvm_unreachable("Switch for an expected token was not provided");
+ }
+ return true;
+}
+
+bool RootSignatureParser::ParseDescriptorTable() {
+ DescriptorTable Table;
+
+ if (ConsumeExpectedToken(TokenKind::pu_l_paren))
+ return true;
+
+ // Empty case:
+ if (!ConsumeExpectedToken(TokenKind::pu_r_paren)) {
+ Elements.push_back(Table);
+ return false;
+ }
+
+ return true;
+}
+
+// Is given token 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::ConsumeExpectedToken(TokenKind Expected) {
+ return ConsumeExpectedToken(ArrayRef{Expected});
+}
+
+bool RootSignatureParser::ConsumeExpectedToken(
+ ArrayRef<TokenKind> AnyExpected) {
+ ConsumeNextToken();
+ if (IsExpectedToken(CurToken.Kind, AnyExpected))
+ return false;
+
+ // Report unexpected token kind error
+ Diags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_unexpected_token_kind)
+ << (unsigned)(AnyExpected.size() != 1)
+ << FormatTokenKinds({CurToken.Kind})
+ << FormatTokenKinds(AnyExpected);
+ 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..f2ca933b4f39e
--- /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];
+
+ // Test generated DescriptorTable start has correct default values
+ 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_hlsl_rootsig_unexpected_token_kind);
+ 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_hlsl_rootsig_unexpected_token_kind);
+ 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_hlsl_rootsig_unexpected_token_kind);
+ 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 d444568345ff01da82e9170df3b37690c28a0ce9 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Thu, 13 Feb 2025 23:17:21 +0000
Subject: [PATCH 02/15] add empty Descriptor Table Clause
---
.../clang/Parse/ParseHLSLRootSignature.h | 13 ++++
clang/lib/Parse/ParseHLSLRootSignature.cpp | 69 ++++++++++++++++++-
.../Parse/ParseHLSLRootSignatureTest.cpp | 25 +++++++
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 11 ++-
4 files changed, 114 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index fa46bdd663a2c..94acf5b0ecd94 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -43,10 +43,15 @@ class RootSignatureParser {
// Root Element helpers
bool ParseRootElement();
bool ParseDescriptorTable();
+ bool ParseDescriptorTableClause();
/// 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.
///
@@ -54,6 +59,14 @@ class RootSignatureParser {
bool ConsumeExpectedToken(TokenKind Expected);
bool ConsumeExpectedToken(ArrayRef<TokenKind> AnyExpected);
+ /// 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;
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index aeb33ded4d815..932a8a6263b05 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -100,12 +100,55 @@ bool RootSignatureParser::ParseDescriptorTable() {
return true;
// Empty case:
- if (!ConsumeExpectedToken(TokenKind::pu_r_paren)) {
+ if (TryConsumeExpectedToken(TokenKind::pu_r_paren)) {
Elements.push_back(Table);
return false;
}
- return true;
+ // Iterate through all the defined clauses
+ do {
+ if (ParseDescriptorTableClause())
+ return true;
+ Table.NumClauses++;
+ } while (TryConsumeExpectedToken(TokenKind::pu_comma));
+
+ if (ConsumeExpectedToken(TokenKind::pu_r_paren))
+ return true;
+
+ Elements.push_back(Table);
+ return false;
+}
+
+bool RootSignatureParser::ParseDescriptorTableClause() {
+ if (ConsumeExpectedToken({TokenKind::kw_CBV, TokenKind::kw_SRV,
+ TokenKind::kw_UAV, TokenKind::kw_Sampler}))
+ return true;
+
+ DescriptorTableClause Clause;
+ switch (CurToken.Kind) {
+ 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;
+ default:
+ llvm_unreachable("Switch for an expected token was not provided");
+ }
+ if (ConsumeExpectedToken(TokenKind::pu_l_paren))
+ return true;
+
+ if (ConsumeExpectedToken(TokenKind::pu_r_paren))
+ return true;
+
+ Elements.push_back(Clause);
+ return false;
}
// Is given token one of the expected kinds
@@ -116,6 +159,15 @@ static bool IsExpectedToken(TokenKind Kind, ArrayRef<TokenKind> AnyExpected) {
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) {
return ConsumeExpectedToken(ArrayRef{Expected});
}
@@ -134,5 +186,18 @@ bool RootSignatureParser::ConsumeExpectedToken(
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/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index f2ca933b4f39e..8e13467f3dfe7 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";
@@ -144,7 +150,26 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
ASSERT_FALSE(Parser.Parse());
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);
+ Elem = Elements[5];
// Test generated DescriptorTable start has correct default values
ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem));
ASSERT_EQ(std::get<DescriptorTable>(Elem).NumClauses, 0u);
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 a7e36bfb21a6db435f8eade235818bf2496a6efd Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Fri, 14 Feb 2025 00:21:28 +0000
Subject: [PATCH 03/15] add Register and implicitly unsigned integers
---
.../clang/Basic/DiagnosticParseKinds.td | 1 +
.../clang/Parse/ParseHLSLRootSignature.h | 5 ++
clang/lib/Parse/ParseHLSLRootSignature.cpp | 57 +++++++++++++++++++
.../Parse/ParseHLSLRootSignatureTest.cpp | 46 +++++++++++++--
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 8 +++
5 files changed, 113 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 4c6adab80e60e..a5e19ddc75e02 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1820,4 +1820,5 @@ def err_hlsl_packoffset_invalid_reg : Error<"invalid resource class specifier '%
// HLSL Root Signature Parser Diagnostics
def err_hlsl_rootsig_unexpected_token_kind : Error<"got %1 but expected %select{|one of}0 the following token kind%select{|s}0: %2">;
+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 94acf5b0ecd94..01d38261f58ee 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -45,6 +45,11 @@ class RootSignatureParser {
bool ParseDescriptorTable();
bool ParseDescriptorTableClause();
+ /// Use NumericLiteralParser to convert CurToken.NumSpelling into a unsigned
+ /// 32-bit integer
+ bool HandleUIntLiteral(uint32_t &X);
+ bool ParseRegister(llvm::hlsl::rootsig::Register *Reg);
+
/// 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 932a8a6263b05..f470126f5ed51 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -1,5 +1,7 @@
#include "clang/Parse/ParseHLSLRootSignature.h"
+#include "clang/Lex/LiteralSupport.h"
+
#include "llvm/Support/raw_ostream.h"
using namespace llvm::hlsl::rootsig;
@@ -144,6 +146,10 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
if (ConsumeExpectedToken(TokenKind::pu_l_paren))
return true;
+ // Consume mandatory Register paramater
+ if (ParseRegister(&Clause.Register))
+ return true;
+
if (ConsumeExpectedToken(TokenKind::pu_r_paren))
return true;
@@ -151,6 +157,57 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
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::ParseRegister(Register *Register) {
+ if (ConsumeExpectedToken(
+ {TokenKind::bReg, TokenKind::tReg, TokenKind::uReg, TokenKind::sReg}))
+ return true;
+
+ 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:
+ llvm_unreachable("Switch for an expected token was not provided");
+ }
+
+ if (HandleUIntLiteral(Register->Number))
+ return true; // propogate NumericLiteralParser error
+
+ return false;
+}
+
// Is given token 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 8e13467f3dfe7..4beff9e6c9a42 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -129,10 +129,10 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseEmptyTest) {
TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
const llvm::StringLiteral Source = R"cc(
DescriptorTable(
- CBV(),
- SRV(),
- Sampler(),
- UAV()
+ CBV(b0),
+ SRV(t42),
+ Sampler(s987),
+ UAV(u987234)
),
DescriptorTable()
)cc";
@@ -152,18 +152,33 @@ 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, (uint32_t)0);
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,
+ (uint32_t)42);
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,
+ (uint32_t)987);
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,
+ (uint32_t)987234);
Elem = Elements[4];
ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem));
@@ -239,4 +254,27 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedEndOfStreamTest) {
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(s4294967296)
+ )
+ )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());
+}
+
} // anonymous namespace
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index c1b67844c747f..93f83fc824c6e 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,7 @@ struct DescriptorTable {
using ClauseType = llvm::dxil::ResourceClass;
struct DescriptorTableClause {
ClauseType Type;
+ Register Register;
};
// Models RootElement : DescriptorTable | DescriptorTableClause
>From e8f60466ec90bde7a4064b873a91a726855c6b59 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Tue, 28 Jan 2025 19:00:29 +0000
Subject: [PATCH 04/15] add support for optional parameters
- use `numDescriptors` as example
---
.../clang/Basic/DiagnosticParseKinds.td | 1 +
.../clang/Parse/ParseHLSLRootSignature.h | 14 ++++
clang/lib/Parse/ParseHLSLRootSignature.cpp | 65 +++++++++++++++++++
.../Parse/ParseHLSLRootSignatureTest.cpp | 26 +++++++-
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 5 ++
5 files changed, 110 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index a5e19ddc75e02..ce0da28df3976 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1821,4 +1821,5 @@ def err_hlsl_packoffset_invalid_reg : Error<"invalid resource class specifier '%
// HLSL Root Signature Parser Diagnostics
def err_hlsl_rootsig_unexpected_token_kind : Error<"got %1 but expected %select{|one of}0 the following token kind%select{|s}0: %2">;
def err_hlsl_number_literal_overflow : Error<"integer literal is too large to be represented as a 32-bit %select{signed |}0 integer type">;
+def err_hlsl_rootsig_repeat_param : Error<"specified the same parameter '%0' multiple times">;
} // end of Parser diagnostics
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 01d38261f58ee..3587d0acbd739 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -45,10 +45,24 @@ class RootSignatureParser {
bool ParseDescriptorTable();
bool ParseDescriptorTableClause();
+ /// It is helpful to have a generalized dispatch method so that when we need
+ /// to parse multiple optional parameters in any order, we can invoke this
+ /// method.
+ ///
+ /// Each unique ParamType is expected to define a custom Parse method. This
+ /// function will switch on the ParamType using std::visit and dispatch onto
+ /// the corresponding Parse method
+ bool ParseParam(llvm::hlsl::rootsig::ParamType Ref);
+
+ /// Parses as many optional parameters as possible in any order
+ bool ParseOptionalParams(
+ llvm::SmallDenseMap<TokenKind, llvm::hlsl::rootsig::ParamType> &RefMap);
+
/// Use NumericLiteralParser to convert CurToken.NumSpelling into a unsigned
/// 32-bit integer
bool HandleUIntLiteral(uint32_t &X);
bool ParseRegister(llvm::hlsl::rootsig::Register *Reg);
+ bool ParseUInt(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 f470126f5ed51..161ad6b71529b 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -150,6 +150,13 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
if (ParseRegister(&Clause.Register))
return true;
+ // Define optional paramaters
+ llvm::SmallDenseMap<TokenKind, ParamType> RefMap = {
+ {TokenKind::kw_numDescriptors, &Clause.NumDescriptors},
+ };
+ if (ParseOptionalParams({RefMap}))
+ return true;
+
if (ConsumeExpectedToken(TokenKind::pu_r_paren))
return true;
@@ -157,6 +164,52 @@ 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) {
+ if (ConsumeExpectedToken(TokenKind::pu_equal))
+ return true;
+
+ bool Error;
+ std::visit(ParseMethods{
+ [&](uint32_t *X) { Error = ParseUInt(X); },
+ }, Ref);
+
+ return Error;
+}
+
+bool RootSignatureParser::ParseOptionalParams(
+ llvm::SmallDenseMap<TokenKind, ParamType> &RefMap) {
+ SmallVector<TokenKind> ParamKeywords;
+ for (auto RefPair : RefMap)
+ ParamKeywords.push_back(RefPair.first);
+
+ // Keep track of which keywords have been seen to report duplicates
+ llvm::SmallDenseSet<TokenKind> Seen;
+
+ while (TryConsumeExpectedToken(TokenKind::pu_comma)) {
+ if (ConsumeExpectedToken(ParamKeywords))
+ return true;
+
+ TokenKind ParamKind = CurToken.Kind;
+ if (Seen.contains(ParamKind)) {
+ Diags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param)
+ << FormatTokenKinds({ParamKind});
+ return true;
+ }
+ Seen.insert(ParamKind);
+
+ if (ParseParam(RefMap[ParamKind]))
+ return true;
+ }
+
+ 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,
@@ -208,6 +261,18 @@ bool RootSignatureParser::ParseRegister(Register *Register) {
return false;
}
+bool RootSignatureParser::ParseUInt(uint32_t *X) {
+ // Treat a postively signed integer as though it is unsigned to match DXC
+ TryConsumeExpectedToken(TokenKind::pu_plus);
+ if (ConsumeExpectedToken(TokenKind::int_literal))
+ return true;
+
+ if (HandleUIntLiteral(*X))
+ return true; // propogate NumericLiteralParser error
+
+ return false;
+}
+
// Is given token 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 4beff9e6c9a42..88182364ff708 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -130,7 +130,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
const llvm::StringLiteral Source = R"cc(
DescriptorTable(
CBV(b0),
- SRV(t42),
+ SRV(t42, numDescriptors = +4),
Sampler(s987),
UAV(u987234)
),
@@ -155,6 +155,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.ViewType,
RegisterType::BReg);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.Number, (uint32_t)0);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, (uint32_t)1);
Elem = Elements[1];
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
@@ -163,6 +164,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
RegisterType::TReg);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.Number,
(uint32_t)42);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, (uint32_t)4);
Elem = Elements[2];
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
@@ -171,6 +173,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
RegisterType::SReg);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.Number,
(uint32_t)987);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, (uint32_t)1);
Elem = Elements[3];
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
@@ -179,6 +182,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
RegisterType::UReg);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.Number,
(uint32_t)987234);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, (uint32_t)1);
Elem = Elements[4];
ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem));
@@ -277,4 +281,24 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidLexOverflowedNumberTest) {
ASSERT_TRUE(Consumer->IsSatisfied());
}
+TEST_F(ParseHLSLRootSignatureTest, InvalidParseRepeatedParamTest) {
+ const llvm::StringLiteral Source = R"cc(
+ DescriptorTable(
+ CBV(b0, numDescriptors = 3, numDescriptors = 1)
+ )
+ )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());
+}
+
} // anonymous namespace
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 93f83fc824c6e..abc5fe28db32a 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -40,11 +40,16 @@ using ClauseType = llvm::dxil::ResourceClass;
struct DescriptorTableClause {
ClauseType Type;
Register Register;
+ uint32_t NumDescriptors = 1;
};
// Models RootElement : DescriptorTable | DescriptorTableClause
using RootElement = std::variant<DescriptorTable, DescriptorTableClause>;
+// Models a reference to all assignment parameter types that any RootElement
+// may have. Things of the form: Keyword = Param
+using ParamType = std::variant<uint32_t *>;
+
} // namespace rootsig
} // namespace hlsl
} // namespace llvm
>From a8ddf47e7412179bc2ae8a8ef6f53c27b150fb11 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Tue, 28 Jan 2025 19:23:50 +0000
Subject: [PATCH 05/15] add space optional parameter
- demonstrate can specify in any order
---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 1 +
clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp | 8 ++++++--
llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h | 1 +
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 161ad6b71529b..23731439c6766 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -153,6 +153,7 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
// Define optional paramaters
llvm::SmallDenseMap<TokenKind, ParamType> RefMap = {
{TokenKind::kw_numDescriptors, &Clause.NumDescriptors},
+ {TokenKind::kw_space, &Clause.Space},
};
if (ParseOptionalParams({RefMap}))
return true;
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 88182364ff708..5ddc068410a88 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -130,8 +130,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
const llvm::StringLiteral Source = R"cc(
DescriptorTable(
CBV(b0),
- SRV(t42, numDescriptors = +4),
- Sampler(s987),
+ SRV(t42, space = 3, numDescriptors = +4),
+ Sampler(s987, space = 2),
UAV(u987234)
),
DescriptorTable()
@@ -156,6 +156,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
RegisterType::BReg);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.Number, (uint32_t)0);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, (uint32_t)1);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, (uint32_t)0);
Elem = Elements[1];
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
@@ -165,6 +166,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.Number,
(uint32_t)42);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, (uint32_t)4);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, (uint32_t)3);
Elem = Elements[2];
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
@@ -174,6 +176,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.Number,
(uint32_t)987);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, (uint32_t)1);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, (uint32_t)2);
Elem = Elements[3];
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
@@ -183,6 +186,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.Number,
(uint32_t)987234);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, (uint32_t)1);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, (uint32_t)0);
Elem = Elements[4];
ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem));
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index abc5fe28db32a..f3abbb5a917a3 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -41,6 +41,7 @@ struct DescriptorTableClause {
ClauseType Type;
Register Register;
uint32_t NumDescriptors = 1;
+ uint32_t Space = 0;
};
// Models RootElement : DescriptorTable | DescriptorTableClause
>From 76817892625b9681d46ade8f767681c2284aaa81 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Tue, 28 Jan 2025 19:29:21 +0000
Subject: [PATCH 06/15] define custom paramtype for param parsing - descriptor
range offset
---
.../clang/Parse/ParseHLSLRootSignature.h | 2 ++
clang/lib/Parse/ParseHLSLRootSignature.cpp | 20 +++++++++++++++++++
.../Parse/ParseHLSLRootSignatureTest.cpp | 12 +++++++++--
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 9 ++++++++-
4 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 3587d0acbd739..7e0f190695cb6 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -63,6 +63,8 @@ class RootSignatureParser {
bool HandleUIntLiteral(uint32_t &X);
bool ParseRegister(llvm::hlsl::rootsig::Register *Reg);
bool ParseUInt(uint32_t *X);
+ bool
+ ParseDescriptorRangeOffset(llvm::hlsl::rootsig::DescriptorRangeOffset *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 23731439c6766..b6d6efd8fd2d2 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -154,6 +154,7 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
llvm::SmallDenseMap<TokenKind, ParamType> RefMap = {
{TokenKind::kw_numDescriptors, &Clause.NumDescriptors},
{TokenKind::kw_space, &Clause.Space},
+ {TokenKind::kw_offset, &Clause.Offset},
};
if (ParseOptionalParams({RefMap}))
return true;
@@ -178,6 +179,7 @@ bool RootSignatureParser::ParseParam(ParamType Ref) {
bool Error;
std::visit(ParseMethods{
[&](uint32_t *X) { Error = ParseUInt(X); },
+ [&](DescriptorRangeOffset *X) { Error = ParseDescriptorRangeOffset(X); },
}, Ref);
return Error;
@@ -274,6 +276,24 @@ bool RootSignatureParser::ParseUInt(uint32_t *X) {
return false;
}
+bool RootSignatureParser::ParseDescriptorRangeOffset(DescriptorRangeOffset *X) {
+ if (ConsumeExpectedToken(
+ {TokenKind::int_literal, TokenKind::en_DescriptorRangeOffsetAppend}))
+ return true;
+
+ // Edge case for the offset enum -> static value
+ if (CurToken.Kind == TokenKind::en_DescriptorRangeOffsetAppend) {
+ *X = DescriptorTableOffsetAppend;
+ return false;
+ }
+
+ uint32_t Temp;
+ if (HandleUIntLiteral(Temp))
+ return true; // propogate NumericLiteralParser error
+ *X = DescriptorRangeOffset(Temp);
+ return false;
+}
+
// Is given token 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 5ddc068410a88..1198a37223b87 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -130,8 +130,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
const llvm::StringLiteral Source = R"cc(
DescriptorTable(
CBV(b0),
- SRV(t42, space = 3, numDescriptors = +4),
- Sampler(s987, space = 2),
+ SRV(t42, space = 3, offset = 32, numDescriptors = +4),
+ Sampler(s987, space = 2, offset = DESCRIPTOR_RANGE_OFFSET_APPEND),
UAV(u987234)
),
DescriptorTable()
@@ -157,6 +157,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.Number, (uint32_t)0);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, (uint32_t)1);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, (uint32_t)0);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Offset,
+ DescriptorRangeOffset(DescriptorTableOffsetAppend));
Elem = Elements[1];
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
@@ -167,6 +169,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
(uint32_t)42);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, (uint32_t)4);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, (uint32_t)3);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Offset,
+ DescriptorRangeOffset(32));
Elem = Elements[2];
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
@@ -177,6 +181,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
(uint32_t)987);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, (uint32_t)1);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, (uint32_t)2);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Offset,
+ DescriptorRangeOffset(DescriptorTableOffsetAppend));
Elem = Elements[3];
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
@@ -187,6 +193,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
(uint32_t)987234);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, (uint32_t)1);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, (uint32_t)0);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Offset,
+ DescriptorRangeOffset(DescriptorTableOffsetAppend));
Elem = Elements[4];
ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem));
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index f3abbb5a917a3..1b23fed12ea14 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -21,6 +21,10 @@ namespace llvm {
namespace hlsl {
namespace rootsig {
+// Definition of the various enumerations and flags
+
+enum class DescriptorRangeOffset : uint32_t;
+
// Definitions of the in-memory data layout structures
// Models the different registers: bReg | tReg | uReg | sReg
@@ -35,6 +39,8 @@ struct DescriptorTable {
uint32_t NumClauses = 0; // The number of clauses in the table
};
+static const DescriptorRangeOffset DescriptorTableOffsetAppend =
+ DescriptorRangeOffset(0xffffffff);
// Models DTClause : CBV | SRV | UAV | Sampler, by collecting like parameters
using ClauseType = llvm::dxil::ResourceClass;
struct DescriptorTableClause {
@@ -42,6 +48,7 @@ struct DescriptorTableClause {
Register Register;
uint32_t NumDescriptors = 1;
uint32_t Space = 0;
+ DescriptorRangeOffset Offset = DescriptorTableOffsetAppend;
};
// Models RootElement : DescriptorTable | DescriptorTableClause
@@ -49,7 +56,7 @@ using RootElement = std::variant<DescriptorTable, DescriptorTableClause>;
// Models a reference to all assignment parameter types that any RootElement
// may have. Things of the form: Keyword = Param
-using ParamType = std::variant<uint32_t *>;
+using ParamType = std::variant<uint32_t *, DescriptorRangeOffset *>;
} // namespace rootsig
} // namespace hlsl
>From c3297af5f026c0d8be6e3c09f87dfb6780f3a25d Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Tue, 28 Jan 2025 19:38:57 +0000
Subject: [PATCH 07/15] add support for parsing enums
- uses Shader Visibility as an example for demonstrated test cases
---
.../clang/Parse/ParseHLSLRootSignature.h | 13 +++++
clang/lib/Parse/ParseHLSLRootSignature.cpp | 48 +++++++++++++++++++
.../Parse/ParseHLSLRootSignatureTest.cpp | 28 +++++++++++
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 15 +++++-
4 files changed, 103 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 7e0f190695cb6..51048c077836c 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -66,6 +66,19 @@ class RootSignatureParser {
bool
ParseDescriptorRangeOffset(llvm::hlsl::rootsig::DescriptorRangeOffset *X);
+ /// Method for parsing any type of the ENUM defined token kinds (from
+ /// HLSLRootSignatureTokenKinds.def)
+ ///
+ /// EnumMap provides a mapping from the unique TokenKind to the in-memory
+ /// enum value
+ template <typename EnumType>
+ bool ParseEnum(llvm::SmallDenseMap<TokenKind, EnumType> &EnumMap,
+ EnumType *Enum);
+
+ /// Helper methods that define the mappings and invoke ParseEnum for
+ /// different enum types
+ bool ParseShaderVisibility(llvm::hlsl::rootsig::ShaderVisibility *Enum);
+
/// 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 b6d6efd8fd2d2..c5cdf5fa6ce92 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -107,8 +107,23 @@ bool RootSignatureParser::ParseDescriptorTable() {
return false;
}
+ bool SeenVisibility = false;
// Iterate through all the defined clauses
do {
+ // Handle the visibility parameter
+ if (TryConsumeExpectedToken(TokenKind::kw_visibility)) {
+ if (SeenVisibility) {
+ Diags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param)
+ << FormatTokenKinds(CurToken.Kind);
+ return true;
+ }
+ SeenVisibility = true;
+ if (ParseParam(&Table.Visibility))
+ return true;
+ continue;
+ }
+
+ // Otherwise, we expect a clause
if (ParseDescriptorTableClause())
return true;
Table.NumClauses++;
@@ -180,6 +195,7 @@ bool RootSignatureParser::ParseParam(ParamType Ref) {
std::visit(ParseMethods{
[&](uint32_t *X) { Error = ParseUInt(X); },
[&](DescriptorRangeOffset *X) { Error = ParseDescriptorRangeOffset(X); },
+ [&](ShaderVisibility *Enum) { Error = ParseShaderVisibility(Enum); },
}, Ref);
return Error;
@@ -294,6 +310,38 @@ bool RootSignatureParser::ParseDescriptorRangeOffset(DescriptorRangeOffset *X) {
return false;
}
+template <typename EnumType>
+bool RootSignatureParser::ParseEnum(
+ llvm::SmallDenseMap<TokenKind, EnumType> &EnumMap, EnumType *Enum) {
+ SmallVector<TokenKind> EnumToks;
+ for (auto EnumPair : EnumMap)
+ EnumToks.push_back(EnumPair.first);
+
+ // If invoked we expect to have an enum
+ if (ConsumeExpectedToken(EnumToks))
+ return true;
+
+ // Effectively a switch statement on the token kinds
+ for (auto EnumPair : EnumMap)
+ if (CurToken.Kind == EnumPair.first) {
+ *Enum = EnumPair.second;
+ return false;
+ }
+
+ llvm_unreachable("Switch for an expected token was not provided");
+}
+
+bool RootSignatureParser::ParseShaderVisibility(ShaderVisibility *Enum) {
+ // Define the possible flag kinds
+ llvm::SmallDenseMap<TokenKind, ShaderVisibility> EnumMap = {
+#define SHADER_VISIBILITY_ENUM(NAME, LIT) \
+ {TokenKind::en_##NAME, ShaderVisibility::NAME},
+#include "clang/Lex/HLSLRootSignatureTokenKinds.def"
+ };
+
+ return ParseEnum(EnumMap, Enum);
+}
+
// Is given token 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 1198a37223b87..a4b783a7a83fa 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -129,6 +129,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseEmptyTest) {
TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
const llvm::StringLiteral Source = R"cc(
DescriptorTable(
+ visibility = SHADER_VISIBILITY_PIXEL,
CBV(b0),
SRV(t42, space = 3, offset = 32, numDescriptors = +4),
Sampler(s987, space = 2, offset = DESCRIPTOR_RANGE_OFFSET_APPEND),
@@ -199,11 +200,15 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
Elem = Elements[4];
ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem));
ASSERT_EQ(std::get<DescriptorTable>(Elem).NumClauses, (uint32_t)4);
+ ASSERT_EQ(std::get<DescriptorTable>(Elem).Visibility,
+ ShaderVisibility::Pixel);
Elem = Elements[5];
// Test generated DescriptorTable start has correct default values
ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem));
ASSERT_EQ(std::get<DescriptorTable>(Elem).NumClauses, 0u);
+ ASSERT_EQ(std::get<DescriptorTable>(Elem).Visibility, ShaderVisibility::All);
+
ASSERT_TRUE(Consumer->IsSatisfied());
}
@@ -313,4 +318,27 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseRepeatedParamTest) {
ASSERT_TRUE(Consumer->IsSatisfied());
}
+TEST_F(ParseHLSLRootSignatureTest, InvalidParseRepeatedVisibilityTest) {
+ const llvm::StringLiteral Source = R"cc(
+ DescriptorTable(
+ visibility = SHADER_VISIBILITY_GEOMETRY,
+ visibility = SHADER_VISIBILITY_HULL
+ )
+ )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());
+}
+
} // anonymous namespace
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 1b23fed12ea14..a405c75f8c343 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -25,6 +25,17 @@ namespace rootsig {
enum class DescriptorRangeOffset : uint32_t;
+enum class ShaderVisibility {
+ All = 0,
+ Vertex = 1,
+ Hull = 2,
+ Domain = 3,
+ Geometry = 4,
+ Pixel = 5,
+ Amplification = 6,
+ Mesh = 7,
+};
+
// Definitions of the in-memory data layout structures
// Models the different registers: bReg | tReg | uReg | sReg
@@ -36,6 +47,7 @@ struct Register {
// Models the end of a descriptor table and stores its visibility
struct DescriptorTable {
+ ShaderVisibility Visibility = ShaderVisibility::All;
uint32_t NumClauses = 0; // The number of clauses in the table
};
@@ -56,7 +68,8 @@ using RootElement = std::variant<DescriptorTable, DescriptorTableClause>;
// Models a reference to all assignment parameter types that any RootElement
// may have. Things of the form: Keyword = Param
-using ParamType = std::variant<uint32_t *, DescriptorRangeOffset *>;
+using ParamType = std::variant<uint32_t *, DescriptorRangeOffset *,
+ ShaderVisibility *>;
} // namespace rootsig
} // namespace hlsl
>From 8a902fd5a6f4bb82b8e570ab22eb8cf273ce9723 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Tue, 28 Jan 2025 19:46:34 +0000
Subject: [PATCH 08/15] add support for parsing flags
- use descriptor range flags as an example
---
.../clang/Basic/DiagnosticParseKinds.td | 2 +
.../clang/Parse/ParseHLSLRootSignature.h | 16 +++++-
clang/lib/Parse/ParseHLSLRootSignature.cpp | 51 +++++++++++++++++-
.../Parse/ParseHLSLRootSignatureTest.cpp | 38 ++++++++++++-
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 53 ++++++++++++++++++-
5 files changed, 155 insertions(+), 5 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index ce0da28df3976..fe1b0dd0b91cb 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1822,4 +1822,6 @@ def err_hlsl_packoffset_invalid_reg : Error<"invalid resource class specifier '%
def err_hlsl_rootsig_unexpected_token_kind : Error<"got %1 but expected %select{|one of}0 the following token kind%select{|s}0: %2">;
def err_hlsl_number_literal_overflow : Error<"integer literal is too large to be represented as a 32-bit %select{signed |}0 integer type">;
def err_hlsl_rootsig_repeat_param : Error<"specified the same parameter '%0' multiple times">;
+def err_hlsl_rootsig_non_zero_flag : Error<"specified a non-zero integer as a flag">;
+
} // end of Parser diagnostics
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 51048c077836c..c4fb03a3bc3ed 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -71,7 +71,10 @@ class RootSignatureParser {
///
/// EnumMap provides a mapping from the unique TokenKind to the in-memory
/// enum value
- template <typename EnumType>
+ ///
+ /// If AllowZero is true, then the Enum is used as a flag and can also have
+ /// the value of '0' to denote no flag
+ template <bool AllowZero = false, typename EnumType>
bool ParseEnum(llvm::SmallDenseMap<TokenKind, EnumType> &EnumMap,
EnumType *Enum);
@@ -79,6 +82,17 @@ class RootSignatureParser {
/// different enum types
bool ParseShaderVisibility(llvm::hlsl::rootsig::ShaderVisibility *Enum);
+ /// A wrapper method around ParseEnum that will parse an 'or' chain of
+ /// enums, with AllowZero = true
+ template <typename FlagType>
+ bool ParseFlags(llvm::SmallDenseMap<TokenKind, FlagType> &EnumMap,
+ FlagType *Enum);
+
+ /// Helper methods that define the mappings and invoke ParseFlags for
+ /// different enum types
+ bool
+ ParseDescriptorRangeFlags(llvm::hlsl::rootsig::DescriptorRangeFlags *Enum);
+
/// 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 c5cdf5fa6ce92..e2564228de897 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -158,6 +158,8 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
default:
llvm_unreachable("Switch for an expected token was not provided");
}
+ Clause.SetDefaultFlags();
+
if (ConsumeExpectedToken(TokenKind::pu_l_paren))
return true;
@@ -170,6 +172,7 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
{TokenKind::kw_numDescriptors, &Clause.NumDescriptors},
{TokenKind::kw_space, &Clause.Space},
{TokenKind::kw_offset, &Clause.Offset},
+ {TokenKind::kw_flags, &Clause.Flags},
};
if (ParseOptionalParams({RefMap}))
return true;
@@ -196,6 +199,7 @@ bool RootSignatureParser::ParseParam(ParamType Ref) {
[&](uint32_t *X) { Error = ParseUInt(X); },
[&](DescriptorRangeOffset *X) { Error = ParseDescriptorRangeOffset(X); },
[&](ShaderVisibility *Enum) { Error = ParseShaderVisibility(Enum); },
+ [&](DescriptorRangeFlags *Flags) { Error = ParseDescriptorRangeFlags(Flags); },
}, Ref);
return Error;
@@ -310,10 +314,12 @@ bool RootSignatureParser::ParseDescriptorRangeOffset(DescriptorRangeOffset *X) {
return false;
}
-template <typename EnumType>
+template <bool AllowZero, typename EnumType>
bool RootSignatureParser::ParseEnum(
llvm::SmallDenseMap<TokenKind, EnumType> &EnumMap, EnumType *Enum) {
SmallVector<TokenKind> EnumToks;
+ if (AllowZero)
+ EnumToks.push_back(TokenKind::int_literal); // '0' is a valid flag value
for (auto EnumPair : EnumMap)
EnumToks.push_back(EnumPair.first);
@@ -321,6 +327,20 @@ bool RootSignatureParser::ParseEnum(
if (ConsumeExpectedToken(EnumToks))
return true;
+ // Handle the edge case when '0' is used to specify None
+ if (CurToken.Kind == TokenKind::int_literal) {
+ uint32_t Temp;
+ if (HandleUIntLiteral(Temp))
+ return true; // propogate NumericLiteralParser error
+ if (Temp != 0) {
+ Diags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_non_zero_flag);
+ return true;
+ }
+ // Set enum to None equivalent
+ *Enum = EnumType(0);
+ return false;
+ }
+
// Effectively a switch statement on the token kinds
for (auto EnumPair : EnumMap)
if (CurToken.Kind == EnumPair.first) {
@@ -342,6 +362,35 @@ bool RootSignatureParser::ParseShaderVisibility(ShaderVisibility *Enum) {
return ParseEnum(EnumMap, Enum);
}
+template <typename FlagType>
+bool RootSignatureParser::ParseFlags(
+ llvm::SmallDenseMap<TokenKind, FlagType> &FlagMap, FlagType *Flags) {
+ // Override the default value to 0 so that we can correctly 'or' the values
+ *Flags = FlagType(0);
+
+ do {
+ FlagType Flag;
+ if (ParseEnum<true>(FlagMap, &Flag))
+ return true;
+ // Store the 'or'
+ *Flags |= Flag;
+ } while (TryConsumeExpectedToken(TokenKind::pu_or));
+
+ return false;
+}
+
+bool RootSignatureParser::ParseDescriptorRangeFlags(
+ DescriptorRangeFlags *Flags) {
+ // Define the possible flag kinds
+ llvm::SmallDenseMap<TokenKind, DescriptorRangeFlags> FlagMap = {
+#define DESCRIPTOR_RANGE_FLAG_ENUM(NAME, LIT, ON) \
+ {TokenKind::en_##NAME, DescriptorRangeFlags::NAME},
+#include "clang/Lex/HLSLRootSignatureTokenKinds.def"
+ };
+
+ return ParseFlags(FlagMap, Flags);
+}
+
// Is given token 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 a4b783a7a83fa..8fe57244142b0 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -131,9 +131,13 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
DescriptorTable(
visibility = SHADER_VISIBILITY_PIXEL,
CBV(b0),
- SRV(t42, space = 3, offset = 32, numDescriptors = +4),
+ SRV(t42, space = 3, offset = 32, numDescriptors = +4, flags = 0),
Sampler(s987, space = 2, offset = DESCRIPTOR_RANGE_OFFSET_APPEND),
- UAV(u987234)
+ UAV(u987234,
+ flags = Descriptors_Volatile | Data_Volatile
+ | Data_Static_While_Set_At_Execute | Data_Static
+ | Descriptors_Static_Keeping_Buffer_Bounds_Checks
+ )
),
DescriptorTable()
)cc";
@@ -160,6 +164,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, (uint32_t)0);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Offset,
DescriptorRangeOffset(DescriptorTableOffsetAppend));
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
+ DescriptorRangeFlags::DataStaticWhileSetAtExecute);
Elem = Elements[1];
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
@@ -172,6 +178,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, (uint32_t)3);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Offset,
DescriptorRangeOffset(32));
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
+ DescriptorRangeFlags::None);
Elem = Elements[2];
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
@@ -184,6 +192,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, (uint32_t)2);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Offset,
DescriptorRangeOffset(DescriptorTableOffsetAppend));
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
+ DescriptorRangeFlags::None);
Elem = Elements[3];
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
@@ -196,6 +206,8 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, (uint32_t)0);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Offset,
DescriptorRangeOffset(DescriptorTableOffsetAppend));
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
+ DescriptorRangeFlags::ValidFlags);
Elem = Elements[4];
ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem));
@@ -341,4 +353,26 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseRepeatedVisibilityTest) {
ASSERT_TRUE(Consumer->IsSatisfied());
}
+TEST_F(ParseHLSLRootSignatureTest, InvalidParseNonZeroFlagTest) {
+ const llvm::StringLiteral Source = R"cc(
+ DescriptorTable(
+ CBV(b0, flags = 3)
+ )
+ )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_non_zero_flag);
+ 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
index a405c75f8c343..f999a0b5eef33 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -21,10 +21,43 @@ namespace llvm {
namespace hlsl {
namespace rootsig {
+#define RS_DEFINE_ENUM_CLASS_FLAGS_OPERATORS(Class) \
+ inline Class operator|(Class a, Class b) { \
+ return static_cast<Class>(llvm::to_underlying(a) | \
+ llvm::to_underlying(b)); \
+ } \
+ inline Class operator&(Class a, Class b) { \
+ return static_cast<Class>(llvm::to_underlying(a) & \
+ llvm::to_underlying(b)); \
+ } \
+ inline Class operator~(Class a) { \
+ return static_cast<Class>(~llvm::to_underlying(a)); \
+ } \
+ inline Class &operator|=(Class &a, Class b) { \
+ a = a | b; \
+ return a; \
+ } \
+ inline Class &operator&=(Class &a, Class b) { \
+ a = a & b; \
+ return a; \
+ }
+
// Definition of the various enumerations and flags
enum class DescriptorRangeOffset : uint32_t;
+enum class DescriptorRangeFlags : unsigned {
+ None = 0,
+ DescriptorsVolatile = 0x1,
+ DataVolatile = 0x2,
+ DataStaticWhileSetAtExecute = 0x4,
+ DataStatic = 0x8,
+ DescriptorsStaticKeepingBufferBoundsChecks = 0x10000,
+ ValidFlags = 0x1000f,
+ ValidSamplerFlags = DescriptorsVolatile,
+};
+RS_DEFINE_ENUM_CLASS_FLAGS_OPERATORS(DescriptorRangeFlags)
+
enum class ShaderVisibility {
All = 0,
Vertex = 1,
@@ -61,6 +94,24 @@ struct DescriptorTableClause {
uint32_t NumDescriptors = 1;
uint32_t Space = 0;
DescriptorRangeOffset Offset = DescriptorTableOffsetAppend;
+ DescriptorRangeFlags Flags;
+
+ void SetDefaultFlags() {
+ switch (Type) {
+ case ClauseType::CBuffer:
+ Flags = DescriptorRangeFlags::DataStaticWhileSetAtExecute;
+ break;
+ case ClauseType::SRV:
+ Flags = DescriptorRangeFlags::DataStaticWhileSetAtExecute;
+ break;
+ case ClauseType::UAV:
+ Flags = DescriptorRangeFlags::DataVolatile;
+ break;
+ case ClauseType::Sampler:
+ Flags = DescriptorRangeFlags::None;
+ break;
+ }
+ }
};
// Models RootElement : DescriptorTable | DescriptorTableClause
@@ -69,7 +120,7 @@ using RootElement = std::variant<DescriptorTable, DescriptorTableClause>;
// Models a reference to all assignment parameter types that any RootElement
// may have. Things of the form: Keyword = Param
using ParamType = std::variant<uint32_t *, DescriptorRangeOffset *,
- ShaderVisibility *>;
+ DescriptorRangeFlags *, ShaderVisibility *>;
} // namespace rootsig
} // namespace hlsl
>From 2d763fe285d44c1165366d5104727e71d9da9f3f Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Fri, 14 Feb 2025 19:03:33 +0000
Subject: [PATCH 09/15] clang formatting
---
.../clang/Parse/ParseHLSLRootSignature.h | 2 +-
clang/lib/Parse/ParseHLSLRootSignature.cpp | 25 +++++++++++--------
2 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index c4fb03a3bc3ed..8ba5ceaceac98 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 e2564228de897..707a37dbad802 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -185,9 +185,7 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
}
// 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> struct ParseMethods : Ts... { using Ts::operator()...; };
template <class... Ts> ParseMethods(Ts...) -> ParseMethods<Ts...>;
bool RootSignatureParser::ParseParam(ParamType Ref) {
@@ -195,12 +193,18 @@ bool RootSignatureParser::ParseParam(ParamType Ref) {
return true;
bool Error;
- std::visit(ParseMethods{
- [&](uint32_t *X) { Error = ParseUInt(X); },
- [&](DescriptorRangeOffset *X) { Error = ParseDescriptorRangeOffset(X); },
- [&](ShaderVisibility *Enum) { Error = ParseShaderVisibility(Enum); },
- [&](DescriptorRangeFlags *Flags) { Error = ParseDescriptorRangeFlags(Flags); },
- }, Ref);
+ std::visit(
+ ParseMethods{
+ [&](uint32_t *X) { Error = ParseUInt(X); },
+ [&](DescriptorRangeOffset *X) {
+ Error = ParseDescriptorRangeOffset(X);
+ },
+ [&](ShaderVisibility *Enum) { Error = ParseShaderVisibility(Enum); },
+ [&](DescriptorRangeFlags *Flags) {
+ Error = ParseDescriptorRangeFlags(Flags);
+ },
+ },
+ Ref);
return Error;
}
@@ -421,8 +425,7 @@ bool RootSignatureParser::ConsumeExpectedToken(
// Report unexpected token kind error
Diags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_unexpected_token_kind)
<< (unsigned)(AnyExpected.size() != 1)
- << FormatTokenKinds({CurToken.Kind})
- << FormatTokenKinds(AnyExpected);
+ << FormatTokenKinds({CurToken.Kind}) << FormatTokenKinds(AnyExpected);
return true;
}
>From 1d078242ac1f025f209d97268473c12a75d13ef7 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Fri, 14 Feb 2025 19:04:17 +0000
Subject: [PATCH 10/15] review: remove unreachable return true
---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 707a37dbad802..e8e637c1b7977 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -89,10 +89,9 @@ bool RootSignatureParser::ParseRootElement() {
switch (CurToken.Kind) {
case TokenKind::kw_DescriptorTable:
return ParseDescriptorTable();
- default:
- llvm_unreachable("Switch for an expected token was not provided");
+ default: break;
}
- return true;
+ llvm_unreachable("Switch for an expected token was not provided");
}
bool RootSignatureParser::ParseDescriptorTable() {
>From f78fdfab5d2ce89b24173c51f8ea56cd086eea85 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Fri, 14 Feb 2025 19:08:24 +0000
Subject: [PATCH 11/15] review: clean-up casting of test values
---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 3 +-
.../Parse/ParseHLSLRootSignatureTest.cpp | 31 +++++++++----------
2 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index e8e637c1b7977..5bbb35875ceb5 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -89,7 +89,8 @@ bool RootSignatureParser::ParseRootElement() {
switch (CurToken.Kind) {
case TokenKind::kw_DescriptorTable:
return ParseDescriptorTable();
- default: break;
+ default:
+ break;
}
llvm_unreachable("Switch for an expected token was not provided");
}
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 8fe57244142b0..224e16bfa372b 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -121,7 +121,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseEmptyTest) {
Consumer->SetNoDiag();
ASSERT_FALSE(Parser.Parse());
- ASSERT_EQ((int)Elements.size(), 0);
+ ASSERT_EQ(Elements.size(), 0u);
ASSERT_TRUE(Consumer->IsSatisfied());
}
@@ -159,9 +159,9 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
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, (uint32_t)0);
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, (uint32_t)1);
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, (uint32_t)0);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.Number, 0u);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, 1u);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, 0u);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Offset,
DescriptorRangeOffset(DescriptorTableOffsetAppend));
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
@@ -172,10 +172,9 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
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,
- (uint32_t)42);
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, (uint32_t)4);
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, (uint32_t)3);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.Number, 42u);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, 4u);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, 3u);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Offset,
DescriptorRangeOffset(32));
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
@@ -186,10 +185,9 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
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,
- (uint32_t)987);
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, (uint32_t)1);
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, (uint32_t)2);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.Number, 987u);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, 1u);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, 2u);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Offset,
DescriptorRangeOffset(DescriptorTableOffsetAppend));
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
@@ -200,10 +198,9 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
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,
- (uint32_t)987234);
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, (uint32_t)1);
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, (uint32_t)0);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Register.Number, 987234u);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).NumDescriptors, 1u);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Space, 0u);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Offset,
DescriptorRangeOffset(DescriptorTableOffsetAppend));
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
@@ -211,7 +208,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
Elem = Elements[4];
ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem));
- ASSERT_EQ(std::get<DescriptorTable>(Elem).NumClauses, (uint32_t)4);
+ ASSERT_EQ(std::get<DescriptorTable>(Elem).NumClauses, 4u);
ASSERT_EQ(std::get<DescriptorTable>(Elem).Visibility,
ShaderVisibility::Pixel);
>From fda500ca8ebe693caa18b434d86ef07c225a90a9 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Fri, 14 Feb 2025 19:18:55 +0000
Subject: [PATCH 12/15] self-review: update doc
---
clang/include/clang/Parse/ParseHLSLRootSignature.h | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 8ba5ceaceac98..89a02d88d6dfc 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -40,7 +40,17 @@ class RootSignatureParser {
DiagnosticsEngine &Diags() { return PP.getDiagnostics(); }
private:
- // Root Element helpers
+ /// All private Parse.* methods follow a similar pattern:
+ /// - Each method will expect that the the next token is of a certain kind
+ /// and will invoke `ConsumeExpectedToken`
+ /// - As such, an error will be raised if the proceeding tokens are not
+ /// what is expected
+ /// - Therefore, it is the callers responsibility to ensure that you are
+ /// expecting the next element type. Or equivalently, the methods should not
+ /// be called as a way to try and parse an element
+ /// - 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
bool ParseRootElement();
bool ParseDescriptorTable();
bool ParseDescriptorTableClause();
>From d99c599a0b144336243a3788d107bc6e057c7504 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Fri, 14 Feb 2025 19:44:36 +0000
Subject: [PATCH 13/15] self-review: move token spellings to definition
---
.../clang/Lex/HLSLRootSignatureTokenKinds.def | 22 ++++++------
.../include/clang/Lex/LexHLSLRootSignature.h | 2 +-
clang/lib/Parse/ParseHLSLRootSignature.cpp | 35 ++-----------------
.../Lex/LexHLSLRootSignatureTest.cpp | 2 +-
4 files changed, 16 insertions(+), 45 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/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 5bbb35875ceb5..40d6921037f9e 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -17,38 +17,9 @@ static std::string FormatTokenKinds(ArrayRef<TokenKind> Kinds) {
if (!First)
Out << ", ";
switch (Kind) {
- case TokenKind::invalid:
- Out << "invalid identifier";
- break;
- case TokenKind::end_of_stream:
- Out << "end of stream";
- break;
- case TokenKind::int_literal:
- Out << "integer literal";
- break;
- case TokenKind::bReg:
- Out << "b register";
- break;
- case TokenKind::tReg:
- Out << "t register";
- break;
- case TokenKind::uReg:
- Out << "u register";
- break;
- case TokenKind::sReg:
- Out << "s register";
- break;
-#define PUNCTUATOR(X, Y) \
- case TokenKind::pu_##X: \
- Out << #Y; \
- break;
-#define KEYWORD(NAME) \
- case TokenKind::kw_##NAME: \
- Out << #NAME; \
- break;
-#define ENUM(NAME, LIT) \
- case TokenKind::en_##NAME: \
- Out << LIT; \
+#define TOK(X, SPELLING) \
+ case TokenKind::X: \
+ Out << SPELLING; \
break;
#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 3146ebad1269b5580d5e2fd268c6dc5075aaff6c Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 5 Mar 2025 18:17:45 +0000
Subject: [PATCH 14/15] review: add liscence
---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 40d6921037f9e..b68bc935f3581 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -1,3 +1,11 @@
+//=== ParseHLSLRootSignature.cpp - Parse Root Signature -------------------===//
+//
+// 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/Parse/ParseHLSLRootSignature.h"
#include "clang/Lex/LiteralSupport.h"
>From f457b38e8bc4bc342e12e81fb4ad1123b92b97f5 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 5 Mar 2025 22:20:52 +0000
Subject: [PATCH 15/15] review prototype: allow for more context to
ConsumeNextToken error
---
.../clang/Basic/DiagnosticParseKinds.td | 2 +-
.../clang/Parse/ParseHLSLRootSignature.h | 8 ++-
clang/lib/Parse/ParseHLSLRootSignature.cpp | 70 +++++++++++++------
.../Parse/ParseHLSLRootSignatureTest.cpp | 6 +-
4 files changed, 58 insertions(+), 28 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index fe1b0dd0b91cb..f2a5eaba48952 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1819,7 +1819,7 @@ def err_hlsl_unsupported_component : Error<"invalid component '%0' used; expecte
def err_hlsl_packoffset_invalid_reg : Error<"invalid resource class specifier '%0' for packoffset, expected 'c'">;
// HLSL Root Signature Parser Diagnostics
-def err_hlsl_rootsig_unexpected_token_kind : Error<"got %1 but expected %select{|one of}0 the following token kind%select{|s}0: %2">;
+def err_hlsl_expected : Error<"expected %0 as %1">;
def err_hlsl_number_literal_overflow : Error<"integer literal is too large to be represented as a 32-bit %select{signed |}0 integer type">;
def err_hlsl_rootsig_repeat_param : Error<"specified the same parameter '%0' multiple times">;
def err_hlsl_rootsig_non_zero_flag : Error<"specified a non-zero integer as a flag">;
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 89a02d88d6dfc..9d141b0ebfd4f 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -114,8 +114,12 @@ class RootSignatureParser {
/// kind.
///
/// Returns true if there was an error reported.
- bool ConsumeExpectedToken(TokenKind Expected);
- bool ConsumeExpectedToken(ArrayRef<TokenKind> AnyExpected);
+ bool ConsumeExpectedToken(TokenKind Expected,
+ unsigned DiagID = diag::err_expected,
+ StringRef DiagMsg = "");
+ bool ConsumeExpectedToken(ArrayRef<TokenKind> AnyExpected,
+ unsigned DiagID = diag::err_expected,
+ StringRef DiagMsg = "");
/// Peek if the next token is of the expected kind and if it is then consume
/// it.
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index b68bc935f3581..732d37d310750 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -53,7 +53,8 @@ bool RootSignatureParser::Parse() {
while (!ParseRootElement()) {
if (Lexer.EndOfBuffer())
return false;
- if (ConsumeExpectedToken(TokenKind::pu_comma))
+ if (ConsumeExpectedToken(TokenKind::pu_comma, diag::err_expected_either,
+ "end of root signature string"))
return true;
}
@@ -61,7 +62,8 @@ bool RootSignatureParser::Parse() {
}
bool RootSignatureParser::ParseRootElement() {
- if (ConsumeExpectedToken(TokenKind::kw_DescriptorTable))
+ if (ConsumeExpectedToken(TokenKind::kw_DescriptorTable,
+ diag::err_hlsl_expected, "root element"))
return true;
// Dispatch onto the correct parse method
@@ -77,7 +79,8 @@ bool RootSignatureParser::ParseRootElement() {
bool RootSignatureParser::ParseDescriptorTable() {
DescriptorTable Table;
- if (ConsumeExpectedToken(TokenKind::pu_l_paren))
+ if (ConsumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
+ "DescriptorTable"))
return true;
// Empty case:
@@ -108,7 +111,8 @@ bool RootSignatureParser::ParseDescriptorTable() {
Table.NumClauses++;
} while (TryConsumeExpectedToken(TokenKind::pu_comma));
- if (ConsumeExpectedToken(TokenKind::pu_r_paren))
+ if (ConsumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after,
+ "descriptor table clauses"))
return true;
Elements.push_back(Table);
@@ -117,7 +121,8 @@ bool RootSignatureParser::ParseDescriptorTable() {
bool RootSignatureParser::ParseDescriptorTableClause() {
if (ConsumeExpectedToken({TokenKind::kw_CBV, TokenKind::kw_SRV,
- TokenKind::kw_UAV, TokenKind::kw_Sampler}))
+ TokenKind::kw_UAV, TokenKind::kw_Sampler},
+ diag::err_hlsl_expected, "descriptor table clause"))
return true;
DescriptorTableClause Clause;
@@ -139,7 +144,8 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
}
Clause.SetDefaultFlags();
- if (ConsumeExpectedToken(TokenKind::pu_l_paren))
+ if (ConsumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
+ FormatTokenKinds({CurToken.Kind})))
return true;
// Consume mandatory Register paramater
@@ -156,7 +162,8 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
if (ParseOptionalParams({RefMap}))
return true;
- if (ConsumeExpectedToken(TokenKind::pu_r_paren))
+ if (ConsumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after,
+ "clause parameters"))
return true;
Elements.push_back(Clause);
@@ -168,7 +175,8 @@ template <class... Ts> struct ParseMethods : Ts... { using Ts::operator()...; };
template <class... Ts> ParseMethods(Ts...) -> ParseMethods<Ts...>;
bool RootSignatureParser::ParseParam(ParamType Ref) {
- if (ConsumeExpectedToken(TokenKind::pu_equal))
+ if (ConsumeExpectedToken(TokenKind::pu_equal, diag::err_expected_after,
+ FormatTokenKinds(CurToken.Kind)))
return true;
bool Error;
@@ -198,7 +206,8 @@ bool RootSignatureParser::ParseOptionalParams(
llvm::SmallDenseSet<TokenKind> Seen;
while (TryConsumeExpectedToken(TokenKind::pu_comma)) {
- if (ConsumeExpectedToken(ParamKeywords))
+ if (ConsumeExpectedToken(ParamKeywords, diag::err_hlsl_expected,
+ "optional parameter"))
return true;
TokenKind ParamKind = CurToken.Kind;
@@ -241,7 +250,8 @@ bool RootSignatureParser::HandleUIntLiteral(uint32_t &X) {
bool RootSignatureParser::ParseRegister(Register *Register) {
if (ConsumeExpectedToken(
- {TokenKind::bReg, TokenKind::tReg, TokenKind::uReg, TokenKind::sReg}))
+ {TokenKind::bReg, TokenKind::tReg, TokenKind::uReg, TokenKind::sReg},
+ diag::err_hlsl_expected, "a register"))
return true;
switch (CurToken.Kind) {
@@ -270,7 +280,8 @@ bool RootSignatureParser::ParseRegister(Register *Register) {
bool RootSignatureParser::ParseUInt(uint32_t *X) {
// Treat a postively signed integer as though it is unsigned to match DXC
TryConsumeExpectedToken(TokenKind::pu_plus);
- if (ConsumeExpectedToken(TokenKind::int_literal))
+ if (ConsumeExpectedToken(TokenKind::int_literal, diag::err_hlsl_expected,
+ "unsigned integer"))
return true;
if (HandleUIntLiteral(*X))
@@ -281,7 +292,8 @@ bool RootSignatureParser::ParseUInt(uint32_t *X) {
bool RootSignatureParser::ParseDescriptorRangeOffset(DescriptorRangeOffset *X) {
if (ConsumeExpectedToken(
- {TokenKind::int_literal, TokenKind::en_DescriptorRangeOffsetAppend}))
+ {TokenKind::int_literal, TokenKind::en_DescriptorRangeOffsetAppend},
+ diag::err_hlsl_expected, "descriptor range offset"))
return true;
// Edge case for the offset enum -> static value
@@ -307,7 +319,8 @@ bool RootSignatureParser::ParseEnum(
EnumToks.push_back(EnumPair.first);
// If invoked we expect to have an enum
- if (ConsumeExpectedToken(EnumToks))
+ if (ConsumeExpectedToken(EnumToks, diag::err_hlsl_expected,
+ "parameter value"))
return true;
// Handle the edge case when '0' is used to specify None
@@ -391,20 +404,33 @@ bool RootSignatureParser::PeekExpectedToken(ArrayRef<TokenKind> AnyExpected) {
return IsExpectedToken(Result.Kind, AnyExpected);
}
-bool RootSignatureParser::ConsumeExpectedToken(TokenKind Expected) {
- return ConsumeExpectedToken(ArrayRef{Expected});
+bool RootSignatureParser::ConsumeExpectedToken(TokenKind Expected,
+ unsigned DiagID,
+ StringRef DiagMsg) {
+ return ConsumeExpectedToken(ArrayRef{Expected}, DiagID, DiagMsg);
}
-bool RootSignatureParser::ConsumeExpectedToken(
- ArrayRef<TokenKind> AnyExpected) {
- ConsumeNextToken();
- if (IsExpectedToken(CurToken.Kind, AnyExpected))
+bool RootSignatureParser::ConsumeExpectedToken(ArrayRef<TokenKind> AnyExpected,
+ unsigned DiagID,
+ StringRef DiagMsg) {
+ if (TryConsumeExpectedToken(AnyExpected))
return false;
// Report unexpected token kind error
- Diags().Report(CurToken.TokLoc, diag::err_hlsl_rootsig_unexpected_token_kind)
- << (unsigned)(AnyExpected.size() != 1)
- << FormatTokenKinds({CurToken.Kind}) << FormatTokenKinds(AnyExpected);
+ DiagnosticBuilder DB = Diags().Report(CurToken.TokLoc, DiagID);
+ switch (DiagID) {
+ case diag::err_expected:
+ DB << FormatTokenKinds(AnyExpected);
+ break;
+ case diag::err_hlsl_expected:
+ case diag::err_expected_either:
+ case diag::err_expected_after:
+ DB << FormatTokenKinds(AnyExpected) << DiagMsg;
+ break;
+ default:
+ DB << DiagMsg;
+ break;
+ }
return true;
}
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 224e16bfa372b..35ad4bfff14d0 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -238,7 +238,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedTokenTest) {
hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
// Test correct diagnostic produced
- Consumer->SetExpected(diag::err_hlsl_rootsig_unexpected_token_kind);
+ Consumer->SetExpected(diag::err_expected_either);
ASSERT_TRUE(Parser.Parse());
ASSERT_TRUE(Consumer->IsSatisfied());
@@ -258,7 +258,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseInvalidTokenTest) {
hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
// Test correct diagnostic produced - invalid token
- Consumer->SetExpected(diag::err_hlsl_rootsig_unexpected_token_kind);
+ Consumer->SetExpected(diag::err_hlsl_expected);
ASSERT_TRUE(Parser.Parse());
ASSERT_TRUE(Consumer->IsSatisfied());
@@ -278,7 +278,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedEndOfStreamTest) {
hlsl::RootSignatureParser Parser(Elements, Lexer, *PP);
// Test correct diagnostic produced - end of stream
- Consumer->SetExpected(diag::err_hlsl_rootsig_unexpected_token_kind);
+ Consumer->SetExpected(diag::err_hlsl_expected);
ASSERT_TRUE(Parser.Parse());
ASSERT_TRUE(Consumer->IsSatisfied());
More information about the cfe-commits
mailing list