Skip to content

Conversation

@MarijnS95
Copy link
Collaborator

@MarijnS95 MarijnS95 commented Sep 29, 2025

We've had a lot of bugs surfacing from the fact that the manual rspirv::grammar::reflect module required hand-adding various spirv ops to match statements to let the code behave, despite being able to very trivially autogenerate this based on the SPIR-V grammar. By doing so we find quite a few instructions that weren't handled by the hand-written table. For is_type() those are:

  • OpTypePipeStorage
  • OpTypeNamedBarrier
  • OpTypeUntypedPointerKHR
  • OpTypeNodePayloadArrayAMDX
  • OpTypeHitObjectNV
  • OpTypeCooperativeVectorNV
  • OpTypeCooperativeMatrixNV
  • OpTypeTensorLayoutNV
  • OpTypeTensorViewNV
  • OpTypeBufferSurfaceINTEL
  • OpTypeStructContinuedINTEL

For is_constant():

  • OpConstantCompositeReplicateEXT
  • OpSpecConstantCompositeReplicateEXT

For is_annotation():

  • OpDecorateId

For is_debug() (via is_nonlocation_debug()):

  • OpModuleProcessed

Unfortunately classes only convey the Control-Flow marker which doesn't provide enough detail to distinguish between returns, aborts or branches (or anything not specified). Even if our crates only ever call is_block_terminator() which is a composite of all those checks, not all Control-Flow opcodes are supposed to be treated as a terminator.

@MarijnS95 MarijnS95 changed the title Generate reflect::is_type() directly in the spirv header Generate reflect::is_xxx() helpers directly in the spirv header Sep 29, 2025
@MarijnS95 MarijnS95 requested a review from kvark September 29, 2025 10:39
We've had a *lot* of bugs surfacing from the fact that the manual
`rspirv::grammar::reflect` module required hand-adding various `spirv`
ops to `match` statements to let the code behave, despite being able
to very trivially autogenerate this based on the SPIR-V grammar.  By
doing so we find quite a few instructions that weren't handled by the
hand-written table.  For `is_type()` those are:

- `OpTypePipeStorage`
- `OpTypeNamedBarrier`
- `OpTypeUntypedPointerKHR`
- `OpTypeNodePayloadArrayAMDX`
- `OpTypeHitObjectNV`
- `OpTypeCooperativeVectorNV`
- `OpTypeCooperativeMatrixNV`
- `OpTypeTensorLayoutNV`
- `OpTypeTensorViewNV`
- `OpTypeBufferSurfaceINTEL`
- `OpTypeStructContinuedINTEL`

For `is_constant()`:

- `OpConstantCompositeReplicateEXT`
- `OpSpecConstantCompositeReplicateEXT`

For `is_annotation()`:

- `OpDecorateId`

For `is_debug()` (via `is_nonlocation_debug()`):

- `OpModuleProcessed`

Unfortunately classes only convey the `Control-Flow` marker which
doesn't provide enough detail to distinguish between returns, aborts or
branches (or anything not specified).  Even if our crates only ever call
`is_block_terminator()` which is a composite of all those checks, not
all `Control-Flow` opcodes are supposed to be treated as a terminator.
@MarijnS95 MarijnS95 merged commit 3e8814d into master Oct 1, 2025
30 of 36 checks passed
@MarijnS95 MarijnS95 deleted the inline-grammar branch October 1, 2025 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants