[0031] Proposal for shader semantics#296
Conversation
| HLSLAnnotationAttr *Semantic; | ||
|
|
||
| // Info about this field/scalar. | ||
| DeclaratorDecl *Decl; |
There was a problem hiding this comment.
This would be useful to handle the [[vk::location(X)]] attribute. Would be handled later, on top of this.
|
Thanks for the review! |
tex3d
left a comment
There was a problem hiding this comment.
Partial review of comments, thanks for all this work!
|
Thanks, rephrased mostly as suggested, reorganized a bit and added more intermediate examples, should be clearer |
llvm-beanz
left a comment
There was a problem hiding this comment.
A couple comments, but I think this looks like a good direction to me.
| It is also possible to explicitly set the index, using the | ||
| `[[vk::location(/* Index */)]]` attribute. \ | ||
| Mixing implicit and explicit location assignment is **not legal**. | ||
| Hence interaction between both mechanism is out of scope. |
There was a problem hiding this comment.
Do we want to have Clang generate the HlslSemanticGOOGLE decorations?
There was a problem hiding this comment.
I don't think we need to implement this for now, but we might want to look into reflection helpers like HLSLSemantic or HLSLCounter decorations in the future.
s-perron
left a comment
There was a problem hiding this comment.
Just one more case to consider for SPIR-V. That can be in a follow up PR. It will not affect the design dramatically, if at all.
| In the example above, there are no system semantics, meaning every | ||
| parameter would get a `Location` decorated variable associated. | ||
| Each scalar field/parameter is associated with a unique index starting at 0, | ||
| from the first parameter's field to the last parameter's field. | ||
| Each scalar takes one index, and arrays of size N takes N indices. | ||
| The semantic index does not impact the Location assignment. | ||
| Indices are independent between `Input` and `Output` semantics. |
There was a problem hiding this comment.
Minor issue that we can fix up in a follow up PR.
How will locations be assigned in libraries with multiple shaders? I think we should restart at 0 for each shader, but I'm not sure that DXC does that.
There was a problem hiding this comment.
From what I see, DXC assigned no Location at all when targeting library, and this causes validation issues.
See https://godbolt.org/z/jbf9zz5sE
I also think this should be thought more once we have a plan for libraries (same with decorations, capabilities, etc, multiple shaders in 1 spir-v module is a bit special)
|
Thanks all for the review! |
This is another proposal on how to implement semantic input in Clang, given DXIL & SPIR-V have drasticly different handlings, but some parts could be shared. Another proposal exists: llvm#112 which also suggest a sema change.
This is another proposal on how to implement semantic input in Clang, given DXIL & SPIR-V have drasticly different handlings, but some parts could be shared.
The POC for this implementation is in llvm/llvm-project#149363
Another proposal exists: #112 which also suggest a sema change.