Skip to content

Commit

Permalink
sr: Deduplicate match ranges like spirv header (#242)
Browse files Browse the repository at this point in the history
* sr: Deduplicate `match` ranges like `spirv` header

Solve some clippy lints by getting rid of these aliases of overlapping
`Op`s.  Unfortunately, just like in `spirv`, this biases towards the
`NV` rather than `KHR` stabilized raytracing ops.

Note that in `spirv` the `Op`s are aliases in code rather than omitted
altogether: don't think we can do that for the range match here though
unless deducing whether the shader was compiled with the NV or KHR
variant, even though the KHR variant is already stabilized for quite
some time.

* Globally sort and prefer stabilized over KHR over EXT over vendor opnames
  • Loading branch information
MarijnS95 committed Apr 6, 2023
1 parent 3a19780 commit aa260e0
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 89 deletions.
62 changes: 58 additions & 4 deletions autogen/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,68 @@ fn map_reserved_instructions(grammar: &mut structs::Grammar) {
.iter_mut()
.filter(|i| i.class == Some(structs::Class::Reserved))
{
if instruction.opname.starts_with("OpType")
// Ignore AccelerationStructureNV which has the same opcode as AccelerationStructureKHR
&& instruction.opname != "OpTypeAccelerationStructureNV"
{
if instruction.opname.starts_with("OpType") {
instruction.class = Some(structs::Class::Type);
}
}
}

fn opname_suffix_weight(opname: &str) -> u32 {
const VENDORS: [&str; 36] = [
"AMD",
"AMDX",
"ANDROID",
"ARM",
"BRCM",
"CHROMIUM",
"EXT",
"FB",
"FSL",
"FUCHSIA",
"GGP",
"GOOGLE",
"HUAWEI",
"IMG",
"INTEL",
"JUICE",
"KDAB",
"KHX",
"LUNARG",
"MESA",
"MVK",
"NN",
"NV",
"NVX",
"NXP",
"NZXT",
"QCOM",
"QNX",
"RASTERGRID",
"RENDERDOC",
"SAMSUNG",
"SEC",
"TIZEN",
"VALVE",
"VIV",
"VSI",
];

match opname {
_ if opname.ends_with("KHR") => 1,
_ if opname.ends_with("EXT") => 2,
_ if VENDORS.iter().any(|v| opname.ends_with(v)) => 3,
_ => 0,
}
}

fn sort_instructions(grammar: &mut structs::Grammar) {
grammar.instructions.sort_by(|l, r| {
l.opcode
.cmp(&r.opcode)
.then_with(|| opname_suffix_weight(&l.opname).cmp(&opname_suffix_weight(&r.opname)))
})
}

fn main() {
// Path to the SPIR-V core grammar file.
let env_var = env::var("CARGO_MANIFEST_DIR").unwrap();
Expand All @@ -73,6 +126,7 @@ fn main() {
let grammar: structs::Grammar = {
let mut original = serde_json::from_str(&contents).unwrap();
map_reserved_instructions(&mut original);
sort_instructions(&mut original);
original
};

Expand Down
12 changes: 10 additions & 2 deletions autogen/src/sr.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::collections::BTreeSet;

use crate::structs;
use crate::utils::*;

Expand Down Expand Up @@ -319,12 +321,18 @@ pub fn gen_sr_code_from_instruction_grammar(
let mut field_types = Vec::new();
let mut field_lifts = Vec::new();

let mut seen_discriminator = BTreeSet::new();

// Compose the token stream for all instructions
for inst in grammar_instructions
.iter() // Loop over all instructions
.iter()
// Skip constants
.filter(|i| i.class != Some(structs::Class::Constant))
// Skip constants
{
if !seen_discriminator.insert(inst.opcode) {
continue;
}

// Get the token for its enumerant
let inst_name = &inst.opname[2..];

Expand Down
24 changes: 12 additions & 12 deletions rspirv/dr/build/autogen_norm_insts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13729,8 +13729,8 @@ impl Builder {
self.insert_into_block(insert_point, inst)?;
Ok(())
}
#[doc = "Appends an OpReportIntersectionNV instruction to the current block."]
pub fn report_intersection_nv(
#[doc = "Appends an OpReportIntersectionKHR instruction to the current block."]
pub fn report_intersection_khr(
&mut self,
result_type: spirv::Word,
result_id: Option<spirv::Word>,
Expand All @@ -13740,16 +13740,16 @@ impl Builder {
let _id = result_id.unwrap_or_else(|| self.id());
#[allow(unused_mut)]
let mut inst = dr::Instruction::new(
spirv::Op::ReportIntersectionNV,
spirv::Op::ReportIntersectionKHR,
Some(result_type),
Some(_id),
vec![dr::Operand::IdRef(hit), dr::Operand::IdRef(hit_kind)],
);
self.insert_into_block(InsertPoint::End, inst)?;
Ok(_id)
}
#[doc = "Appends an OpReportIntersectionNV instruction to the current block."]
pub fn insert_report_intersection_nv(
#[doc = "Appends an OpReportIntersectionKHR instruction to the current block."]
pub fn insert_report_intersection_khr(
&mut self,
insert_point: InsertPoint,
result_type: spirv::Word,
Expand All @@ -13760,16 +13760,16 @@ impl Builder {
let _id = result_id.unwrap_or_else(|| self.id());
#[allow(unused_mut)]
let mut inst = dr::Instruction::new(
spirv::Op::ReportIntersectionNV,
spirv::Op::ReportIntersectionKHR,
Some(result_type),
Some(_id),
vec![dr::Operand::IdRef(hit), dr::Operand::IdRef(hit_kind)],
);
self.insert_into_block(insert_point, inst)?;
Ok(_id)
}
#[doc = "Appends an OpReportIntersectionKHR instruction to the current block."]
pub fn report_intersection_khr(
#[doc = "Appends an OpReportIntersectionNV instruction to the current block."]
pub fn report_intersection_nv(
&mut self,
result_type: spirv::Word,
result_id: Option<spirv::Word>,
Expand All @@ -13779,16 +13779,16 @@ impl Builder {
let _id = result_id.unwrap_or_else(|| self.id());
#[allow(unused_mut)]
let mut inst = dr::Instruction::new(
spirv::Op::ReportIntersectionKHR,
spirv::Op::ReportIntersectionNV,
Some(result_type),
Some(_id),
vec![dr::Operand::IdRef(hit), dr::Operand::IdRef(hit_kind)],
);
self.insert_into_block(InsertPoint::End, inst)?;
Ok(_id)
}
#[doc = "Appends an OpReportIntersectionKHR instruction to the current block."]
pub fn insert_report_intersection_khr(
#[doc = "Appends an OpReportIntersectionNV instruction to the current block."]
pub fn insert_report_intersection_nv(
&mut self,
insert_point: InsertPoint,
result_type: spirv::Word,
Expand All @@ -13799,7 +13799,7 @@ impl Builder {
let _id = result_id.unwrap_or_else(|| self.id());
#[allow(unused_mut)]
let mut inst = dr::Instruction::new(
spirv::Op::ReportIntersectionKHR,
spirv::Op::ReportIntersectionNV,
Some(result_type),
Some(_id),
vec![dr::Operand::IdRef(hit), dr::Operand::IdRef(hit_kind)],
Expand Down
27 changes: 27 additions & 0 deletions rspirv/dr/build/autogen_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,33 @@ impl Builder {
new_id
}
}
#[doc = "Appends an OpTypeAccelerationStructureNV instruction and returns the result id, or return the existing id if the instruction was already present."]
pub fn type_acceleration_structure_nv(&mut self) -> spirv::Word {
self.type_acceleration_structure_nv_id(None)
}
#[doc = "Appends an OpTypeAccelerationStructureNV instruction and returns the result id, or return the existing id if the instruction was already present."]
pub fn type_acceleration_structure_nv_id(
&mut self,
result_id: Option<spirv::Word>,
) -> spirv::Word {
let mut inst = dr::Instruction::new(
spirv::Op::TypeAccelerationStructureNV,
None,
result_id,
vec![],
);
if let Some(result_id) = result_id {
self.module.types_global_values.push(inst);
result_id
} else if let Some(id) = self.dedup_insert_type(&inst) {
id
} else {
let new_id = self.id();
inst.result_id = Some(new_id);
self.module.types_global_values.push(inst);
new_id
}
}
#[doc = "Appends an OpTypeCooperativeMatrixNV instruction and returns the result id, or return the existing id if the instruction was already present."]
pub fn type_cooperative_matrix_nv(
&mut self,
Expand Down
8 changes: 4 additions & 4 deletions rspirv/grammar/autogen_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3595,7 +3595,7 @@ static INSTRUCTION_TABLE: &[Instruction<'static>] = &[
[(IdRef, One), (IdRef, One)]
),
inst!(
ReportIntersectionNV,
ReportIntersectionKHR,
[RayTracingNV, RayTracingKHR],
["SPV_NV_ray_tracing", "SPV_KHR_ray_tracing"],
[
Expand All @@ -3606,7 +3606,7 @@ static INSTRUCTION_TABLE: &[Instruction<'static>] = &[
]
),
inst!(
ReportIntersectionKHR,
ReportIntersectionNV,
[RayTracingNV, RayTracingKHR],
["SPV_NV_ray_tracing", "SPV_KHR_ray_tracing"],
[
Expand Down Expand Up @@ -3680,7 +3680,7 @@ static INSTRUCTION_TABLE: &[Instruction<'static>] = &[
]
),
inst!(
TypeAccelerationStructureNV,
TypeAccelerationStructureKHR,
[RayTracingNV, RayTracingKHR, RayQueryKHR],
[
"SPV_NV_ray_tracing",
Expand All @@ -3690,7 +3690,7 @@ static INSTRUCTION_TABLE: &[Instruction<'static>] = &[
[(IdResult, One)]
),
inst!(
TypeAccelerationStructureKHR,
TypeAccelerationStructureNV,
[RayTracingNV, RayTracingKHR, RayQueryKHR],
[
"SPV_NV_ray_tracing",
Expand Down
49 changes: 0 additions & 49 deletions rspirv/lift/autogen_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5835,20 +5835,6 @@ impl LiftContext {
})
.ok_or(OperandError::Missing)?,
}),
5334u32 => Ok(ops::Op::ReportIntersectionNV {
hit: (match operands.next() {
Some(dr::Operand::IdRef(value)) => Some(*value),
Some(_) => return Err(OperandError::WrongType.into()),
None => None,
})
.ok_or(OperandError::Missing)?,
hit_kind: (match operands.next() {
Some(dr::Operand::IdRef(value)) => Some(*value),
Some(_) => return Err(OperandError::WrongType.into()),
None => None,
})
.ok_or(OperandError::Missing)?,
}),
5334u32 => Ok(ops::Op::ReportIntersectionKHR {
hit: (match operands.next() {
Some(dr::Operand::IdRef(value)) => Some(*value),
Expand Down Expand Up @@ -6081,7 +6067,6 @@ impl LiftContext {
})
.ok_or(OperandError::Missing)?,
}),
5341u32 => Ok(ops::Op::TypeAccelerationStructureNV),
5344u32 => Ok(ops::Op::ExecuteCallableNV {
sbt_index: (match operands.next() {
Some(dr::Operand::IdRef(value)) => Some(*value),
Expand Down Expand Up @@ -6768,20 +6753,6 @@ impl LiftContext {
})
.ok_or(OperandError::Missing)?,
}),
5632u32 => Ok(ops::Op::DecorateStringGOOGLE {
target: (match operands.next() {
Some(dr::Operand::IdRef(value)) => Some(*value),
Some(_) => return Err(OperandError::WrongType.into()),
None => None,
})
.ok_or(OperandError::Missing)?,
decoration: (match operands.next() {
Some(dr::Operand::Decoration(value)) => Some(*value),
Some(_) => return Err(OperandError::WrongType.into()),
None => None,
})
.ok_or(OperandError::Missing)?,
}),
5633u32 => Ok(ops::Op::MemberDecorateString {
struct_type: (match operands.next() {
Some(dr::Operand::IdRef(value)) => Some(self.types.lookup_token(*value)),
Expand All @@ -6802,26 +6773,6 @@ impl LiftContext {
})
.ok_or(OperandError::Missing)?,
}),
5633u32 => Ok(ops::Op::MemberDecorateStringGOOGLE {
struct_type: (match operands.next() {
Some(dr::Operand::IdRef(value)) => Some(self.types.lookup_token(*value)),
Some(_) => return Err(OperandError::WrongType.into()),
None => None,
})
.ok_or(OperandError::Missing)?,
member: (match operands.next() {
Some(dr::Operand::LiteralBit32(value)) => Some(*value),
Some(_) => return Err(OperandError::WrongType.into()),
None => None,
})
.ok_or(OperandError::Missing)?,
decoration: (match operands.next() {
Some(dr::Operand::Decoration(value)) => Some(*value),
Some(_) => return Err(OperandError::WrongType.into()),
None => None,
})
.ok_or(OperandError::Missing)?,
}),
5699u32 => Ok(ops::Op::VmeImageINTEL {
image_type: (match operands.next() {
Some(dr::Operand::IdRef(value)) => Some(self.types.lookup_token(*value)),
Expand Down
14 changes: 0 additions & 14 deletions rspirv/sr/autogen_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1491,10 +1491,6 @@ pub enum Op {
index_offset: spirv::Word,
packed_indices: spirv::Word,
},
ReportIntersectionNV {
hit: spirv::Word,
hit_kind: spirv::Word,
},
ReportIntersectionKHR {
hit: spirv::Word,
hit_kind: spirv::Word,
Expand Down Expand Up @@ -1542,7 +1538,6 @@ pub enum Op {
time: spirv::Word,
payload: spirv::Word,
},
TypeAccelerationStructureNV,
ExecuteCallableNV {
sbt_index: spirv::Word,
callable_data_id: spirv::Word,
Expand Down Expand Up @@ -1736,20 +1731,11 @@ pub enum Op {
target: spirv::Word,
decoration: spirv::Decoration,
},
DecorateStringGOOGLE {
target: spirv::Word,
decoration: spirv::Decoration,
},
MemberDecorateString {
struct_type: Token<Type>,
member: u32,
decoration: spirv::Decoration,
},
MemberDecorateStringGOOGLE {
struct_type: Token<Type>,
member: u32,
decoration: spirv::Decoration,
},
VmeImageINTEL {
image_type: Token<Type>,
sampler: spirv::Word,
Expand Down

0 comments on commit aa260e0

Please sign in to comment.