================
@@ -89,37 +88,178 @@ bool RootSignatureParser::parseDescriptorTableClause() {
CurToken.TokKind == TokenKind::kw_UAV ||
CurToken.TokKind == TokenKind::kw_Sampler) &&
"Expects to only be invoked starting at given keyword");
+ TokenKind ParamKind = CurToken.TokKind; // retain for diagnostics
DescriptorTableClause Clause;
- switch (CurToken.TokKind) {
+ TokenKind ExpectedRegister;
+ switch (ParamKind) {
default:
llvm_unreachable("Switch for consumed token was not provided");
case TokenKind::kw_CBV:
Clause.Type = ClauseType::CBuffer;
+ ExpectedRegister = TokenKind::bReg;
break;
case TokenKind::kw_SRV:
Clause.Type = ClauseType::SRV;
+ ExpectedRegister = TokenKind::tReg;
break;
case TokenKind::kw_UAV:
Clause.Type = ClauseType::UAV;
+ ExpectedRegister = TokenKind::uReg;
break;
case TokenKind::kw_Sampler:
Clause.Type = ClauseType::Sampler;
+ ExpectedRegister = TokenKind::sReg;
break;
}
if (consumeExpectedToken(TokenKind::pu_l_paren, diag::err_expected_after,
- CurToken.TokKind))
+ ParamKind))
return true;
- if (consumeExpectedToken(TokenKind::pu_r_paren, diag::err_expected_after,
- CurToken.TokKind))
+ llvm::SmallDenseMap<TokenKind, ParamType> Params = {
----------------
llvm-beanz wrote:
> Hm, okay. I think that is where we disagree then: whether it is worthwhile to
> make this generic or not.
>
> I will make the (opinionated) argument for having it implemented generically:
>
> * There isn't much of a difference in how we will parse the different root
> parameter types, `RootSBV(...), CBV(...), StaticSampler(...),
> RootConstants(...), etc`, so, adding their respective parse methods in future
> prs will just be something like:
I don't think what you've described int he code example here is where the
genericness becomes a problem.
> * We can contrast that with DXC,
> [here](https://github.com/microsoft/DirectXShaderCompiler/blob/c940161bb3398ff988fafc343ed1623d4a3fad6c/tools/clang/lib/Parse/HLSLRootSignature.cpp#L1301),
> where it needs to redefine the same "seen" and "mandatory" functionality
> over and over again.
> * I assume that if we don't want to have a generic method in clang then the
> code flow would follow a similar pattern as DXC (maybe I don't understand the
> struct approach correctly and that is a wrong assumption?).
I think you draw a false equivalence. You seem to be saying we either do this
generically, or we do this like DXC... That's not what I'm saying. The approach
that Clang takes generally in parsing is to parse things out, and from the
parsed information construct a declaration or statement, which then gets
validated during construction.
That is not how DXC's parser for root signatures works, nor is it what you're
proposing, but maybe it's the better approach.
> * Therefore, using a generic way to parse the parameters of root parameter
> types will be clearer in its intent (it is declarative) and easier to extend
> (instead of copying functionality over) when we add the other parts of the
> RootSignatureParser
I think the bigger question is at what layer of abstraction does being generic
help and at what layer of abstraction is it a hinderance.
Having a `parseRootParam` function that "generically" parses a root-level
parameter seems like a good idea, but should it remain generic based on the
type of the parameter? Should we have a single "parseArgument" function?
Maybe... Should these functions produce the same structural result for every
parameter? These things are less clear to me.
> These reasons are why I went with the current generic implementation.
>
> Regarding scalability, the struct of arrays or the map will have a statically
> known `N` elements (or pairs), where `N` is the number of parameters for a
> given root parameter type. (`N` is not equal to the total number of token
> kinds). The largest `N` would be is for `StaticSamplers` with 13, and then
> for example `DescriptorTableClause` it is 5. And we could make
> `Mandatory/Seen` just be two `uint32_t` bitfields.
The reasons that led you to a generic solution, might be reasons why following
existing patterns that Clang uses for order-independent parsing (like
attributes), might be the better architectural decision. Maybe we should borrow
more from Clang's AST design and have polymorphic returned objects rather than
variants.
https://github.com/llvm/llvm-project/pull/133800
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
