[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Implement Parsing of Descriptor Tables (PR #122982)
Finn Plummer via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Wed Feb 12 09:31:36 PST 2025
https://github.com/inbelic updated https://github.com/llvm/llvm-project/pull/122982
>From 479dcca55ea4f494751ad85d023e6efeb6e6c35e Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Tue, 28 Jan 2025 17:49:21 +0000
Subject: [PATCH 01/22] [HLSL][RootSignature] Handle an empty root signature
- Define the Parser struct
- Model RootElements as a variant of the different types
- Create a basic test case for unit testing
---
.../clang/Parse/ParseHLSLRootSignature.h | 26 ++++++++++++++++
clang/lib/Parse/ParseHLSLRootSignature.cpp | 20 +++++++++++++
.../Parse/ParseHLSLRootSignatureTest.cpp | 26 ++++++++++++++++
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 30 +++++++++++++++++++
4 files changed, 102 insertions(+)
create mode 100644 llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 15898f8c24bba..8a3fbc1d3e25b 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -22,9 +22,13 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSwitch.h"
+#include "llvm/Frontend/HLSL/HLSLRootSignature.h"
+
namespace clang {
namespace hlsl {
+namespace rs = llvm::hlsl::root_signature;
+
struct RootSignatureToken {
enum Kind {
#define TOK(X) X,
@@ -93,6 +97,28 @@ class RootSignatureLexer {
}
};
+class RootSignatureParser {
+public:
+ RootSignatureParser(SmallVector<rs::RootElement> &Elements,
+ const SmallVector<RootSignatureToken> &Tokens,
+ DiagnosticsEngine &Diags);
+
+ // Iterates over the provided tokens and constructs the in-memory
+ // representations of the RootElements.
+ //
+ // The return value denotes if there was a failure and the method will
+ // return on the first encountered failure, or, return false if it
+ // can sucessfully reach the end of the tokens.
+ bool Parse();
+
+private:
+ SmallVector<rs::RootElement> &Elements;
+ SmallVector<RootSignatureToken>::const_iterator CurTok;
+ SmallVector<RootSignatureToken>::const_iterator LastTok;
+
+ DiagnosticsEngine &Diags;
+};
+
} // namespace hlsl
} // namespace clang
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 1f556523c6334..8a73c442a3ea9 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -1,5 +1,7 @@
#include "clang/Parse/ParseHLSLRootSignature.h"
+using namespace llvm::hlsl::root_signature;
+
namespace clang {
namespace hlsl {
@@ -162,5 +164,23 @@ std::optional<RootSignatureToken> RootSignatureLexer::PeekNextToken() {
return Result;
}
+// Parser Definitions
+
+RootSignatureParser::RootSignatureParser(
+ SmallVector<RootElement> &Elements,
+ const SmallVector<RootSignatureToken> &Tokens, DiagnosticsEngine &Diags)
+ : Elements(Elements), Diags(Diags) {
+ CurTok = Tokens.begin();
+ LastTok = Tokens.end();
+}
+
+bool RootSignatureParser::Parse() {
+ // Handle edge-case of empty RootSignature()
+ if (CurTok == LastTok)
+ return false;
+
+ return true;
+}
+
} // namespace hlsl
} // namespace clang
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index c5006b59575b6..199f36cb46520 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -24,6 +24,7 @@
#include "gtest/gtest.h"
using namespace clang;
+using namespace llvm::hlsl::root_signature;
namespace {
@@ -306,4 +307,29 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidLexRegNumberTest) {
ASSERT_FALSE(Consumer->IsSatisfied());
}
+// Valid Parser Tests
+
+TEST_F(ParseHLSLRootSignatureTest, ValidParseEmptyTest) {
+ const llvm::StringLiteral Source = R"cc()cc";
+
+ TrivialModuleLoader ModLoader;
+ auto PP = CreatePP(Source, ModLoader);
+ auto TokLoc = SourceLocation();
+
+ // Test no diagnostics produced
+ Consumer->SetNoDiag();
+ hlsl::RootSignatureLexer Lexer(Source, TokLoc, *PP);
+
+ SmallVector<hlsl::RootSignatureToken> Tokens;
+ ASSERT_FALSE(Lexer.Lex(Tokens));
+
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(Elements, Tokens, Diags);
+
+ ASSERT_FALSE(Parser.Parse());
+ ASSERT_EQ((int)Elements.size(), 0);
+
+ 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..4c196d29a01bb
--- /dev/null
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -0,0 +1,30 @@
+//===- 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 root_signature {
+
+// Models RootElement
+using RootElement = std::variant<std::monostate>;
+
+} // namespace root_signature
+} // namespace hlsl
+} // namespace llvm
+
+#endif // LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H
>From 4772ecc671843cf7d885215e47903a73edebccd5 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Tue, 28 Jan 2025 18:04:00 +0000
Subject: [PATCH 02/22] add support for an empty descriptor table
---
.../clang/Parse/ParseHLSLRootSignature.h | 21 +++++
clang/lib/Parse/ParseHLSLRootSignature.cpp | 83 +++++++++++++++++++
.../Parse/ParseHLSLRootSignatureTest.cpp | 30 +++++++
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 11 ++-
4 files changed, 143 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 8a3fbc1d3e25b..a235a74765184 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -111,6 +111,27 @@ class RootSignatureParser {
// can sucessfully reach the end of the tokens.
bool Parse();
+private:
+ // Root Element helpers
+ bool ParseRootElement(bool First);
+ bool ParseDescriptorTable();
+
+ // Increment the token iterator if we have not reached the end.
+ // Return value denotes if we were already at the last token.
+ bool ConsumeNextToken();
+
+ // Is the current token one of the expected kinds
+ bool EnsureExpectedToken(TokenKind AnyExpected);
+ bool EnsureExpectedToken(ArrayRef<TokenKind> AnyExpected);
+
+ // Consume the next token and report an error if it is not of the expected
+ // kind.
+ //
+ // Return value denotes if it failed to match the expected kind, either it is
+ // the end of the stream or it didn't match any of the expected kinds.
+ bool ConsumeExpectedToken(TokenKind Expected);
+ bool ConsumeExpectedToken(ArrayRef<TokenKind> AnyExpected);
+
private:
SmallVector<rs::RootElement> &Elements;
SmallVector<RootSignatureToken>::const_iterator CurTok;
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 8a73c442a3ea9..a6267b4431902 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -179,7 +179,90 @@ bool RootSignatureParser::Parse() {
if (CurTok == LastTok)
return false;
+ bool First = true;
+ // Iterate as many RootElements as possible
+ while (!ParseRootElement(First)) {
+ First = false;
+ // Avoid use of ConsumeNextToken here to skip incorrect end of tokens error
+ CurTok++;
+ if (CurTok == LastTok)
+ return false;
+ if (EnsureExpectedToken(TokenKind::pu_comma))
+ return true;
+ }
+
+ return true;
+}
+
+bool RootSignatureParser::ParseRootElement(bool First) {
+ if (First && EnsureExpectedToken(TokenKind::kw_DescriptorTable))
+ return true;
+ if (!First && ConsumeExpectedToken(TokenKind::kw_DescriptorTable))
+ return true;
+
+ // Dispatch onto the correct parse method
+ switch (CurTok->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;
+
+}
+
+bool RootSignatureParser::ConsumeNextToken() {
+ CurTok++;
+ if (LastTok == CurTok) {
+ return true;
+ }
+ return false;
+}
+
+// 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::EnsureExpectedToken(TokenKind Expected) {
+ return EnsureExpectedToken(ArrayRef{Expected});
+}
+
+bool RootSignatureParser::EnsureExpectedToken(ArrayRef<TokenKind> AnyExpected) {
+ if (IsExpectedToken(CurTok->Kind, AnyExpected))
+ return false;
+
+ return true;
+}
+
+bool RootSignatureParser::ConsumeExpectedToken(TokenKind Expected) {
+ return ConsumeExpectedToken(ArrayRef{Expected});
+}
+
+bool RootSignatureParser::ConsumeExpectedToken(
+ ArrayRef<TokenKind> AnyExpected) {
+ if (ConsumeNextToken())
+ return true;
+
+ return EnsureExpectedToken(AnyExpected);
}
} // namespace hlsl
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 199f36cb46520..f2d723abea94f 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -332,4 +332,34 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseEmptyTest) {
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, *PP);
+
+ SmallVector<hlsl::RootSignatureToken> Tokens;
+
+ // Test no diagnostics produced
+ Consumer->SetNoDiag();
+ ASSERT_FALSE(Lexer.Lex(Tokens));
+
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(Elements, Tokens, Diags);
+
+ 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, (uint32_t)0);
+
+ 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 4c196d29a01bb..8f844d672fd55 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -20,8 +20,15 @@ namespace llvm {
namespace hlsl {
namespace root_signature {
-// Models RootElement
-using RootElement = std::variant<std::monostate>;
+// 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 root_signature
} // namespace hlsl
>From 17c2618218c9d3893758e7ad07ce2580512a5f26 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Tue, 28 Jan 2025 18:16:43 +0000
Subject: [PATCH 03/22] add diagnostics to methods
---
.../clang/Basic/DiagnosticParseKinds.td | 3 +
.../clang/Parse/ParseHLSLRootSignature.h | 1 +
clang/lib/Parse/ParseHLSLRootSignature.cpp | 55 ++++++++++++++++++-
.../Parse/ParseHLSLRootSignatureTest.cpp | 50 +++++++++++++++++
4 files changed, 108 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 86fcae209c40d..52c3fcce14076 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1806,4 +1806,7 @@ 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_eos : Error<"unexpected end to token stream">;
+def err_hlsl_rootsig_unexpected_token_kind : Error<"expected the %select{following|one of the following}0 token kinds '%1'">;
} // end of Parser diagnostics
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index a235a74765184..47a63df311e63 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -15,6 +15,7 @@
#include "clang/AST/APValue.h"
#include "clang/Basic/DiagnosticLex.h"
+#include "clang/Basic/DiagnosticParse.h"
#include "clang/Lex/LiteralSupport.h"
#include "clang/Lex/Preprocessor.h"
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index a6267b4431902..9d434f481fda7 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -1,10 +1,59 @@
#include "clang/Parse/ParseHLSLRootSignature.h"
+#include "llvm/Support/raw_ostream.h"
+
using namespace llvm::hlsl::root_signature;
namespace clang {
namespace hlsl {
+// Helper definitions
+
+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:
+ 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/Parse/HLSLRootSignatureTokenKinds.def"
+ }
+ First = false;
+ }
+
+ return TokenString;
+}
+
// Lexer Definitions
static bool IsNumberChar(char C) {
@@ -223,12 +272,13 @@ bool RootSignatureParser::ParseDescriptorTable() {
}
return true;
-
}
bool RootSignatureParser::ConsumeNextToken() {
CurTok++;
if (LastTok == CurTok) {
+ // Report unexpected end of tokens error
+ Diags.Report(CurTok->TokLoc, diag::err_hlsl_rootsig_unexpected_eos);
return true;
}
return false;
@@ -250,6 +300,9 @@ bool RootSignatureParser::EnsureExpectedToken(ArrayRef<TokenKind> AnyExpected) {
if (IsExpectedToken(CurTok->Kind, AnyExpected))
return false;
+ // Report unexpected token kind error
+ Diags.Report(CurTok->TokLoc, diag::err_hlsl_rootsig_unexpected_token_kind)
+ << (unsigned)(AnyExpected.size() != 1) << FormatTokenKinds(AnyExpected);
return true;
}
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index f2d723abea94f..3970f6948677b 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -358,6 +358,56 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
// Test generated DescriptorTable start has correct default values
ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem));
ASSERT_EQ(std::get<DescriptorTable>(Elem).NumClauses, (uint32_t)0);
+ ASSERT_TRUE(Consumer->IsSatisfied());
+}
+
+// Invalid Parser Tests
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedEOSTest) {
+ const llvm::StringLiteral Source = R"cc(
+ DescriptorTable
+ )cc";
+
+ TrivialModuleLoader ModLoader;
+ auto PP = CreatePP(Source, ModLoader);
+ auto TokLoc = SourceLocation();
+
+ // Test correct diagnostic produced
+ Consumer->SetExpected(diag::err_hlsl_rootsig_unexpected_eos);
+ hlsl::RootSignatureLexer Lexer(Source, TokLoc, *PP);
+
+ SmallVector<hlsl::RootSignatureToken> Tokens;
+ ASSERT_FALSE(Lexer.Lex(Tokens));
+
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(Elements, Tokens, Diags);
+
+ ASSERT_TRUE(Parser.Parse());
+
+ ASSERT_TRUE(Consumer->IsSatisfied());
+}
+
+TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedTokenTest) {
+ const llvm::StringLiteral Source = R"cc(
+ DescriptorTable()
+ space
+ )cc";
+
+ TrivialModuleLoader ModLoader;
+ auto PP = CreatePP(Source, ModLoader);
+ auto TokLoc = SourceLocation();
+
+ // Test correct diagnostic produced
+ Consumer->SetExpected(diag::err_hlsl_rootsig_unexpected_token_kind);
+ hlsl::RootSignatureLexer Lexer(Source, TokLoc, *PP);
+
+ SmallVector<hlsl::RootSignatureToken> Tokens;
+ ASSERT_FALSE(Lexer.Lex(Tokens));
+
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(Elements, Tokens, Diags);
+
+ ASSERT_TRUE(Parser.Parse());
ASSERT_TRUE(Consumer->IsSatisfied());
}
>From c873b3265d8d4f5c7795bbbf00686166d768b6cc Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Tue, 28 Jan 2025 18:37:42 +0000
Subject: [PATCH 04/22] add support for empty descriptor table clause
---
.../clang/Parse/ParseHLSLRootSignature.h | 22 +++++
clang/lib/Parse/ParseHLSLRootSignature.cpp | 80 ++++++++++++++++++-
.../Parse/ParseHLSLRootSignatureTest.cpp | 25 ++++++
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 11 ++-
4 files changed, 134 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 47a63df311e63..ec2a9ec0f8d27 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -116,15 +116,28 @@ class RootSignatureParser {
// Root Element helpers
bool ParseRootElement(bool First);
bool ParseDescriptorTable();
+ bool ParseDescriptorTableClause();
+ // Helper dispatch method
// Increment the token iterator if we have not reached the end.
// Return value denotes if we were already at the last token.
bool ConsumeNextToken();
+ // Attempt to retrieve the next token, if TokenKind is invalid then there was
+ // no next token.
+ RootSignatureToken PeekNextToken();
+
// Is the current token one of the expected kinds
bool EnsureExpectedToken(TokenKind AnyExpected);
bool EnsureExpectedToken(ArrayRef<TokenKind> AnyExpected);
+ // Peek if the next token is of the expected kind.
+ //
+ // Return value denotes if it failed to match the expected kind, either it is
+ // the end of the stream or it didn't match any of the expected kinds.
+ bool PeekExpectedToken(TokenKind Expected);
+ bool PeekExpectedToken(ArrayRef<TokenKind> AnyExpected);
+
// Consume the next token and report an error if it is not of the expected
// kind.
//
@@ -133,6 +146,15 @@ 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.
+ //
+ // Return value denotes if it failed to match the expected kind, either it is
+ // the end of the stream or it didn't match any of the expected kinds. It will
+ // not report an error if there isn't a match.
+ bool TryConsumeExpectedToken(TokenKind Expected);
+ bool TryConsumeExpectedToken(ArrayRef<TokenKind> Expected);
+
private:
SmallVector<rs::RootElement> &Elements;
SmallVector<RootSignatureToken>::const_iterator CurTok;
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 9d434f481fda7..1bc539558e4d8 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -266,12 +266,64 @@ 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 (CurTok->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");
+ return true;
+ }
+ if (ConsumeExpectedToken(TokenKind::pu_l_paren))
+ return true;
+
+ if (ConsumeExpectedToken(TokenKind::pu_r_paren))
+ return true;
+
+ Elements.push_back(Clause);
+ return false;
+}
+
+RootSignatureToken RootSignatureParser::PeekNextToken() {
+ // Create an invalid token
+ RootSignatureToken Token = RootSignatureToken(SourceLocation());
+ if (CurTok != LastTok)
+ Token = *(CurTok + 1);
+ return Token;
}
bool RootSignatureParser::ConsumeNextToken() {
@@ -306,6 +358,19 @@ bool RootSignatureParser::EnsureExpectedToken(ArrayRef<TokenKind> AnyExpected) {
return true;
}
+bool RootSignatureParser::PeekExpectedToken(TokenKind Expected) {
+ return PeekExpectedToken(ArrayRef{Expected});
+}
+
+bool RootSignatureParser::PeekExpectedToken(ArrayRef<TokenKind> AnyExpected) {
+ RootSignatureToken Token = PeekNextToken();
+ if (Token.Kind == TokenKind::invalid)
+ return true;
+ if (IsExpectedToken(Token.Kind, AnyExpected))
+ return false;
+ return true;
+}
+
bool RootSignatureParser::ConsumeExpectedToken(TokenKind Expected) {
return ConsumeExpectedToken(ArrayRef{Expected});
}
@@ -318,5 +383,16 @@ bool RootSignatureParser::ConsumeExpectedToken(
return EnsureExpectedToken(AnyExpected);
}
+bool RootSignatureParser::TryConsumeExpectedToken(TokenKind Expected) {
+ return TryConsumeExpectedToken(ArrayRef{Expected});
+}
+
+bool RootSignatureParser::TryConsumeExpectedToken(
+ ArrayRef<TokenKind> AnyExpected) {
+ if (PeekExpectedToken(AnyExpected))
+ return true;
+ return ConsumeNextToken();
+}
+
} // namespace hlsl
} // namespace clang
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 3970f6948677b..ebea292065230 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -334,6 +334,12 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseEmptyTest) {
TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
const llvm::StringLiteral Source = R"cc(
+ DescriptorTable(
+ CBV(),
+ SRV(),
+ Sampler(),
+ UAV()
+ ),
DescriptorTable()
)cc";
@@ -354,7 +360,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, (uint32_t)0);
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 8f844d672fd55..9b7916107c1f1 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 root_signature
} // namespace hlsl
>From 0305544288edd623c011a3e027f05f0f7d7d1f3a Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Tue, 28 Jan 2025 18:52:04 +0000
Subject: [PATCH 05/22] add support for parsing registers
---
.../clang/Parse/ParseHLSLRootSignature.h | 2 ++
clang/lib/Parse/ParseHLSLRootSignature.cpp | 31 +++++++++++++++++++
.../Parse/ParseHLSLRootSignatureTest.cpp | 23 +++++++++++---
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 8 +++++
4 files changed, 60 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index ec2a9ec0f8d27..876fefe9453c9 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -119,6 +119,8 @@ class RootSignatureParser {
bool ParseDescriptorTableClause();
// Helper dispatch method
+ bool ParseRegister(rs::Register *Reg);
+
// Increment the token iterator if we have not reached the end.
// Return value denotes if we were already at the last token.
bool ConsumeNextToken();
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 1bc539558e4d8..a7823f71515e1 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -311,6 +311,13 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
if (ConsumeExpectedToken(TokenKind::pu_l_paren))
return true;
+ // Consume mandatory Register paramater
+ if (ConsumeExpectedToken(
+ {TokenKind::bReg, TokenKind::tReg, TokenKind::uReg, TokenKind::sReg}))
+ return true;
+ if (ParseRegister(&Clause.Register))
+ return true;
+
if (ConsumeExpectedToken(TokenKind::pu_r_paren))
return true;
@@ -318,6 +325,30 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
return false;
}
+bool RootSignatureParser::ParseRegister(Register *Register) {
+ switch (CurTok->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");
+ return true;
+ }
+
+ Register->Number = CurTok->NumLiteral.getInt().getExtValue();
+
+ return false;
+}
+
RootSignatureToken RootSignatureParser::PeekNextToken() {
// Create an invalid token
RootSignatureToken Token = RootSignatureToken(SourceLocation());
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index ebea292065230..455c1dee01d0f 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -335,10 +335,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";
@@ -362,18 +362,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));
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 9b7916107c1f1..7779fd2e803ac 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -23,6 +23,13 @@ namespace root_signature {
// 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 742e62c89f6c255ad9a351348064551c5971baa1 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 06/22] add support for optional parameters
- use numDescriptors as an example
---
.../clang/Parse/ParseHLSLRootSignature.h | 15 +++++
clang/lib/Parse/ParseHLSLRootSignature.cpp | 58 +++++++++++++++++++
.../Parse/ParseHLSLRootSignatureTest.cpp | 6 +-
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 4 ++
4 files changed, 82 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 876fefe9453c9..e72bc10b03825 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -119,7 +119,22 @@ class RootSignatureParser {
bool ParseDescriptorTableClause();
// Helper dispatch method
+ //
+ // These will switch on the Variant kind to dispatch to the respective Parse
+ // method and store the parsed value back into Ref.
+ //
+ // 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
+ bool ParseParam(rs::ParamType Ref);
+
+ // Parse as many optional parameters as possible in any order
+ bool
+ ParseOptionalParams(llvm::SmallDenseMap<TokenKind, rs::ParamType> RefMap);
+
+ // Common parsing helpers
bool ParseRegister(rs::Register *Reg);
+ bool ParseUInt(uint32_t *X);
// Increment the token iterator if we have not reached the end.
// Return value denotes if we were already at the last token.
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index a7823f71515e1..4eb520f4107fb 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -318,6 +318,13 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
if (ParseRegister(&Clause.Register))
return true;
+ // Parse optional paramaters
+ llvm::SmallDenseMap<TokenKind, rs::ParamType> RefMap = {
+ {TokenKind::kw_numDescriptors, &Clause.NumDescriptors},
+ };
+ if (ParseOptionalParams({RefMap}))
+ return true;
+
if (ConsumeExpectedToken(TokenKind::pu_r_paren))
return true;
@@ -325,6 +332,57 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
return false;
}
+// Helper struct so that we can use the overloaded notation of std::visit
+template <class... Ts> struct OverloadedMethods : Ts... {
+ using Ts::operator()...;
+};
+template <class... Ts> OverloadedMethods(Ts...) -> OverloadedMethods<Ts...>;
+
+bool RootSignatureParser::ParseParam(ParamType Ref) {
+ if (ConsumeExpectedToken(TokenKind::pu_equal))
+ return true;
+
+ bool Error;
+ std::visit(OverloadedMethods{[&](uint32_t *X) { Error = ParseUInt(X); },
+ }, Ref);
+
+ return Error;
+}
+
+bool RootSignatureParser::ParseOptionalParams(
+ llvm::SmallDenseMap<TokenKind, rs::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 = CurTok->Kind;
+ if (Seen.contains(ParamKind)) {
+ return true;
+ }
+ Seen.insert(ParamKind);
+
+ if (ParseParam(RefMap[ParamKind]))
+ return true;
+ }
+
+ return false;
+}
+
+bool RootSignatureParser::ParseUInt(uint32_t *X) {
+ if (ConsumeExpectedToken(TokenKind::int_literal))
+ return true;
+
+ *X = CurTok->NumLiteral.getInt().getExtValue();
+ return false;
+}
+
bool RootSignatureParser::ParseRegister(Register *Register) {
switch (CurTok->Kind) {
case TokenKind::bReg:
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 455c1dee01d0f..83ebef72fa6bf 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -336,7 +336,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
const llvm::StringLiteral Source = R"cc(
DescriptorTable(
CBV(b0),
- SRV(t42),
+ SRV(t42, numDescriptors = 4),
Sampler(s987),
UAV(u987234)
),
@@ -365,6 +365,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));
@@ -373,6 +374,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));
@@ -381,6 +383,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));
@@ -389,6 +392,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));
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 7779fd2e803ac..8f5655681c7b7 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -40,11 +40,15 @@ 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 root_signature
} // namespace hlsl
} // namespace llvm
>From c8a1e3826aa2e9bc70892f40c906db96f615ca6f Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Tue, 28 Jan 2025 19:19:33 +0000
Subject: [PATCH 07/22] add diagnostic for repeated parametr
---
.../clang/Basic/DiagnosticParseKinds.td | 1 +
clang/lib/Parse/ParseHLSLRootSignature.cpp | 2 ++
.../Parse/ParseHLSLRootSignatureTest.cpp | 26 +++++++++++++++++++
3 files changed, 29 insertions(+)
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 52c3fcce14076..4994d2339ce57 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1809,4 +1809,5 @@ def err_hlsl_packoffset_invalid_reg : Error<"invalid resource class specifier '%
// HLSL Root Signature Parser Diagnostics
def err_hlsl_rootsig_unexpected_eos : Error<"unexpected end to token stream">;
def err_hlsl_rootsig_unexpected_token_kind : Error<"expected the %select{following|one of the following}0 token kinds '%1'">;
+def err_hlsl_rootsig_repeat_param : Error<"specified the same parameter '%0' multiple times">;
} // end of Parser diagnostics
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 4eb520f4107fb..096f541a17c55 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -364,6 +364,8 @@ bool RootSignatureParser::ParseOptionalParams(
TokenKind ParamKind = CurTok->Kind;
if (Seen.contains(ParamKind)) {
+ Diags.Report(CurTok->TokLoc, diag::err_hlsl_rootsig_repeat_param)
+ << FormatTokenKinds(ParamKind);
return true;
}
Seen.insert(ParamKind);
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 83ebef72fa6bf..74b57cd891faf 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -456,4 +456,30 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedTokenTest) {
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();
+
+ // Test correct diagnostic produced
+ Consumer->SetExpected(diag::err_hlsl_rootsig_repeat_param);
+ hlsl::RootSignatureLexer Lexer(Source, TokLoc, *PP);
+
+ SmallVector<hlsl::RootSignatureToken> Tokens;
+ ASSERT_FALSE(Lexer.Lex(Tokens));
+
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(Elements, Tokens, Diags);
+
+ ASSERT_TRUE(Parser.Parse());
+
+ ASSERT_TRUE(Consumer->IsSatisfied());
+}
+
} // anonymous namespace
>From 75271b57054125966b5571b87c3b801d77fd1013 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 08/22] 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 096f541a17c55..1e8b027742c57 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -321,6 +321,7 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
// Parse optional paramaters
llvm::SmallDenseMap<TokenKind, rs::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 74b57cd891faf..c35590406794d 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -336,8 +336,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()
@@ -366,6 +366,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));
@@ -375,6 +376,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));
@@ -384,6 +386,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));
@@ -393,6 +396,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 8f5655681c7b7..608e79494fa64 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 e8e074a769c37d2ab06db6e46b5409a9d2f9a679 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 09/22] add support for custom parameter parsing -
DescriptorRangeOffset
---
.../clang/Parse/ParseHLSLRootSignature.h | 1 +
clang/lib/Parse/ParseHLSLRootSignature.cpp | 19 +++++++++++++++++++
.../Parse/ParseHLSLRootSignatureTest.cpp | 12 ++++++++++--
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 9 ++++++++-
4 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index e72bc10b03825..89d3c89bd3b45 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -135,6 +135,7 @@ class RootSignatureParser {
// Common parsing helpers
bool ParseRegister(rs::Register *Reg);
bool ParseUInt(uint32_t *X);
+ bool ParseDescriptorRangeOffset(rs::DescriptorRangeOffset *X);
// Increment the token iterator if we have not reached the end.
// Return value denotes if we were already at the last token.
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 1e8b027742c57..d93a921aaa694 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -322,6 +322,7 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
llvm::SmallDenseMap<TokenKind, rs::ParamType> RefMap = {
{TokenKind::kw_numDescriptors, &Clause.NumDescriptors},
{TokenKind::kw_space, &Clause.Space},
+ {TokenKind::kw_offset, &Clause.Offset},
};
if (ParseOptionalParams({RefMap}))
return true;
@@ -345,6 +346,9 @@ bool RootSignatureParser::ParseParam(ParamType Ref) {
bool Error;
std::visit(OverloadedMethods{[&](uint32_t *X) { Error = ParseUInt(X); },
+ [&](DescriptorRangeOffset *X) {
+ Error = ParseDescriptorRangeOffset(X);
+ },
}, Ref);
return Error;
@@ -378,6 +382,21 @@ bool RootSignatureParser::ParseOptionalParams(
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 (CurTok->Kind == TokenKind::en_DescriptorRangeOffsetAppend) {
+ *X = DescriptorTableOffsetAppend;
+ return false;
+ }
+
+ *X = DescriptorRangeOffset(CurTok->NumLiteral.getInt().getExtValue());
+ return false;
+}
+
bool RootSignatureParser::ParseUInt(uint32_t *X) {
if (ConsumeExpectedToken(TokenKind::int_literal))
return true;
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index c35590406794d..f6c8e99aced1e 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -336,8 +336,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()
@@ -367,6 +367,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));
@@ -377,6 +379,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));
@@ -387,6 +391,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));
@@ -397,6 +403,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 608e79494fa64..fc0fa160eebd9 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 root_signature {
+// 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 root_signature
} // namespace hlsl
} // namespace llvm
>From 4ed3abf4c99d61211f77f01fea416da05cab4b23 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 10/22] add support for shader visibility
- introduces the ParseEnum function that will parse any of the ENUM
token definitions
---
.../clang/Parse/ParseHLSLRootSignature.h | 6 +++
clang/lib/Parse/ParseHLSLRootSignature.cpp | 51 +++++++++++++++++++
.../Parse/ParseHLSLRootSignatureTest.cpp | 32 ++++++++++++
.../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 89d3c89bd3b45..50ecfe2255d76 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -137,6 +137,12 @@ class RootSignatureParser {
bool ParseUInt(uint32_t *X);
bool ParseDescriptorRangeOffset(rs::DescriptorRangeOffset *X);
+ // Various flags/enum parsing helpers
+ template <typename EnumType>
+ bool ParseEnum(llvm::SmallDenseMap<TokenKind, EnumType> EnumMap,
+ EnumType *Enum);
+ bool ParseShaderVisibility(rs::ShaderVisibility *Enum);
+
// Increment the token iterator if we have not reached the end.
// Return value denotes if we were already at the last token.
bool ConsumeNextToken();
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index d93a921aaa694..ef81c65b8d7d4 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -271,8 +271,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(CurTok->TokLoc, diag::err_hlsl_rootsig_repeat_param)
+ << FormatTokenKinds(CurTok->Kind);
+ return true;
+ }
+ SeenVisibility = true;
+ if (ParseParam(&Table.Visibility))
+ return true;
+ continue;
+ }
+
+ // Otherwise, we expect a clause
if (ParseDescriptorTableClause())
return true;
Table.NumClauses++;
@@ -349,6 +364,9 @@ bool RootSignatureParser::ParseParam(ParamType Ref) {
[&](DescriptorRangeOffset *X) {
Error = ParseDescriptorRangeOffset(X);
},
+ [&](ShaderVisibility *Enum) {
+ Error = ParseShaderVisibility(Enum);
+ },
}, Ref);
return Error;
@@ -429,6 +447,39 @@ bool RootSignatureParser::ParseRegister(Register *Register) {
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 (CurTok->Kind == EnumPair.first) {
+ *Enum = EnumPair.second;
+ return false;
+ }
+
+ llvm_unreachable("Switch for an expected token was not provided");
+ return true;
+}
+
+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/Parse/HLSLRootSignatureTokenKinds.def"
+ };
+
+ return ParseEnum(EnumMap, Enum);
+}
+
RootSignatureToken RootSignatureParser::PeekNextToken() {
// Create an invalid token
RootSignatureToken Token = RootSignatureToken(SourceLocation());
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index f6c8e99aced1e..19e5b03b3c3d1 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -335,6 +335,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),
@@ -409,11 +410,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, (uint32_t)0);
+ ASSERT_EQ(std::get<DescriptorTable>(Elem).Visibility, ShaderVisibility::All);
+
ASSERT_TRUE(Consumer->IsSatisfied());
}
@@ -494,4 +499,31 @@ 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();
+
+ // Test correct diagnostic produced
+ Consumer->SetExpected(diag::err_hlsl_rootsig_repeat_param);
+ hlsl::RootSignatureLexer Lexer(Source, TokLoc, *PP);
+
+ SmallVector<hlsl::RootSignatureToken> Tokens;
+ ASSERT_FALSE(Lexer.Lex(Tokens));
+
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(Elements, Tokens, Diags);
+
+ 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 fc0fa160eebd9..6b1feca48801d 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -25,6 +25,17 @@ namespace root_signature {
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 root_signature
} // namespace hlsl
} // namespace llvm
>From 3c928d5feaa085e74b3b0e68e5f2684e3d035a5f 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 11/22] add support for parsing Flag parameters
- use DescriptorRangeFlags to demonstrate valid functionality
---
.../clang/Parse/ParseHLSLRootSignature.h | 6 ++-
clang/lib/Parse/ParseHLSLRootSignature.cpp | 50 ++++++++++++++++-
.../Parse/ParseHLSLRootSignatureTest.cpp | 16 +++++-
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 54 ++++++++++++++++++-
4 files changed, 121 insertions(+), 5 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 50ecfe2255d76..07358c37423b9 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -138,9 +138,13 @@ class RootSignatureParser {
bool ParseDescriptorRangeOffset(rs::DescriptorRangeOffset *X);
// Various flags/enum parsing helpers
- template <typename EnumType>
+ template <bool AllowZero = false, typename EnumType>
bool ParseEnum(llvm::SmallDenseMap<TokenKind, EnumType> EnumMap,
EnumType *Enum);
+ template <typename FlagType>
+ bool ParseFlags(llvm::SmallDenseMap<TokenKind, FlagType> EnumMap,
+ FlagType *Enum);
+ bool ParseDescriptorRangeFlags(rs::DescriptorRangeFlags *Enum);
bool ParseShaderVisibility(rs::ShaderVisibility *Enum);
// Increment the token iterator if we have not reached the end.
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index ef81c65b8d7d4..2478a65bd31cf 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -323,6 +323,8 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
llvm_unreachable("Switch for an expected token was not provided");
return true;
}
+ Clause.SetDefaultFlags();
+
if (ConsumeExpectedToken(TokenKind::pu_l_paren))
return true;
@@ -338,6 +340,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;
@@ -364,6 +367,9 @@ bool RootSignatureParser::ParseParam(ParamType Ref) {
[&](DescriptorRangeOffset *X) {
Error = ParseDescriptorRangeOffset(X);
},
+ [&](DescriptorRangeFlags *Flags) {
+ Error = ParseDescriptorRangeFlags(Flags);
+ },
[&](ShaderVisibility *Enum) {
Error = ParseShaderVisibility(Enum);
},
@@ -447,10 +453,12 @@ bool RootSignatureParser::ParseRegister(Register *Register) {
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);
@@ -458,6 +466,16 @@ bool RootSignatureParser::ParseEnum(
if (ConsumeExpectedToken(EnumToks))
return true;
+ // Handle the edge case when '0' is used to specify None
+ if (CurTok->Kind == TokenKind::int_literal) {
+ if (CurTok->NumLiteral.getInt() != 0) {
+ 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 (CurTok->Kind == EnumPair.first) {
@@ -469,6 +487,36 @@ bool RootSignatureParser::ParseEnum(
return true;
}
+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/Parse/HLSLRootSignatureTokenKinds.def"
+ };
+
+ return ParseFlags(FlagMap, Flags);
+}
+
bool RootSignatureParser::ParseShaderVisibility(ShaderVisibility *Enum) {
// Define the possible flag kinds
llvm::SmallDenseMap<TokenKind, ShaderVisibility> EnumMap = {
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 19e5b03b3c3d1..7a88ed64e48bc 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -337,9 +337,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";
@@ -370,6 +374,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));
@@ -382,6 +388,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));
@@ -394,6 +402,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));
@@ -406,6 +416,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));
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 6b1feca48801d..511ffd96a6d16 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 root_signature {
+#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,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 *,
- ShaderVisibility *>;
+ DescriptorRangeFlags *, ShaderVisibility *>;
+
} // namespace root_signature
} // namespace hlsl
} // namespace llvm
>From d037590f585f1ac5bdd40d015b80fe781360d423 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Tue, 28 Jan 2025 19:47:44 +0000
Subject: [PATCH 12/22] add diagnostic for non-zero int literal as flag
---
.../clang/Basic/DiagnosticParseKinds.td | 2 ++
clang/lib/Parse/ParseHLSLRootSignature.cpp | 1 +
.../Parse/ParseHLSLRootSignatureTest.cpp | 26 +++++++++++++++++++
3 files changed, 29 insertions(+)
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 4994d2339ce57..5cb5b3b404c7a 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1810,4 +1810,6 @@ def err_hlsl_packoffset_invalid_reg : Error<"invalid resource class specifier '%
def err_hlsl_rootsig_unexpected_eos : Error<"unexpected end to token stream">;
def err_hlsl_rootsig_unexpected_token_kind : Error<"expected the %select{following|one of the following}0 token kinds '%1'">;
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/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 2478a65bd31cf..20d938a29963c 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -469,6 +469,7 @@ bool RootSignatureParser::ParseEnum(
// Handle the edge case when '0' is used to specify None
if (CurTok->Kind == TokenKind::int_literal) {
if (CurTok->NumLiteral.getInt() != 0) {
+ Diags.Report(CurTok->TokLoc, diag::err_hlsl_rootsig_non_zero_flag);
return true;
}
// Set enum to None equivalent
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 7a88ed64e48bc..3253625ad491d 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -538,4 +538,30 @@ 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();
+
+ // Test correct diagnostic produced
+ Consumer->SetExpected(diag::err_hlsl_rootsig_non_zero_flag);
+ hlsl::RootSignatureLexer Lexer(Source, TokLoc, *PP);
+
+ SmallVector<hlsl::RootSignatureToken> Tokens;
+ ASSERT_FALSE(Lexer.Lex(Tokens));
+
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(Elements, Tokens, Diags);
+
+ ASSERT_TRUE(Parser.Parse());
+
+ ASSERT_TRUE(Consumer->IsSatisfied());
+}
+
} // anonymous namespace
>From 9ee3e654c6784a8a565f051200e31d2cf3d2175d Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Tue, 28 Jan 2025 19:50:45 +0000
Subject: [PATCH 13/22] visit clang-format
---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 23 +++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 20d938a29963c..2cdede00dfac7 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -363,17 +363,18 @@ bool RootSignatureParser::ParseParam(ParamType Ref) {
return true;
bool Error;
- std::visit(OverloadedMethods{[&](uint32_t *X) { Error = ParseUInt(X); },
- [&](DescriptorRangeOffset *X) {
- Error = ParseDescriptorRangeOffset(X);
- },
- [&](DescriptorRangeFlags *Flags) {
- Error = ParseDescriptorRangeFlags(Flags);
- },
- [&](ShaderVisibility *Enum) {
- Error = ParseShaderVisibility(Enum);
- },
- }, Ref);
+ std::visit(
+ OverloadedMethods{
+ [&](uint32_t *X) { Error = ParseUInt(X); },
+ [&](DescriptorRangeOffset *X) {
+ Error = ParseDescriptorRangeOffset(X);
+ },
+ [&](DescriptorRangeFlags *Flags) {
+ Error = ParseDescriptorRangeFlags(Flags);
+ },
+ [&](ShaderVisibility *Enum) { Error = ParseShaderVisibility(Enum); },
+ },
+ Ref);
return Error;
}
>From e030faf81b2b0558db8836bd1f722794cc629bac Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 29 Jan 2025 00:34:12 +0000
Subject: [PATCH 14/22] small missed update from restructuring
---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 2cdede00dfac7..981656fe11dba 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -539,10 +539,11 @@ RootSignatureToken RootSignatureParser::PeekNextToken() {
}
bool RootSignatureParser::ConsumeNextToken() {
+ SourceLocation EndLoc = CurTok->TokLoc;
CurTok++;
if (LastTok == CurTok) {
// Report unexpected end of tokens error
- Diags.Report(CurTok->TokLoc, diag::err_hlsl_rootsig_unexpected_eos);
+ Diags.Report(EndLoc, diag::err_hlsl_rootsig_unexpected_eos);
return true;
}
return false;
>From d0b815b2268cf6f6792bf90b3d58e53572684dff Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 5 Feb 2025 17:28:48 +0000
Subject: [PATCH 15/22] review: update namespace
- reduce namespace for brevity
- remove introducing rs namespace and use new namespace instead
---
.../clang/Parse/ParseHLSLRootSignature.h | 22 +++++++++----------
clang/lib/Parse/ParseHLSLRootSignature.cpp | 6 ++---
.../Parse/ParseHLSLRootSignatureTest.cpp | 2 +-
.../llvm/Frontend/HLSL/HLSLRootSignature.h | 4 ++--
4 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index 07358c37423b9..c2dc7530a5a3d 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -28,8 +28,6 @@
namespace clang {
namespace hlsl {
-namespace rs = llvm::hlsl::root_signature;
-
struct RootSignatureToken {
enum Kind {
#define TOK(X) X,
@@ -100,7 +98,7 @@ class RootSignatureLexer {
class RootSignatureParser {
public:
- RootSignatureParser(SmallVector<rs::RootElement> &Elements,
+ RootSignatureParser(SmallVector<llvm::hlsl::rootsig::RootElement> &Elements,
const SmallVector<RootSignatureToken> &Tokens,
DiagnosticsEngine &Diags);
@@ -126,16 +124,17 @@ class RootSignatureParser {
// 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
- bool ParseParam(rs::ParamType Ref);
+ bool ParseParam(llvm::hlsl::rootsig::ParamType Ref);
// Parse as many optional parameters as possible in any order
- bool
- ParseOptionalParams(llvm::SmallDenseMap<TokenKind, rs::ParamType> RefMap);
+ bool ParseOptionalParams(
+ llvm::SmallDenseMap<TokenKind, llvm::hlsl::rootsig::ParamType> RefMap);
// Common parsing helpers
- bool ParseRegister(rs::Register *Reg);
+ bool ParseRegister(llvm::hlsl::rootsig::Register *Reg);
bool ParseUInt(uint32_t *X);
- bool ParseDescriptorRangeOffset(rs::DescriptorRangeOffset *X);
+ bool
+ ParseDescriptorRangeOffset(llvm::hlsl::rootsig::DescriptorRangeOffset *X);
// Various flags/enum parsing helpers
template <bool AllowZero = false, typename EnumType>
@@ -144,8 +143,9 @@ class RootSignatureParser {
template <typename FlagType>
bool ParseFlags(llvm::SmallDenseMap<TokenKind, FlagType> EnumMap,
FlagType *Enum);
- bool ParseDescriptorRangeFlags(rs::DescriptorRangeFlags *Enum);
- bool ParseShaderVisibility(rs::ShaderVisibility *Enum);
+ bool
+ ParseDescriptorRangeFlags(llvm::hlsl::rootsig::DescriptorRangeFlags *Enum);
+ bool ParseShaderVisibility(llvm::hlsl::rootsig::ShaderVisibility *Enum);
// Increment the token iterator if we have not reached the end.
// Return value denotes if we were already at the last token.
@@ -184,7 +184,7 @@ class RootSignatureParser {
bool TryConsumeExpectedToken(ArrayRef<TokenKind> Expected);
private:
- SmallVector<rs::RootElement> &Elements;
+ SmallVector<llvm::hlsl::rootsig::RootElement> &Elements;
SmallVector<RootSignatureToken>::const_iterator CurTok;
SmallVector<RootSignatureToken>::const_iterator LastTok;
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 981656fe11dba..2e9729f16b78a 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -2,7 +2,7 @@
#include "llvm/Support/raw_ostream.h"
-using namespace llvm::hlsl::root_signature;
+using namespace llvm::hlsl::rootsig;
namespace clang {
namespace hlsl {
@@ -336,7 +336,7 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
return true;
// Parse optional paramaters
- llvm::SmallDenseMap<TokenKind, rs::ParamType> RefMap = {
+ llvm::SmallDenseMap<TokenKind, ParamType> RefMap = {
{TokenKind::kw_numDescriptors, &Clause.NumDescriptors},
{TokenKind::kw_space, &Clause.Space},
{TokenKind::kw_offset, &Clause.Offset},
@@ -380,7 +380,7 @@ bool RootSignatureParser::ParseParam(ParamType Ref) {
}
bool RootSignatureParser::ParseOptionalParams(
- llvm::SmallDenseMap<TokenKind, rs::ParamType> RefMap) {
+ llvm::SmallDenseMap<TokenKind, ParamType> RefMap) {
SmallVector<TokenKind> ParamKeywords;
for (auto RefPair : RefMap)
ParamKeywords.push_back(RefPair.first);
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 3253625ad491d..1efc72085538f 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -24,7 +24,7 @@
#include "gtest/gtest.h"
using namespace clang;
-using namespace llvm::hlsl::root_signature;
+using namespace llvm::hlsl::rootsig;
namespace {
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 511ffd96a6d16..f999a0b5eef33 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -19,7 +19,7 @@
namespace llvm {
namespace hlsl {
-namespace root_signature {
+namespace rootsig {
#define RS_DEFINE_ENUM_CLASS_FLAGS_OPERATORS(Class) \
inline Class operator|(Class a, Class b) { \
@@ -122,7 +122,7 @@ using RootElement = std::variant<DescriptorTable, DescriptorTableClause>;
using ParamType = std::variant<uint32_t *, DescriptorRangeOffset *,
DescriptorRangeFlags *, ShaderVisibility *>;
-} // namespace root_signature
+} // namespace rootsig
} // namespace hlsl
} // namespace llvm
>From 8a464ecbb0d0128a91daaa225a6a6e2ccb575214 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 5 Feb 2025 17:33:18 +0000
Subject: [PATCH 16/22] review: remove unneeded returns after llvm_unreachable
---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 2e9729f16b78a..b653992f51377 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -255,8 +255,8 @@ bool RootSignatureParser::ParseRootElement(bool First) {
return ParseDescriptorTable();
default:
llvm_unreachable("Switch for an expected token was not provided");
- return true;
}
+ return true;
}
bool RootSignatureParser::ParseDescriptorTable() {
@@ -321,7 +321,6 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
break;
default:
llvm_unreachable("Switch for an expected token was not provided");
- return true;
}
Clause.SetDefaultFlags();
@@ -446,7 +445,6 @@ bool RootSignatureParser::ParseRegister(Register *Register) {
break;
default:
llvm_unreachable("Switch for an expected token was not provided");
- return true;
}
Register->Number = CurTok->NumLiteral.getInt().getExtValue();
>From afabdda333daffd63b96ca4e8dca0fd90b3a4ec5 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 5 Feb 2025 17:34:42 +0000
Subject: [PATCH 17/22] review: pass DenseMap by reference
---
clang/include/clang/Parse/ParseHLSLRootSignature.h | 6 +++---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index c2dc7530a5a3d..c51268f5923ad 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -128,7 +128,7 @@ class RootSignatureParser {
// Parse as many optional parameters as possible in any order
bool ParseOptionalParams(
- llvm::SmallDenseMap<TokenKind, llvm::hlsl::rootsig::ParamType> RefMap);
+ llvm::SmallDenseMap<TokenKind, llvm::hlsl::rootsig::ParamType> &RefMap);
// Common parsing helpers
bool ParseRegister(llvm::hlsl::rootsig::Register *Reg);
@@ -138,10 +138,10 @@ class RootSignatureParser {
// Various flags/enum parsing helpers
template <bool AllowZero = false, typename EnumType>
- bool ParseEnum(llvm::SmallDenseMap<TokenKind, EnumType> EnumMap,
+ bool ParseEnum(llvm::SmallDenseMap<TokenKind, EnumType> &EnumMap,
EnumType *Enum);
template <typename FlagType>
- bool ParseFlags(llvm::SmallDenseMap<TokenKind, FlagType> EnumMap,
+ bool ParseFlags(llvm::SmallDenseMap<TokenKind, FlagType> &EnumMap,
FlagType *Enum);
bool
ParseDescriptorRangeFlags(llvm::hlsl::rootsig::DescriptorRangeFlags *Enum);
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index b653992f51377..abef7762ee0f7 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -379,7 +379,7 @@ bool RootSignatureParser::ParseParam(ParamType Ref) {
}
bool RootSignatureParser::ParseOptionalParams(
- llvm::SmallDenseMap<TokenKind, ParamType> RefMap) {
+ llvm::SmallDenseMap<TokenKind, ParamType> &RefMap) {
SmallVector<TokenKind> ParamKeywords;
for (auto RefPair : RefMap)
ParamKeywords.push_back(RefPair.first);
@@ -454,7 +454,7 @@ bool RootSignatureParser::ParseRegister(Register *Register) {
template <bool AllowZero, typename EnumType>
bool RootSignatureParser::ParseEnum(
- llvm::SmallDenseMap<TokenKind, EnumType> EnumMap, EnumType *Enum) {
+ llvm::SmallDenseMap<TokenKind, EnumType> &EnumMap, EnumType *Enum) {
SmallVector<TokenKind> EnumToks;
if (AllowZero)
EnumToks.push_back(TokenKind::int_literal); // '0' is a valid flag value
@@ -489,7 +489,7 @@ bool RootSignatureParser::ParseEnum(
template <typename FlagType>
bool RootSignatureParser::ParseFlags(
- llvm::SmallDenseMap<TokenKind, FlagType> FlagMap, FlagType *Flags) {
+ llvm::SmallDenseMap<TokenKind, FlagType> &FlagMap, FlagType *Flags) {
// Override the default value to 0 so that we can correctly 'or' the values
*Flags = FlagType(0);
>From 99f9afd89c02ec54025a2da0390895fffb50f1c6 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 5 Feb 2025 17:37:33 +0000
Subject: [PATCH 18/22] review: adjust comments for doxygen
---
.../clang/Parse/ParseHLSLRootSignature.h | 68 +++++++++----------
1 file changed, 34 insertions(+), 34 deletions(-)
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
index c51268f5923ad..a76e9ef27af95 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -102,12 +102,12 @@ class RootSignatureParser {
const SmallVector<RootSignatureToken> &Tokens,
DiagnosticsEngine &Diags);
- // Iterates over the provided tokens and constructs the in-memory
- // representations of the RootElements.
- //
- // The return value denotes if there was a failure and the method will
- // return on the first encountered failure, or, return false if it
- // can sucessfully reach the end of the tokens.
+ /// Iterates over the provided tokens and constructs the in-memory
+ /// representations of the RootElements.
+ ///
+ /// The return value denotes if there was a failure and the method will
+ /// return on the first encountered failure, or, return false if it
+ /// can sucessfully reach the end of the tokens.
bool Parse();
private:
@@ -116,14 +116,14 @@ class RootSignatureParser {
bool ParseDescriptorTable();
bool ParseDescriptorTableClause();
- // Helper dispatch method
- //
- // These will switch on the Variant kind to dispatch to the respective Parse
- // method and store the parsed value back into Ref.
- //
- // 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
+ /// Helper dispatch method
+ ///
+ /// These will switch on the Variant kind to dispatch to the respective Parse
+ /// method and store the parsed value back into Ref.
+ ///
+ /// 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
bool ParseParam(llvm::hlsl::rootsig::ParamType Ref);
// Parse as many optional parameters as possible in any order
@@ -147,39 +147,39 @@ class RootSignatureParser {
ParseDescriptorRangeFlags(llvm::hlsl::rootsig::DescriptorRangeFlags *Enum);
bool ParseShaderVisibility(llvm::hlsl::rootsig::ShaderVisibility *Enum);
- // Increment the token iterator if we have not reached the end.
- // Return value denotes if we were already at the last token.
+ /// Increment the token iterator if we have not reached the end.
+ /// Return value denotes if we were already at the last token.
bool ConsumeNextToken();
- // Attempt to retrieve the next token, if TokenKind is invalid then there was
- // no next token.
+ /// Attempt to retrieve the next token, if TokenKind is invalid then there was
+ /// no next token.
RootSignatureToken PeekNextToken();
- // Is the current token one of the expected kinds
+ /// Is the current token one of the expected kinds
bool EnsureExpectedToken(TokenKind AnyExpected);
bool EnsureExpectedToken(ArrayRef<TokenKind> AnyExpected);
- // Peek if the next token is of the expected kind.
- //
- // Return value denotes if it failed to match the expected kind, either it is
- // the end of the stream or it didn't match any of the expected kinds.
+ /// Peek if the next token is of the expected kind.
+ ///
+ /// Return value denotes if it failed to match the expected kind, either it is
+ /// the end of the stream or it didn't match any of the expected kinds.
bool PeekExpectedToken(TokenKind Expected);
bool PeekExpectedToken(ArrayRef<TokenKind> AnyExpected);
- // Consume the next token and report an error if it is not of the expected
- // kind.
- //
- // Return value denotes if it failed to match the expected kind, either it is
- // the end of the stream or it didn't match any of the expected kinds.
+ /// Consume the next token and report an error if it is not of the expected
+ /// kind.
+ ///
+ /// Return value denotes if it failed to match the expected kind, either it is
+ /// the end of the stream or it didn't match any of the expected kinds.
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.
- //
- // Return value denotes if it failed to match the expected kind, either it is
- // the end of the stream or it didn't match any of the expected kinds. It will
- // not report an error if there isn't a match.
+ /// Peek if the next token is of the expected kind and if it is then consume
+ /// it.
+ ///
+ /// Return value denotes if it failed to match the expected kind, either it is
+ /// the end of the stream or it didn't match any of the expected kinds. It
+ /// will not report an error if there isn't a match.
bool TryConsumeExpectedToken(TokenKind Expected);
bool TryConsumeExpectedToken(ArrayRef<TokenKind> Expected);
>From 99b569116bddb2929689130eb58c903a6db58b9a Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Mon, 10 Feb 2025 23:42:21 +0000
Subject: [PATCH 19/22] review: rebase onto lexer api changes
- we have changed the api of the lexer to avoid pre-allocating all the
tokens and instead to have the parser only invoke
ConsumeToken/PeekNextToken when needed
- the end of stream diagnostic is also moved to the lexer
---
.../clang/Basic/DiagnosticParseKinds.td | 1 -
.../clang/Parse/ParseHLSLRootSignature.h | 23 +++--
clang/lib/Parse/ParseHLSLRootSignature.cpp | 91 +++++++------------
.../Parse/ParseHLSLRootSignatureTest.cpp | 83 ++++-------------
4 files changed, 66 insertions(+), 132 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 5cb5b3b404c7a..0124bec6085ba 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1807,7 +1807,6 @@ 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_eos : Error<"unexpected end to token stream">;
def err_hlsl_rootsig_unexpected_token_kind : Error<"expected the %select{following|one of the following}0 token kinds '%1'">;
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 a76e9ef27af95..efbe342e49f56 100644
--- a/clang/include/clang/Parse/ParseHLSLRootSignature.h
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -99,8 +99,7 @@ class RootSignatureLexer {
class RootSignatureParser {
public:
RootSignatureParser(SmallVector<llvm::hlsl::rootsig::RootElement> &Elements,
- const SmallVector<RootSignatureToken> &Tokens,
- DiagnosticsEngine &Diags);
+ RootSignatureLexer &Lexer, DiagnosticsEngine &Diags);
/// Iterates over the provided tokens and constructs the in-memory
/// representations of the RootElements.
@@ -112,7 +111,7 @@ class RootSignatureParser {
private:
// Root Element helpers
- bool ParseRootElement(bool First);
+ bool ParseRootElement();
bool ParseDescriptorTable();
bool ParseDescriptorTableClause();
@@ -147,9 +146,17 @@ class RootSignatureParser {
ParseDescriptorRangeFlags(llvm::hlsl::rootsig::DescriptorRangeFlags *Enum);
bool ParseShaderVisibility(llvm::hlsl::rootsig::ShaderVisibility *Enum);
- /// Increment the token iterator if we have not reached the end.
+ /// Invoke the lexer to consume a token and update CurToken with the result
+ ///
/// Return value denotes if we were already at the last token.
- bool ConsumeNextToken();
+ ///
+ /// This is used to avoid having to constantly access the Lexer's CurToken
+ bool ConsumeNextToken() {
+ if (Lexer.ConsumeToken())
+ return true; // Report lexer err
+ CurToken = Lexer.GetCurToken();
+ return false;
+ }
/// Attempt to retrieve the next token, if TokenKind is invalid then there was
/// no next token.
@@ -185,10 +192,10 @@ class RootSignatureParser {
private:
SmallVector<llvm::hlsl::rootsig::RootElement> &Elements;
- SmallVector<RootSignatureToken>::const_iterator CurTok;
- SmallVector<RootSignatureToken>::const_iterator LastTok;
-
+ RootSignatureLexer &Lexer;
DiagnosticsEngine &Diags;
+
+ RootSignatureToken CurToken;
};
} // namespace hlsl
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index abef7762ee0f7..616d78e0657f4 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -18,6 +18,9 @@ static std::string FormatTokenKinds(ArrayRef<TokenKind> Kinds) {
Out << ", ";
switch (Kind) {
case TokenKind::invalid:
+ Out << "uninitialized";
+ break;
+ case TokenKind::end_of_stream:
break;
case TokenKind::int_literal:
Out << "integer literal";
@@ -215,42 +218,33 @@ std::optional<RootSignatureToken> RootSignatureLexer::PeekNextToken() {
// Parser Definitions
-RootSignatureParser::RootSignatureParser(
- SmallVector<RootElement> &Elements,
- const SmallVector<RootSignatureToken> &Tokens, DiagnosticsEngine &Diags)
- : Elements(Elements), Diags(Diags) {
- CurTok = Tokens.begin();
- LastTok = Tokens.end();
-}
+RootSignatureParser::RootSignatureParser(SmallVector<RootElement> &Elements,
+ RootSignatureLexer &Lexer,
+ DiagnosticsEngine &Diags)
+ : Elements(Elements), Lexer(Lexer), Diags(Diags) {}
bool RootSignatureParser::Parse() {
// Handle edge-case of empty RootSignature()
- if (CurTok == LastTok)
+ if (Lexer.EndOfBuffer())
return false;
- bool First = true;
// Iterate as many RootElements as possible
- while (!ParseRootElement(First)) {
- First = false;
- // Avoid use of ConsumeNextToken here to skip incorrect end of tokens error
- CurTok++;
- if (CurTok == LastTok)
+ while (!ParseRootElement()) {
+ if (Lexer.EndOfBuffer())
return false;
- if (EnsureExpectedToken(TokenKind::pu_comma))
+ if (ConsumeExpectedToken(TokenKind::pu_comma))
return true;
}
return true;
}
-bool RootSignatureParser::ParseRootElement(bool First) {
- if (First && EnsureExpectedToken(TokenKind::kw_DescriptorTable))
- return true;
- if (!First && ConsumeExpectedToken(TokenKind::kw_DescriptorTable))
+bool RootSignatureParser::ParseRootElement() {
+ if (ConsumeExpectedToken(TokenKind::kw_DescriptorTable))
return true;
// Dispatch onto the correct parse method
- switch (CurTok->Kind) {
+ switch (CurToken.Kind) {
case TokenKind::kw_DescriptorTable:
return ParseDescriptorTable();
default:
@@ -277,8 +271,8 @@ bool RootSignatureParser::ParseDescriptorTable() {
// Handle the visibility parameter
if (!TryConsumeExpectedToken(TokenKind::kw_visibility)) {
if (SeenVisibility) {
- Diags.Report(CurTok->TokLoc, diag::err_hlsl_rootsig_repeat_param)
- << FormatTokenKinds(CurTok->Kind);
+ Diags.Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param)
+ << FormatTokenKinds(CurToken.Kind);
return true;
}
SeenVisibility = true;
@@ -306,7 +300,7 @@ bool RootSignatureParser::ParseDescriptorTableClause() {
return true;
DescriptorTableClause Clause;
- switch (CurTok->Kind) {
+ switch (CurToken.Kind) {
case TokenKind::kw_CBV:
Clause.Type = ClauseType::CBuffer;
break;
@@ -391,9 +385,9 @@ bool RootSignatureParser::ParseOptionalParams(
if (ConsumeExpectedToken(ParamKeywords))
return true;
- TokenKind ParamKind = CurTok->Kind;
+ TokenKind ParamKind = CurToken.Kind;
if (Seen.contains(ParamKind)) {
- Diags.Report(CurTok->TokLoc, diag::err_hlsl_rootsig_repeat_param)
+ Diags.Report(CurToken.TokLoc, diag::err_hlsl_rootsig_repeat_param)
<< FormatTokenKinds(ParamKind);
return true;
}
@@ -412,12 +406,12 @@ bool RootSignatureParser::ParseDescriptorRangeOffset(DescriptorRangeOffset *X) {
return true;
// Edge case for the offset enum -> static value
- if (CurTok->Kind == TokenKind::en_DescriptorRangeOffsetAppend) {
+ if (CurToken.Kind == TokenKind::en_DescriptorRangeOffsetAppend) {
*X = DescriptorTableOffsetAppend;
return false;
}
- *X = DescriptorRangeOffset(CurTok->NumLiteral.getInt().getExtValue());
+ *X = DescriptorRangeOffset(CurToken.NumLiteral.getInt().getExtValue());
return false;
}
@@ -425,12 +419,12 @@ bool RootSignatureParser::ParseUInt(uint32_t *X) {
if (ConsumeExpectedToken(TokenKind::int_literal))
return true;
- *X = CurTok->NumLiteral.getInt().getExtValue();
+ *X = CurToken.NumLiteral.getInt().getExtValue();
return false;
}
bool RootSignatureParser::ParseRegister(Register *Register) {
- switch (CurTok->Kind) {
+ switch (CurToken.Kind) {
case TokenKind::bReg:
Register->ViewType = RegisterType::BReg;
break;
@@ -447,7 +441,7 @@ bool RootSignatureParser::ParseRegister(Register *Register) {
llvm_unreachable("Switch for an expected token was not provided");
}
- Register->Number = CurTok->NumLiteral.getInt().getExtValue();
+ Register->Number = CurToken.NumLiteral.getInt().getExtValue();
return false;
}
@@ -466,9 +460,9 @@ bool RootSignatureParser::ParseEnum(
return true;
// Handle the edge case when '0' is used to specify None
- if (CurTok->Kind == TokenKind::int_literal) {
- if (CurTok->NumLiteral.getInt() != 0) {
- Diags.Report(CurTok->TokLoc, diag::err_hlsl_rootsig_non_zero_flag);
+ if (CurToken.Kind == TokenKind::int_literal) {
+ if (CurToken.NumLiteral.getInt() != 0) {
+ Diags.Report(CurToken.TokLoc, diag::err_hlsl_rootsig_non_zero_flag);
return true;
}
// Set enum to None equivalent
@@ -478,7 +472,7 @@ bool RootSignatureParser::ParseEnum(
// Effectively a switch statement on the token kinds
for (auto EnumPair : EnumMap)
- if (CurTok->Kind == EnumPair.first) {
+ if (CurToken.Kind == EnumPair.first) {
*Enum = EnumPair.second;
return false;
}
@@ -528,25 +522,6 @@ bool RootSignatureParser::ParseShaderVisibility(ShaderVisibility *Enum) {
return ParseEnum(EnumMap, Enum);
}
-RootSignatureToken RootSignatureParser::PeekNextToken() {
- // Create an invalid token
- RootSignatureToken Token = RootSignatureToken(SourceLocation());
- if (CurTok != LastTok)
- Token = *(CurTok + 1);
- return Token;
-}
-
-bool RootSignatureParser::ConsumeNextToken() {
- SourceLocation EndLoc = CurTok->TokLoc;
- CurTok++;
- if (LastTok == CurTok) {
- // Report unexpected end of tokens error
- Diags.Report(EndLoc, diag::err_hlsl_rootsig_unexpected_eos);
- return true;
- }
- return false;
-}
-
// Is given token one of the expected kinds
static bool IsExpectedToken(TokenKind Kind, ArrayRef<TokenKind> AnyExpected) {
for (auto Expected : AnyExpected)
@@ -560,11 +535,11 @@ bool RootSignatureParser::EnsureExpectedToken(TokenKind Expected) {
}
bool RootSignatureParser::EnsureExpectedToken(ArrayRef<TokenKind> AnyExpected) {
- if (IsExpectedToken(CurTok->Kind, AnyExpected))
+ if (IsExpectedToken(CurToken.Kind, AnyExpected))
return false;
// Report unexpected token kind error
- Diags.Report(CurTok->TokLoc, diag::err_hlsl_rootsig_unexpected_token_kind)
+ Diags.Report(CurToken.TokLoc, diag::err_hlsl_rootsig_unexpected_token_kind)
<< (unsigned)(AnyExpected.size() != 1) << FormatTokenKinds(AnyExpected);
return true;
}
@@ -574,10 +549,10 @@ bool RootSignatureParser::PeekExpectedToken(TokenKind Expected) {
}
bool RootSignatureParser::PeekExpectedToken(ArrayRef<TokenKind> AnyExpected) {
- RootSignatureToken Token = PeekNextToken();
- if (Token.Kind == TokenKind::invalid)
+ auto Result = Lexer.PeekNextToken();
+ if (!Result)
return true;
- if (IsExpectedToken(Token.Kind, AnyExpected))
+ if (IsExpectedToken(Result->Kind, AnyExpected))
return false;
return true;
}
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 1efc72085538f..31c5b6b384d8f 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -316,15 +316,12 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseEmptyTest) {
auto PP = CreatePP(Source, ModLoader);
auto TokLoc = SourceLocation();
- // Test no diagnostics produced
- Consumer->SetNoDiag();
hlsl::RootSignatureLexer Lexer(Source, TokLoc, *PP);
-
- SmallVector<hlsl::RootSignatureToken> Tokens;
- ASSERT_FALSE(Lexer.Lex(Tokens));
-
SmallVector<RootElement> Elements;
- hlsl::RootSignatureParser Parser(Elements, Tokens, Diags);
+ hlsl::RootSignatureParser Parser(Elements, Lexer, Diags);
+
+ // Test no diagnostics produced
+ Consumer->SetNoDiag();
ASSERT_FALSE(Parser.Parse());
ASSERT_EQ((int)Elements.size(), 0);
@@ -353,15 +350,11 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
auto TokLoc = SourceLocation();
hlsl::RootSignatureLexer Lexer(Source, TokLoc, *PP);
-
- SmallVector<hlsl::RootSignatureToken> Tokens;
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(Elements, Lexer, Diags);
// Test no diagnostics produced
Consumer->SetNoDiag();
- ASSERT_FALSE(Lexer.Lex(Tokens));
-
- SmallVector<RootElement> Elements;
- hlsl::RootSignatureParser Parser(Elements, Tokens, Diags);
ASSERT_FALSE(Parser.Parse());
RootElement Elem = Elements[0];
@@ -436,30 +429,6 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
// Invalid Parser Tests
-TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedEOSTest) {
- const llvm::StringLiteral Source = R"cc(
- DescriptorTable
- )cc";
-
- TrivialModuleLoader ModLoader;
- auto PP = CreatePP(Source, ModLoader);
- auto TokLoc = SourceLocation();
-
- // Test correct diagnostic produced
- Consumer->SetExpected(diag::err_hlsl_rootsig_unexpected_eos);
- hlsl::RootSignatureLexer Lexer(Source, TokLoc, *PP);
-
- SmallVector<hlsl::RootSignatureToken> Tokens;
- ASSERT_FALSE(Lexer.Lex(Tokens));
-
- SmallVector<RootElement> Elements;
- hlsl::RootSignatureParser Parser(Elements, Tokens, Diags);
-
- ASSERT_TRUE(Parser.Parse());
-
- ASSERT_TRUE(Consumer->IsSatisfied());
-}
-
TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedTokenTest) {
const llvm::StringLiteral Source = R"cc(
DescriptorTable()
@@ -470,16 +439,12 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedTokenTest) {
auto PP = CreatePP(Source, ModLoader);
auto TokLoc = SourceLocation();
- // Test correct diagnostic produced
- Consumer->SetExpected(diag::err_hlsl_rootsig_unexpected_token_kind);
hlsl::RootSignatureLexer Lexer(Source, TokLoc, *PP);
-
- SmallVector<hlsl::RootSignatureToken> Tokens;
- ASSERT_FALSE(Lexer.Lex(Tokens));
-
SmallVector<RootElement> Elements;
- hlsl::RootSignatureParser Parser(Elements, Tokens, Diags);
+ hlsl::RootSignatureParser Parser(Elements, Lexer, Diags);
+ // Test correct diagnostic produced
+ Consumer->SetExpected(diag::err_hlsl_rootsig_unexpected_token_kind);
ASSERT_TRUE(Parser.Parse());
ASSERT_TRUE(Consumer->IsSatisfied());
@@ -496,16 +461,12 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseRepeatedParamTest) {
auto PP = CreatePP(Source, ModLoader);
auto TokLoc = SourceLocation();
- // Test correct diagnostic produced
- Consumer->SetExpected(diag::err_hlsl_rootsig_repeat_param);
hlsl::RootSignatureLexer Lexer(Source, TokLoc, *PP);
-
- SmallVector<hlsl::RootSignatureToken> Tokens;
- ASSERT_FALSE(Lexer.Lex(Tokens));
-
SmallVector<RootElement> Elements;
- hlsl::RootSignatureParser Parser(Elements, Tokens, Diags);
+ hlsl::RootSignatureParser Parser(Elements, Lexer, Diags);
+ // Test correct diagnostic produced
+ Consumer->SetExpected(diag::err_hlsl_rootsig_repeat_param);
ASSERT_TRUE(Parser.Parse());
ASSERT_TRUE(Consumer->IsSatisfied());
@@ -523,16 +484,12 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseRepeatedVisibilityTest) {
auto PP = CreatePP(Source, ModLoader);
auto TokLoc = SourceLocation();
- // Test correct diagnostic produced
- Consumer->SetExpected(diag::err_hlsl_rootsig_repeat_param);
hlsl::RootSignatureLexer Lexer(Source, TokLoc, *PP);
-
- SmallVector<hlsl::RootSignatureToken> Tokens;
- ASSERT_FALSE(Lexer.Lex(Tokens));
-
SmallVector<RootElement> Elements;
- hlsl::RootSignatureParser Parser(Elements, Tokens, Diags);
+ hlsl::RootSignatureParser Parser(Elements, Lexer, Diags);
+ // Test correct diagnostic produced
+ Consumer->SetExpected(diag::err_hlsl_rootsig_repeat_param);
ASSERT_TRUE(Parser.Parse());
ASSERT_TRUE(Consumer->IsSatisfied());
@@ -549,16 +506,12 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseNonZeroFlagTest) {
auto PP = CreatePP(Source, ModLoader);
auto TokLoc = SourceLocation();
- // Test correct diagnostic produced
- Consumer->SetExpected(diag::err_hlsl_rootsig_non_zero_flag);
hlsl::RootSignatureLexer Lexer(Source, TokLoc, *PP);
-
- SmallVector<hlsl::RootSignatureToken> Tokens;
- ASSERT_FALSE(Lexer.Lex(Tokens));
-
SmallVector<RootElement> Elements;
- hlsl::RootSignatureParser Parser(Elements, Tokens, Diags);
+ hlsl::RootSignatureParser Parser(Elements, Lexer, Diags);
+ // Test correct diagnostic produced
+ Consumer->SetExpected(diag::err_hlsl_rootsig_non_zero_flag);
ASSERT_TRUE(Parser.Parse());
ASSERT_TRUE(Consumer->IsSatisfied());
>From 5ca354764cf66b67e1379cfc7bd48865b0497b20 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Tue, 11 Feb 2025 21:07:52 +0000
Subject: [PATCH 20/22] lexer review: let end_of_stream error be unexpected
token instead
---
.../Parse/ParseHLSLRootSignatureTest.cpp | 20 +++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 31c5b6b384d8f..c48b4844201f0 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -450,6 +450,26 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedTokenTest) {
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, *PP);
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(Elements, Lexer, Diags);
+
+ // Test correct diagnostic produced - end of stream
+ Consumer->SetExpected(diag::err_hlsl_rootsig_unexpected_token_kind);
+ ASSERT_TRUE(Parser.Parse());
+
+ ASSERT_TRUE(Consumer->IsSatisfied());
+}
+
TEST_F(ParseHLSLRootSignatureTest, InvalidParseRepeatedParamTest) {
const llvm::StringLiteral Source = R"cc(
DescriptorTable(
>From c6c9a12bc7a23f2a18352625c8e5ae9b637d8dad Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Tue, 11 Feb 2025 21:13:22 +0000
Subject: [PATCH 21/22] lexer review: let invalid_token error be an
unexpected_token error
---
.../Parse/ParseHLSLRootSignatureTest.cpp | 20 +++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index c48b4844201f0..82aa775b74e78 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -450,6 +450,26 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedTokenTest) {
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, *PP);
+ SmallVector<RootElement> Elements;
+ hlsl::RootSignatureParser Parser(Elements, Lexer, Diags);
+
+ // 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(
>From 217cb76bdc68cb7493f797d51a41b51d1fc60da8 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Tue, 11 Feb 2025 21:14:56 +0000
Subject: [PATCH 22/22] lexer review: handle parsing of unsigned integers
---
clang/lib/Parse/ParseHLSLRootSignature.cpp | 2 ++
clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 616d78e0657f4..b63f5fdfd3280 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -416,6 +416,8 @@ bool RootSignatureParser::ParseDescriptorRangeOffset(DescriptorRangeOffset *X) {
}
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;
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 82aa775b74e78..04a8876b5cce3 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -334,7 +334,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
DescriptorTable(
visibility = SHADER_VISIBILITY_PIXEL,
CBV(b0),
- SRV(t42, space = 3, offset = 32, numDescriptors = 4, flags = 0),
+ SRV(t42, space = +3, offset = 32, numDescriptors = 4, flags = 0),
Sampler(s987, space = 2, offset = DESCRIPTOR_RANGE_OFFSET_APPEND),
UAV(u987234,
flags = Descriptors_Volatile | Data_Volatile
More information about the llvm-branch-commits
mailing list