Update DXIL.rst #6078
Update DXIL.rst #6078
Conversation
Make the table with supported shader model, major and minor numbers visible.
| Mesh shader (MS) no support ms_6_5 | ||
| Amplification shader (AS) no support as_6_5 | ||
| ========================= ===================================== =========== | ||
| The following values of ``<shaderModelName>``, ``<major>``, ``<minor>`` are supported: |
There was a problem hiding this comment.
Thanks for noticing and updating this!
I think the formatting with _ between these was meant to match the values in the table below, however this section is quite out of date anyway.
Here are some problems with the table as it stands:
- We don't actually support these "Legacy Models" in DXIL. When this was originally written, we did, but later decided to upgrade the shader model in dxc.exe, and not support legacy values at the API, nor capture them into the DXIL.
- The "DXIL Models" column only lists the very first shader model supported for various shader stages.
- lib_6_1 was added here before we officially supported libraries as a target the runtime would accept at lib_6_3, so while you can technically compile to lib_6_1 and lib_6_2, you must disable validation to do so.
Given all of this, I feel like we should do away with this specific table format. Perhaps we should change columns to something like this:
| Shader Kind | shaderModelName | Minimum major, minor |
+=======================+=======================================+=============+
| Vertex shader (VS) | vs | 6, 0 |
...
| Dxil library | lib | 6, 3 |
... and so on ...
(note: corrected library line here)
What do you think?
There was a problem hiding this comment.
Thanks @tex3d, for the suggestions regarding the correctness of the content. Happy to update the content.
Are Mesh Shader (MS) and Amplification Shader (AS) supported? If not, do they need to be listed as unsupported (for completeness??)?
There was a problem hiding this comment.
Yes, they are supported in shader model 6.5 and above.
The "no support" in the previous table was for the "Legacy Models" column. These legacy models are FXC/DXBC profiles (the previous compiler/bytecode). MS and AS were introduced more recently, so they are only supported in newer DXIL versions.
| | Mesh shader (MS) | no support | ms_6_5 | | ||
| +-----------------------+---------------------------------------+-------------+ | ||
| | Amplification shader (AS) | no support | as_6_5 | | ||
| +-----------------------+---------------------------------------+-------------+ | ||
|
|
||
| The DXIL verifier ensures that DXIL conforms to the specified shader model. |
There was a problem hiding this comment.
Side note: "verifier" should really be "validator"
Plus, the following paragraph seems odd when you remove the legacy shader models from the picture.
For shader models prior to 6.0, only the rules applicable to the DXIL representation are valid. For example, the limits on maximum number of resources is honored, but the limits on registers aren't because DXIL does not have a representation for registers.
Maybe we should just delete this?
There was a problem hiding this comment.
OK.
Let me know of your feedback on the accuracy of this changed version of the section.
There was a problem hiding this comment.
Side note: "verifier" should really be "validator"
Is the "verifier" now referred to as "validator" now? If so, should the term be changed throughout the document?
There was a problem hiding this comment.
Oh, missed that last comment. Yes, where it uses "verifier" elsewhere in the document, it should probably say "validator".
I think "verifier" was just the earliest terminology used when drafting this document before the validator actually existed. I believe we chose "validator" partly because that's what we used prior to DXC, and also because verifier is already an overloaded term used to refer to verifying that an llvm module is well-formed, as well as the clang verifier test which checks front-end diagnostics.
Make the table with supported shader model, major and minor numbers visible.