diff --git a/Cargo.lock b/Cargo.lock index ee0abc2c3a..82ea325325 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2434,15 +2434,15 @@ dependencies = [ [[package]] name = "spirt" -version = "0.3.0" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "64e1f7903720ff818d6da824edf2c4082c6e7a029a99317fd10c39dd7c40c7ff" +checksum = "f2d5968bd2a36466468aac637b355776f080edfb0c6f769b2b99b9708260c42a" dependencies = [ "arrayvec", "bytemuck", "derive_more", "elsa", - "indexmap 1.9.3", + "indexmap 2.5.0", "internal-iterator", "itertools", "lazy_static", diff --git a/crates/rustc_codegen_spirv/Cargo.toml b/crates/rustc_codegen_spirv/Cargo.toml index bed7ad6e88..a5735a6b01 100644 --- a/crates/rustc_codegen_spirv/Cargo.toml +++ b/crates/rustc_codegen_spirv/Cargo.toml @@ -55,7 +55,7 @@ rustc_codegen_spirv-types.workspace = true rustc-demangle = "0.1.21" sanitize-filename = "0.4" smallvec = { version = "1.6.1", features = ["union"] } -spirt = "0.3.0" +spirt = "0.4.0" spirv-tools.workspace = true lazy_static = "1.4.0" itertools = "0.10.5" diff --git a/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs b/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs index 7732d13ee2..c3175155df 100644 --- a/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs +++ b/crates/rustc_codegen_spirv/src/codegen_cx/mod.rs @@ -460,6 +460,11 @@ impl CodegenArgs { "spirt-keep-debug-sources-in-dumps", "keep file contents debuginfo when dumping SPIR-T", ); + opts.optflag( + "", + "spirt-keep-unstructured-cfg-in-dumps", + "include initial unstructured CFG when dumping SPIR-T", + ); opts.optflag( "", "specializer-debug", @@ -629,6 +634,8 @@ impl CodegenArgs { .opt_present("spirt-strip-custom-debuginfo-from-dumps"), spirt_keep_debug_sources_in_dumps: matches .opt_present("spirt-keep-debug-sources-in-dumps"), + spirt_keep_unstructured_cfg_in_dumps: matches + .opt_present("spirt-keep-unstructured-cfg-in-dumps"), specializer_debug: matches.opt_present("specializer-debug"), specializer_dump_instances: matches_opt_path("specializer-dump-instances"), print_all_zombie: matches.opt_present("print-all-zombie"), diff --git a/crates/rustc_codegen_spirv/src/linker/mod.rs b/crates/rustc_codegen_spirv/src/linker/mod.rs index 1dffd724db..a11fdf593b 100644 --- a/crates/rustc_codegen_spirv/src/linker/mod.rs +++ b/crates/rustc_codegen_spirv/src/linker/mod.rs @@ -62,6 +62,7 @@ pub struct Options { pub dump_spirt_passes: Option, pub spirt_strip_custom_debuginfo_from_dumps: bool, pub spirt_keep_debug_sources_in_dumps: bool, + pub spirt_keep_unstructured_cfg_in_dumps: bool, pub specializer_debug: bool, pub specializer_dump_instances: Option, pub print_all_zombie: bool, @@ -434,9 +435,16 @@ pub fn link( } } }; - after_pass("lower_from_spv", &module); + // HACK(eddyb) don't dump the unstructured state if not requested, as + // after SPIR-T 0.4.0 it's extremely verbose (due to def-use hermeticity). + if opts.spirt_keep_unstructured_cfg_in_dumps || !opts.structurize { + after_pass("lower_from_spv", &module); + } // NOTE(eddyb) this *must* run on unstructured CFGs, to do its job. + // FIXME(eddyb) no longer relying on structurization, try porting this + // to replace custom aborts in `Block`s and inject `ExitInvocation`s + // after them (truncating the `Block` and/or parent region if necessary). { let _timer = sess.timer("spirt_passes::controlflow::convert_custom_aborts_to_unstructured_returns_in_entry_points"); spirt_passes::controlflow::convert_custom_aborts_to_unstructured_returns_in_entry_points(opts, &mut module); diff --git a/crates/rustc_codegen_spirv/src/linker/spirt_passes/controlflow.rs b/crates/rustc_codegen_spirv/src/linker/spirt_passes/controlflow.rs index 5a173e43a4..7c5155cbe8 100644 --- a/crates/rustc_codegen_spirv/src/linker/spirt_passes/controlflow.rs +++ b/crates/rustc_codegen_spirv/src/linker/spirt_passes/controlflow.rs @@ -4,13 +4,17 @@ use crate::custom_insts::{self, CustomInst, CustomOp}; use smallvec::SmallVec; use spirt::func_at::FuncAt; use spirt::{ - cfg, spv, Attr, AttrSet, ConstCtor, ConstDef, ControlNodeKind, DataInstFormDef, DataInstKind, - DeclDef, EntityDefs, ExportKey, Exportee, Module, Type, TypeCtor, TypeCtorArg, TypeDef, Value, + cfg, spv, Attr, AttrSet, ConstDef, ConstKind, ControlNodeKind, DataInstFormDef, DataInstKind, + DeclDef, EntityDefs, ExportKey, Exportee, Module, Type, TypeDef, TypeKind, TypeOrConst, Value, }; use std::fmt::Write as _; /// Replace our custom extended instruction `Abort`s with standard `OpReturn`s, /// but only in entry-points (and only before CFG structurization). +// +// FIXME(eddyb) no longer relying on structurization, try porting this +// to replace custom aborts in `Block`s and inject `ExitInvocation`s +// after them (truncating the `Block` and/or parent region if necessary). pub fn convert_custom_aborts_to_unstructured_returns_in_entry_points( linker_options: &crate::linker::Options, module: &mut Module, @@ -66,8 +70,8 @@ pub fn convert_custom_aborts_to_unstructured_returns_in_entry_points( }; let func_decl = &mut module.funcs[func]; - assert!(match &cx[func_decl.ret_type].ctor { - TypeCtor::SpvInst(spv_inst) => spv_inst.opcode == wk.OpTypeVoid, + assert!(match &cx[func_decl.ret_type].kind { + TypeKind::SpvInst { spv_inst, .. } => spv_inst.opcode == wk.OpTypeVoid, _ => false, }); @@ -112,7 +116,7 @@ pub fn convert_custom_aborts_to_unstructured_returns_in_entry_points( if let DataInstKind::SpvInst(spv_inst) = &data_inst_form_def.kind { if spv_inst.opcode == wk.OpLoad { if let Value::Const(ct) = data_inst_def.inputs[0] { - if let ConstCtor::PtrToGlobalVar(gv) = cx[ct].ctor { + if let ConstKind::PtrToGlobalVar(gv) = cx[ct].kind { if interface_global_vars.contains(&gv) { return Some(( gv, @@ -129,8 +133,8 @@ pub fn convert_custom_aborts_to_unstructured_returns_in_entry_points( if inputs { let mut first_input = true; for (gv, ty, value) in loaded_inputs { - let scalar_type = |ty: Type| match &cx[ty].ctor { - TypeCtor::SpvInst(spv_inst) => match spv_inst.imms[..] { + let scalar_type = |ty: Type| match &cx[ty].kind { + TypeKind::SpvInst { spv_inst, .. } => match spv_inst.imms[..] { [spv::Imm::Short(_, 32), spv::Imm::Short(_, signedness)] if spv_inst.opcode == wk.OpTypeInt => { @@ -145,14 +149,16 @@ pub fn convert_custom_aborts_to_unstructured_returns_in_entry_points( }; let vector_or_scalar_type = |ty: Type| { let ty_def = &cx[ty]; - match (&ty_def.ctor, &ty_def.ctor_args[..]) { - (TypeCtor::SpvInst(spv_inst), &[TypeCtorArg::Type(elem)]) - if spv_inst.opcode == wk.OpTypeVector => - { - match spv_inst.imms[..] { - [spv::Imm::Short(_, vlen @ 2..=4)] => { - Some((scalar_type(elem)?, Some(vlen))) - } + match &ty_def.kind { + TypeKind::SpvInst { + spv_inst, + type_and_const_inputs, + } if spv_inst.opcode == wk.OpTypeVector => { + match (&type_and_const_inputs[..], &spv_inst.imms[..]) { + ( + &[TypeOrConst::Type(elem)], + &[spv::Imm::Short(_, vlen @ 2..=4)], + ) => Some((scalar_type(elem)?, Some(vlen))), _ => None, } } @@ -250,7 +256,9 @@ pub fn convert_custom_aborts_to_unstructured_returns_in_entry_points( )) = custom_terminator_inst { let abort_inst = func_at_abort_inst.position; - terminator.kind = cfg::ControlInstKind::Return; + terminator.kind = cfg::ControlInstKind::ExitInvocation( + cfg::ExitInvocationKind::SpvInst(wk.OpReturn.into()), + ); match abort_strategy { Some(Strategy::Unreachable) => { @@ -260,16 +268,19 @@ pub fn convert_custom_aborts_to_unstructured_returns_in_entry_points( inputs: _, backtrace, }) => { - let const_ctor = |v: Value| match v { - Value::Const(ct) => &cx[ct].ctor, + let const_kind = |v: Value| match v { + Value::Const(ct) => &cx[ct].kind, _ => unreachable!(), }; - let const_str = |v: Value| match const_ctor(v) { - &ConstCtor::SpvStringLiteralForExtInst(s) => s, + let const_str = |v: Value| match const_kind(v) { + &ConstKind::SpvStringLiteralForExtInst(s) => s, _ => unreachable!(), }; - let const_u32 = |v: Value| match const_ctor(v) { - ConstCtor::SpvInst(spv_inst) => { + let const_u32 = |v: Value| match const_kind(v) { + ConstKind::SpvInst { + spv_inst_and_const_inputs, + } => { + let (spv_inst, _const_inputs) = &**spv_inst_and_const_inputs; assert!(spv_inst.opcode == wk.OpConstant); match spv_inst.imms[..] { [spv::Imm::Short(_, x)] => x, @@ -283,11 +294,9 @@ pub fn convert_custom_aborts_to_unstructured_returns_in_entry_points( attrs: Default::default(), ty: cx.intern(TypeDef { attrs: Default::default(), - ctor: TypeCtor::SpvStringLiteralForExtInst, - ctor_args: Default::default(), + kind: TypeKind::SpvStringLiteralForExtInst, }), - ctor: ConstCtor::SpvStringLiteralForExtInst(s), - ctor_args: Default::default(), + kind: ConstKind::SpvStringLiteralForExtInst(s), }) }; diff --git a/crates/rustc_codegen_spirv/src/linker/spirt_passes/debuginfo.rs b/crates/rustc_codegen_spirv/src/linker/spirt_passes/debuginfo.rs index 6121d01122..9d1944ced8 100644 --- a/crates/rustc_codegen_spirv/src/linker/spirt_passes/debuginfo.rs +++ b/crates/rustc_codegen_spirv/src/linker/spirt_passes/debuginfo.rs @@ -6,7 +6,7 @@ use smallvec::SmallVec; use spirt::transform::{InnerInPlaceTransform, Transformer}; use spirt::visit::InnerVisit; use spirt::{ - spv, Attr, AttrSetDef, ConstCtor, Context, ControlNode, ControlNodeKind, DataInstKind, + spv, Attr, AttrSetDef, ConstKind, Context, ControlNode, ControlNodeKind, DataInstKind, InternedStr, Module, OrdAssertEq, Value, }; @@ -95,16 +95,20 @@ impl Transformer for CustomDebuginfoToSpv<'_> { col_start: col, col_end: _, } => { - let const_ctor = |v: Value| match v { - Value::Const(ct) => &self.cx[ct].ctor, + let const_kind = |v: Value| match v { + Value::Const(ct) => &self.cx[ct].kind, _ => unreachable!(), }; - let const_str = |v: Value| match const_ctor(v) { - &ConstCtor::SpvStringLiteralForExtInst(s) => s, + let const_str = |v: Value| match const_kind(v) { + &ConstKind::SpvStringLiteralForExtInst(s) => s, _ => unreachable!(), }; - let const_u32 = |v: Value| match const_ctor(v) { - ConstCtor::SpvInst(spv_inst) => { + let const_u32 = |v: Value| match const_kind(v) { + ConstKind::SpvInst { + spv_inst_and_const_inputs, + } => { + let (spv_inst, _const_inputs) = + &**spv_inst_and_const_inputs; assert!(spv_inst.opcode == self.wk.OpConstant); match spv_inst.imms[..] { [spv::Imm::Short(_, x)] => x, diff --git a/crates/rustc_codegen_spirv/src/linker/spirt_passes/diagnostics.rs b/crates/rustc_codegen_spirv/src/linker/spirt_passes/diagnostics.rs index 4ccdc7218c..c44bd2db09 100644 --- a/crates/rustc_codegen_spirv/src/linker/spirt_passes/diagnostics.rs +++ b/crates/rustc_codegen_spirv/src/linker/spirt_passes/diagnostics.rs @@ -10,7 +10,7 @@ use smallvec::SmallVec; use spirt::func_at::FuncAt; use spirt::visit::{InnerVisit, Visitor}; use spirt::{ - spv, Attr, AttrSet, AttrSetDef, Const, ConstCtor, Context, ControlNode, ControlNodeKind, + spv, Attr, AttrSet, AttrSetDef, Const, ConstKind, Context, ControlNode, ControlNodeKind, DataInstDef, DataInstForm, DataInstKind, Diag, DiagLevel, ExportKey, Exportee, Func, FuncDecl, GlobalVar, InternedStr, Module, Type, Value, }; @@ -275,16 +275,19 @@ impl UseOrigin<'_> { } => (file, line_start, line_end, col_start, col_end), _ => unreachable!(), }; - let const_ctor = |v: Value| match v { - Value::Const(ct) => &cx[ct].ctor, + let const_kind = |v: Value| match v { + Value::Const(ct) => &cx[ct].kind, _ => unreachable!(), }; - let const_str = |v: Value| match const_ctor(v) { - &ConstCtor::SpvStringLiteralForExtInst(s) => s, + let const_str = |v: Value| match const_kind(v) { + &ConstKind::SpvStringLiteralForExtInst(s) => s, _ => unreachable!(), }; - let const_u32 = |v: Value| match const_ctor(v) { - ConstCtor::SpvInst(spv_inst) => { + let const_u32 = |v: Value| match const_kind(v) { + ConstKind::SpvInst { + spv_inst_and_const_inputs, + } => { + let (spv_inst, _const_inputs) = &**spv_inst_and_const_inputs; assert!(spv_inst.opcode == wk.OpConstant); match spv_inst.imms[..] { [spv::Imm::Short(_, x)] => x, @@ -505,9 +508,9 @@ impl<'a> Visitor<'a> for DiagnosticReporter<'a> { fn visit_const_use(&mut self, ct: Const) { if self.seen_consts.insert(ct) { let ct_def = &self.cx[ct]; - match ct_def.ctor { + match ct_def.kind { // HACK(eddyb) don't push an `UseOrigin` for `GlobalVar` pointers. - ConstCtor::PtrToGlobalVar(_) if ct_def.attrs == AttrSet::default() => { + ConstKind::PtrToGlobalVar(_) if ct_def.attrs == AttrSet::default() => { self.visit_const_def(ct_def); } _ => { @@ -642,12 +645,12 @@ impl<'a> Visitor<'a> for DiagnosticReporter<'a> { // Treat this like a call, in the caller. replace_origin(self, IntraFuncUseOrigin::CallCallee); - let const_ctor = |v: Value| match v { - Value::Const(ct) => &self.cx[ct].ctor, + let const_kind = |v: Value| match v { + Value::Const(ct) => &self.cx[ct].kind, _ => unreachable!(), }; - let const_str = |v: Value| match const_ctor(v) { - &ConstCtor::SpvStringLiteralForExtInst(s) => s, + let const_str = |v: Value| match const_kind(v) { + &ConstKind::SpvStringLiteralForExtInst(s) => s, _ => unreachable!(), }; self.use_stack.push(UseOrigin::IntraFunc { diff --git a/crates/rustc_codegen_spirv/src/linker/spirt_passes/mod.rs b/crates/rustc_codegen_spirv/src/linker/spirt_passes/mod.rs index 78ef09725a..e2e3d4f972 100644 --- a/crates/rustc_codegen_spirv/src/linker/spirt_passes/mod.rs +++ b/crates/rustc_codegen_spirv/src/linker/spirt_passes/mod.rs @@ -17,7 +17,6 @@ use spirt::{ FuncDefBody, GlobalVar, Module, Type, Value, }; use std::collections::VecDeque; -use std::hash::Hash; use std::iter; // HACK(eddyb) `spv::spec::Spec` with extra `WellKnown`s (that should be upstreamed). @@ -264,88 +263,11 @@ const _: () = { } fn visit_control_node_def(&mut self, func_at_control_node: FuncAt<'a, ControlNode>) { (self.visit_control_node)(&mut self.state, func_at_control_node); - // HACK(eddyb) accidentally private `inner_visit_with` method. - fn control_node_inner_visit_with<'a>( - self_: FuncAt<'a, ControlNode>, - visitor: &mut impl Visitor<'a>, - ) { - let ControlNodeDef { kind, outputs } = self_.def(); - - match kind { - ControlNodeKind::Block { insts } => { - for func_at_inst in self_.at(*insts) { - visitor.visit_data_inst_def(func_at_inst.def()); - } - } - ControlNodeKind::Select { - kind: SelectionKind::BoolCond | SelectionKind::SpvInst(_), - scrutinee, - cases, - } => { - visitor.visit_value_use(scrutinee); - for &case in cases { - visitor.visit_control_region_def(self_.at(case)); - } - } - ControlNodeKind::Loop { - initial_inputs, - body, - repeat_condition, - } => { - for v in initial_inputs { - visitor.visit_value_use(v); - } - visitor.visit_control_region_def(self_.at(*body)); - visitor.visit_value_use(repeat_condition); - } - } - for output in outputs { - output.inner_visit_with(visitor); - } - } - control_node_inner_visit_with(func_at_control_node, self); + func_at_control_node.inner_visit_with(self); } } }; -// HACK(eddyb) this works around the accidental lack of `spirt::Value: Hash`. -#[derive(Copy, Clone, PartialEq, Eq)] -struct HashableValue(Value); -#[allow(clippy::derived_hash_with_manual_eq)] -impl Hash for HashableValue { - fn hash(&self, state: &mut H) { - use spirt::*; - #[derive(Hash)] - enum ValueH { - Const(Const), - ControlRegionInput { - region: ControlRegion, - input_idx: u32, - }, - ControlNodeOutput { - control_node: ControlNode, - output_idx: u32, - }, - DataInstOutput(DataInst), - } - match self.0 { - Value::Const(ct) => ValueH::Const(ct), - Value::ControlRegionInput { region, input_idx } => { - ValueH::ControlRegionInput { region, input_idx } - } - Value::ControlNodeOutput { - control_node, - output_idx, - } => ValueH::ControlNodeOutput { - control_node, - output_idx, - }, - Value::DataInstOutput(inst) => ValueH::DataInstOutput(inst), - } - .hash(state); - } -} - // FIXME(eddyb) maybe this should be provided by `spirt::transform`. struct ReplaceValueWith(F); const _: () = { @@ -378,7 +300,7 @@ fn remove_unused_values_in_func(cx: &Context, func_def_body: &mut FuncDefBody) { // FIXME(eddyb) entity-keyed dense sets might be better for performance, // but would require separate sets/maps for separate `Value` cases. - used: FxHashSet, + used: FxHashSet, queue: VecDeque, } @@ -396,7 +318,7 @@ fn remove_unused_values_in_func(cx: &Context, func_def_body: &mut FuncDefBody) { return; } } - if self.used.insert(HashableValue(v)) { + if self.used.insert(v) { self.queue.push_back(v); } } @@ -476,8 +398,8 @@ fn remove_unused_values_in_func(cx: &Context, func_def_body: &mut FuncDefBody) { propagator.mark_used(v); propagator.propagate_used(func_at_control_node.at(())); }; - match func_at_control_node.def().kind { - ControlNodeKind::Block { insts } => { + match &func_at_control_node.def().kind { + &ControlNodeKind::Block { insts } => { for func_at_inst in func_at_control_node.at(insts) { // Ignore pure instructions (i.e. they're only used // if their output value is used, from somewhere else). @@ -496,11 +418,20 @@ fn remove_unused_values_in_func(cx: &Context, func_def_body: &mut FuncDefBody) { } } - ControlNodeKind::Select { scrutinee: v, .. } - | ControlNodeKind::Loop { + &ControlNodeKind::Select { scrutinee: v, .. } + | &ControlNodeKind::Loop { repeat_condition: v, .. } => mark_used_and_propagate(v), + + ControlNodeKind::ExitInvocation { + kind: spirt::cfg::ExitInvocationKind::SpvInst(_), + inputs, + } => { + for &v in inputs { + mark_used_and_propagate(v); + } + } } }, }; @@ -535,9 +466,7 @@ fn remove_unused_values_in_func(cx: &Context, func_def_body: &mut FuncDefBody) { continue; } } - if !used_values - .contains(&HashableValue(Value::DataInstOutput(func_at_inst.position))) - { + if !used_values.contains(&Value::DataInstOutput(func_at_inst.position)) { // Replace the removed `DataInstDef` itself with `OpNop`, // removing the ability to use its "name" as a value. // @@ -574,7 +503,7 @@ fn remove_unused_values_in_func(cx: &Context, func_def_body: &mut FuncDefBody) { output_idx: original_idx as u32, }; - if !used_values.contains(&HashableValue(original_output)) { + if !used_values.contains(&original_output) { // Remove the output definition and corresponding value from all cases. func_def_body .at_mut(control_node) @@ -593,11 +522,12 @@ fn remove_unused_values_in_func(cx: &Context, func_def_body: &mut FuncDefBody) { control_node, output_idx: new_idx as u32, }; - value_replacements.insert(HashableValue(original_output), new_output); + value_replacements.insert(original_output, new_output); } new_idx += 1; } } + ControlNodeKind::Loop { body, initial_inputs, @@ -612,7 +542,7 @@ fn remove_unused_values_in_func(cx: &Context, func_def_body: &mut FuncDefBody) { input_idx: original_idx as u32, }; - if !used_values.contains(&HashableValue(original_input)) { + if !used_values.contains(&original_input) { // Remove the input definition and corresponding values. match &mut func_def_body.at_mut(control_node).def().kind { ControlNodeKind::Loop { initial_inputs, .. } => { @@ -632,18 +562,20 @@ fn remove_unused_values_in_func(cx: &Context, func_def_body: &mut FuncDefBody) { region: body, input_idx: new_idx as u32, }; - value_replacements.insert(HashableValue(original_input), new_input); + value_replacements.insert(original_input, new_input); } new_idx += 1; } } + + ControlNodeKind::ExitInvocation { .. } => {} } } if !value_replacements.is_empty() { func_def_body.inner_in_place_transform_with(&mut ReplaceValueWith(|v| match v { Value::Const(_) => None, - _ => value_replacements.get(&HashableValue(v)).copied(), + _ => value_replacements.get(&v).copied(), })); } } diff --git a/crates/rustc_codegen_spirv/src/linker/spirt_passes/reduce.rs b/crates/rustc_codegen_spirv/src/linker/spirt_passes/reduce.rs index a4750586e7..aa1b96b2cb 100644 --- a/crates/rustc_codegen_spirv/src/linker/spirt_passes/reduce.rs +++ b/crates/rustc_codegen_spirv/src/linker/spirt_passes/reduce.rs @@ -4,16 +4,16 @@ use spirt::func_at::{FuncAt, FuncAtMut}; use spirt::transform::InnerInPlaceTransform; use spirt::visit::InnerVisit; use spirt::{ - spv, Const, ConstCtor, ConstDef, Context, ControlNode, ControlNodeDef, ControlNodeKind, + spv, Const, ConstDef, ConstKind, Context, ControlNode, ControlNodeDef, ControlNodeKind, ControlNodeOutputDecl, ControlRegion, ControlRegionInputDecl, DataInst, DataInstDef, DataInstFormDef, DataInstKind, EntityOrientedDenseMap, FuncDefBody, SelectionKind, Type, - TypeCtor, TypeDef, Value, + TypeDef, TypeKind, Value, }; use std::collections::hash_map::Entry; -use std::hash::Hash; use std::{iter, slice}; -use super::{HashableValue, ReplaceValueWith, VisitAllControlRegionsAndNodes}; +use super::{ReplaceValueWith, VisitAllControlRegionsAndNodes}; +use std::rc::Rc; /// Apply "reduction rules" to `func_def_body`, replacing (pure) computations /// with one of their inputs or a constant (e.g. `x + 0 => x` or `1 + 2 => 3`), @@ -87,9 +87,7 @@ pub(crate) fn reduce_in_func(cx: &Context, func_def_body: &mut FuncDefBody) { control_node: func_at_control_node.position, output_idx: i, }; - if let Entry::Vacant(entry) = - value_replacements.entry(HashableValue(output)) - { + if let Entry::Vacant(entry) = value_replacements.entry(output) { let per_case_value = cases.iter().map(|&case| { func_at_control_node.at(case).def().outputs[i as usize] }); @@ -132,8 +130,10 @@ pub(crate) fn reduce_in_func(cx: &Context, func_def_body: &mut FuncDefBody) { op: PureOp::IntToBool, output_type: cx.intern(TypeDef { attrs: Default::default(), - ctor: TypeCtor::SpvInst(wk.OpTypeBool.into()), - ctor_args: iter::empty().collect(), + kind: TypeKind::SpvInst { + spv_inst: wk.OpTypeBool.into(), + type_and_const_inputs: iter::empty().collect(), + }, }), input: *scrutinee, }; @@ -166,11 +166,16 @@ pub(crate) fn reduce_in_func(cx: &Context, func_def_body: &mut FuncDefBody) { }; if body_output == body_input { value_replacements - .entry(HashableValue(body_input)) + .entry(body_input) .or_insert(initial_input); } } } + + &ControlNodeDef { + kind: ControlNodeKind::ExitInvocation { .. }, + .. + } => {} }; func_def_body.inner_visit_with(&mut VisitAllControlRegionsAndNodes { state: (), @@ -197,7 +202,7 @@ pub(crate) fn reduce_in_func(cx: &Context, func_def_body: &mut FuncDefBody) { any_changes = true; match redu_target { ReductionTarget::DataInst(inst) => { - value_replacements.insert(HashableValue(Value::DataInstOutput(inst)), v); + value_replacements.insert(Value::DataInstOutput(inst), v); // Replace the reduced `DataInstDef` itself with `OpNop`, // removing the ability to use its "name" as a value. @@ -243,7 +248,7 @@ pub(crate) fn reduce_in_func(cx: &Context, func_def_body: &mut FuncDefBody) { loop { match v { Value::Const(_) => break, - _ => match value_replacements.get(&HashableValue(v)) { + _ => match value_replacements.get(&v) { Some(&new) => v = new, None => break, }, @@ -292,6 +297,7 @@ impl ParentMap { ControlNodeKind::Select { cases, .. } => cases, ControlNodeKind::Loop { body, .. } => slice::from_ref(body), + ControlNodeKind::ExitInvocation { .. } => &[][..], }; for &child_region in child_regions { this.control_region_parent @@ -319,8 +325,13 @@ fn try_reduce_select( let wk = &super::SpvSpecWithExtras::get().well_known; let as_spv_const = |v: Value| match v { - Value::Const(ct) => match &cx[ct].ctor { - ConstCtor::SpvInst(spv_inst) => Some(spv_inst.opcode), + Value::Const(ct) => match &cx[ct].kind { + ConstKind::SpvInst { + spv_inst_and_const_inputs, + } => { + let (spv_inst, _const_inputs) = &**spv_inst_and_const_inputs; + Some(spv_inst.opcode) + } _ => None, }, _ => None, @@ -476,22 +487,13 @@ impl TryFrom for spv::Inst { } /// Potentially-reducible application of a `PureOp` (`op`) to `input`. -#[derive(Copy, Clone, PartialEq, Eq)] +#[derive(Copy, Clone, PartialEq, Eq, Hash)] struct Reducible { op: PureOp, output_type: Type, input: V, } -// HACK(eddyb) this works around the accidental lack of `spirt::Value: Hash`. -impl Hash for Reducible { - fn hash(&self, state: &mut H) { - self.op.hash(state); - self.output_type.hash(state); - HashableValue(self.input).hash(state); - } -} - impl Reducible { fn with_input(self, new_input: V2) -> Reducible { Reducible { @@ -565,20 +567,27 @@ impl Reducible { let wk = &super::SpvSpecWithExtras::get().well_known; let ct_def = &cx[self.input]; - match (self.op, &ct_def.ctor) { - (_, ConstCtor::SpvInst(spv_inst)) if spv_inst.opcode == wk.OpUndef => { - Some(cx.intern(ConstDef { - attrs: ct_def.attrs, - ty: self.output_type, - ctor: ct_def.ctor.clone(), - ctor_args: iter::empty().collect(), - })) - } + match (self.op, &ct_def.kind) { + ( + _, + ConstKind::SpvInst { + spv_inst_and_const_inputs, + }, + ) if spv_inst_and_const_inputs.0.opcode == wk.OpUndef => Some(cx.intern(ConstDef { + attrs: ct_def.attrs, + ty: self.output_type, + kind: ct_def.kind.clone(), + })), - (PureOp::BitCast, ConstCtor::SpvInst(spv_inst)) if spv_inst.opcode == wk.OpConstant => { + ( + PureOp::BitCast, + ConstKind::SpvInst { + spv_inst_and_const_inputs, + }, + ) if spv_inst_and_const_inputs.0.opcode == wk.OpConstant => { // `OpTypeInt`/`OpTypeFloat` bit width. - let scalar_width = |ty: Type| match &cx[ty].ctor { - TypeCtor::SpvInst(spv_inst) + let scalar_width = |ty: Type| match &cx[ty].kind { + TypeKind::SpvInst { spv_inst, .. } if [wk.OpTypeInt, wk.OpTypeFloat].contains(&spv_inst.opcode) => { Some(spv_inst.imms[0]) @@ -590,8 +599,7 @@ impl Reducible { (Some(from), Some(to)) if from == to => Some(cx.intern(ConstDef { attrs: ct_def.attrs, ty: self.output_type, - ctor: ct_def.ctor.clone(), - ctor_args: ct_def.ctor_args.clone(), + kind: ct_def.kind.clone(), })), _ => None, } @@ -601,14 +609,21 @@ impl Reducible { PureOp::CompositeExtract { elem_idx: spv::Imm::Short(_, elem_idx), }, - ConstCtor::SpvInst(spv_inst), - ) if spv_inst.opcode == wk.OpConstantComposite => { - Some(ct_def.ctor_args[elem_idx as usize]) + ConstKind::SpvInst { + spv_inst_and_const_inputs, + }, + ) if spv_inst_and_const_inputs.0.opcode == wk.OpConstantComposite => { + let (_spv_inst, const_inputs) = &**spv_inst_and_const_inputs; + Some(const_inputs[elem_idx as usize]) } - (PureOp::IntToBool, ConstCtor::SpvInst(spv_inst)) - if spv_inst.opcode == wk.OpConstant => - { + ( + PureOp::IntToBool, + ConstKind::SpvInst { + spv_inst_and_const_inputs, + }, + ) if spv_inst_and_const_inputs.0.opcode == wk.OpConstant => { + let (spv_inst, _const_inputs) = &**spv_inst_and_const_inputs; let bool_const_op = match spv_imm_checked_trunc32(&spv_inst.imms[..]) { Some(0) => wk.OpConstantFalse, Some(1) => wk.OpConstantTrue, @@ -617,8 +632,12 @@ impl Reducible { Some(cx.intern(ConstDef { attrs: Default::default(), ty: self.output_type, - ctor: ConstCtor::SpvInst(bool_const_op.into()), - ctor_args: iter::empty().collect(), + kind: ConstKind::SpvInst { + spv_inst_and_const_inputs: Rc::new(( + bool_const_op.into(), + iter::empty().collect(), + )), + }, })) } @@ -680,7 +699,7 @@ impl Reducible { // FIXME(eddyb) come up with a better convention for this! func: FuncAtMut<'_, ()>, - value_replacements: &FxHashMap, + value_replacements: &FxHashMap, parent_map: &ParentMap, @@ -693,7 +712,7 @@ impl Reducible { // the first time they're encountered, but also, if this process was more // "demand-driven" (recursing into use->def, instead of processing defs), // it might not require any of this complication. - while let Some(&replacement) = value_replacements.get(&HashableValue(self.input)) { + while let Some(&replacement) = value_replacements.get(&self.input) { self.input = replacement; } @@ -715,7 +734,7 @@ impl Reducible { // FIXME(eddyb) come up with a better convention for this! mut func: FuncAtMut<'_, ()>, - value_replacements: &FxHashMap, + value_replacements: &FxHashMap, parent_map: &ParentMap, diff --git a/docs/src/codegen-args.md b/docs/src/codegen-args.md index d1f610366d..18794a7dfe 100644 --- a/docs/src/codegen-args.md +++ b/docs/src/codegen-args.md @@ -208,3 +208,11 @@ all the "file contents debuginfo" (i.e. from SPIR-V `OpSource` instructions), which will end up being included, in full, at the start of the dump. The default (of hiding the file contents) is less verbose, but (arguably) lossier. + +### `--spirt-keep-unstructured-cfg-in-dumps` + +When dumping (pretty-printed) `SPIR-🇹` (e.g. with `--dump-spirt-passes`), include +the initial unstructured state, as well (i.e. just after lowering from SPIR-V). + +The default (of only dumping structured SPIR-T) can have far less noisy dataflow, +but unstructured SPIR-T may be needed for e.g. debugging the structurizer itself. diff --git a/tests/ui/dis/entry-pass-mode-cast-array.stderr b/tests/ui/dis/entry-pass-mode-cast-array.stderr index 3dd6085685..959aaa2ddc 100644 --- a/tests/ui/dis/entry-pass-mode-cast-array.stderr +++ b/tests/ui/dis/entry-pass-mode-cast-array.stderr @@ -8,6 +8,10 @@ OpNoLine OpSelectionMerge %13 None OpBranchConditional %9 %14 %15 %14 = OpLabel +OpBranch %13 +%15 = OpLabel +OpReturn +%13 = OpLabel OpLine %5 14 4 %16 = OpCompositeExtract %17 %6 0 %18 = OpFAdd %17 %16 %19 @@ -15,9 +19,5 @@ OpLine %5 14 4 OpLine %5 15 4 OpStore %21 %20 OpNoLine -OpBranch %13 -%15 = OpLabel -OpBranch %13 -%13 = OpLabel OpReturn OpFunctionEnd diff --git a/tests/ui/dis/index_user_dst.stderr b/tests/ui/dis/index_user_dst.stderr index 7cff35e049..a05bf5ada5 100644 --- a/tests/ui/dis/index_user_dst.stderr +++ b/tests/ui/dis/index_user_dst.stderr @@ -9,13 +9,13 @@ OpNoLine OpSelectionMerge %14 None OpBranchConditional %12 %15 %16 %15 = OpLabel +OpBranch %14 +%16 = OpLabel +OpReturn +%14 = OpLabel OpLine %5 10 21 %17 = OpInBoundsAccessChain %18 %6 %9 %19 = OpLoad %20 %17 OpNoLine -OpBranch %14 -%16 = OpLabel -OpBranch %14 -%14 = OpLabel OpReturn OpFunctionEnd diff --git a/tests/ui/dis/issue-731.stderr b/tests/ui/dis/issue-731.stderr index 807400a153..4972c4a50a 100644 --- a/tests/ui/dis/issue-731.stderr +++ b/tests/ui/dis/issue-731.stderr @@ -8,6 +8,10 @@ OpNoLine OpSelectionMerge %13 None OpBranchConditional %9 %14 %15 %14 = OpLabel +OpBranch %13 +%15 = OpLabel +OpReturn +%13 = OpLabel OpLine %5 12 4 %16 = OpCompositeExtract %17 %6 0 %18 = OpFAdd %17 %16 %19 @@ -15,9 +19,5 @@ OpLine %5 12 4 OpLine %5 13 4 OpStore %21 %20 OpNoLine -OpBranch %13 -%15 = OpLabel -OpBranch %13 -%13 = OpLabel OpReturn OpFunctionEnd diff --git a/tests/ui/dis/panic_builtin_bounds_check.stderr b/tests/ui/dis/panic_builtin_bounds_check.stderr index 279553b922..f1a394f28f 100644 --- a/tests/ui/dis/panic_builtin_bounds_check.stderr +++ b/tests/ui/dis/panic_builtin_bounds_check.stderr @@ -32,7 +32,7 @@ OpBranch %14 OpLine %4 275 4 %17 = OpExtInst %6 %1 1 %3 %11 %10 OpNoLine -OpBranch %14 +OpReturn %14 = OpLabel OpReturn OpFunctionEnd diff --git a/tests/ui/dis/panic_sequential_many.rs b/tests/ui/dis/panic_sequential_many.rs new file mode 100644 index 0000000000..4cb7db174d --- /dev/null +++ b/tests/ui/dis/panic_sequential_many.rs @@ -0,0 +1,30 @@ +#![crate_name = "panic_sequential_many"] + +// Test a long sequence of conditional panics, which has historically generated +// very nested structured control-flow (instead of a single merged chain). + +// build-pass +// compile-flags: -C target-feature=+ext:SPV_KHR_non_semantic_info +// compile-flags: -C llvm-args=--abort-strategy=debug-printf +// compile-flags: -C llvm-args=--disassemble +// +// FIXME(eddyb) consider using such replacements also for dealing +// with `OpLine` changing all the time (esp. in libcore functions). +// +// normalize-stderr-test "; (SPIR-V|Generator: rspirv|Version: 1\.\d+|Bound: \d+)\n" -> "" +// normalize-stderr-test "OpCapability VulkanMemoryModel\n" -> "" +// normalize-stderr-test "OpSource .*\n" -> "" +// normalize-stderr-test "OpExtension .SPV_KHR_vulkan_memory_model.\n" -> "" +// normalize-stderr-test "OpMemoryModel Logical Vulkan" -> "OpMemoryModel Logical Simple" +// normalize-stderr-test "\S:\S*/panic_sequential_many.rs" -> "$$DIR/panic_sequential_many.rs" +// FIXME(eddyb) handle this one in the test runner. +// normalize-stderr-test "\S*/lib/rustlib/" -> "$$SYSROOT/lib/rustlib/" + +use spirv_std::spirv; + +#[spirv(fragment)] +pub fn main(#[spirv(flat)] x: u32, #[spirv(flat)] y: u32, o: &mut u32) { + // HACK(eddyb) this might stop working if the checks get optimized out, + // after the first `y != 0` (which dominates the rest of the function). + *o = x / y / y / y / y / y / y / y / y / y / y / y / y / y / y / y / y / y; +} diff --git a/tests/ui/dis/panic_sequential_many.stderr b/tests/ui/dis/panic_sequential_many.stderr new file mode 100644 index 0000000000..d748d2fd49 --- /dev/null +++ b/tests/ui/dis/panic_sequential_many.stderr @@ -0,0 +1,281 @@ +OpCapability Shader +OpCapability Float64 +OpCapability Int64 +OpCapability Int16 +OpCapability Int8 +OpCapability ShaderClockKHR +OpExtension "SPV_KHR_non_semantic_info" +OpExtension "SPV_KHR_shader_clock" +%1 = OpExtInstImport "NonSemantic.DebugPrintf" +OpMemoryModel Logical Simple +OpEntryPoint Fragment %2 "main" %3 %4 %5 +OpExecutionMode %2 OriginUpperLeft +%6 = OpString "/n[Rust panicked at $DIR/panic_sequential_many.rs:29:10]/n attempt to divide by zero/n in main()/n" +%7 = OpString "$DIR/panic_sequential_many.rs" +OpName %3 "x" +OpName %4 "y" +OpName %5 "o" +OpDecorate %3 Flat +OpDecorate %3 Location 0 +OpDecorate %4 Flat +OpDecorate %4 Location 1 +OpDecorate %5 Location 0 +%8 = OpTypeInt 32 0 +%9 = OpTypePointer Input %8 +%10 = OpTypePointer Output %8 +%11 = OpTypeVoid +%12 = OpTypeFunction %11 +%3 = OpVariable %9 Input +%4 = OpVariable %9 Input +%13 = OpTypeBool +%14 = OpConstant %8 0 +%5 = OpVariable %10 Output +%2 = OpFunction %11 None %12 +%15 = OpLabel +OpLine %7 26 12 +%16 = OpLoad %8 %3 +OpLine %7 26 35 +%17 = OpLoad %8 %4 +OpLine %7 29 9 +%18 = OpIEqual %13 %17 %14 +OpNoLine +OpSelectionMerge %19 None +OpBranchConditional %18 %20 %21 +%20 = OpLabel +OpLine %7 29 9 +%22 = OpExtInst %11 %1 1 %6 +OpNoLine +OpReturn +%21 = OpLabel +OpBranch %19 +%19 = OpLabel +OpLine %7 29 9 +%23 = OpUDiv %8 %16 %17 +%24 = OpIEqual %13 %17 %14 +OpNoLine +OpSelectionMerge %25 None +OpBranchConditional %24 %26 %27 +%26 = OpLabel +OpLine %7 29 9 +%28 = OpExtInst %11 %1 1 %6 +OpNoLine +OpReturn +%27 = OpLabel +OpBranch %25 +%25 = OpLabel +OpLine %7 29 9 +%29 = OpUDiv %8 %23 %17 +%30 = OpIEqual %13 %17 %14 +OpNoLine +OpSelectionMerge %31 None +OpBranchConditional %30 %32 %33 +%32 = OpLabel +OpLine %7 29 9 +%34 = OpExtInst %11 %1 1 %6 +OpNoLine +OpReturn +%33 = OpLabel +OpBranch %31 +%31 = OpLabel +OpLine %7 29 9 +%35 = OpUDiv %8 %29 %17 +%36 = OpIEqual %13 %17 %14 +OpNoLine +OpSelectionMerge %37 None +OpBranchConditional %36 %38 %39 +%38 = OpLabel +OpLine %7 29 9 +%40 = OpExtInst %11 %1 1 %6 +OpNoLine +OpReturn +%39 = OpLabel +OpBranch %37 +%37 = OpLabel +OpLine %7 29 9 +%41 = OpUDiv %8 %35 %17 +%42 = OpIEqual %13 %17 %14 +OpNoLine +OpSelectionMerge %43 None +OpBranchConditional %42 %44 %45 +%44 = OpLabel +OpLine %7 29 9 +%46 = OpExtInst %11 %1 1 %6 +OpNoLine +OpReturn +%45 = OpLabel +OpBranch %43 +%43 = OpLabel +OpLine %7 29 9 +%47 = OpUDiv %8 %41 %17 +%48 = OpIEqual %13 %17 %14 +OpNoLine +OpSelectionMerge %49 None +OpBranchConditional %48 %50 %51 +%50 = OpLabel +OpLine %7 29 9 +%52 = OpExtInst %11 %1 1 %6 +OpNoLine +OpReturn +%51 = OpLabel +OpBranch %49 +%49 = OpLabel +OpLine %7 29 9 +%53 = OpUDiv %8 %47 %17 +%54 = OpIEqual %13 %17 %14 +OpNoLine +OpSelectionMerge %55 None +OpBranchConditional %54 %56 %57 +%56 = OpLabel +OpLine %7 29 9 +%58 = OpExtInst %11 %1 1 %6 +OpNoLine +OpReturn +%57 = OpLabel +OpBranch %55 +%55 = OpLabel +OpLine %7 29 9 +%59 = OpUDiv %8 %53 %17 +%60 = OpIEqual %13 %17 %14 +OpNoLine +OpSelectionMerge %61 None +OpBranchConditional %60 %62 %63 +%62 = OpLabel +OpLine %7 29 9 +%64 = OpExtInst %11 %1 1 %6 +OpNoLine +OpReturn +%63 = OpLabel +OpBranch %61 +%61 = OpLabel +OpLine %7 29 9 +%65 = OpUDiv %8 %59 %17 +%66 = OpIEqual %13 %17 %14 +OpNoLine +OpSelectionMerge %67 None +OpBranchConditional %66 %68 %69 +%68 = OpLabel +OpLine %7 29 9 +%70 = OpExtInst %11 %1 1 %6 +OpNoLine +OpReturn +%69 = OpLabel +OpBranch %67 +%67 = OpLabel +OpLine %7 29 9 +%71 = OpUDiv %8 %65 %17 +%72 = OpIEqual %13 %17 %14 +OpNoLine +OpSelectionMerge %73 None +OpBranchConditional %72 %74 %75 +%74 = OpLabel +OpLine %7 29 9 +%76 = OpExtInst %11 %1 1 %6 +OpNoLine +OpReturn +%75 = OpLabel +OpBranch %73 +%73 = OpLabel +OpLine %7 29 9 +%77 = OpUDiv %8 %71 %17 +%78 = OpIEqual %13 %17 %14 +OpNoLine +OpSelectionMerge %79 None +OpBranchConditional %78 %80 %81 +%80 = OpLabel +OpLine %7 29 9 +%82 = OpExtInst %11 %1 1 %6 +OpNoLine +OpReturn +%81 = OpLabel +OpBranch %79 +%79 = OpLabel +OpLine %7 29 9 +%83 = OpUDiv %8 %77 %17 +%84 = OpIEqual %13 %17 %14 +OpNoLine +OpSelectionMerge %85 None +OpBranchConditional %84 %86 %87 +%86 = OpLabel +OpLine %7 29 9 +%88 = OpExtInst %11 %1 1 %6 +OpNoLine +OpReturn +%87 = OpLabel +OpBranch %85 +%85 = OpLabel +OpLine %7 29 9 +%89 = OpUDiv %8 %83 %17 +%90 = OpIEqual %13 %17 %14 +OpNoLine +OpSelectionMerge %91 None +OpBranchConditional %90 %92 %93 +%92 = OpLabel +OpLine %7 29 9 +%94 = OpExtInst %11 %1 1 %6 +OpNoLine +OpReturn +%93 = OpLabel +OpBranch %91 +%91 = OpLabel +OpLine %7 29 9 +%95 = OpUDiv %8 %89 %17 +%96 = OpIEqual %13 %17 %14 +OpNoLine +OpSelectionMerge %97 None +OpBranchConditional %96 %98 %99 +%98 = OpLabel +OpLine %7 29 9 +%100 = OpExtInst %11 %1 1 %6 +OpNoLine +OpReturn +%99 = OpLabel +OpBranch %97 +%97 = OpLabel +OpLine %7 29 9 +%101 = OpUDiv %8 %95 %17 +%102 = OpIEqual %13 %17 %14 +OpNoLine +OpSelectionMerge %103 None +OpBranchConditional %102 %104 %105 +%104 = OpLabel +OpLine %7 29 9 +%106 = OpExtInst %11 %1 1 %6 +OpNoLine +OpReturn +%105 = OpLabel +OpBranch %103 +%103 = OpLabel +OpLine %7 29 9 +%107 = OpUDiv %8 %101 %17 +%108 = OpIEqual %13 %17 %14 +OpNoLine +OpSelectionMerge %109 None +OpBranchConditional %108 %110 %111 +%110 = OpLabel +OpLine %7 29 9 +%112 = OpExtInst %11 %1 1 %6 +OpNoLine +OpReturn +%111 = OpLabel +OpBranch %109 +%109 = OpLabel +OpLine %7 29 9 +%113 = OpUDiv %8 %107 %17 +%114 = OpIEqual %13 %17 %14 +OpNoLine +OpSelectionMerge %115 None +OpBranchConditional %114 %116 %117 +%116 = OpLabel +OpLine %7 29 9 +%118 = OpExtInst %11 %1 1 %6 +OpNoLine +OpReturn +%117 = OpLabel +OpBranch %115 +%115 = OpLabel +OpLine %7 29 4 +%119 = OpUDiv %8 %113 %17 +OpStore %5 %119 +OpNoLine +OpReturn +OpFunctionEnd diff --git a/tests/ui/lang/core/unwrap_or.stderr b/tests/ui/lang/core/unwrap_or.stderr index 7349d6eb0e..0b39d5b926 100644 --- a/tests/ui/lang/core/unwrap_or.stderr +++ b/tests/ui/lang/core/unwrap_or.stderr @@ -15,17 +15,9 @@ OpBranch %18 %20 = OpLabel OpBranch %18 %18 = OpLabel -%21 = OpPhi %16 %22 %19 %23 %20 -%24 = OpPhi %11 %25 %19 %10 %20 -OpSelectionMerge %26 None -OpBranchConditional %21 %27 %28 -%27 = OpLabel -OpBranch %26 -%28 = OpLabel -OpBranch %26 -%26 = OpLabel +%21 = OpPhi %11 %22 %19 %10 %20 OpLine %5 13 4 -OpStore %29 %24 +OpStore %23 %21 OpNoLine OpReturn OpFunctionEnd