Skip to content

Commit 2c7d1b9

Browse files
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 2c7d1b9

File tree

6 files changed

+190
-110
lines changed

6 files changed

+190
-110
lines changed

autogen/src/header.rs

Lines changed: 54 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,32 @@ 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+
/// <https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_type_declaration_instructions>
357+
pub fn is_type(self) -> bool {
358+
matches!(self, #(#types)|*)
359+
}
360+
/// Returns [`true`] if the given opcode is a constant-defining instruction.
361+
/// <https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_constant_creation_instructions>
362+
pub fn is_constant(self) -> bool {
363+
matches!(self, #(#constants)|*)
364+
}
365+
/// Returns [`true`] if the given opcode is an annotation instruction.
366+
/// <https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#Annotation>
367+
pub fn is_annotation(self) -> bool {
368+
matches!(self, #(#annotations)|*)
369+
}
370+
/// Returns [`true`] if the given opcode is a debug instruction.
371+
/// <https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_debug_instructions>
372+
pub fn is_debug(self) -> bool {
373+
matches!(self, #(#debugs)|*)
374+
}
375+
/// Returns [`true`] if the given opcode is a control-flow instruction.
376+
/// <https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_control_flow_instructions>
377+
pub fn is_control_flow(self) -> bool {
378+
matches!(self, #(#control_flows)|*)
379+
}
328380
}
329381
}
330382
}

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: 10 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,23 @@ 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+
/// <https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#FunctionTermination>
11835
pub fn is_return_or_abort(opcode: spirv::Op) -> bool {
11936
is_return(opcode) || is_abort(opcode)
12037
}
12138

122-
/// Returns true if the given opcode is a branch instruction.
39+
/// Returns [`true`] if the given opcode is a branch instruction.
40+
/// <https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#Branch> and <https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#ConditionalBranch>
12341
pub fn is_branch(opcode: spirv::Op) -> bool {
12442
matches!(
12543
opcode,
12644
spirv::Op::Branch | spirv::Op::BranchConditional | spirv::Op::Switch
12745
)
12846
}
12947

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

spirv/autogen_spirv.rs

Lines changed: 123 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4199,17 +4199,129 @@ impl Op {
41994199
#[allow(clippy::upper_case_acronyms)]
42004200
#[allow(non_upper_case_globals)]
42014201
impl Op {
4202-
pub const SDotKHR: Op = Op::SDot;
4203-
pub const UDotKHR: Op = Op::UDot;
4204-
pub const SUDotKHR: Op = Op::SUDot;
4205-
pub const SDotAccSatKHR: Op = Op::SDotAccSat;
4206-
pub const UDotAccSatKHR: Op = Op::UDotAccSat;
4207-
pub const SUDotAccSatKHR: Op = Op::SUDotAccSat;
4208-
pub const ReportIntersectionNV: Op = Op::ReportIntersectionKHR;
4209-
pub const TypeAccelerationStructureNV: Op = Op::TypeAccelerationStructureKHR;
4210-
pub const DemoteToHelperInvocationEXT: Op = Op::DemoteToHelperInvocation;
4211-
pub const DecorateStringGOOGLE: Op = Op::DecorateString;
4212-
pub const MemberDecorateStringGOOGLE: Op = Op::MemberDecorateString;
4202+
pub const SDotKHR: Self = Self::SDot;
4203+
pub const UDotKHR: Self = Self::UDot;
4204+
pub const SUDotKHR: Self = Self::SUDot;
4205+
pub const SDotAccSatKHR: Self = Self::SDotAccSat;
4206+
pub const UDotAccSatKHR: Self = Self::UDotAccSat;
4207+
pub const SUDotAccSatKHR: Self = Self::SUDotAccSat;
4208+
pub const ReportIntersectionNV: Self = Self::ReportIntersectionKHR;
4209+
pub const TypeAccelerationStructureNV: Self = Self::TypeAccelerationStructureKHR;
4210+
pub const DemoteToHelperInvocationEXT: Self = Self::DemoteToHelperInvocation;
4211+
pub const DecorateStringGOOGLE: Self = Self::DecorateString;
4212+
pub const MemberDecorateStringGOOGLE: Self = Self::MemberDecorateString;
4213+
#[doc = r" Returns [`true`] if the given opcode is a type-declaring instruction."]
4214+
pub fn is_type(self) -> bool {
4215+
matches!(
4216+
self,
4217+
Self::TypeVoid
4218+
| Self::TypeBool
4219+
| Self::TypeInt
4220+
| Self::TypeFloat
4221+
| Self::TypeVector
4222+
| Self::TypeMatrix
4223+
| Self::TypeImage
4224+
| Self::TypeSampler
4225+
| Self::TypeSampledImage
4226+
| Self::TypeArray
4227+
| Self::TypeRuntimeArray
4228+
| Self::TypeStruct
4229+
| Self::TypeOpaque
4230+
| Self::TypePointer
4231+
| Self::TypeFunction
4232+
| Self::TypeEvent
4233+
| Self::TypeDeviceEvent
4234+
| Self::TypeReserveId
4235+
| Self::TypeQueue
4236+
| Self::TypePipe
4237+
| Self::TypeForwardPointer
4238+
| Self::TypePipeStorage
4239+
| Self::TypeNamedBarrier
4240+
| Self::TypeUntypedPointerKHR
4241+
| Self::TypeCooperativeMatrixKHR
4242+
| Self::TypeRayQueryKHR
4243+
| Self::TypeNodePayloadArrayAMDX
4244+
| Self::TypeHitObjectNV
4245+
| Self::TypeCooperativeVectorNV
4246+
| Self::TypeAccelerationStructureKHR
4247+
| Self::TypeCooperativeMatrixNV
4248+
| Self::TypeTensorLayoutNV
4249+
| Self::TypeTensorViewNV
4250+
| Self::TypeBufferSurfaceINTEL
4251+
| Self::TypeStructContinuedINTEL
4252+
)
4253+
}
4254+
#[doc = r" Returns [`true`] if the given opcode is a constant-defining instruction."]
4255+
pub fn is_constant(self) -> bool {
4256+
matches!(
4257+
self,
4258+
Self::ConstantTrue
4259+
| Self::ConstantFalse
4260+
| Self::Constant
4261+
| Self::ConstantComposite
4262+
| Self::ConstantSampler
4263+
| Self::ConstantNull
4264+
| Self::SpecConstantTrue
4265+
| Self::SpecConstantFalse
4266+
| Self::SpecConstant
4267+
| Self::SpecConstantComposite
4268+
| Self::SpecConstantOp
4269+
| Self::ConstantCompositeReplicateEXT
4270+
| Self::SpecConstantCompositeReplicateEXT
4271+
| Self::ConstantCompositeContinuedINTEL
4272+
| Self::SpecConstantCompositeContinuedINTEL
4273+
)
4274+
}
4275+
#[doc = r" Returns [`true`] if the given opcode is an annotation instruction."]
4276+
pub fn is_annotation(self) -> bool {
4277+
matches!(
4278+
self,
4279+
Self::Decorate
4280+
| Self::MemberDecorate
4281+
| Self::DecorationGroup
4282+
| Self::GroupDecorate
4283+
| Self::GroupMemberDecorate
4284+
| Self::DecorateId
4285+
| Self::DecorateString
4286+
| Self::MemberDecorateString
4287+
)
4288+
}
4289+
#[doc = r" Returns [`true`] if the given opcode is a debug instruction."]
4290+
pub fn is_debug(self) -> bool {
4291+
matches!(
4292+
self,
4293+
Self::SourceContinued
4294+
| Self::Source
4295+
| Self::SourceExtension
4296+
| Self::Name
4297+
| Self::MemberName
4298+
| Self::String
4299+
| Self::Line
4300+
| Self::NoLine
4301+
| Self::ModuleProcessed
4302+
)
4303+
}
4304+
#[doc = r" Returns [`true`] if the given opcode is a control-flow instruction."]
4305+
pub fn is_control_flow(self) -> bool {
4306+
matches!(
4307+
self,
4308+
Self::Phi
4309+
| Self::LoopMerge
4310+
| Self::SelectionMerge
4311+
| Self::Label
4312+
| Self::Branch
4313+
| Self::BranchConditional
4314+
| Self::Switch
4315+
| Self::Kill
4316+
| Self::Return
4317+
| Self::ReturnValue
4318+
| Self::Unreachable
4319+
| Self::LifetimeStart
4320+
| Self::LifetimeStop
4321+
| Self::TerminateInvocation
4322+
| Self::DemoteToHelperInvocation
4323+
)
4324+
}
42134325
}
42144326
#[doc = "[GLSL.std.450](https://www.khronos.org/registry/spir-v/specs/unified1/GLSL.std.450.html) extended instruction opcode"]
42154327
#[repr(u32)]

0 commit comments

Comments
 (0)