Skip to content

Commit 3e8814d

Browse files
MarijnS95MarijnS95
committed
Generate reflect::is_xxx() helpers directly in the spirv header
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.
1 parent 89ce4d0 commit 3e8814d

File tree

6 files changed

+208
-110
lines changed

6 files changed

+208
-110
lines changed

‎autogen/src/header.rs‎

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -291,16 +291,42 @@ pub fn gen_spirv_header(grammar: &structs::Grammar) -> TokenStream {
291291
// Use associated constants for these aliases.
292292
let mut aliases = vec![];
293293
let mut variants = vec![];
294+
let mut types = vec![];
295+
let mut constants = vec![];
296+
let mut annotations = vec![];
297+
let mut debugs = vec![];
298+
let mut control_flows = vec![];
294299

295300
// Get the instruction table.
296301
for inst in &grammar.instructions {
297302
let opname = as_ident(inst.opname.strip_prefix("Op").unwrap());
298303
let opcode = inst.opcode;
304+
variants.push((opcode, opname.clone()));
305+
306+
let opname = quote!(Self::#opname);
299307
for alias in &inst.aliases {
300308
let alias = as_ident(alias.strip_prefix("Op").unwrap());
301-
aliases.push(quote! { pub const #alias: Op = Op::#opname; });
309+
aliases.push(quote! { pub const #alias: Self = #opname; });
310+
}
311+
312+
match inst.class {
313+
Some(structs::Class::Type) => {
314+
types.push(opname);
315+
}
316+
Some(structs::Class::Constant) => {
317+
constants.push(opname);
318+
}
319+
Some(structs::Class::Annotation) => {
320+
annotations.push(opname);
321+
}
322+
Some(structs::Class::Debug) => {
323+
debugs.push(opname);
324+
}
325+
Some(structs::Class::Branch) => {
326+
control_flows.push(opname);
327+
}
328+
_ => {}
302329
}
303-
variants.push((opcode, opname.clone()));
304330
}
305331

306332
let the_enum = generate_enum(
@@ -325,6 +351,37 @@ pub fn gen_spirv_header(grammar: &structs::Grammar) -> TokenStream {
325351
#[allow(non_upper_case_globals)]
326352
impl Op {
327353
#(#aliases)*
354+
355+
/// Returns [`true`] if the given opcode is a type-declaring instruction.
356+
///
357+
/// <https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_type_declaration_instructions>
358+
pub fn is_type(self) -> bool {
359+
matches!(self, #(#types)|*)
360+
}
361+
/// Returns [`true`] if the given opcode is a constant-defining instruction.
362+
///
363+
/// <https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_constant_creation_instructions>
364+
pub fn is_constant(self) -> bool {
365+
matches!(self, #(#constants)|*)
366+
}
367+
/// Returns [`true`] if the given opcode is an annotation instruction.
368+
///
369+
/// <https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#Annotation>
370+
pub fn is_annotation(self) -> bool {
371+
matches!(self, #(#annotations)|*)
372+
}
373+
/// Returns [`true`] if the given opcode is a debug instruction.
374+
///
375+
/// <https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_debug_instructions>
376+
pub fn is_debug(self) -> bool {
377+
matches!(self, #(#debugs)|*)
378+
}
379+
/// Returns [`true`] if the given opcode is a control-flow instruction.
380+
///
381+
/// <https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_control_flow_instructions>
382+
pub fn is_control_flow(self) -> bool {
383+
matches!(self, #(#control_flows)|*)
384+
}
328385
}
329386
}
330387
}

‎autogen/src/structs.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ pub enum Class {
122122
#[serde(rename = "Constant-Creation")]
123123
Constant,
124124
Debug,
125-
DebugLine,
126125
#[serde(rename = "Extension")]
127126
ExtensionDecl,
128127
#[serde(rename = "Function")]

‎rspirv/binary/tracker.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ impl TypeTracker {
3838

3939
pub fn track(&mut self, inst: &dr::Instruction) {
4040
if let Some(rid) = inst.result_id {
41-
if grammar::reflect::is_type(inst.class.opcode) {
41+
if inst.class.opcode.is_type() {
4242
match inst.class.opcode {
4343
spirv::Op::TypeInt => {
4444
if let (

‎rspirv/dr/loader.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,10 +150,8 @@ impl binary::Consumer for Loader {
150150
None => self.module.types_global_values.push(inst),
151151
}
152152
}
153-
opcode if grammar::reflect::is_annotation(opcode) => self.module.annotations.push(inst),
154-
opcode
155-
if grammar::reflect::is_type(opcode) || grammar::reflect::is_constant(opcode) =>
156-
{
153+
opcode if opcode.is_annotation() => self.module.annotations.push(inst),
154+
opcode if opcode.is_type() || opcode.is_constant() => {
157155
self.module.types_global_values.push(inst)
158156
}
159157
spirv::Op::Variable if self.function.is_none() => {

‎rspirv/grammar/reflect.rs‎

Lines changed: 13 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -2,105 +2,22 @@
22
33
use crate::spirv;
44

5-
/// Returns true if the given opcode is for a location debug instruction.
5+
/// Returns [`true`] if the given opcode is for a location debug instruction.
66
pub fn is_location_debug(opcode: spirv::Op) -> bool {
77
matches!(opcode, spirv::Op::Line | spirv::Op::NoLine)
88
}
99

10-
/// Returns true if the given opcode is for a non-location debug instruction.
11-
pub fn is_nonlocation_debug(opcode: spirv::Op) -> bool {
12-
matches!(
13-
opcode,
14-
spirv::Op::SourceContinued
15-
| spirv::Op::Source
16-
| spirv::Op::SourceExtension
17-
| spirv::Op::Name
18-
| spirv::Op::MemberName
19-
| spirv::Op::String
20-
)
21-
}
22-
23-
/// Returns true if the given opcode is for a debug instruction.
24-
pub fn is_debug(opcode: spirv::Op) -> bool {
25-
is_location_debug(opcode) || is_nonlocation_debug(opcode)
26-
}
27-
28-
/// Returns true if the given opcode is for an annotation instruction.
29-
pub fn is_annotation(opcode: spirv::Op) -> bool {
30-
matches!(
31-
opcode,
32-
spirv::Op::Decorate
33-
| spirv::Op::MemberDecorate
34-
| spirv::Op::DecorationGroup
35-
| spirv::Op::GroupDecorate
36-
| spirv::Op::GroupMemberDecorate
37-
| spirv::Op::DecorateString
38-
| spirv::Op::MemberDecorateStringGOOGLE
39-
)
40-
}
41-
42-
/// Returns true if the given opcode is for a type-declaring instruction.
43-
pub fn is_type(opcode: spirv::Op) -> bool {
44-
matches!(
45-
opcode,
46-
spirv::Op::TypeVoid
47-
| spirv::Op::TypeBool
48-
| spirv::Op::TypeInt
49-
| spirv::Op::TypeFloat
50-
| spirv::Op::TypeVector
51-
| spirv::Op::TypeMatrix
52-
| spirv::Op::TypeImage
53-
| spirv::Op::TypeSampler
54-
| spirv::Op::TypeSampledImage
55-
| spirv::Op::TypeArray
56-
| spirv::Op::TypeRuntimeArray
57-
| spirv::Op::TypeStruct
58-
| spirv::Op::TypeOpaque
59-
| spirv::Op::TypePointer
60-
| spirv::Op::TypeFunction
61-
| spirv::Op::TypeEvent
62-
| spirv::Op::TypeDeviceEvent
63-
| spirv::Op::TypeReserveId
64-
| spirv::Op::TypeQueue
65-
| spirv::Op::TypePipe
66-
| spirv::Op::TypeAccelerationStructureKHR
67-
| spirv::Op::TypeRayQueryKHR
68-
| spirv::Op::TypeForwardPointer
69-
| spirv::Op::TypeCooperativeMatrixKHR
70-
)
71-
}
72-
73-
/// Returns true if the given opcode is for a constant-defining instruction.
74-
pub fn is_constant(opcode: spirv::Op) -> bool {
75-
matches!(
76-
opcode,
77-
spirv::Op::ConstantTrue
78-
| spirv::Op::ConstantFalse
79-
| spirv::Op::Constant
80-
| spirv::Op::ConstantComposite
81-
| spirv::Op::ConstantSampler
82-
| spirv::Op::ConstantNull
83-
| spirv::Op::SpecConstantTrue
84-
| spirv::Op::SpecConstantFalse
85-
| spirv::Op::SpecConstant
86-
| spirv::Op::SpecConstantComposite
87-
| spirv::Op::SpecConstantOp
88-
| spirv::Op::ConstantCompositeContinuedINTEL
89-
| spirv::Op::SpecConstantCompositeContinuedINTEL
90-
)
91-
}
92-
93-
/// Returns true if the given opcode is for a variable-defining instruction.
10+
/// Returns [`true`] if the given opcode is for a variable-defining instruction.
9411
pub fn is_variable(opcode: spirv::Op) -> bool {
9512
opcode == spirv::Op::Variable
9613
}
9714

98-
/// Returns true if the given opcode is a return instruction.
15+
/// Returns [`true`] if the given opcode is a return instruction.
9916
pub fn is_return(opcode: spirv::Op) -> bool {
10017
matches!(opcode, spirv::Op::Return | spirv::Op::ReturnValue)
10118
}
10219

103-
/// Returns true if the given opcode aborts execution.
20+
/// Returns [`true`] if the given opcode aborts execution.
10421
pub fn is_abort(opcode: spirv::Op) -> bool {
10522
matches!(
10623
opcode,
@@ -113,21 +30,26 @@ pub fn is_abort(opcode: spirv::Op) -> bool {
11330
)
11431
}
11532

116-
/// Returns true if the given opcode is a return instruction or it aborts
117-
/// execution.
33+
/// Returns [`true`] if the given opcode is a return instruction or it aborts execution.
34+
///
35+
/// <https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#FunctionTermination>
11836
pub fn is_return_or_abort(opcode: spirv::Op) -> bool {
11937
is_return(opcode) || is_abort(opcode)
12038
}
12139

122-
/// Returns true if the given opcode is a branch instruction.
40+
/// Returns [`true`] if the given opcode is a branch instruction.
41+
///
42+
/// <https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#Branch> and <https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#ConditionalBranch>
12343
pub fn is_branch(opcode: spirv::Op) -> bool {
12444
matches!(
12545
opcode,
12646
spirv::Op::Branch | spirv::Op::BranchConditional | spirv::Op::Switch
12747
)
12848
}
12949

130-
/// Returns true if the given opcode is for a terminator instruction.
50+
/// Returns [`true`] if the given opcode is for a terminator instruction.
51+
///
52+
/// <https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#Termination>
13153
pub fn is_block_terminator(opcode: spirv::Op) -> bool {
13254
is_branch(opcode) || is_return_or_abort(opcode)
13355
}

0 commit comments

Comments
 (0)