-
Notifications
You must be signed in to change notification settings - Fork 17
[0031] Proposal for shader semantics #296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
proposals/NNNN-input-semantics.md
Outdated
HLSLAnnotationAttr *Semantic; | ||
|
||
// Info about this field/scalar. | ||
DeclaratorDecl *Decl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be useful to handle the [[vk::location(X)]]
attribute. Would be handled later, on top of this.
Thanks for the review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple comments, but I think this looks like a good direction to me.
proposals/NNNN-semantics.md
Outdated
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to have Clang generate the HlslSemanticGOOGLE
decorations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
proposals/NNNN-semantics.md
Outdated
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.