From a4681a1e896440c44cb6c2096368c5e723284788 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Mon, 1 May 2023 15:35:23 +0300 Subject: [PATCH] Intern, as `DataInstForm`, the `kind` and `output_type` fields of `DataInstDef`. --- CHANGELOG.md | 2 + src/context.rs | 2 + src/func_at.rs | 2 +- src/lib.rs | 24 +++++++-- src/passes/legalize.rs | 15 +++++- src/passes/link.rs | 42 ++++++++++++++-- src/passes/qptr.rs | 14 +++++- src/print/mod.rs | 25 +++++++--- src/qptr/analyze.rs | 12 +++-- src/qptr/lift.rs | 108 ++++++++++++++++++++++++++--------------- src/qptr/lower.rs | 51 +++++++++++++------ src/qptr/mod.rs | 2 +- src/qptr/shapes.rs | 2 +- src/spv/lift.rs | 32 +++++++----- src/spv/lower.rs | 43 ++++++++++------ src/spv/mod.rs | 2 + src/transform.rs | 73 +++++++++++++++++++--------- src/visit.rs | 29 +++++++---- 18 files changed, 341 insertions(+), 139 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index af23045..ec8a622 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] - ReleaseDate ### Changed 🛠 +- [PR#28](https://github.com/EmbarkStudios/spirt/pull/28) moved two `DataInstDef` + fields (`kind` and `output_type`) to `DataInstForm`, a new interned type - [PR#30](https://github.com/EmbarkStudios/spirt/pull/30) replaced the old `spv-lower-dump` example (which only dumped plaintext, not HTML) with a more useful `spv-lower-print` one diff --git a/src/context.rs b/src/context.rs index 26d0dc8..8f3a5bb 100644 --- a/src/context.rs +++ b/src/context.rs @@ -900,6 +900,7 @@ interners! { crate::AttrSetDef, crate::TypeDef, crate::ConstDef, + crate::DataInstFormDef, } // FIXME(eddyb) consider a more uniform naming scheme than the combination @@ -908,6 +909,7 @@ interners! { AttrSet default(crate::AttrSetDef::default()) => crate::AttrSetDef, Type => crate::TypeDef, Const => crate::ConstDef, + DataInstForm => crate::DataInstFormDef, } impl InternInCx for I::Def diff --git a/src/func_at.rs b/src/func_at.rs index fba3eea..e9d9ce2 100644 --- a/src/func_at.rs +++ b/src/func_at.rs @@ -117,7 +117,7 @@ impl FuncAt<'_, Value> { control_node, output_idx, } => self.at(control_node).def().outputs[output_idx as usize].ty, - Value::DataInstOutput(inst) => self.at(inst).def().output_type.unwrap(), + Value::DataInstOutput(inst) => cx[self.at(inst).def().form].output_type.unwrap(), } } } diff --git a/src/lib.rs b/src/lib.rs index 195054c..9c3a056 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -870,15 +870,31 @@ pub use context::DataInst; pub struct DataInstDef { pub attrs: AttrSet, - pub kind: DataInstKind, - - pub output_type: Option, + pub form: DataInstForm, // FIXME(eddyb) change the inline size of this to fit most instructions. pub inputs: SmallVec<[Value; 2]>, } -#[derive(Clone, PartialEq, Eq)] +/// Interned handle for a [`DataInstFormDef`](crate::DataInstFormDef) +/// (a "form", or "template", for [`DataInstDef`](crate::DataInstDef)s). +pub use context::DataInstForm; + +/// "Form" (or "template") definition for [`DataInstFormDef`]s, which includes +/// most of their common *static* information (notably excluding `attrs`, as +/// they vary more often due to handling diagnostics, debuginfo, refinement etc.). +// +// FIXME(eddyb) now that this is interned, try to find all the code that was +// working around needing to borrow `DataInstKind`, just because it was owned +// by a `FuncDefBody` (instead of interned in the `Context`). +#[derive(Clone, PartialEq, Eq, Hash)] +pub struct DataInstFormDef { + pub kind: DataInstKind, + + pub output_type: Option, +} + +#[derive(Clone, PartialEq, Eq, Hash)] pub enum DataInstKind { // FIXME(eddyb) try to split this into recursive and non-recursive calls, // to avoid needing special handling for recursion where it's impossible. diff --git a/src/passes/legalize.rs b/src/passes/legalize.rs index c7cb430..6bdc6d6 100644 --- a/src/passes/legalize.rs +++ b/src/passes/legalize.rs @@ -1,5 +1,7 @@ use crate::visit::{InnerVisit, Visitor}; -use crate::{cfg, AttrSet, Const, Context, DeclDef, Func, FxIndexSet, GlobalVar, Module, Type}; +use crate::{ + cfg, AttrSet, Const, Context, DataInstForm, DeclDef, Func, FxIndexSet, GlobalVar, Module, Type, +}; /// Apply the [`cfg::Structurizer`] algorithm to all function definitions in `module`. pub fn structurize_func_cfgs(module: &mut Module) { @@ -12,6 +14,7 @@ pub fn structurize_func_cfgs(module: &mut Module) { seen_types: FxIndexSet::default(), seen_consts: FxIndexSet::default(), + seen_data_inst_forms: FxIndexSet::default(), seen_global_vars: FxIndexSet::default(), seen_funcs: FxIndexSet::default(), }; @@ -34,13 +37,16 @@ struct ReachableUseCollector<'a> { // FIXME(eddyb) build some automation to avoid ever repeating these. seen_types: FxIndexSet, seen_consts: FxIndexSet, + seen_data_inst_forms: FxIndexSet, seen_global_vars: FxIndexSet, seen_funcs: FxIndexSet, } impl Visitor<'_> for ReachableUseCollector<'_> { // FIXME(eddyb) build some automation to avoid ever repeating these. - fn visit_attr_set_use(&mut self, _attrs: AttrSet) {} + fn visit_attr_set_use(&mut self, _attrs: AttrSet) { + // FIXME(eddyb) if `AttrSet`s are ignored, why not `Type`s too? + } fn visit_type_use(&mut self, ty: Type) { if self.seen_types.insert(ty) { self.visit_type_def(&self.cx[ty]); @@ -51,6 +57,11 @@ impl Visitor<'_> for ReachableUseCollector<'_> { self.visit_const_def(&self.cx[ct]); } } + fn visit_data_inst_form_use(&mut self, data_inst_form: DataInstForm) { + if self.seen_data_inst_forms.insert(data_inst_form) { + self.visit_data_inst_form_def(&self.cx[data_inst_form]); + } + } fn visit_global_var_use(&mut self, gv: GlobalVar) { if self.seen_global_vars.insert(gv) { diff --git a/src/passes/link.rs b/src/passes/link.rs index 59f8c1e..21dfdd4 100644 --- a/src/passes/link.rs +++ b/src/passes/link.rs @@ -1,8 +1,8 @@ use crate::transform::{InnerTransform, Transformed, Transformer}; use crate::visit::{InnerVisit, Visitor}; use crate::{ - AttrSet, Const, Context, DeclDef, ExportKey, Exportee, Func, FxIndexSet, GlobalVar, Import, - Module, Type, + AttrSet, Const, Context, DataInstForm, DeclDef, ExportKey, Exportee, Func, FxIndexSet, + GlobalVar, Import, Module, Type, }; use rustc_hash::{FxHashMap, FxHashSet}; use std::collections::VecDeque; @@ -33,6 +33,7 @@ pub fn minimize_exports(module: &mut Module, is_root: impl Fn(&ExportKey) -> boo seen_types: FxHashSet::default(), seen_consts: FxHashSet::default(), + seen_data_inst_forms: FxHashSet::default(), seen_global_vars: FxHashSet::default(), seen_funcs: FxHashSet::default(), }; @@ -60,13 +61,16 @@ struct LiveExportCollector<'a> { // FIXME(eddyb) build some automation to avoid ever repeating these. seen_types: FxHashSet, seen_consts: FxHashSet, + seen_data_inst_forms: FxHashSet, seen_global_vars: FxHashSet, seen_funcs: FxHashSet, } impl Visitor<'_> for LiveExportCollector<'_> { // FIXME(eddyb) build some automation to avoid ever repeating these. - fn visit_attr_set_use(&mut self, _attrs: AttrSet) {} + fn visit_attr_set_use(&mut self, _attrs: AttrSet) { + // FIXME(eddyb) if `AttrSet`s are ignored, why not `Type`s too? + } fn visit_type_use(&mut self, ty: Type) { if self.seen_types.insert(ty) { self.visit_type_def(&self.cx[ty]); @@ -77,6 +81,11 @@ impl Visitor<'_> for LiveExportCollector<'_> { self.visit_const_def(&self.cx[ct]); } } + fn visit_data_inst_form_use(&mut self, data_inst_form: DataInstForm) { + if self.seen_data_inst_forms.insert(data_inst_form) { + self.visit_data_inst_form_def(&self.cx[data_inst_form]); + } + } fn visit_global_var_use(&mut self, gv: GlobalVar) { if self.seen_global_vars.insert(gv) { @@ -119,6 +128,7 @@ pub fn resolve_imports(module: &mut Module) { seen_types: FxHashSet::default(), seen_consts: FxHashSet::default(), + seen_data_inst_forms: FxHashSet::default(), seen_global_vars: FxHashSet::default(), seen_funcs: FxHashSet::default(), }; @@ -134,6 +144,7 @@ pub fn resolve_imports(module: &mut Module) { transformed_types: FxHashMap::default(), transformed_consts: FxHashMap::default(), + transformed_data_inst_forms: FxHashMap::default(), transformed_global_vars: FxHashMap::default(), global_var_queue: VecDeque::new(), transformed_funcs: FxHashMap::default(), @@ -170,13 +181,16 @@ struct ImportResolutionCollector<'a> { // FIXME(eddyb) build some automation to avoid ever repeating these. seen_types: FxHashSet, seen_consts: FxHashSet, + seen_data_inst_forms: FxHashSet, seen_global_vars: FxHashSet, seen_funcs: FxHashSet, } impl Visitor<'_> for ImportResolutionCollector<'_> { // FIXME(eddyb) build some automation to avoid ever repeating these. - fn visit_attr_set_use(&mut self, _attrs: AttrSet) {} + fn visit_attr_set_use(&mut self, _attrs: AttrSet) { + // FIXME(eddyb) if `AttrSet`s are ignored, why not `Type`s too? + } fn visit_type_use(&mut self, ty: Type) { if self.seen_types.insert(ty) { self.visit_type_def(&self.cx[ty]); @@ -187,6 +201,11 @@ impl Visitor<'_> for ImportResolutionCollector<'_> { self.visit_const_def(&self.cx[ct]); } } + fn visit_data_inst_form_use(&mut self, data_inst_form: DataInstForm) { + if self.seen_data_inst_forms.insert(data_inst_form) { + self.visit_data_inst_form_def(&self.cx[data_inst_form]); + } + } fn visit_global_var_use(&mut self, gv: GlobalVar) { if self.seen_global_vars.insert(gv) { @@ -233,6 +252,7 @@ struct ImportResolver<'a> { // FIXME(eddyb) build some automation to avoid ever repeating these. transformed_types: FxHashMap>, transformed_consts: FxHashMap>, + transformed_data_inst_forms: FxHashMap>, transformed_global_vars: FxHashMap>, global_var_queue: VecDeque, transformed_funcs: FxHashMap>, @@ -261,6 +281,20 @@ impl Transformer for ImportResolver<'_> { self.transformed_consts.insert(ct, transformed); transformed } + fn transform_data_inst_form_use( + &mut self, + data_inst_form: DataInstForm, + ) -> Transformed { + if let Some(&cached) = self.transformed_data_inst_forms.get(&data_inst_form) { + return cached; + } + let transformed = self + .transform_data_inst_form_def(&self.cx[data_inst_form]) + .map(|data_inst_form_def| self.cx.intern(data_inst_form_def)); + self.transformed_data_inst_forms + .insert(data_inst_form, transformed); + transformed + } fn transform_global_var_use(&mut self, gv: GlobalVar) -> Transformed { if let Some(&cached) = self.transformed_global_vars.get(&gv) { diff --git a/src/passes/qptr.rs b/src/passes/qptr.rs index 0342f9f..2ddc936 100644 --- a/src/passes/qptr.rs +++ b/src/passes/qptr.rs @@ -1,7 +1,7 @@ //! [`QPtr`](crate::TypeCtor::QPtr) transforms. -use crate::qptr; use crate::visit::{InnerVisit, Visitor}; +use crate::{qptr, DataInstForm}; use crate::{AttrSet, Const, Context, Func, FxIndexSet, GlobalVar, Module, Type}; pub fn lower_from_spv_ptrs(module: &mut Module, layout_config: &qptr::LayoutConfig) { @@ -15,6 +15,7 @@ pub fn lower_from_spv_ptrs(module: &mut Module, layout_config: &qptr::LayoutConf seen_types: FxIndexSet::default(), seen_consts: FxIndexSet::default(), + seen_data_inst_forms: FxIndexSet::default(), seen_global_vars: FxIndexSet::default(), seen_funcs: FxIndexSet::default(), }; @@ -49,6 +50,7 @@ pub fn lift_to_spv_ptrs(module: &mut Module, layout_config: &qptr::LayoutConfig) seen_types: FxIndexSet::default(), seen_consts: FxIndexSet::default(), + seen_data_inst_forms: FxIndexSet::default(), seen_global_vars: FxIndexSet::default(), seen_funcs: FxIndexSet::default(), }; @@ -73,13 +75,16 @@ struct ReachableUseCollector<'a> { // FIXME(eddyb) build some automation to avoid ever repeating these. seen_types: FxIndexSet, seen_consts: FxIndexSet, + seen_data_inst_forms: FxIndexSet, seen_global_vars: FxIndexSet, seen_funcs: FxIndexSet, } impl Visitor<'_> for ReachableUseCollector<'_> { // FIXME(eddyb) build some automation to avoid ever repeating these. - fn visit_attr_set_use(&mut self, _attrs: AttrSet) {} + fn visit_attr_set_use(&mut self, _attrs: AttrSet) { + // FIXME(eddyb) if `AttrSet`s are ignored, why not `Type`s too? + } fn visit_type_use(&mut self, ty: Type) { if self.seen_types.insert(ty) { self.visit_type_def(&self.cx[ty]); @@ -90,6 +95,11 @@ impl Visitor<'_> for ReachableUseCollector<'_> { self.visit_const_def(&self.cx[ct]); } } + fn visit_data_inst_form_use(&mut self, data_inst_form: DataInstForm) { + if self.seen_data_inst_forms.insert(data_inst_form) { + self.visit_data_inst_form_def(&self.cx[data_inst_form]); + } + } fn visit_global_var_use(&mut self, gv: GlobalVar) { if self.seen_global_vars.insert(gv) { diff --git a/src/print/mod.rs b/src/print/mod.rs index 129e402..37961b1 100644 --- a/src/print/mod.rs +++ b/src/print/mod.rs @@ -26,10 +26,11 @@ use crate::visit::{DynVisit, InnerVisit, Visit, Visitor}; use crate::{ cfg, spv, AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstCtor, ConstDef, Context, ControlNode, ControlNodeDef, ControlNodeKind, ControlNodeOutputDecl, ControlRegion, - ControlRegionDef, ControlRegionInputDecl, DataInst, DataInstDef, DataInstKind, DeclDef, Diag, - DiagLevel, DiagMsgPart, EntityListIter, ExportKey, Exportee, Func, FuncDecl, FuncParam, - FxIndexMap, GlobalVar, GlobalVarDecl, GlobalVarDefBody, Import, Module, ModuleDebugInfo, - ModuleDialect, OrdAssertEq, SelectionKind, Type, TypeCtor, TypeCtorArg, TypeDef, Value, + ControlRegionDef, ControlRegionInputDecl, DataInst, DataInstDef, DataInstForm, DataInstFormDef, + DataInstKind, DeclDef, Diag, DiagLevel, DiagMsgPart, EntityListIter, ExportKey, Exportee, Func, + FuncDecl, FuncParam, FxIndexMap, GlobalVar, GlobalVarDecl, GlobalVarDefBody, Import, Module, + ModuleDebugInfo, ModuleDialect, OrdAssertEq, SelectionKind, Type, TypeCtor, TypeCtorArg, + TypeDef, Value, }; use rustc_hash::FxHashMap; use smallvec::SmallVec; @@ -385,6 +386,12 @@ impl<'a> Visitor<'a> for Plan<'a> { fn visit_const_use(&mut self, ct: Const) { self.use_interned(CxInterned::Const(ct)); } + fn visit_data_inst_form_use(&mut self, data_inst_form: DataInstForm) { + // NOTE(eddyb) this contains no deduplication because each `DataInstDef` + // will be pretty-printed separately, so everything in its `form` also + // needs to get use counts incremented separately per-`DataInstDef`. + self.visit_data_inst_form_def(&self.cx[data_inst_form]); + } fn visit_global_var_use(&mut self, gv: GlobalVar) { if let Some(module) = self.current_module { @@ -785,7 +792,7 @@ impl<'a> Printer<'a> { if let ControlNodeKind::Block { insts } = *kind { for func_at_inst in func_def_body.at(insts) { define(Use::AlignmentAnchorForDataInst(func_at_inst.position)); - if func_at_inst.def().output_type.is_some() { + if cx[func_at_inst.def().form].output_type.is_some() { define(Use::DataInstOutput(func_at_inst.position)); } } @@ -808,6 +815,7 @@ impl<'a> Printer<'a> { fn visit_attr_set_use(&mut self, _: AttrSet) {} fn visit_type_use(&mut self, _: Type) {} fn visit_const_use(&mut self, _: Const) {} + fn visit_data_inst_form_use(&mut self, _: DataInstForm) {} fn visit_global_var_use(&mut self, _: GlobalVar) {} fn visit_func_use(&mut self, _: Func) {} @@ -2687,7 +2695,7 @@ impl Print for FuncAt<'_, ControlNode> { data_inst_attrs_and_def.def_without_name, ]); data_inst_attrs_and_def.insert_name_before_def( - if data_inst_def.output_type.is_none() { + if printer.cx[data_inst_def.form].output_type.is_none() { pretty::Fragment::default() } else { pretty::Fragment::new([ @@ -2834,13 +2842,14 @@ impl Print for DataInstDef { fn print(&self, printer: &Printer<'_>) -> AttrsAndDef { let Self { attrs, - kind, - output_type, + form, inputs, } = self; let attrs = attrs.print(printer); + let DataInstFormDef { kind, output_type } = &printer.cx[*form]; + let header = match kind { &DataInstKind::FuncCall(func) => pretty::Fragment::new([ printer.declarative_keyword_style().apply("call").into(), diff --git a/src/qptr/analyze.rs b/src/qptr/analyze.rs index 8c9bfbf..6600df5 100644 --- a/src/qptr/analyze.rs +++ b/src/qptr/analyze.rs @@ -8,8 +8,8 @@ use crate::func_at::FuncAt; use crate::visit::{InnerVisit, Visitor}; use crate::{ AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstCtor, Context, ControlNode, ControlNodeKind, - DataInst, DataInstKind, DeclDef, Diag, EntityList, ExportKey, Exportee, Func, FxIndexMap, - GlobalVar, Module, OrdAssertEq, Type, TypeCtor, Value, + DataInst, DataInstForm, DataInstKind, DeclDef, Diag, EntityList, ExportKey, Exportee, Func, + FxIndexMap, GlobalVar, Module, OrdAssertEq, Type, TypeCtor, Value, }; use itertools::Either; use rustc_hash::FxHashMap; @@ -908,6 +908,7 @@ impl<'a> InferUsage<'a> { for func_at_inst in func_def_body.at(insts).into_iter().rev() { let data_inst = func_at_inst.position; let data_inst_def = func_at_inst.def(); + let data_inst_form_def = &cx[data_inst_def.form]; let output_usage = data_inst_output_usages.remove(&data_inst).flatten(); let mut generate_usage = |this: &mut Self, ptr: Value, new_usage| { @@ -947,7 +948,7 @@ impl<'a> InferUsage<'a> { None => new_usage, }); }; - match &data_inst_def.kind { + match &data_inst_form_def.kind { &DataInstKind::FuncCall(callee) => { match self.infer_usage_in_func(module, callee) { FuncInferUsageState::Complete(callee_results) => { @@ -970,7 +971,7 @@ impl<'a> InferUsage<'a> { )); } }; - if data_inst_def.output_type.map_or(false, is_qptr) { + if data_inst_form_def.output_type.map_or(false, is_qptr) { if let Some(usage) = output_usage { usage_or_err_attrs_to_attach .push((Value::DataInstOutput(data_inst), usage)); @@ -1156,7 +1157,7 @@ impl<'a> InferUsage<'a> { } DataInstKind::QPtr(op @ (QPtrOp::Load | QPtrOp::Store)) => { let (op_name, access_type) = match op { - QPtrOp::Load => ("Load", data_inst_def.output_type.unwrap()), + QPtrOp::Load => ("Load", data_inst_form_def.output_type.unwrap()), QPtrOp::Store => ( "Store", func_at_inst.at(data_inst_def.inputs[1]).type_of(&cx), @@ -1302,6 +1303,7 @@ impl Visitor<'_> for CollectAllDataInsts { fn visit_attr_set_use(&mut self, _: AttrSet) {} fn visit_type_use(&mut self, _: Type) {} fn visit_const_use(&mut self, _: Const) {} + fn visit_data_inst_form_use(&mut self, _: DataInstForm) {} fn visit_global_var_use(&mut self, _: GlobalVar) {} fn visit_func_use(&mut self, _: Func) {} diff --git a/src/qptr/lift.rs b/src/qptr/lift.rs index bd172d9..f6238a5 100644 --- a/src/qptr/lift.rs +++ b/src/qptr/lift.rs @@ -8,9 +8,9 @@ use crate::qptr::{shapes, QPtrAttr, QPtrMemUsage, QPtrMemUsageKind, QPtrOp, QPtr use crate::transform::{InnerInPlaceTransform, InnerTransform, Transformed, Transformer}; use crate::{ spv, AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstCtor, ConstDef, Context, ControlNode, - ControlNodeKind, DataInst, DataInstDef, DataInstKind, DeclDef, Diag, DiagLevel, EntityDefs, - EntityOrientedDenseMap, Func, FuncDecl, FxIndexMap, GlobalVar, GlobalVarDecl, Module, Type, - TypeCtor, TypeCtorArg, TypeDef, Value, + ControlNodeKind, DataInst, DataInstDef, DataInstFormDef, DataInstKind, DeclDef, Diag, + DiagLevel, EntityDefs, EntityOrientedDenseMap, Func, FuncDecl, FxIndexMap, GlobalVar, + GlobalVarDecl, Module, Type, TypeCtor, TypeCtorArg, TypeDef, Value, }; use smallvec::SmallVec; use std::cell::Cell; @@ -439,6 +439,7 @@ impl LiftToSpvPtrInstsInFunc<'_> { let func_at_data_inst_frozen = func_at_data_inst.reborrow().freeze(); let data_inst = func_at_data_inst_frozen.position; let data_inst_def = func_at_data_inst_frozen.def(); + let data_inst_form_def = &cx[data_inst_def.form]; let func = func_at_data_inst_frozen.at(()); let type_of_val = |v: Value| func.at(v).type_of(cx); // FIXME(eddyb) maybe all this data should be packaged up together in a @@ -460,7 +461,7 @@ impl LiftToSpvPtrInstsInFunc<'_> { Ok((addr_space, self.lifter.layout_of(pointee_type)?)) }; - let replacement_data_inst_def = match &data_inst_def.kind { + let replacement_data_inst_def = match &data_inst_form_def.kind { &DataInstKind::FuncCall(_callee) => { for &v in &data_inst_def.inputs { if self.lifter.as_spv_ptr_type(type_of_val(v)).is_some() { @@ -479,16 +480,20 @@ impl LiftToSpvPtrInstsInFunc<'_> { let pointee_type = self.lifter.pointee_type_for_usage(qptr_usage)?; DataInstDef { attrs: self.lifter.strip_qptr_usage_attr(data_inst_def.attrs), - kind: DataInstKind::SpvInst(spv::Inst { - opcode: wk.OpVariable, - imms: [spv::Imm::Short(wk.StorageClass, wk.Function)] - .into_iter() - .collect(), + form: cx.intern(DataInstFormDef { + kind: DataInstKind::SpvInst(spv::Inst { + opcode: wk.OpVariable, + imms: [spv::Imm::Short(wk.StorageClass, wk.Function)] + .into_iter() + .collect(), + }), + output_type: Some( + self.lifter.spv_ptr_type( + AddrSpace::SpvStorageClass(wk.Function), + pointee_type, + ), + ), }), - output_type: Some( - self.lifter - .spv_ptr_type(AddrSpace::SpvStorageClass(wk.Function), pointee_type), - ), inputs: data_inst_def.inputs.clone(), } } @@ -513,8 +518,10 @@ impl LiftToSpvPtrInstsInFunc<'_> { }; DataInstDef { attrs: data_inst_def.attrs, - kind: DataInstKind::SpvInst(wk.OpAccessChain.into()), - output_type: Some(self.lifter.spv_ptr_type(addr_space, handle_type)), + form: cx.intern(DataInstFormDef { + kind: DataInstKind::SpvInst(wk.OpAccessChain.into()), + output_type: Some(self.lifter.spv_ptr_type(addr_space, handle_type)), + }), inputs: data_inst_def.inputs.clone(), } } @@ -538,9 +545,13 @@ impl LiftToSpvPtrInstsInFunc<'_> { ); DataInstDef { - // FIXME(eddyb) avoid the repeated call to `type_of_val`, + // FIXME(eddyb) avoid the repeated call to `type_of_val` + // (and the interning of a temporary `DataInstFormDef`), // maybe don't even replace the `QPtrOp::Buffer` instruction? - output_type: Some(type_of_val(buf_ptr)), + form: cx.intern(DataInstFormDef { + kind: QPtrOp::BufferData.into(), + output_type: Some(type_of_val(buf_ptr)), + }), ..data_inst_def.clone() } } @@ -583,11 +594,14 @@ impl LiftToSpvPtrInstsInFunc<'_> { }; DataInstDef { - kind: DataInstKind::SpvInst(spv::Inst { - opcode: wk.OpArrayLength, - imms: [spv::Imm::Short(wk.LiteralInteger, field_idx)] - .into_iter() - .collect(), + form: cx.intern(DataInstFormDef { + kind: DataInstKind::SpvInst(spv::Inst { + opcode: wk.OpArrayLength, + imms: [spv::Imm::Short(wk.LiteralInteger, field_idx)] + .into_iter() + .collect(), + }), + output_type: data_inst_form_def.output_type, }), ..data_inst_def.clone() } @@ -677,18 +691,24 @@ impl LiftToSpvPtrInstsInFunc<'_> { }, ); DataInstDef { - // FIXME(eddyb) avoid the repeated call to `type_of_val`, + // FIXME(eddyb) avoid the repeated call to `type_of_val` + // (and the interning of a temporary `DataInstFormDef`), // maybe don't even replace the `QPtrOp::Offset` instruction? - output_type: Some(type_of_val(base_ptr)), + form: cx.intern(DataInstFormDef { + kind: QPtrOp::Offset(0).into(), + output_type: Some(type_of_val(base_ptr)), + }), ..data_inst_def.clone() } } else { DataInstDef { attrs: data_inst_def.attrs, - kind: DataInstKind::SpvInst(wk.OpAccessChain.into()), - output_type: Some( - self.lifter.spv_ptr_type(addr_space, layout.original_type), - ), + form: cx.intern(DataInstFormDef { + kind: DataInstKind::SpvInst(wk.OpAccessChain.into()), + output_type: Some( + self.lifter.spv_ptr_type(addr_space, layout.original_type), + ), + }), inputs: access_chain_inputs, } } @@ -775,14 +795,18 @@ impl LiftToSpvPtrInstsInFunc<'_> { } DataInstDef { attrs: data_inst_def.attrs, - kind: DataInstKind::SpvInst(wk.OpAccessChain.into()), - output_type: Some(self.lifter.spv_ptr_type(addr_space, layout.original_type)), + form: cx.intern(DataInstFormDef { + kind: DataInstKind::SpvInst(wk.OpAccessChain.into()), + output_type: Some( + self.lifter.spv_ptr_type(addr_space, layout.original_type), + ), + }), inputs: access_chain_inputs, } } DataInstKind::QPtr(op @ (QPtrOp::Load | QPtrOp::Store)) => { let (spv_opcode, access_type) = match op { - QPtrOp::Load => (wk.OpLoad, data_inst_def.output_type.unwrap()), + QPtrOp::Load => (wk.OpLoad, data_inst_form_def.output_type.unwrap()), QPtrOp::Store => (wk.OpStore, type_of_val(data_inst_def.inputs[1])), _ => unreachable!(), }; @@ -803,7 +827,10 @@ impl LiftToSpvPtrInstsInFunc<'_> { }; let mut new_data_inst_def = DataInstDef { - kind: DataInstKind::SpvInst(spv_opcode.into()), + form: cx.intern(DataInstFormDef { + kind: DataInstKind::SpvInst(spv_opcode.into()), + output_type: data_inst_form_def.output_type, + }), ..data_inst_def.clone() }; @@ -916,8 +943,10 @@ impl LiftToSpvPtrInstsInFunc<'_> { } if let Some((addr_space, pointee_type)) = from_spv_ptr_output { - new_data_inst_def.output_type = - Some(self.lifter.spv_ptr_type(addr_space, pointee_type)); + new_data_inst_def.form = cx.intern(DataInstFormDef { + output_type: Some(self.lifter.spv_ptr_type(addr_space, pointee_type)), + ..cx[new_data_inst_def.form].clone() + }); } new_data_inst_def @@ -1039,8 +1068,10 @@ impl LiftToSpvPtrInstsInFunc<'_> { Ok(if access_chain_inputs.len() > 1 { Some(DataInstDef { attrs: Default::default(), - kind: DataInstKind::SpvInst(wk.OpAccessChain.into()), - output_type: Some(self.lifter.spv_ptr_type(addr_space, access_type)), + form: self.lifter.cx.intern(DataInstFormDef { + kind: DataInstKind::SpvInst(wk.OpAccessChain.into()), + output_type: Some(self.lifter.spv_ptr_type(addr_space, access_type)), + }), inputs: access_chain_inputs, }) } else { @@ -1137,11 +1168,12 @@ impl Transformer for LiftToSpvPtrInstsInFunc<'_> { let mut lifted = self.try_lift_data_inst_def(func_at_inst.reborrow(), control_node); if let Ok(Transformed::Unchanged) = lifted { let data_inst_def = func_at_inst.reborrow().def(); - if let DataInstKind::QPtr(_) = data_inst_def.kind { + let data_inst_form_def = &self.lifter.cx[data_inst_def.form]; + if let DataInstKind::QPtr(_) = data_inst_form_def.kind { lifted = Err(LiftError(Diag::bug([ "unimplemented qptr instruction".into() ]))); - } else if let Some(ty) = data_inst_def.output_type { + } else if let Some(ty) = data_inst_form_def.output_type { if matches!(self.lifter.cx[ty].ctor, TypeCtor::QPtr) { lifted = Err(LiftError(Diag::bug([ "unimplemented qptr-producing instruction".into(), diff --git a/src/qptr/lower.rs b/src/qptr/lower.rs index 3a10c3f..d80da72 100644 --- a/src/qptr/lower.rs +++ b/src/qptr/lower.rs @@ -8,8 +8,8 @@ use crate::qptr::{shapes, QPtrAttr, QPtrOp}; use crate::transform::{InnerInPlaceTransform, Transformed, Transformer}; use crate::{ spv, AddrSpace, AttrSet, AttrSetDef, Const, ConstCtor, ConstDef, Context, ControlNode, - ControlNodeKind, DataInst, DataInstDef, DataInstKind, Diag, FuncDecl, GlobalVarDecl, - OrdAssertEq, Type, TypeCtor, TypeCtorArg, TypeDef, Value, + ControlNodeKind, DataInst, DataInstDef, DataInstForm, DataInstFormDef, DataInstKind, Diag, + FuncDecl, GlobalVarDecl, OrdAssertEq, Type, TypeCtor, TypeCtorArg, TypeDef, Value, }; use smallvec::SmallVec; use std::cell::Cell; @@ -241,6 +241,19 @@ impl Transformer for EraseSpvPtrs<'_> { Transformed::Unchanged } } + + // FIXME(eddyb) because this is now interned, it might be better to + // temporarily track the old output types in a map, and not actually + // intern the non-`qptr`-output `qptr.*` instructions, only to replace + // the output type with `qptr` here. + fn transform_data_inst_form_use( + &mut self, + data_inst_form: DataInstForm, + ) -> Transformed { + // FIXME(eddyb) maybe cache this remap (in `LowerFromSpvPtrs`, globally). + self.transform_data_inst_form_def(&self.lowerer.cx[data_inst_form]) + .map(|data_inst_form_def| self.lowerer.cx.intern(data_inst_form_def)) + } } struct LowerFromSpvPtrInstsInFunc<'a> { @@ -414,17 +427,17 @@ impl LowerFromSpvPtrInstsInFunc<'_> { // FIXME(eddyb) is this a good convention? let func = func_at_data_inst_frozen.at(()); - let spv_inst = match &data_inst_def.kind { + let mut attrs = data_inst_def.attrs; + let DataInstFormDef { + ref kind, + output_type, + } = cx[data_inst_def.form]; + + let spv_inst = match kind { DataInstKind::SpvInst(spv_inst) => spv_inst, _ => return Ok(Transformed::Unchanged), }; - let DataInstDef { - mut attrs, - output_type, - .. - } = *data_inst_def; - let replacement_kind_and_inputs = if spv_inst.opcode == wk.OpVariable { assert!(data_inst_def.inputs.len() <= 1); let (_, var_data_type) = self @@ -573,8 +586,10 @@ impl LowerFromSpvPtrInstsInFunc<'_> { cx, DataInstDef { attrs: Default::default(), - kind, - output_type: Some(self.lowerer.qptr_type()), + form: cx.intern(DataInstFormDef { + kind, + output_type: Some(self.lowerer.qptr_type()), + }), inputs, } .into(), @@ -628,8 +643,13 @@ impl LowerFromSpvPtrInstsInFunc<'_> { let (new_kind, new_inputs) = replacement_kind_and_inputs; Ok(Transformed::Changed(DataInstDef { attrs, - kind: new_kind, - output_type, + // FIXME(eddyb) because this is now interned, it might be better to + // temporarily track the old output types in a map, and not actually + // intern the non-`qptr`-output `qptr.*` instructions. + form: cx.intern(DataInstFormDef { + kind: new_kind, + output_type, + }), inputs: new_inputs, })) } @@ -643,11 +663,12 @@ impl LowerFromSpvPtrInstsInFunc<'_> { let func_at_data_inst_frozen = func_at_data_inst.reborrow().freeze(); let data_inst_def = func_at_data_inst_frozen.def(); + let data_inst_form_def = &cx[data_inst_def.form]; // FIXME(eddyb) is this a good convention? let func = func_at_data_inst_frozen.at(()); - match data_inst_def.kind { + match data_inst_form_def.kind { // Known semantics, no need to preserve SPIR-V pointer information. DataInstKind::FuncCall(_) | DataInstKind::QPtr(_) => return, @@ -673,7 +694,7 @@ impl LowerFromSpvPtrInstsInFunc<'_> { ); } } - if let Some(output_type) = data_inst_def.output_type { + if let Some(output_type) = data_inst_form_def.output_type { if let Some((addr_space, pointee)) = self.lowerer.as_spv_ptr_type(output_type) { old_and_new_attrs .get_or_insert_with(get_old_attrs) diff --git a/src/qptr/mod.rs b/src/qptr/mod.rs index 25ffe93..b001c3c 100644 --- a/src/qptr/mod.rs +++ b/src/qptr/mod.rs @@ -149,7 +149,7 @@ pub enum QPtrMemUsageKind { } /// `QPtr`-specific operations ([`DataInstKind::QPtr`]). -#[derive(Clone, PartialEq, Eq)] +#[derive(Clone, PartialEq, Eq, Hash)] pub enum QPtrOp { // HACK(eddyb) `OpVariable` replacement, which itself should not be kept as // a `SpvInst` - once fn-local variables are lowered, this should go there. diff --git a/src/qptr/shapes.rs b/src/qptr/shapes.rs index 7c6b965..ac53d43 100644 --- a/src/qptr/shapes.rs +++ b/src/qptr/shapes.rs @@ -88,7 +88,7 @@ pub enum Handle { /// Only `align` is *required*, that is `size % align == 0` must be always enforced. // // FIXME(eddyb) consider supporting specialization-constant-length arrays. -#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] pub struct MemLayout { // FIXME(eddyb) use proper newtypes (and log2 for align!). pub align: u32, diff --git a/src/spv/lift.rs b/src/spv/lift.rs index f3d08df..effe720 100644 --- a/src/spv/lift.rs +++ b/src/spv/lift.rs @@ -6,9 +6,10 @@ use crate::visit::{InnerVisit, Visitor}; use crate::{ cfg, AddrSpace, Attr, AttrSet, Const, ConstCtor, ConstDef, Context, ControlNode, ControlNodeKind, ControlNodeOutputDecl, ControlRegion, ControlRegionInputDecl, DataInst, - DataInstDef, DataInstKind, DeclDef, EntityList, ExportKey, Exportee, Func, FuncDecl, FuncParam, - FxIndexMap, FxIndexSet, GlobalVar, GlobalVarDefBody, Import, Module, ModuleDebugInfo, - ModuleDialect, SelectionKind, Type, TypeCtor, TypeCtorArg, TypeDef, Value, + DataInstDef, DataInstForm, DataInstFormDef, DataInstKind, DeclDef, EntityList, ExportKey, + Exportee, Func, FuncDecl, FuncParam, FxIndexMap, FxIndexSet, GlobalVar, GlobalVarDefBody, + Import, Module, ModuleDebugInfo, ModuleDialect, SelectionKind, Type, TypeCtor, TypeCtorArg, + TypeDef, Value, }; use rustc_hash::FxHashMap; use smallvec::SmallVec; @@ -101,6 +102,7 @@ struct NeedsIdsCollector<'a> { debug_strings: BTreeSet<&'a str>, globals: FxIndexSet, + data_inst_forms_seen: FxIndexSet, global_vars_seen: FxIndexSet, funcs: FxIndexSet, } @@ -177,6 +179,11 @@ impl Visitor<'_> for NeedsIdsCollector<'_> { } } } + fn visit_data_inst_form_use(&mut self, data_inst_form: DataInstForm) { + if self.data_inst_forms_seen.insert(data_inst_form) { + self.visit_data_inst_form_def(&self.cx[data_inst_form]); + } + } fn visit_global_var_use(&mut self, gv: GlobalVar) { if self.global_vars_seen.insert(gv) { @@ -218,9 +225,9 @@ impl Visitor<'_> for NeedsIdsCollector<'_> { attr.inner_visit_with(self); } - fn visit_data_inst_def(&mut self, data_inst_def: &DataInstDef) { + fn visit_data_inst_form_def(&mut self, data_inst_form_def: &DataInstFormDef) { #[allow(clippy::match_same_arms)] - match data_inst_def.kind { + match data_inst_form_def.kind { // FIXME(eddyb) this should be a proper `Result`-based error instead, // and/or `spv::lift` should mutate the module for legalization. DataInstKind::QPtr(_) => { @@ -234,7 +241,7 @@ impl Visitor<'_> for NeedsIdsCollector<'_> { self.ext_inst_imports.insert(&self.cx[ext_set]); } } - data_inst_def.inner_visit_with(self); + data_inst_form_def.inner_visit_with(self); } } @@ -363,6 +370,7 @@ impl<'a> NeedsIdsCollector<'a> { ext_inst_imports, debug_strings, globals, + data_inst_forms_seen: _, global_vars_seen: _, funcs, } = self; @@ -1034,7 +1042,7 @@ impl<'a> FuncLifting<'a> { .values() .flat_map(|block| block.insts.iter().copied()) .flat_map(|insts| func_def_body.at(insts)) - .filter(|&func_at_inst| func_at_inst.def().output_type.is_some()) + .filter(|&func_at_inst| cx[func_at_inst.def().form].output_type.is_some()) .map(|func_at_inst| func_at_inst.position); Ok(Self { @@ -1326,7 +1334,8 @@ impl LazyInst<'_, '_> { result_id: _, data_inst_def, } => { - let (inst, extra_initial_id_operand) = match &data_inst_def.kind { + let DataInstFormDef { kind, output_type } = &cx[data_inst_def.form]; + let (inst, extra_initial_id_operand) = match kind { // Disallowed while visiting. DataInstKind::QPtr(_) => unreachable!(), @@ -1345,9 +1354,7 @@ impl LazyInst<'_, '_> { }; spv::InstWithIds { without_ids: inst, - result_type_id: data_inst_def - .output_type - .map(|ty| ids.globals[&Global::Type(ty)]), + result_type_id: output_type.map(|ty| ids.globals[&Global::Type(ty)]), result_id, ids: extra_initial_id_operand .into_iter() @@ -1470,6 +1477,7 @@ impl Module { ext_inst_imports: BTreeSet::new(), debug_strings: BTreeSet::new(), globals: FxIndexSet::default(), + data_inst_forms_seen: FxIndexSet::default(), global_vars_seen: FxIndexSet::default(), funcs: FxIndexSet::default(), }; @@ -1559,7 +1567,7 @@ impl Module { let data_inst_def = func_at_inst.def(); LazyInst::DataInst { parent_func: func_lifting, - result_id: data_inst_def.output_type.map(|_| { + result_id: cx[data_inst_def.form].output_type.map(|_| { func_lifting.data_inst_output_ids [&func_at_inst.position] }), diff --git a/src/spv/lower.rs b/src/spv/lower.rs index 104b9be..939d066 100644 --- a/src/spv/lower.rs +++ b/src/spv/lower.rs @@ -5,9 +5,9 @@ use crate::spv::{self, spec}; use crate::{ cfg, print, AddrSpace, Attr, AttrSet, Const, ConstCtor, ConstDef, Context, ControlNodeDef, ControlNodeKind, ControlRegion, ControlRegionDef, ControlRegionInputDecl, DataInstDef, - DataInstKind, DeclDef, EntityDefs, EntityList, ExportKey, Exportee, Func, FuncDecl, - FuncDefBody, FuncParam, FxIndexMap, GlobalVarDecl, GlobalVarDefBody, Import, InternedStr, - Module, SelectionKind, Type, TypeCtor, TypeCtorArg, TypeDef, Value, + DataInstFormDef, DataInstKind, DeclDef, EntityDefs, EntityList, ExportKey, Exportee, Func, + FuncDecl, FuncDefBody, FuncParam, FxIndexMap, GlobalVarDecl, GlobalVarDefBody, Import, + InternedStr, Module, SelectionKind, Type, TypeCtor, TypeCtorArg, TypeDef, Value, }; use rustc_hash::FxHashMap; use smallvec::SmallVec; @@ -866,6 +866,18 @@ impl Module { return Err(invalid("OpFunction without matching OpFunctionEnd")); } + // HACK(eddyb) `OpNop` is useful for defining `DataInst`s before they're + // actually lowered (to be able to refer to their outputs `Value`s). + let mut cached_op_nop_form = None; + let mut get_op_nop_form = || { + *cached_op_nop_form.get_or_insert_with(|| { + cx.intern(DataInstFormDef { + kind: DataInstKind::SpvInst(wk.OpNop.into()), + output_type: None, + }) + }) + }; + // Process function bodies, having seen the whole module. for func_body in pending_func_bodies { let FuncBody { @@ -995,8 +1007,8 @@ impl Module { &cx, DataInstDef { attrs: AttrSet::default(), - kind: DataInstKind::SpvInst(wk.OpNop.into()), - output_type: None, + // FIXME(eddyb) cache this form locally. + form: get_op_nop_form(), inputs: [].into_iter().collect(), } .into(), @@ -1375,16 +1387,19 @@ impl Module { let data_inst_def = DataInstDef { attrs, - kind, - output_type: result_id - .map(|_| { - result_type.ok_or_else(|| { - invalid( - "expected value-producing instruction, with a result type", - ) + form: cx.intern(DataInstFormDef { + kind, + output_type: result_id + .map(|_| { + result_type.ok_or_else(|| { + invalid( + "expected value-producing instruction, \ + with a result type", + ) + }) }) - }) - .transpose()?, + .transpose()?, + }), inputs: ids .iter() .map(|&id| { diff --git a/src/spv/mod.rs b/src/spv/mod.rs index d5e87e3..4bb92ef 100644 --- a/src/spv/mod.rs +++ b/src/spv/mod.rs @@ -58,6 +58,8 @@ pub struct Inst { // FIXME(eddyb) change the inline size of this to fit most instructions. // FIXME(eddyb) it might be worth investigating the performance implications // of interning "long immediates", compared to the flattened representation. + // NOTE(eddyb) interning these separately is likely unnecessary in many cases, + // now that `DataInstForm`s are interned, and `Const`s etc. were already. pub imms: SmallVec<[Imm; 2]>, } diff --git a/src/transform.rs b/src/transform.rs index 6f6a0dc..6f8f9b2 100644 --- a/src/transform.rs +++ b/src/transform.rs @@ -5,10 +5,10 @@ use crate::qptr::{self, QPtrAttr, QPtrMemUsage, QPtrMemUsageKind, QPtrOp, QPtrUs use crate::{ cfg, spv, AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstCtor, ConstDef, ControlNode, ControlNodeDef, ControlNodeKind, ControlNodeOutputDecl, ControlRegion, ControlRegionDef, - ControlRegionInputDecl, DataInst, DataInstDef, DataInstKind, DeclDef, EntityListIter, - ExportKey, Exportee, Func, FuncDecl, FuncDefBody, FuncParam, GlobalVar, GlobalVarDecl, - GlobalVarDefBody, Import, Module, ModuleDebugInfo, ModuleDialect, OrdAssertEq, SelectionKind, - Type, TypeCtor, TypeCtorArg, TypeDef, Value, + ControlRegionInputDecl, DataInst, DataInstDef, DataInstForm, DataInstFormDef, DataInstKind, + DeclDef, EntityListIter, ExportKey, Exportee, Func, FuncDecl, FuncDefBody, FuncParam, + GlobalVar, GlobalVarDecl, GlobalVarDefBody, Import, Module, ModuleDebugInfo, ModuleDialect, + OrdAssertEq, SelectionKind, Type, TypeCtor, TypeCtorArg, TypeDef, Value, }; use std::cmp::Ordering; use std::rc::Rc; @@ -145,6 +145,12 @@ pub trait Transformer: Sized { fn transform_const_use(&mut self, _ct: Const) -> Transformed { Transformed::Unchanged } + fn transform_data_inst_form_use( + &mut self, + _data_inst_form: DataInstForm, + ) -> Transformed { + Transformed::Unchanged + } // Module-stored entity leaves (noop default behavior). fn transform_global_var_use(&mut self, _gv: GlobalVar) -> Transformed { @@ -172,6 +178,12 @@ pub trait Transformer: Sized { fn transform_const_def(&mut self, ct_def: &ConstDef) -> Transformed { ct_def.inner_transform_with(self) } + fn transform_data_inst_form_def( + &mut self, + data_inst_form_def: &DataInstFormDef, + ) -> Transformed { + data_inst_form_def.inner_transform_with(self) + } fn transform_value_use(&mut self, v: &Value) -> Transformed { v.inner_transform_with(self) } @@ -740,35 +752,50 @@ impl InnerInPlaceTransform for FuncAtMut<'_, DataInst> { fn inner_in_place_transform_with(&mut self, transformer: &mut impl Transformer) { let DataInstDef { attrs, - kind, - output_type, + form, inputs, } = self.reborrow().def(); transformer.transform_attr_set_use(*attrs).apply_to(attrs); - match kind { - DataInstKind::FuncCall(func) => transformer.transform_func_use(*func).apply_to(func), - DataInstKind::QPtr(op) => match op { - QPtrOp::FuncLocalVar(_) - | QPtrOp::HandleArrayIndex - | QPtrOp::BufferData - | QPtrOp::BufferDynLen { .. } - | QPtrOp::Offset(_) - | QPtrOp::DynOffset { .. } - | QPtrOp::Load - | QPtrOp::Store => {} - }, - DataInstKind::SpvInst(_) | DataInstKind::SpvExtInst { .. } => {} - } - if let Some(ty) = output_type { - transformer.transform_type_use(*ty).apply_to(ty); - } + transformer + .transform_data_inst_form_use(*form) + .apply_to(form); for v in inputs { transformer.transform_value_use(v).apply_to(v); } } } +impl InnerTransform for DataInstFormDef { + fn inner_transform_with(&self, transformer: &mut impl Transformer) -> Transformed { + let Self { kind, output_type } = self; + + transform!({ + kind -> match kind { + DataInstKind::FuncCall(func) => transformer.transform_func_use(*func).map(DataInstKind::FuncCall), + DataInstKind::QPtr(op) => match op { + QPtrOp::FuncLocalVar(_) + | QPtrOp::HandleArrayIndex + | QPtrOp::BufferData + | QPtrOp::BufferDynLen { .. } + | QPtrOp::Offset(_) + | QPtrOp::DynOffset { .. } + | QPtrOp::Load + | QPtrOp::Store => Transformed::Unchanged, + }, + DataInstKind::SpvInst(_) | DataInstKind::SpvExtInst { .. } => Transformed::Unchanged, + }, + // FIXME(eddyb) this should be replaced with an impl of `InnerTransform` + // for `Option` or some other helper, to avoid "manual transpose". + output_type -> output_type.map(|ty| transformer.transform_type_use(ty)) + .map_or(Transformed::Unchanged, |t| t.map(Some)), + } => Self { + kind, + output_type, + }) + } +} + impl InnerInPlaceTransform for cfg::ControlInst { fn inner_in_place_transform_with(&mut self, transformer: &mut impl Transformer) { let Self { diff --git a/src/visit.rs b/src/visit.rs index 0d2d230..67b3584 100644 --- a/src/visit.rs +++ b/src/visit.rs @@ -5,10 +5,10 @@ use crate::qptr::{self, QPtrAttr, QPtrMemUsage, QPtrMemUsageKind, QPtrOp, QPtrUs use crate::{ cfg, spv, AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstCtor, ConstDef, ControlNode, ControlNodeDef, ControlNodeKind, ControlNodeOutputDecl, ControlRegion, ControlRegionDef, - ControlRegionInputDecl, DataInstDef, DataInstKind, DeclDef, DiagMsgPart, EntityListIter, - ExportKey, Exportee, Func, FuncDecl, FuncDefBody, FuncParam, GlobalVar, GlobalVarDecl, - GlobalVarDefBody, Import, Module, ModuleDebugInfo, ModuleDialect, SelectionKind, Type, - TypeCtor, TypeCtorArg, TypeDef, Value, + ControlRegionInputDecl, DataInstDef, DataInstForm, DataInstFormDef, DataInstKind, DeclDef, + DiagMsgPart, EntityListIter, ExportKey, Exportee, Func, FuncDecl, FuncDefBody, FuncParam, + GlobalVar, GlobalVarDecl, GlobalVarDefBody, Import, Module, ModuleDebugInfo, ModuleDialect, + SelectionKind, Type, TypeCtor, TypeCtorArg, TypeDef, Value, }; // FIXME(eddyb) `Sized` bound shouldn't be needed but removing it requires @@ -20,6 +20,7 @@ pub trait Visitor<'a>: Sized { fn visit_attr_set_use(&mut self, attrs: AttrSet); fn visit_type_use(&mut self, ty: Type); fn visit_const_use(&mut self, ct: Const); + fn visit_data_inst_form_use(&mut self, data_inst_form: DataInstForm); // Module-stored entity leaves (no default provided). fn visit_global_var_use(&mut self, gv: GlobalVar); @@ -67,6 +68,9 @@ pub trait Visitor<'a>: Sized { fn visit_data_inst_def(&mut self, data_inst_def: &'a DataInstDef) { data_inst_def.inner_visit_with(self); } + fn visit_data_inst_form_def(&mut self, data_inst_form_def: &'a DataInstFormDef) { + data_inst_form_def.inner_visit_with(self); + } fn visit_value_use(&mut self, v: &'a Value) { v.inner_visit_with(self); } @@ -556,12 +560,22 @@ impl InnerVisit for DataInstDef { fn inner_visit_with<'a>(&'a self, visitor: &mut impl Visitor<'a>) { let Self { attrs, - kind, - output_type, + form, inputs, } = self; visitor.visit_attr_set_use(*attrs); + visitor.visit_data_inst_form_use(*form); + for v in inputs { + visitor.visit_value_use(v); + } + } +} + +impl InnerVisit for DataInstFormDef { + fn inner_visit_with<'a>(&'a self, visitor: &mut impl Visitor<'a>) { + let Self { kind, output_type } = self; + match kind { &DataInstKind::FuncCall(func) => visitor.visit_func_use(func), DataInstKind::QPtr(op) => match *op { @@ -579,9 +593,6 @@ impl InnerVisit for DataInstDef { if let Some(ty) = *output_type { visitor.visit_type_use(ty); } - for v in inputs { - visitor.visit_value_use(v); - } } }