[HLSL][RootSignature] Implement Parsing of Descriptor Tables #122982
+936
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-hlsl Author: Finn Plummer (inbelic) Changes
This an initial part of #120472 Patch is 38.92 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/122982.diff 8 Files Affected:
diff --git a/clang/include/clang/Parse/HLSLRootSignatureTokenKinds.def b/clang/include/clang/Parse/HLSLRootSignatureTokenKinds.def
new file mode 100644
index 00000000000000..aaa2dabcb43f4e
--- /dev/null
+++ b/clang/include/clang/Parse/HLSLRootSignatureTokenKinds.def
@@ -0,0 +1,121 @@
+//===--- HLSLRootSignature.def - Tokens and Enum Database -------*- C++ -*-===//
+
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines the TokenKinds used in the Root Signature DSL. This
+// includes keywords, enums and a small subset of punctuators. Users of this
+// file must optionally #define the TOK, KEYWORD, ENUM or specific ENUM macros
+// to make use of this file.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef TOK
+#define TOK(X)
+#endif
+#ifndef PUNCTUATOR
+#define PUNCTUATOR(X,Y) TOK(pu_ ## X)
+#endif
+#ifndef KEYWORD
+#define KEYWORD(X) TOK(kw_ ## X)
+#endif
+#ifndef ENUM
+#define ENUM(NAME, LIT) TOK(en_ ## NAME)
+#endif
+
+// Defines the various types of enum
+#ifndef DESCRIPTOR_RANGE_OFFSET_ENUM
+#define DESCRIPTOR_RANGE_OFFSET_ENUM(NAME, LIT) ENUM(NAME, LIT)
+#endif
+#ifndef ROOT_DESCRIPTOR_FLAG_ENUM
+#define ROOT_DESCRIPTOR_FLAG_ENUM(NAME, LIT) ENUM(NAME, LIT)
+#endif
+// Note: ON denotes that the flag unique from the above Root Descriptor Flags.
+// This is required to avoid token kind enum conflicts.
+#ifndef DESCRIPTOR_RANGE_FLAG_ENUM_OFF
+#define DESCRIPTOR_RANGE_FLAG_ENUM_OFF(NAME, LIT)
+#endif
+#ifndef DESCRIPTOR_RANGE_FLAG_ENUM_ON
+#define DESCRIPTOR_RANGE_FLAG_ENUM_ON(NAME, LIT) ENUM(NAME, LIT)
+#endif
+#ifndef DESCRIPTOR_RANGE_FLAG_ENUM
+#define DESCRIPTOR_RANGE_FLAG_ENUM(NAME, LIT, ON) DESCRIPTOR_RANGE_FLAG_ENUM_##ON(NAME, LIT)
+#endif
+#ifndef SHADER_VISIBILITY_ENUM
+#define SHADER_VISIBILITY_ENUM(NAME, LIT) ENUM(NAME, LIT)
+#endif
+
+// General Tokens:
+TOK(invalid)
+TOK(int_literal)
+
+// Register Tokens:
+TOK(bReg)
+TOK(tReg)
+TOK(uReg)
+TOK(sReg)
+
+// Punctuators:
+PUNCTUATOR(l_paren, '(')
+PUNCTUATOR(r_paren, ')')
+PUNCTUATOR(comma, ',')
+PUNCTUATOR(or, '|')
+PUNCTUATOR(equal, '=')
+
+// RootElement Keywords:
+KEYWORD(DescriptorTable)
+
+// DescriptorTable Keywords:
+KEYWORD(CBV)
+KEYWORD(SRV)
+KEYWORD(UAV)
+KEYWORD(Sampler)
+
+// General Parameter Keywords:
+KEYWORD(space)
+KEYWORD(visibility)
+KEYWORD(flags)
+
+// View Parameter Keywords:
+KEYWORD(numDescriptors)
+KEYWORD(offset)
+
+// Descriptor Range Offset Enum:
+DESCRIPTOR_RANGE_OFFSET_ENUM(DescriptorRangeOffsetAppend, "DESCRIPTOR_RANGE_OFFSET_APPEND")
+
+// Root Descriptor Flag Enums:
+ROOT_DESCRIPTOR_FLAG_ENUM(DataVolatile, "DATA_VOLATILE")
+ROOT_DESCRIPTOR_FLAG_ENUM(DataStaticWhileSetAtExecute, "DATA_STATIC_WHILE_SET_AT_EXECUTE")
+ROOT_DESCRIPTOR_FLAG_ENUM(DataStatic, "DATA_STATIC")
+
+// Descriptor Range Flag Enums:
+DESCRIPTOR_RANGE_FLAG_ENUM(DescriptorsVolatile, "DESCRIPTORS_VOLATILE", ON)
+DESCRIPTOR_RANGE_FLAG_ENUM(DataVolatile, "DATA_VOLATILE", OFF)
+DESCRIPTOR_RANGE_FLAG_ENUM(DataStaticWhileSetAtExecute, "DATA_STATIC_WHILE_SET_AT_EXECUTE", OFF)
+DESCRIPTOR_RANGE_FLAG_ENUM(DataStatic, "DATA_STATIC", OFF)
+DESCRIPTOR_RANGE_FLAG_ENUM(DescriptorsStaticKeepingBufferBoundsChecks, "DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS", ON)
+
+// Shader Visibiliy Enums:
+SHADER_VISIBILITY_ENUM(All, "SHADER_VISIBILITY_ALL")
+SHADER_VISIBILITY_ENUM(Vertex, "SHADER_VISIBILITY_VERTEX")
+SHADER_VISIBILITY_ENUM(Hull, "SHADER_VISIBILITY_HULL")
+SHADER_VISIBILITY_ENUM(Domain, "SHADER_VISIBILITY_DOMAIN")
+SHADER_VISIBILITY_ENUM(Geometry, "SHADER_VISIBILITY_GEOMETRY")
+SHADER_VISIBILITY_ENUM(Pixel, "SHADER_VISIBILITY_PIXEL")
+SHADER_VISIBILITY_ENUM(Amplification, "SHADER_VISIBILITY_AMPLIFICATION")
+SHADER_VISIBILITY_ENUM(Mesh, "SHADER_VISIBILITY_MESH")
+
+#undef SHADER_VISIBILITY_ENUM
+#undef DESCRIPTOR_RANGE_FLAG_ENUM
+#undef DESCRIPTOR_RANGE_FLAG_ENUM_OFF
+#undef DESCRIPTOR_RANGE_FLAG_ENUM_ON
+#undef ROOT_DESCRIPTOR_FLAG_ENUM
+#undef DESCRIPTOR_RANGE_OFFSET_ENUM
+#undef ENUM
+#undef KEYWORD
+#undef PUNCTUATOR
+#undef TOK
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
new file mode 100644
index 00000000000000..9464bd8f2f9e0f
--- /dev/null
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -0,0 +1,164 @@
+//===--- ParseHLSLRootSignature.h -------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines the ParseHLSLRootSignature interface.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_PARSE_PARSEHLSLROOTSIGNATURE_H
+#define LLVM_CLANG_PARSE_PARSEHLSLROOTSIGNATURE_H
+
+#include "clang/Lex/LiteralSupport.h"
+#include "clang/Lex/Preprocessor.h"
+
+#include "llvm/ADT/APInt.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
+
+#include "llvm/Frontend/HLSL/HLSLRootSignature.h"
+
+namespace llvm {
+namespace hlsl {
+namespace root_signature {
+
+struct RootSignatureToken {
+ enum Kind {
+#define TOK(X) X,
+#include "clang/Parse/HLSLRootSignatureTokenKinds.def"
+ };
+
+ Kind Kind = Kind::invalid;
+
+ // Retain the SouceLocation of the token for diagnostics
+ clang::SourceLocation TokLoc;
+
+ // Retain if the uint32_t bits represent a signed integer
+ bool Signed = false;
+ union {
+ uint32_t IntLiteral = 0;
+ float FloatLiteral;
+ };
+
+ // Constructors
+ RootSignatureToken() {}
+ RootSignatureToken(clang::SourceLocation TokLoc) : TokLoc(TokLoc) {}
+};
+using TokenKind = enum RootSignatureToken::Kind;
+
+class RootSignatureLexer {
+public:
+ RootSignatureLexer(StringRef Signature, clang::SourceLocation SourceLoc,
+ clang::Preprocessor &PP)
+ : Buffer(Signature), SourceLoc(SourceLoc), PP(PP) {}
+
+ // Consumes the internal buffer as a list of tokens and will emplace them
+ // onto the given tokens.
+ //
+ // It will consume until it successfully reaches the end of the buffer,
+ // or, until the first error is encountered. The return value denotes if
+ // there was a failure.
+ bool Lex(SmallVector<RootSignatureToken> &Tokens);
+
+ // Get the current source location of the lexer
+ clang::SourceLocation GetLocation() { return SourceLoc; };
+
+private:
+ // Internal buffer to iterate over
+ StringRef Buffer;
+
+ // Passed down parameters from Sema
+ clang::SourceLocation SourceLoc;
+ clang::Preprocessor &PP;
+
+ bool LexNumber(RootSignatureToken &Result);
+
+ // Consumes the internal buffer for a single token.
+ //
+ // The return value denotes if there was a failure.
+ bool LexToken(RootSignatureToken &Token);
+
+ // Advance the buffer by the specified number of characters. Updates the
+ // SourceLocation appropriately.
+ void AdvanceBuffer(unsigned NumCharacters = 1) {
+ Buffer = Buffer.drop_front(NumCharacters);
+ SourceLoc = SourceLoc.getLocWithOffset(NumCharacters);
+ }
+};
+
+class RootSignatureParser {
+public:
+ RootSignatureParser(SmallVector<RootElement> &Elements,
+ const SmallVector<RootSignatureToken> &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:
+ bool ReportError(); // TODO: Implement this to report error through Diags
+
+ // Root Element helpers
+ bool ParseRootElement();
+ bool ParseDescriptorTable();
+ bool ParseDescriptorTableClause();
+
+ // Common parsing helpers
+ bool ParseRegister(Register &Register);
+
+ // Various flags/enum parsing helpers
+ bool ParseDescriptorRangeFlags(DescriptorRangeFlags &Flags);
+ bool ParseShaderVisibility(ShaderVisibility &Flag);
+
+ // 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();
+
+ // 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.
+ 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<RootElement> &Elements;
+ SmallVector<RootSignatureToken>::const_iterator CurTok;
+ SmallVector<RootSignatureToken>::const_iterator LastTok;
+};
+
+} // namespace root_signature
+} // namespace hlsl
+} // namespace llvm
+
+#endif // LLVM_CLANG_PARSE_PARSEHLSLROOTSIGNATURE_H
diff --git a/clang/lib/Parse/CMakeLists.txt b/clang/lib/Parse/CMakeLists.txt
index 22e902f7e1bc50..00fde537bb9c60 100644
--- a/clang/lib/Parse/CMakeLists.txt
+++ b/clang/lib/Parse/CMakeLists.txt
@@ -14,6 +14,7 @@ add_clang_library(clangParse
ParseExpr.cpp
ParseExprCXX.cpp
ParseHLSL.cpp
+ ParseHLSLRootSignature.cpp
ParseInit.cpp
ParseObjc.cpp
ParseOpenMP.cpp
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
new file mode 100644
index 00000000000000..9f12ff12866dcd
--- /dev/null
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -0,0 +1,479 @@
+#include "clang/Parse/ParseHLSLRootSignature.h"
+
+namespace llvm {
+namespace hlsl {
+namespace root_signature {
+
+// Lexer Definitions
+
+static bool IsPreprocessorNumberChar(char C) {
+ // TODO: extend for float support with or without hexadecimal/exponent
+ return isdigit(C); // integer support
+}
+
+bool RootSignatureLexer::LexNumber(RootSignatureToken &Result) {
+ // NumericLiteralParser does not handle the sign so we will manually apply it
+ Result.Signed = Buffer.front() == '-';
+ if (Result.Signed)
+ AdvanceBuffer();
+
+ // Retrieve the possible number
+ StringRef NumSpelling = Buffer.take_while(IsPreprocessorNumberChar);
+
+ // Parse the numeric value and so semantic checks on its specification
+ clang::NumericLiteralParser Literal(NumSpelling, SourceLoc,
+ PP.getSourceManager(), PP.getLangOpts(),
+ PP.getTargetInfo(), PP.getDiagnostics());
+ if (Literal.hadError)
+ return true; // Error has already been reported so just return
+
+ // Retrieve the number value to store into the token
+ if (Literal.isIntegerLiteral()) {
+ Result.Kind = TokenKind::int_literal;
+
+ APSInt X = APSInt(32, Result.Signed);
+ if (Literal.GetIntegerValue(X))
+ return true; // TODO: Report overflow error
+
+ X = Result.Signed ? -X : X;
+ Result.IntLiteral = (uint32_t)X.getZExtValue();
+ } else {
+ return true; // TODO: report unsupported number literal specification
+ }
+
+ AdvanceBuffer(NumSpelling.size());
+ return false;
+}
+
+bool RootSignatureLexer::Lex(SmallVector<RootSignatureToken> &Tokens) {
+ // Discard any leading whitespace
+ AdvanceBuffer(Buffer.take_while(isspace).size());
+
+ while (!Buffer.empty()) {
+ RootSignatureToken Result;
+ if (LexToken(Result))
+ return true;
+
+ // Successfully Lexed the token so we can store it
+ Tokens.push_back(Result);
+
+ // Discard any trailing whitespace
+ AdvanceBuffer(Buffer.take_while(isspace).size());
+ }
+
+ return false;
+}
+
+bool RootSignatureLexer::LexToken(RootSignatureToken &Result) {
+ // Record where this token is in the text of diagnostics
+ Result.TokLoc = SourceLoc;
+
+ char C = Buffer.front();
+
+ // Punctuators
+ switch (C) {
+#define PUNCTUATOR(X, Y) \
+ case Y: { \
+ Result.Kind = TokenKind::pu_##X; \
+ AdvanceBuffer(); \
+ return false; \
+ }
+#include "clang/Parse/HLSLRootSignatureTokenKinds.def"
+ default:
+ break;
+ }
+
+ // Numeric constant
+ if (isdigit(C) || C == '-')
+ return LexNumber(Result);
+
+ // All following tokens require at least one additional character
+ if (Buffer.size() <= 1)
+ return true; // TODO: Report invalid token error
+
+ // Peek at the next character to deteremine token type
+ char NextC = Buffer[1];
+
+ // Registers: [tsub][0-9+]
+ if ((C == 't' || C == 's' || C == 'u' || C == 'b') && isdigit(NextC)) {
+ AdvanceBuffer();
+
+ if (LexNumber(Result))
+ return true;
+
+ // Lex number could also parse a float so ensure it was an unsigned int
+ if (Result.Kind != TokenKind::int_literal || Result.Signed)
+ return true; // Return invalid number literal for register error
+
+ // Convert character to the register type.
+ // This is done after LexNumber to override the TokenKind
+ switch (C) {
+ case 'b':
+ Result.Kind = TokenKind::bReg;
+ break;
+ case 't':
+ Result.Kind = TokenKind::tReg;
+ break;
+ case 'u':
+ Result.Kind = TokenKind::uReg;
+ break;
+ case 's':
+ Result.Kind = TokenKind::sReg;
+ break;
+ default:
+ llvm_unreachable("Switch for an expected token was not provided");
+ return true;
+ }
+ return false;
+ }
+
+ // Keywords and Enums:
+ StringRef TokSpelling =
+ Buffer.take_while([](char C) { return isalnum(C) || C == '_'; });
+
+ // Define a large string switch statement for all the keywords and enums
+ auto Switch = llvm::StringSwitch<TokenKind>(TokSpelling);
+#define KEYWORD(NAME) Switch.Case(#NAME, TokenKind::kw_##NAME);
+#define ENUM(NAME, LIT) Switch.CaseLower(LIT, TokenKind::en_##NAME);
+#include "clang/Parse/HLSLRootSignatureTokenKinds.def"
+
+ // Then attempt to retreive a string from it
+ auto Kind = Switch.Default(TokenKind::invalid);
+ if (Kind == TokenKind::invalid)
+ return true; // TODO: Report invalid identifier
+
+ Result.Kind = Kind;
+ AdvanceBuffer(TokSpelling.size());
+ return false;
+}
+
+// Parser Definitions
+
+RootSignatureParser::RootSignatureParser(
+ SmallVector<RootElement> &Elements,
+ const SmallVector<RootSignatureToken> &Tokens)
+ : Elements(Elements) {
+ CurTok = Tokens.begin();
+ LastTok = Tokens.end();
+}
+
+bool RootSignatureParser::ReportError() { return true; }
+
+bool RootSignatureParser::Parse() {
+ CurTok--; // Decrement once here so we can use the ...ExpectedToken api
+
+ // Iterate as many RootElements as possible
+ bool HasComma = true;
+ while (HasComma &&
+ !TryConsumeExpectedToken(ArrayRef{TokenKind::kw_DescriptorTable})) {
+ if (ParseRootElement())
+ return true;
+ HasComma = !TryConsumeExpectedToken(TokenKind::pu_comma);
+ }
+ if (HasComma)
+ return ReportError(); // report 'comma' denotes a required extra item
+
+ // Ensure that we are at the end of the tokens
+ CurTok++;
+ if (CurTok != LastTok)
+ return ReportError(); // report expected end of input but got more
+ return false;
+}
+
+bool RootSignatureParser::ParseRootElement() {
+ // 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;
+
+ // Iterate as many DescriptorTableClaues as possible
+ bool HasComma = true;
+ while (!TryConsumeExpectedToken({TokenKind::kw_CBV, TokenKind::kw_SRV,
+ TokenKind::kw_UAV, TokenKind::kw_Sampler})) {
+ if (ParseDescriptorTableClause())
+ return true;
+ Table.NumClauses++;
+ HasComma = !TryConsumeExpectedToken(TokenKind::pu_comma);
+ }
+
+ // Consume optional 'visibility' paramater
+ if (HasComma && !TryConsumeExpectedToken(TokenKind::kw_visibility)) {
+ if (ConsumeExpectedToken(TokenKind::pu_equal))
+ return true;
+
+ if (ParseShaderVisibility(Table.Visibility))
+ return true;
+
+ HasComma = !TryConsumeExpectedToken(TokenKind::pu_comma);
+ }
+
+ if (HasComma)
+ return ReportError(); // report 'comma' denotes a required extra item
+
+ if (ConsumeExpectedToken(TokenKind::pu_r_paren))
+ return true;
+
+ Elements.push_back(RootElement(Table));
+ return false;
+}
+
+bool RootSignatureParser::ParseDescriptorTableClause() {
+ // Determine the type of Clause first so we can initialize the struct with
+ // the correct default flags
+ ClauseType CT;
+ switch (CurTok->Kind) {
+ case TokenKind::kw_CBV:
+ CT = ClauseType::CBV;
+ break;
+ case TokenKind::kw_SRV:
+ CT = ClauseType::SRV;
+ break;
+ case TokenKind::kw_UAV:
+ CT = ClauseType::UAV;
+ break;
+ case TokenKind::kw_Sampler:
+ CT = ClauseType::Sampler;
+ break;
+ default:
+ llvm_unreachable("Switch for an expected token was not provided");
+ return true;
+ }
+ DescriptorTableClause Clause(CT);
+
+ 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;
+
+ // Start parsing the optional parameters
+ bool HasComma = !TryConsumeExpectedToken(TokenKind::pu_comma);
+
+ // Consume optional 'numDescriptors' paramater
+ if (HasComma && !TryConsumeExpectedToken(TokenKind::kw_numDescriptors)) {
+ if (ConsumeExpectedToken(TokenKind::pu_equal))
+ return true;
+ if (ConsumeExpectedToken(TokenKind::int_literal))
+ return true;
+
+ Clause.NumDescriptors = CurTok->IntLiteral;
+
+ HasComma = !TryConsumeExpectedToken(TokenKind::pu_comma);
+ }
+
+ // Consume optional 'space' paramater
+ if (HasComma && !TryConsumeExpectedToken(TokenKind::kw_space)) {
+ if (ConsumeExpectedToken(TokenKind::pu_equal))
+ return true;
+ if (ConsumeExpectedToken(TokenKind::int_literal))
+ return true;
+
+ Clause.Space = CurTok->IntLiteral;
+
+ HasComma = !TryConsumeExpectedToken(TokenKind::pu_comma);
+ }
+
+ // Consume optional 'offset' paramater
+ if (HasComma && !TryConsumeExpectedToken(TokenKind::kw_offset)) {
+ if (ConsumeExpectedToken(TokenKind::pu_equal))
+ return true;
+ if (ConsumeExpectedToken(ArrayRef{
+ TokenKind::int_literal, TokenKind::en_DescriptorRangeOffsetAppend}))
+ return true;
+
+ // Offset defaults to DescriptorTableOffsetAppend so only change if we have
+ // an int arg
+ if (CurTok->Kind == TokenKind::int_literal)
+ Clause.Offset = CurTok->IntLiteral;
+
+ HasComma = !TryConsumeExpectedToken(TokenKind::pu_comma);
+ }
+
+ // Consume optional 'flags' paramater
+ if (HasComma && !TryConsumeExpectedToken(TokenKind::kw_flags)) {
+ if (ConsumeExpectedToken(TokenKind::pu_equal))
+ ...
[truncated]
|
|
@llvm/pr-subscribers-clang Author: Finn Plummer (inbelic) Changes
This an initial part of #120472 Patch is 38.92 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/122982.diff 8 Files Affected:
diff --git a/clang/include/clang/Parse/HLSLRootSignatureTokenKinds.def b/clang/include/clang/Parse/HLSLRootSignatureTokenKinds.def
new file mode 100644
index 00000000000000..aaa2dabcb43f4e
--- /dev/null
+++ b/clang/include/clang/Parse/HLSLRootSignatureTokenKinds.def
@@ -0,0 +1,121 @@
+//===--- HLSLRootSignature.def - Tokens and Enum Database -------*- C++ -*-===//
+
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines the TokenKinds used in the Root Signature DSL. This
+// includes keywords, enums and a small subset of punctuators. Users of this
+// file must optionally #define the TOK, KEYWORD, ENUM or specific ENUM macros
+// to make use of this file.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef TOK
+#define TOK(X)
+#endif
+#ifndef PUNCTUATOR
+#define PUNCTUATOR(X,Y) TOK(pu_ ## X)
+#endif
+#ifndef KEYWORD
+#define KEYWORD(X) TOK(kw_ ## X)
+#endif
+#ifndef ENUM
+#define ENUM(NAME, LIT) TOK(en_ ## NAME)
+#endif
+
+// Defines the various types of enum
+#ifndef DESCRIPTOR_RANGE_OFFSET_ENUM
+#define DESCRIPTOR_RANGE_OFFSET_ENUM(NAME, LIT) ENUM(NAME, LIT)
+#endif
+#ifndef ROOT_DESCRIPTOR_FLAG_ENUM
+#define ROOT_DESCRIPTOR_FLAG_ENUM(NAME, LIT) ENUM(NAME, LIT)
+#endif
+// Note: ON denotes that the flag unique from the above Root Descriptor Flags.
+// This is required to avoid token kind enum conflicts.
+#ifndef DESCRIPTOR_RANGE_FLAG_ENUM_OFF
+#define DESCRIPTOR_RANGE_FLAG_ENUM_OFF(NAME, LIT)
+#endif
+#ifndef DESCRIPTOR_RANGE_FLAG_ENUM_ON
+#define DESCRIPTOR_RANGE_FLAG_ENUM_ON(NAME, LIT) ENUM(NAME, LIT)
+#endif
+#ifndef DESCRIPTOR_RANGE_FLAG_ENUM
+#define DESCRIPTOR_RANGE_FLAG_ENUM(NAME, LIT, ON) DESCRIPTOR_RANGE_FLAG_ENUM_##ON(NAME, LIT)
+#endif
+#ifndef SHADER_VISIBILITY_ENUM
+#define SHADER_VISIBILITY_ENUM(NAME, LIT) ENUM(NAME, LIT)
+#endif
+
+// General Tokens:
+TOK(invalid)
+TOK(int_literal)
+
+// Register Tokens:
+TOK(bReg)
+TOK(tReg)
+TOK(uReg)
+TOK(sReg)
+
+// Punctuators:
+PUNCTUATOR(l_paren, '(')
+PUNCTUATOR(r_paren, ')')
+PUNCTUATOR(comma, ',')
+PUNCTUATOR(or, '|')
+PUNCTUATOR(equal, '=')
+
+// RootElement Keywords:
+KEYWORD(DescriptorTable)
+
+// DescriptorTable Keywords:
+KEYWORD(CBV)
+KEYWORD(SRV)
+KEYWORD(UAV)
+KEYWORD(Sampler)
+
+// General Parameter Keywords:
+KEYWORD(space)
+KEYWORD(visibility)
+KEYWORD(flags)
+
+// View Parameter Keywords:
+KEYWORD(numDescriptors)
+KEYWORD(offset)
+
+// Descriptor Range Offset Enum:
+DESCRIPTOR_RANGE_OFFSET_ENUM(DescriptorRangeOffsetAppend, "DESCRIPTOR_RANGE_OFFSET_APPEND")
+
+// Root Descriptor Flag Enums:
+ROOT_DESCRIPTOR_FLAG_ENUM(DataVolatile, "DATA_VOLATILE")
+ROOT_DESCRIPTOR_FLAG_ENUM(DataStaticWhileSetAtExecute, "DATA_STATIC_WHILE_SET_AT_EXECUTE")
+ROOT_DESCRIPTOR_FLAG_ENUM(DataStatic, "DATA_STATIC")
+
+// Descriptor Range Flag Enums:
+DESCRIPTOR_RANGE_FLAG_ENUM(DescriptorsVolatile, "DESCRIPTORS_VOLATILE", ON)
+DESCRIPTOR_RANGE_FLAG_ENUM(DataVolatile, "DATA_VOLATILE", OFF)
+DESCRIPTOR_RANGE_FLAG_ENUM(DataStaticWhileSetAtExecute, "DATA_STATIC_WHILE_SET_AT_EXECUTE", OFF)
+DESCRIPTOR_RANGE_FLAG_ENUM(DataStatic, "DATA_STATIC", OFF)
+DESCRIPTOR_RANGE_FLAG_ENUM(DescriptorsStaticKeepingBufferBoundsChecks, "DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS", ON)
+
+// Shader Visibiliy Enums:
+SHADER_VISIBILITY_ENUM(All, "SHADER_VISIBILITY_ALL")
+SHADER_VISIBILITY_ENUM(Vertex, "SHADER_VISIBILITY_VERTEX")
+SHADER_VISIBILITY_ENUM(Hull, "SHADER_VISIBILITY_HULL")
+SHADER_VISIBILITY_ENUM(Domain, "SHADER_VISIBILITY_DOMAIN")
+SHADER_VISIBILITY_ENUM(Geometry, "SHADER_VISIBILITY_GEOMETRY")
+SHADER_VISIBILITY_ENUM(Pixel, "SHADER_VISIBILITY_PIXEL")
+SHADER_VISIBILITY_ENUM(Amplification, "SHADER_VISIBILITY_AMPLIFICATION")
+SHADER_VISIBILITY_ENUM(Mesh, "SHADER_VISIBILITY_MESH")
+
+#undef SHADER_VISIBILITY_ENUM
+#undef DESCRIPTOR_RANGE_FLAG_ENUM
+#undef DESCRIPTOR_RANGE_FLAG_ENUM_OFF
+#undef DESCRIPTOR_RANGE_FLAG_ENUM_ON
+#undef ROOT_DESCRIPTOR_FLAG_ENUM
+#undef DESCRIPTOR_RANGE_OFFSET_ENUM
+#undef ENUM
+#undef KEYWORD
+#undef PUNCTUATOR
+#undef TOK
diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h
new file mode 100644
index 00000000000000..9464bd8f2f9e0f
--- /dev/null
+++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h
@@ -0,0 +1,164 @@
+//===--- ParseHLSLRootSignature.h -------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines the ParseHLSLRootSignature interface.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_PARSE_PARSEHLSLROOTSIGNATURE_H
+#define LLVM_CLANG_PARSE_PARSEHLSLROOTSIGNATURE_H
+
+#include "clang/Lex/LiteralSupport.h"
+#include "clang/Lex/Preprocessor.h"
+
+#include "llvm/ADT/APInt.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
+
+#include "llvm/Frontend/HLSL/HLSLRootSignature.h"
+
+namespace llvm {
+namespace hlsl {
+namespace root_signature {
+
+struct RootSignatureToken {
+ enum Kind {
+#define TOK(X) X,
+#include "clang/Parse/HLSLRootSignatureTokenKinds.def"
+ };
+
+ Kind Kind = Kind::invalid;
+
+ // Retain the SouceLocation of the token for diagnostics
+ clang::SourceLocation TokLoc;
+
+ // Retain if the uint32_t bits represent a signed integer
+ bool Signed = false;
+ union {
+ uint32_t IntLiteral = 0;
+ float FloatLiteral;
+ };
+
+ // Constructors
+ RootSignatureToken() {}
+ RootSignatureToken(clang::SourceLocation TokLoc) : TokLoc(TokLoc) {}
+};
+using TokenKind = enum RootSignatureToken::Kind;
+
+class RootSignatureLexer {
+public:
+ RootSignatureLexer(StringRef Signature, clang::SourceLocation SourceLoc,
+ clang::Preprocessor &PP)
+ : Buffer(Signature), SourceLoc(SourceLoc), PP(PP) {}
+
+ // Consumes the internal buffer as a list of tokens and will emplace them
+ // onto the given tokens.
+ //
+ // It will consume until it successfully reaches the end of the buffer,
+ // or, until the first error is encountered. The return value denotes if
+ // there was a failure.
+ bool Lex(SmallVector<RootSignatureToken> &Tokens);
+
+ // Get the current source location of the lexer
+ clang::SourceLocation GetLocation() { return SourceLoc; };
+
+private:
+ // Internal buffer to iterate over
+ StringRef Buffer;
+
+ // Passed down parameters from Sema
+ clang::SourceLocation SourceLoc;
+ clang::Preprocessor &PP;
+
+ bool LexNumber(RootSignatureToken &Result);
+
+ // Consumes the internal buffer for a single token.
+ //
+ // The return value denotes if there was a failure.
+ bool LexToken(RootSignatureToken &Token);
+
+ // Advance the buffer by the specified number of characters. Updates the
+ // SourceLocation appropriately.
+ void AdvanceBuffer(unsigned NumCharacters = 1) {
+ Buffer = Buffer.drop_front(NumCharacters);
+ SourceLoc = SourceLoc.getLocWithOffset(NumCharacters);
+ }
+};
+
+class RootSignatureParser {
+public:
+ RootSignatureParser(SmallVector<RootElement> &Elements,
+ const SmallVector<RootSignatureToken> &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:
+ bool ReportError(); // TODO: Implement this to report error through Diags
+
+ // Root Element helpers
+ bool ParseRootElement();
+ bool ParseDescriptorTable();
+ bool ParseDescriptorTableClause();
+
+ // Common parsing helpers
+ bool ParseRegister(Register &Register);
+
+ // Various flags/enum parsing helpers
+ bool ParseDescriptorRangeFlags(DescriptorRangeFlags &Flags);
+ bool ParseShaderVisibility(ShaderVisibility &Flag);
+
+ // 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();
+
+ // 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.
+ 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<RootElement> &Elements;
+ SmallVector<RootSignatureToken>::const_iterator CurTok;
+ SmallVector<RootSignatureToken>::const_iterator LastTok;
+};
+
+} // namespace root_signature
+} // namespace hlsl
+} // namespace llvm
+
+#endif // LLVM_CLANG_PARSE_PARSEHLSLROOTSIGNATURE_H
diff --git a/clang/lib/Parse/CMakeLists.txt b/clang/lib/Parse/CMakeLists.txt
index 22e902f7e1bc50..00fde537bb9c60 100644
--- a/clang/lib/Parse/CMakeLists.txt
+++ b/clang/lib/Parse/CMakeLists.txt
@@ -14,6 +14,7 @@ add_clang_library(clangParse
ParseExpr.cpp
ParseExprCXX.cpp
ParseHLSL.cpp
+ ParseHLSLRootSignature.cpp
ParseInit.cpp
ParseObjc.cpp
ParseOpenMP.cpp
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
new file mode 100644
index 00000000000000..9f12ff12866dcd
--- /dev/null
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -0,0 +1,479 @@
+#include "clang/Parse/ParseHLSLRootSignature.h"
+
+namespace llvm {
+namespace hlsl {
+namespace root_signature {
+
+// Lexer Definitions
+
+static bool IsPreprocessorNumberChar(char C) {
+ // TODO: extend for float support with or without hexadecimal/exponent
+ return isdigit(C); // integer support
+}
+
+bool RootSignatureLexer::LexNumber(RootSignatureToken &Result) {
+ // NumericLiteralParser does not handle the sign so we will manually apply it
+ Result.Signed = Buffer.front() == '-';
+ if (Result.Signed)
+ AdvanceBuffer();
+
+ // Retrieve the possible number
+ StringRef NumSpelling = Buffer.take_while(IsPreprocessorNumberChar);
+
+ // Parse the numeric value and so semantic checks on its specification
+ clang::NumericLiteralParser Literal(NumSpelling, SourceLoc,
+ PP.getSourceManager(), PP.getLangOpts(),
+ PP.getTargetInfo(), PP.getDiagnostics());
+ if (Literal.hadError)
+ return true; // Error has already been reported so just return
+
+ // Retrieve the number value to store into the token
+ if (Literal.isIntegerLiteral()) {
+ Result.Kind = TokenKind::int_literal;
+
+ APSInt X = APSInt(32, Result.Signed);
+ if (Literal.GetIntegerValue(X))
+ return true; // TODO: Report overflow error
+
+ X = Result.Signed ? -X : X;
+ Result.IntLiteral = (uint32_t)X.getZExtValue();
+ } else {
+ return true; // TODO: report unsupported number literal specification
+ }
+
+ AdvanceBuffer(NumSpelling.size());
+ return false;
+}
+
+bool RootSignatureLexer::Lex(SmallVector<RootSignatureToken> &Tokens) {
+ // Discard any leading whitespace
+ AdvanceBuffer(Buffer.take_while(isspace).size());
+
+ while (!Buffer.empty()) {
+ RootSignatureToken Result;
+ if (LexToken(Result))
+ return true;
+
+ // Successfully Lexed the token so we can store it
+ Tokens.push_back(Result);
+
+ // Discard any trailing whitespace
+ AdvanceBuffer(Buffer.take_while(isspace).size());
+ }
+
+ return false;
+}
+
+bool RootSignatureLexer::LexToken(RootSignatureToken &Result) {
+ // Record where this token is in the text of diagnostics
+ Result.TokLoc = SourceLoc;
+
+ char C = Buffer.front();
+
+ // Punctuators
+ switch (C) {
+#define PUNCTUATOR(X, Y) \
+ case Y: { \
+ Result.Kind = TokenKind::pu_##X; \
+ AdvanceBuffer(); \
+ return false; \
+ }
+#include "clang/Parse/HLSLRootSignatureTokenKinds.def"
+ default:
+ break;
+ }
+
+ // Numeric constant
+ if (isdigit(C) || C == '-')
+ return LexNumber(Result);
+
+ // All following tokens require at least one additional character
+ if (Buffer.size() <= 1)
+ return true; // TODO: Report invalid token error
+
+ // Peek at the next character to deteremine token type
+ char NextC = Buffer[1];
+
+ // Registers: [tsub][0-9+]
+ if ((C == 't' || C == 's' || C == 'u' || C == 'b') && isdigit(NextC)) {
+ AdvanceBuffer();
+
+ if (LexNumber(Result))
+ return true;
+
+ // Lex number could also parse a float so ensure it was an unsigned int
+ if (Result.Kind != TokenKind::int_literal || Result.Signed)
+ return true; // Return invalid number literal for register error
+
+ // Convert character to the register type.
+ // This is done after LexNumber to override the TokenKind
+ switch (C) {
+ case 'b':
+ Result.Kind = TokenKind::bReg;
+ break;
+ case 't':
+ Result.Kind = TokenKind::tReg;
+ break;
+ case 'u':
+ Result.Kind = TokenKind::uReg;
+ break;
+ case 's':
+ Result.Kind = TokenKind::sReg;
+ break;
+ default:
+ llvm_unreachable("Switch for an expected token was not provided");
+ return true;
+ }
+ return false;
+ }
+
+ // Keywords and Enums:
+ StringRef TokSpelling =
+ Buffer.take_while([](char C) { return isalnum(C) || C == '_'; });
+
+ // Define a large string switch statement for all the keywords and enums
+ auto Switch = llvm::StringSwitch<TokenKind>(TokSpelling);
+#define KEYWORD(NAME) Switch.Case(#NAME, TokenKind::kw_##NAME);
+#define ENUM(NAME, LIT) Switch.CaseLower(LIT, TokenKind::en_##NAME);
+#include "clang/Parse/HLSLRootSignatureTokenKinds.def"
+
+ // Then attempt to retreive a string from it
+ auto Kind = Switch.Default(TokenKind::invalid);
+ if (Kind == TokenKind::invalid)
+ return true; // TODO: Report invalid identifier
+
+ Result.Kind = Kind;
+ AdvanceBuffer(TokSpelling.size());
+ return false;
+}
+
+// Parser Definitions
+
+RootSignatureParser::RootSignatureParser(
+ SmallVector<RootElement> &Elements,
+ const SmallVector<RootSignatureToken> &Tokens)
+ : Elements(Elements) {
+ CurTok = Tokens.begin();
+ LastTok = Tokens.end();
+}
+
+bool RootSignatureParser::ReportError() { return true; }
+
+bool RootSignatureParser::Parse() {
+ CurTok--; // Decrement once here so we can use the ...ExpectedToken api
+
+ // Iterate as many RootElements as possible
+ bool HasComma = true;
+ while (HasComma &&
+ !TryConsumeExpectedToken(ArrayRef{TokenKind::kw_DescriptorTable})) {
+ if (ParseRootElement())
+ return true;
+ HasComma = !TryConsumeExpectedToken(TokenKind::pu_comma);
+ }
+ if (HasComma)
+ return ReportError(); // report 'comma' denotes a required extra item
+
+ // Ensure that we are at the end of the tokens
+ CurTok++;
+ if (CurTok != LastTok)
+ return ReportError(); // report expected end of input but got more
+ return false;
+}
+
+bool RootSignatureParser::ParseRootElement() {
+ // 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;
+
+ // Iterate as many DescriptorTableClaues as possible
+ bool HasComma = true;
+ while (!TryConsumeExpectedToken({TokenKind::kw_CBV, TokenKind::kw_SRV,
+ TokenKind::kw_UAV, TokenKind::kw_Sampler})) {
+ if (ParseDescriptorTableClause())
+ return true;
+ Table.NumClauses++;
+ HasComma = !TryConsumeExpectedToken(TokenKind::pu_comma);
+ }
+
+ // Consume optional 'visibility' paramater
+ if (HasComma && !TryConsumeExpectedToken(TokenKind::kw_visibility)) {
+ if (ConsumeExpectedToken(TokenKind::pu_equal))
+ return true;
+
+ if (ParseShaderVisibility(Table.Visibility))
+ return true;
+
+ HasComma = !TryConsumeExpectedToken(TokenKind::pu_comma);
+ }
+
+ if (HasComma)
+ return ReportError(); // report 'comma' denotes a required extra item
+
+ if (ConsumeExpectedToken(TokenKind::pu_r_paren))
+ return true;
+
+ Elements.push_back(RootElement(Table));
+ return false;
+}
+
+bool RootSignatureParser::ParseDescriptorTableClause() {
+ // Determine the type of Clause first so we can initialize the struct with
+ // the correct default flags
+ ClauseType CT;
+ switch (CurTok->Kind) {
+ case TokenKind::kw_CBV:
+ CT = ClauseType::CBV;
+ break;
+ case TokenKind::kw_SRV:
+ CT = ClauseType::SRV;
+ break;
+ case TokenKind::kw_UAV:
+ CT = ClauseType::UAV;
+ break;
+ case TokenKind::kw_Sampler:
+ CT = ClauseType::Sampler;
+ break;
+ default:
+ llvm_unreachable("Switch for an expected token was not provided");
+ return true;
+ }
+ DescriptorTableClause Clause(CT);
+
+ 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;
+
+ // Start parsing the optional parameters
+ bool HasComma = !TryConsumeExpectedToken(TokenKind::pu_comma);
+
+ // Consume optional 'numDescriptors' paramater
+ if (HasComma && !TryConsumeExpectedToken(TokenKind::kw_numDescriptors)) {
+ if (ConsumeExpectedToken(TokenKind::pu_equal))
+ return true;
+ if (ConsumeExpectedToken(TokenKind::int_literal))
+ return true;
+
+ Clause.NumDescriptors = CurTok->IntLiteral;
+
+ HasComma = !TryConsumeExpectedToken(TokenKind::pu_comma);
+ }
+
+ // Consume optional 'space' paramater
+ if (HasComma && !TryConsumeExpectedToken(TokenKind::kw_space)) {
+ if (ConsumeExpectedToken(TokenKind::pu_equal))
+ return true;
+ if (ConsumeExpectedToken(TokenKind::int_literal))
+ return true;
+
+ Clause.Space = CurTok->IntLiteral;
+
+ HasComma = !TryConsumeExpectedToken(TokenKind::pu_comma);
+ }
+
+ // Consume optional 'offset' paramater
+ if (HasComma && !TryConsumeExpectedToken(TokenKind::kw_offset)) {
+ if (ConsumeExpectedToken(TokenKind::pu_equal))
+ return true;
+ if (ConsumeExpectedToken(ArrayRef{
+ TokenKind::int_literal, TokenKind::en_DescriptorRangeOffsetAppend}))
+ return true;
+
+ // Offset defaults to DescriptorTableOffsetAppend so only change if we have
+ // an int arg
+ if (CurTok->Kind == TokenKind::int_literal)
+ Clause.Offset = CurTok->IntLiteral;
+
+ HasComma = !TryConsumeExpectedToken(TokenKind::pu_comma);
+ }
+
+ // Consume optional 'flags' paramater
+ if (HasComma && !TryConsumeExpectedToken(TokenKind::kw_flags)) {
+ if (ConsumeExpectedToken(TokenKind::pu_equal))
+ ...
[truncated]
|
inbelic
commented
Jan 15, 2025
| bool Parse(); | ||
|
|
||
| private: | ||
| bool ReportError(); // TODO: Implement this to report error through Diags |
There was a problem hiding this comment.
I have left this as a todo just to help with pr size, as it will require adding a good chunk of additional testing
There was a problem hiding this comment.
I don't think this should be separate. It's going to be really hard to ensure that any follow-up that adds error reporting properly covers all the cases.
inbelic
force-pushed
the
inbelic/rootsig-parse-dt
branch
from
2661569 to
ab390a2
Compare
inbelic
force-pushed
the
inbelic/rootsig-parse-dt
branch
from
ab390a2 to
cac95d1
Compare
V-FEXrt
reviewed
Jan 22, 2025
V-FEXrt
reviewed
Jan 22, 2025
| if (ParseRegister(Clause.Register)) | ||
| return true; | ||
|
|
||
| // Start parsing the optional parameters |
There was a problem hiding this comment.
Is the order of the optional flags set (i.e must be numDescriptors, then space, then offset, then flags ...) or can they be provided in any order?
There was a problem hiding this comment.
My understanding of the grammar specified here was that they need to be in the given order. Although looking at DXC's implementation it seems they can be specified in any order.
There was a problem hiding this comment.
Confirmed that we should accept the parameters in any order. So will need to refactor this.
llvm-beanz
reviewed
Jan 23, 2025
There was a problem hiding this comment.
I think that the way you're breaking up this change is sub-optimal from a review perspective. You've added a lot of code that partially handles parsing a very complex root signature. The problem is that to complete this implementation you're going to go back over this code over and over again fleshing it out, and from a reviewer's perspective we're going to need to keep paging back in extra context.
If instead you started with a much simpler root signature (even just an empty one), but implement more complete handling for it, we can review that and incrementally build up without revisiting the same code over and over again in each subsequent patch.
|
|
||
| static const uint32_t DescriptorTableOffsetAppend = 0xffffffff; | ||
| // Models DTClause : CBV | SRV | UAV | Sampler by collecting like parameters | ||
| enum class ClauseType { CBV, SRV, UAV, Sampler }; |
There was a problem hiding this comment.
Can we do this instead?
Suggested change
| enum class ClauseType { CBV, SRV, UAV, Sampler }; | |
| using ClauseType = llvm::dxil::ResourceClass |
This will change CBV to CBuffer, but otherwise those enums need to be the same right?
| bool Parse(); | ||
|
|
||
| private: | ||
| bool ReportError(); // TODO: Implement this to report error through Diags |
There was a problem hiding this comment.
I don't think this should be separate. It's going to be really hard to ensure that any follow-up that adds error reporting properly covers all the cases.
|
Sounds good, and I appreciate the feedback. I will restructure the changes to be of smaller granularity, which will be better self-contained and directly include their diagnostics testing. |
damyanp
reviewed
Jan 24, 2025
| RootSignatureToken PeekNextToken(); | ||
|
|
||
| // Is the current token one of the expected kinds | ||
| bool IsCurExpectedToken(ArrayRef<TokenKind> AnyExpected); |
There was a problem hiding this comment.
Any reason this doesn't have an overload that takes a single Expected like all the ones below?
Alternatively, if this one doesn't need the overload then do we need the other ones?
| return ParseDescriptorTable(); | ||
| default: | ||
| llvm_unreachable("Switch for an expected token was not provided"); | ||
| return true; |
There was a problem hiding this comment.
Is the return true after llvm_unreachable the right thing to do here?
There was a problem hiding this comment.
Return true denotes that we encountered an error, so after hitting the unreachable state, this will just let the parser error out.
| }; | ||
|
|
||
| // Models RootElement : DescriptorTable | DescriptorTableClause | ||
| struct RootElement { |
There was a problem hiding this comment.
Is there a reason not to use std::variant?
There was a problem hiding this comment.
Nope, good idea! Switched
| enum class ClauseType { CBV, SRV, UAV, Sampler }; | ||
| struct DescriptorTableClause { | ||
| ClauseType Type; | ||
| Register Register; |
There was a problem hiding this comment.
It looks like there's nothing enforcing Register's initialization? Since this struct has a constructor, I'd expect to get a fully initialized object back when I construct it.
There was a problem hiding this comment.
Switched to use a non-constructor function to avoid implying that the register would be initialized.
3 tasks
inbelic
force-pushed
the
users/inbelic/pr-122981
branch
from
a76b907 to
dc784ff
Compare
inbelic
force-pushed
the
inbelic/rootsig-parse-dt
branch
2 times, most recently
from
03d7592 to
65ebfe1
Compare
- Define the Parser struct - Model RootElements as a variant of the different types - Create a basic test case for unit testing
- use numDescriptors as an example
- demonstrate can specify in any order
- introduces the ParseEnum function that will parse any of the ENUM token definitions
- use DescriptorRangeFlags to demonstrate valid functionality
inbelic
force-pushed
the
inbelic/rootsig-parse-dt
branch
from
65ebfe1 to
b643259
Compare
|
Updated to include diagnostics output and the relevant testing. I have rebased to split the pr into smaller incremental changes, to help with review. And I have taken care to address all previous comments. Unfortunately the rebase causes these comments to get lost :/ I have implemented the suggestion to use Notably though, there was some bigger changes introduced to handle that the optional parameters can be specified in any order. |
joaosaffran
reviewed
Jan 29, 2025
| return true; | ||
|
|
||
| // Dispatch onto the correct parse method | ||
| switch (CurTok->Kind) { |
There was a problem hiding this comment.
I was expecting this switch to happen earlier in this method. What is the reason to ensure that descriptor tables are the first element of Root Signatures? For example, this is valid SRV(t0), DescriptorTable(CBV(b0, numDescriptors=5)) and doesn't start with a DescriptorTable.
Comment on lines
+311
to
+333
| 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; | ||
| } | ||
| Clause.SetDefaultFlags(); |
There was a problem hiding this comment.
It might be interesting to move this to its own method, since it can be reused when handling root descriptors.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This an initial part of #120472