Initial support for NonSemantic.Shader.DebugInfo 101#4220
Initial support for NonSemantic.Shader.DebugInfo 101#4220dnovillo merged 3 commits intoKhronosGroup:mainfrom
Conversation
|
For reference, these are the PRs adding NSDI.101 support:
|
|
LGTM aside from Jeremy's comment. Before I merge, could you squash this into just two or three commits? One for the rename of the header file and associated enumeration values and the other one for the "initial implementation"? And the tests can go in their own commit or together with the implementation. |
Sure. I'll re-do the commits so they are split that way. |
Drop the version number from the file name and all C identifiers No semantic change.
- DebugTypeVectorIdEXT: emitted for OpTypeCooperativeVectorNV types whose component count is a SPIR-V Id rather than a literal. - DebugTypeCooperativeMatrixKHR: emitted for OpTypeCooperativeMatrixKHR types, replacing the former opaque-composite workaround. - Optional FPEncoding operand on DebugTypeBasic: identifies non-standard float formats (bfloat16, float8 E4M3, float8 E5M2). The NonSemantic.Shader.DebugInfo import string is only promoted to .101 when a version-101 opcode is actually emitted.
Done. I split it in two commits: the rename with no functional changes and the new implementation with tests. I also realized that the tests were not covering the coopmat test I removed from Test/ (spec constants with all three use types simultaneously). I added it to the new gtests/ implementation. PTAL. Thanks. |
jeffbolznv
left a comment
There was a problem hiding this comment.
I primarily reviewed the spvbuilder changes, I didn't go through the test code.
This implements NSDI.101 (KhronosGroup/SPIRV-Registry#390) in glslang. It updates SPIRV-Tools and SPIRV-Headers dependencies to bring in the new headers for NSDI.101.
Version promotion
To minimize disruption with existing tools, glslang will always generate an import of
NonSemantic.Shader.DebugInfo.100. If the module uses an opcode that requires NSDI.101 (cooperative types and new floating point types), it then changes the import string to.101. The promotion is done by patching theOpExtInstImportinstruction's string operand.The entry point is
Builder::requireNonSemanticShaderDebugInfoVersion. Each of the three new emitter functionscalls it unconditionally before emitting the instruction.
New emitters
Builder::makeVectorIdDebugType: emitsDebugTypeVectorIdEXT. Called frommakeCooperativeVectorTypeNVwhenemitNonSemanticShaderDebugInfois set.Builder::makeCooperativeMatrixDebugTypeKHR: emitsDebugTypeCooperativeMatrixKHR. Replaces the former opaque-composite workaround inmakeCooperativeMatrixTypeKHR.Builder::makeFloatDebugType: extended with an optionalfpEncodingoperand. The three float-8 and bfloat16 type builders (makeBFloat16Type,makeFloatE5M2Type,makeFloatE4M3Type) now handle the new floating point types.Header rename
This needed to import the new
NonSemanticShaderDebugInfo.hfrom SPIRV-Headers. This means that all the previous C identifiers that had100embedded in them needed to change. Those changes are mechanical, there is no functional change there.I also added a section in
README.mdto document how to update this headers in future revisions of NSDI.Tests
I replaced the previous golden output tests
spv.debuginfo.coopmatKHR.compwith a new unit test filegtests/SpvDebugInfoTest.cpp. All the NSDI.101 opcodes are tested there.