Conversation
We already generate the `enum *Op` structures for these extensions; reference them directly instead of hardcoding the number in these instruction tables again.
These were global zero-sized structures with getters that walk the generated `INSTRUCTION_TABLE` lists, which share a very common pattern that lends itself perfectly for being abstracted away to a single implementation of the lookup
`autogen_nonsemantic_debugprintf.rs` was added in commit 267882d ("Unify extended instruction parsing and add `NonSemantic.DebugPrintF` (#216)") but never `include!()`d making it unreachable. It was also never made available in `ExtInstSetTracker`, somewhat defeating the purpose of adding the extension in the first place.
Extensions like DebugInfo and OpenCL.debuginfo.100 define their own operand kinds with potentially clashing names but different discriminant values. Generate a separate `ExtOperandKind` enum per extension module and add wrapper variants (e.g. `OperandKind::DebugInfo(ExtOperandKind)`) to the core `OperandKind` enum. The `ext_inst!` macro gains a second arm disambiguated by a trailing comma: the ident arm prepends `OperandKind::` for extensions without own operand kinds, while the expr arm passes through pre-wrapped `OperandKind::Variant(ExtOperandKind::Kind)` expressions. All extension tables are now wrapped in `pub mod` for consistent namespacing.
Instead of maintaining a hardcoded list of extended instruction sets, automatically glob for all `extinst.*.grammar.json` files in the SPIRV-Headers submodule. Module names, op enum names, and table names are derived systematically from the grammar filename. The `ext_inst_table()` lookup normalises its input (lowercasing and converting underscores to hyphens) so that any casing variant of the canonical import name resolves correctly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
Let claude fix that now, though in a slightly different way. |
Collaborator
Author
|
@cwfitzgerald can you review this once more? |
cwfitzgerald
approved these changes
Mar 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previous PRs made a mess out of extended instructions by adding some boilerplate for them but never updating it in
ExtInstSetTrackermaking it impossible to be used in the binary parser. This generalizesInstructionandExtendedInstructiona little bit to handle both cases.Disclaimer: most commits were written by me, but I've asked claude to help me with the indefinite churn in the last two commits.