================
@@ -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 = {
----------------
inbelic 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:
```
bool RootSignatureParser::parseStaticSampler() {
using StaticSamplerParams = ParamMap<13>; // N = # of params locally
StaticSampler Sampler;
StaticSamplerParams Params = {
/* Kinds=*/ = {
TokenKind::sReg,
TokenKind::kw_filter,
...
},
/* ParamType=*/ = {
&Sampler.Register
&Sampler.Filter,
...
},
/*Seen=*/ 0x0,
/*Mandatory=*/0x1 // only register is required
};
if (parseParams(Params))
return true;
...
}
```
- 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?).
- 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
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 4. And we could make `Mandatory/Seen`
just be two `uint32_t` bitfields.
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
