From 7647a15a6832e1673925470858953ef97198e6df Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Thu, 26 Oct 2023 22:24:15 +0300 Subject: [PATCH 1/4] Fix new `clippy::get_first` warnings. --- src/print/mod.rs | 4 ++-- src/spv/read.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/print/mod.rs b/src/print/mod.rs index a7b821c..d0dad47 100644 --- a/src/print/mod.rs +++ b/src/print/mod.rs @@ -728,7 +728,7 @@ impl<'a> Printer<'a> { .collect::>>() .filter(|all_names| all_names.iter().map(|(_, spv_name)| spv_name).all_equal()) .and_then(|all_names| { - let &(_, spv_name) = all_names.get(0)?; + let &(_, spv_name) = all_names.first()?; let name = spv::extract_literal_string(&spv_name.imms).ok()?; // This is the point of no return: these `insert`s will @@ -3118,7 +3118,7 @@ impl Print for FuncAt<'_, DataInst> { ]), ] .into_iter() - .chain(extra_inputs.get(0).map(|&init| { + .chain(extra_inputs.first().map(|&init| { pretty::Fragment::new([ printer.pretty_named_argument_prefix("initializer"), init.print(printer), diff --git a/src/spv/read.rs b/src/spv/read.rs index da8769e..5817805 100644 --- a/src/spv/read.rs +++ b/src/spv/read.rs @@ -168,7 +168,7 @@ impl InstParser<'_> { .result_type_id .or_else(|| { // `OpSwitch` takes its literal type from the first operand. - let &id = self.inst.ids.get(0)?; + let &id = self.inst.ids.first()?; self.known_ids.get(&id)?.result_type_id() }) .and_then(|id| self.known_ids.get(&id)) From 1ec65110ffd1255768896eec1b770cbfee95a68a Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Sun, 15 Oct 2023 13:35:40 +0300 Subject: [PATCH 2/4] Combine `ConstCtor` and "ctor args" into a single `ConstKind`. --- src/cfg.rs | 26 +++++++++---- src/lib.rs | 15 +++++--- src/print/mod.rs | 94 +++++++++++++++++++++++++++------------------ src/qptr/analyze.rs | 6 +-- src/qptr/layout.rs | 13 +++---- src/qptr/lift.rs | 23 ++++++----- src/qptr/lower.rs | 18 ++++----- src/spv/lift.rs | 70 +++++++++++++++++---------------- src/spv/lower.rs | 23 ++++++----- src/transform.rs | 31 ++++++++------- src/visit.rs | 19 +++++---- 11 files changed, 190 insertions(+), 148 deletions(-) diff --git a/src/cfg.rs b/src/cfg.rs index 3dc70ae..9746280 100644 --- a/src/cfg.rs +++ b/src/cfg.rs @@ -1,13 +1,15 @@ //! Control-flow graph (CFG) abstractions and utilities. use crate::{ - spv, AttrSet, Const, ConstCtor, ConstDef, Context, ControlNode, ControlNodeDef, + spv, AttrSet, Const, ConstDef, ConstKind, Context, ControlNode, ControlNodeDef, ControlNodeKind, ControlNodeOutputDecl, ControlRegion, ControlRegionDef, EntityList, EntityOrientedDenseMap, FuncDefBody, FxIndexMap, FxIndexSet, SelectionKind, Type, TypeCtor, TypeDef, Value, }; +use itertools::Either; use smallvec::SmallVec; use std::mem; +use std::rc::Rc; /// The control-flow graph (CFG) of a function, as control-flow instructions /// ([`ControlInst`]s) attached to [`ControlRegion`]s, as an "action on exit", i.e. @@ -128,7 +130,6 @@ mod sealed { } } } -use itertools::Either; use sealed::IncomingEdgeCount; struct TraversalState { @@ -552,14 +553,22 @@ impl<'a> Structurizer<'a> { let const_true = cx.intern(ConstDef { attrs: AttrSet::default(), ty: type_bool, - ctor: ConstCtor::SpvInst(wk.OpConstantTrue.into()), - ctor_args: [].into_iter().collect(), + kind: ConstKind::SpvInst { + spv_inst_and_const_inputs: Rc::new(( + wk.OpConstantTrue.into(), + [].into_iter().collect(), + )), + }, }); let const_false = cx.intern(ConstDef { attrs: AttrSet::default(), ty: type_bool, - ctor: ConstCtor::SpvInst(wk.OpConstantFalse.into()), - ctor_args: [].into_iter().collect(), + kind: ConstKind::SpvInst { + spv_inst_and_const_inputs: Rc::new(( + wk.OpConstantFalse.into(), + [].into_iter().collect(), + )), + }, }); let (loop_header_to_exit_targets, incoming_edge_counts_including_loop_exits) = @@ -1435,8 +1444,9 @@ impl<'a> Structurizer<'a> { self.cx.intern(ConstDef { attrs: AttrSet::default(), ty, - ctor: ConstCtor::SpvInst(wk.OpUndef.into()), - ctor_args: [].into_iter().collect(), + kind: ConstKind::SpvInst { + spv_inst_and_const_inputs: Rc::new((wk.OpUndef.into(), [].into_iter().collect())), + }, }) } } diff --git a/src/lib.rs b/src/lib.rs index 4da6b12..c87fd03 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -173,6 +173,7 @@ pub mod spv; use smallvec::SmallVec; use std::borrow::Cow; use std::collections::BTreeSet; +use std::rc::Rc; // HACK(eddyb) work around the lack of `FxIndex{Map,Set}` type aliases elsewhere. #[doc(hidden)] @@ -482,7 +483,7 @@ pub enum TypeCtor { SpvInst(spv::Inst), - /// The type of a [`ConstCtor::SpvStringLiteralForExtInst`] constant, i.e. + /// The type of a [`ConstKind::SpvStringLiteralForExtInst`] constant, i.e. /// a SPIR-V `OpString` with no actual type in SPIR-V. SpvStringLiteralForExtInst, } @@ -503,16 +504,18 @@ pub use context::Const; pub struct ConstDef { pub attrs: AttrSet, pub ty: Type, - pub ctor: ConstCtor, - pub ctor_args: SmallVec<[Const; 2]>, + pub kind: ConstKind, } -/// [`Const`] "constructor": a [`ConstDef`] wiithout any nested [`Const`]s. #[derive(Clone, PartialEq, Eq, Hash)] -pub enum ConstCtor { +pub enum ConstKind { PtrToGlobalVar(GlobalVar), - SpvInst(spv::Inst), + // HACK(eddyb) this is a fallback case that should become increasingly rare + // (especially wrt recursive consts), `Rc` means it can't bloat `ConstDef`. + SpvInst { + spv_inst_and_const_inputs: Rc<(spv::Inst, SmallVec<[Const; 4]>)>, + }, /// SPIR-V `OpString`, but only when used as an operand for an `OpExtInst`, /// which can't have literals itself - for non-string literals `OpConstant*` diff --git a/src/print/mod.rs b/src/print/mod.rs index d0dad47..d9a0878 100644 --- a/src/print/mod.rs +++ b/src/print/mod.rs @@ -24,7 +24,7 @@ use crate::print::multiversion::Versions; use crate::qptr::{self, QPtrAttr, QPtrMemUsage, QPtrMemUsageKind, QPtrOp, QPtrUsage}; use crate::visit::{InnerVisit, Visit, Visitor}; use crate::{ - cfg, spv, AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstCtor, ConstDef, Context, + cfg, spv, AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstDef, ConstKind, Context, ControlNode, ControlNodeDef, ControlNodeKind, ControlNodeOutputDecl, ControlRegion, ControlRegionDef, ControlRegionInputDecl, DataInst, DataInstDef, DataInstForm, DataInstFormDef, DataInstKind, DeclDef, Diag, DiagLevel, DiagMsgPart, EntityListIter, ExportKey, Exportee, Func, @@ -525,7 +525,7 @@ impl<'a> Visitor<'a> for Plan<'a> { fn visit_const_def(&mut self, ct_def: &'a ConstDef) { // HACK(eddyb) the type of a `PtrToGlobalVar` is never printed, skip it. - if let ConstCtor::PtrToGlobalVar(gv) = ct_def.ctor { + if let ConstKind::PtrToGlobalVar(gv) = ct_def.kind { self.visit_attr_set_use(ct_def.attrs); self.visit_global_var_use(gv); } else { @@ -836,16 +836,26 @@ impl<'a> Printer<'a> { // FIXME(eddyb) remove the duplication between // here and `ConstDef`'s `Print` impl. - let has_compact_print = match &ct_def.ctor { - ConstCtor::SpvInst(inst) => { - [wk.OpConstantFalse, wk.OpConstantTrue, wk.OpConstant] - .contains(&inst.opcode) + let (has_compact_print, has_nested_consts) = match &ct_def.kind + { + ConstKind::SpvInst { spv_inst_and_const_inputs } => { + let (spv_inst, const_inputs) = + &**spv_inst_and_const_inputs; + ( + [ + wk.OpConstantFalse, + wk.OpConstantTrue, + wk.OpConstant, + ] + .contains(&spv_inst.opcode), + !const_inputs.is_empty(), + ) } - _ => false, + _ => (false, false), }; ct_def.attrs == AttrSet::default() - && (has_compact_print || ct_def.ctor_args.is_empty()) + && (has_compact_print || !has_nested_consts) } } } @@ -2439,7 +2449,7 @@ impl Print for TypeDef { impl Print for ConstDef { type Output = AttrsAndDef; fn print(&self, printer: &Printer<'_>) -> AttrsAndDef { - let Self { attrs, ty, ctor, ctor_args } = self; + let Self { attrs, ty, kind } = self; let wk = &spv::spec::Spec::get().well_known; @@ -2453,7 +2463,10 @@ impl Print for ConstDef { } .apply(ty) }; - let compact_def = if let &ConstCtor::SpvInst(spv::Inst { opcode, ref imms }) = ctor { + let compact_def = if let ConstKind::SpvInst { spv_inst_and_const_inputs } = kind { + let (spv_inst, _const_inputs) = &**spv_inst_and_const_inputs; + let &spv::Inst { opcode, ref imms } = spv_inst; + if opcode == wk.OpConstantFalse { Some(kw("false")) } else if opcode == wk.OpConstantTrue { @@ -2556,20 +2569,23 @@ impl Print for ConstDef { AttrsAndDef { attrs: attrs.print(printer), - def_without_name: compact_def.unwrap_or_else(|| match *ctor { - ConstCtor::PtrToGlobalVar(gv) => { + def_without_name: compact_def.unwrap_or_else(|| match kind { + &ConstKind::PtrToGlobalVar(gv) => { pretty::Fragment::new(["&".into(), gv.print(printer)]) } - ConstCtor::SpvInst(spv::Inst { opcode, ref imms }) => pretty::Fragment::new([ - printer.pretty_spv_inst( - printer.spv_op_style(), - opcode, - imms, - ctor_args.iter().map(|arg| arg.print(printer)), - ), - printer.pretty_type_ascription_suffix(*ty), - ]), - ConstCtor::SpvStringLiteralForExtInst(s) => pretty::Fragment::new([ + ConstKind::SpvInst { spv_inst_and_const_inputs } => { + let (spv_inst, const_inputs) = &**spv_inst_and_const_inputs; + pretty::Fragment::new([ + printer.pretty_spv_inst( + printer.spv_op_style(), + spv_inst.opcode, + &spv_inst.imms, + const_inputs.iter().map(|ct| ct.print(printer)), + ), + printer.pretty_type_ascription_suffix(*ty), + ]) + } + &ConstKind::SpvStringLiteralForExtInst(s) => pretty::Fragment::new([ printer.pretty_spv_opcode(printer.spv_op_style(), wk.OpString), "(".into(), printer.pretty_string_literal(&printer.cx[s]), @@ -3273,25 +3289,27 @@ impl Print for FuncAt<'_, DataInst> { Str(&'a str), U32(u32), } - let pseudo_imm_from_value = |v: Value| match v { - Value::Const(ct) => match &printer.cx[ct].ctor { - &ConstCtor::SpvStringLiteralForExtInst(s) => { - Some(PseudoImm::Str(&printer.cx[s])) - } - ConstCtor::SpvInst(spv_inst) if spv_inst.opcode == wk.OpConstant => { - match spv_inst.imms[..] { - // HACK(eddyb) only allow unambiguously positive values. - [spv::Imm::Short(_, x)] - if i32::try_from(x).and_then(u32::try_from) == Ok(x) => - { - Some(PseudoImm::U32(x)) + let pseudo_imm_from_value = |v: Value| { + if let Value::Const(ct) = v { + match &printer.cx[ct].kind { + &ConstKind::SpvStringLiteralForExtInst(s) => { + return Some(PseudoImm::Str(&printer.cx[s])); + } + ConstKind::SpvInst { spv_inst_and_const_inputs } => { + let (spv_inst, _const_inputs) = &**spv_inst_and_const_inputs; + if spv_inst.opcode == wk.OpConstant { + if let [spv::Imm::Short(_, x)] = spv_inst.imms[..] { + // HACK(eddyb) only allow unambiguously positive values. + if i32::try_from(x).and_then(u32::try_from) == Ok(x) { + return Some(PseudoImm::U32(x)); + } + } } - _ => None, } + ConstKind::PtrToGlobalVar(_) => {} } - _ => None, - }, - _ => None, + } + None }; let debuginfo_with_pseudo_imm_inputs: Option> = known_inst_desc diff --git a/src/qptr/analyze.rs b/src/qptr/analyze.rs index ecedab3..a9a4c7a 100644 --- a/src/qptr/analyze.rs +++ b/src/qptr/analyze.rs @@ -7,7 +7,7 @@ use super::{shapes, QPtrAttr, QPtrMemUsage, QPtrOp, QPtrUsage}; use crate::func_at::FuncAt; use crate::visit::{InnerVisit, Visitor}; use crate::{ - AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstCtor, Context, ControlNode, ControlNodeKind, + AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstKind, Context, ControlNode, ControlNodeKind, DataInst, DataInstForm, DataInstKind, DeclDef, Diag, EntityList, ExportKey, Exportee, Func, FxIndexMap, GlobalVar, Module, OrdAssertEq, Type, TypeCtor, Value, }; @@ -872,8 +872,8 @@ impl<'a> InferUsage<'a> { let mut generate_usage = |this: &mut Self, ptr: Value, new_usage| { let slot = match ptr { - Value::Const(ct) => match cx[ct].ctor { - ConstCtor::PtrToGlobalVar(gv) => { + Value::Const(ct) => match cx[ct].kind { + ConstKind::PtrToGlobalVar(gv) => { this.global_var_usages.entry(gv).or_default() } // FIXME(eddyb) may be relevant? diff --git a/src/qptr/layout.rs b/src/qptr/layout.rs index cdec2cd..cfea017 100644 --- a/src/qptr/layout.rs +++ b/src/qptr/layout.rs @@ -2,7 +2,7 @@ use crate::qptr::shapes; use crate::{ - spv, AddrSpace, Attr, Const, ConstCtor, Context, Diag, FxIndexMap, Type, TypeCtor, TypeCtorArg, + spv, AddrSpace, Attr, Const, ConstKind, Context, Diag, FxIndexMap, Type, TypeCtor, TypeCtorArg, }; use itertools::Either; use smallvec::SmallVec; @@ -184,17 +184,16 @@ impl<'a> LayoutCache<'a> { // FIXME(eddyb) properly distinguish between zero-extension and sign-extension. fn const_as_u32(&self, ct: Const) -> Option { - match &self.cx[ct].ctor { - ConstCtor::SpvInst(spv_inst) - if spv_inst.opcode == self.wk.OpConstant && spv_inst.imms.len() == 1 => - { + if let ConstKind::SpvInst { spv_inst_and_const_inputs } = &self.cx[ct].kind { + let (spv_inst, _const_inputs) = &**spv_inst_and_const_inputs; + if spv_inst.opcode == self.wk.OpConstant && spv_inst.imms.len() == 1 { match spv_inst.imms[..] { - [spv::Imm::Short(_, x)] => Some(x), + [spv::Imm::Short(_, x)] => return Some(x), _ => unreachable!(), } } - _ => None, } + None } /// Attempt to compute a `TypeLayout` for a given (SPIR-V) `Type`. diff --git a/src/qptr/lift.rs b/src/qptr/lift.rs index 291401f..d30497c 100644 --- a/src/qptr/lift.rs +++ b/src/qptr/lift.rs @@ -7,7 +7,7 @@ use crate::func_at::FuncAtMut; use crate::qptr::{shapes, QPtrAttr, QPtrMemUsage, QPtrMemUsageKind, QPtrOp, QPtrUsage}; use crate::transform::{InnerInPlaceTransform, InnerTransform, Transformed, Transformer}; use crate::{ - spv, AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstCtor, ConstDef, Context, ControlNode, + spv, AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstDef, ConstKind, Context, ControlNode, ControlNodeKind, DataInst, DataInstDef, DataInstFormDef, DataInstKind, DeclDef, Diag, DiagLevel, EntityDefs, EntityOrientedDenseMap, Func, FuncDecl, FxIndexMap, GlobalVar, GlobalVarDecl, Module, Type, TypeCtor, TypeCtorArg, TypeDef, Value, @@ -356,11 +356,17 @@ impl<'a> LiftToSpvPtrs<'a> { self.cx.intern(ConstDef { attrs: AttrSet::default(), ty: self.u32_type(), - ctor: ConstCtor::SpvInst(spv::Inst { - opcode: wk.OpConstant, - imms: [spv::Imm::Short(wk.LiteralContextDependentNumber, x)].into_iter().collect(), - }), - ctor_args: [].into_iter().collect(), + kind: ConstKind::SpvInst { + spv_inst_and_const_inputs: Rc::new(( + spv::Inst { + opcode: wk.OpConstant, + imms: [spv::Imm::Short(wk.LiteralContextDependentNumber, x)] + .into_iter() + .collect(), + }, + [].into_iter().collect(), + )), + }, }) } @@ -1081,12 +1087,11 @@ impl Transformer for LiftToSpvPtrInstsInFunc<'_> { fn transform_const_use(&mut self, ct: Const) -> Transformed { // FIXME(eddyb) maybe cache this remap (in `LiftToSpvPtrs`, globally). let ct_def = &self.lifter.cx[ct]; - if let ConstCtor::PtrToGlobalVar(gv) = ct_def.ctor { + if let ConstKind::PtrToGlobalVar(gv) = ct_def.kind { Transformed::Changed(self.lifter.cx.intern(ConstDef { attrs: ct_def.attrs, ty: self.global_vars[gv].type_of_ptr_to, - ctor: ct_def.ctor.clone(), - ctor_args: ct_def.ctor_args.clone(), + kind: ct_def.kind.clone(), })) } else { Transformed::Unchanged diff --git a/src/qptr/lower.rs b/src/qptr/lower.rs index 7aeaee5..d28bd14 100644 --- a/src/qptr/lower.rs +++ b/src/qptr/lower.rs @@ -7,7 +7,7 @@ use crate::func_at::FuncAtMut; use crate::qptr::{shapes, QPtrAttr, QPtrOp}; use crate::transform::{InnerInPlaceTransform, Transformed, Transformer}; use crate::{ - spv, AddrSpace, AttrSet, AttrSetDef, Const, ConstCtor, ConstDef, Context, ControlNode, + spv, AddrSpace, AttrSet, AttrSetDef, Const, ConstDef, ConstKind, Context, ControlNode, ControlNodeKind, DataInst, DataInstDef, DataInstForm, DataInstFormDef, DataInstKind, Diag, FuncDecl, GlobalVarDecl, OrdAssertEq, Type, TypeCtor, TypeCtorArg, TypeDef, Value, }; @@ -172,17 +172,16 @@ impl<'a> LowerFromSpvPtrs<'a> { // FIXME(eddyb) properly distinguish between zero-extension and sign-extension. fn const_as_u32(&self, ct: Const) -> Option { - match &self.cx[ct].ctor { - ConstCtor::SpvInst(spv_inst) - if spv_inst.opcode == self.wk.OpConstant && spv_inst.imms.len() == 1 => - { + if let ConstKind::SpvInst { spv_inst_and_const_inputs } = &self.cx[ct].kind { + let (spv_inst, _const_inputs) = &**spv_inst_and_const_inputs; + if spv_inst.opcode == self.wk.OpConstant && spv_inst.imms.len() == 1 { match spv_inst.imms[..] { - [spv::Imm::Short(_, x)] => Some(x), + [spv::Imm::Short(_, x)] => return Some(x), _ => unreachable!(), } } - _ => None, } + None } /// Get the (likely cached) `QPtr` type. @@ -226,12 +225,11 @@ impl Transformer for EraseSpvPtrs<'_> { fn transform_const_use(&mut self, ct: Const) -> Transformed { // FIXME(eddyb) maybe cache this remap (in `LowerFromSpvPtrs`, globally). let ct_def = &self.lowerer.cx[ct]; - if let ConstCtor::PtrToGlobalVar(_) = ct_def.ctor { + if let ConstKind::PtrToGlobalVar(_) = ct_def.kind { Transformed::Changed(self.lowerer.cx.intern(ConstDef { attrs: ct_def.attrs, ty: self.lowerer.qptr_type(), - ctor: ct_def.ctor.clone(), - ctor_args: ct_def.ctor_args.clone(), + kind: ct_def.kind.clone(), })) } else { Transformed::Unchanged diff --git a/src/spv/lift.rs b/src/spv/lift.rs index 6d8264c..7072e4b 100644 --- a/src/spv/lift.rs +++ b/src/spv/lift.rs @@ -4,7 +4,7 @@ use crate::func_at::FuncAt; use crate::spv::{self, spec}; use crate::visit::{InnerVisit, Visitor}; use crate::{ - cfg, AddrSpace, Attr, AttrSet, Const, ConstCtor, ConstDef, Context, ControlNode, + cfg, AddrSpace, Attr, AttrSet, Const, ConstDef, ConstKind, Context, ControlNode, ControlNodeKind, ControlNodeOutputDecl, ControlRegion, ControlRegionInputDecl, DataInst, DataInstDef, DataInstForm, DataInstFormDef, DataInstKind, DeclDef, EntityList, ExportKey, Exportee, Func, FuncDecl, FuncParam, FxIndexMap, FxIndexSet, GlobalVar, GlobalVarDefBody, @@ -130,7 +130,7 @@ impl Visitor<'_> for NeedsIdsCollector<'_> { TypeCtor::SpvStringLiteralForExtInst => { unreachable!( "`TypeCtor::SpvStringLiteralForExtInst` should not be used \ - as a type outside of `ConstCtor::SpvStringLiteralForExtInst`" + as a type outside of `ConstKind::SpvStringLiteralForExtInst`" ); } } @@ -143,8 +143,8 @@ impl Visitor<'_> for NeedsIdsCollector<'_> { return; } let ct_def = &self.cx[ct]; - match ct_def.ctor { - ConstCtor::PtrToGlobalVar(_) | ConstCtor::SpvInst(_) => { + match ct_def.kind { + ConstKind::PtrToGlobalVar(_) | ConstKind::SpvInst { .. } => { self.visit_const_def(ct_def); self.globals.insert(global); } @@ -152,8 +152,8 @@ impl Visitor<'_> for NeedsIdsCollector<'_> { // HACK(eddyb) because this is an `OpString` and needs to go earlier // in the module than any `OpConstant*`, it needs to be special-cased, // without visiting its type, or an entry in `self.globals`. - ConstCtor::SpvStringLiteralForExtInst(s) => { - let ConstDef { attrs, ty, ctor: _, ctor_args } = ct_def; + ConstKind::SpvStringLiteralForExtInst(s) => { + let ConstDef { attrs, ty, kind: _ } = ct_def; assert!(*attrs == AttrSet::default()); assert!( @@ -164,7 +164,6 @@ impl Visitor<'_> for NeedsIdsCollector<'_> { ctor_args: SmallVec::new(), } ); - assert!(ctor_args.is_empty()); self.debug_strings.insert(&self.cx[s]); } @@ -758,9 +757,14 @@ impl<'a> FuncLifting<'a> { .collect(); let is_infinite_loop = match repeat_condition { - Value::Const(cond) => { - cx[cond].ctor == ConstCtor::SpvInst(wk.OpConstantTrue.into()) - } + Value::Const(cond) => match &cx[cond].kind { + ConstKind::SpvInst { spv_inst_and_const_inputs } => { + let (spv_inst, _const_inputs) = + &**spv_inst_and_const_inputs; + spv_inst.opcode == wk.OpConstantTrue + } + _ => false, + }, _ => false, }; @@ -1022,8 +1026,8 @@ impl LazyInst<'_, '_> { Global::Type(ty) => (cx[ty].attrs, None), Global::Const(ct) => { let ct_def = &cx[ct]; - match ct_def.ctor { - ConstCtor::PtrToGlobalVar(gv) => { + match ct_def.kind { + ConstKind::PtrToGlobalVar(gv) => { let gv_decl = &module.global_vars[gv]; let import = match gv_decl.def { DeclDef::Imported(import) => Some(import), @@ -1031,10 +1035,10 @@ impl LazyInst<'_, '_> { }; (gv_decl.attrs, import) } - ConstCtor::SpvInst { .. } => (ct_def.attrs, None), + ConstKind::SpvInst { .. } => (ct_def.attrs, None), // Not inserted into `globals` while visiting. - ConstCtor::SpvStringLiteralForExtInst(_) => unreachable!(), + ConstKind::SpvStringLiteralForExtInst(_) => unreachable!(), } } }; @@ -1068,8 +1072,8 @@ impl LazyInst<'_, '_> { let cx = module.cx_ref(); let value_to_id = |parent_func: &FuncLifting<'_>, v| match v { - Value::Const(ct) => match cx[ct].ctor { - ConstCtor::SpvStringLiteralForExtInst(s) => ids.debug_strings[&cx[s]], + Value::Const(ct) => match cx[ct].kind { + ConstKind::SpvStringLiteralForExtInst(s) => ids.debug_strings[&cx[s]], _ => ids.globals[&Global::Const(ct)], }, @@ -1122,10 +1126,9 @@ impl LazyInst<'_, '_> { } Global::Const(ct) => { let ct_def = &cx[ct]; - match &ct_def.ctor { - &ConstCtor::PtrToGlobalVar(gv) => { + match &ct_def.kind { + &ConstKind::PtrToGlobalVar(gv) => { assert!(ct_def.attrs == AttrSet::default()); - assert!(ct_def.ctor_args.is_empty()); let gv_decl = &module.global_vars[gv]; @@ -1157,19 +1160,21 @@ impl LazyInst<'_, '_> { } } - ConstCtor::SpvInst(inst) => spv::InstWithIds { - without_ids: inst.clone(), - result_type_id: Some(ids.globals[&Global::Type(ct_def.ty)]), - result_id, - ids: ct_def - .ctor_args - .iter() - .map(|&ct| ids.globals[&Global::Const(ct)]) - .collect(), - }, + ConstKind::SpvInst { spv_inst_and_const_inputs } => { + let (spv_inst, const_inputs) = &**spv_inst_and_const_inputs; + spv::InstWithIds { + without_ids: spv_inst.clone(), + result_type_id: Some(ids.globals[&Global::Type(ct_def.ty)]), + result_id, + ids: const_inputs + .iter() + .map(|&ct| ids.globals[&Global::Const(ct)]) + .collect(), + } + } // Not inserted into `globals` while visiting. - ConstCtor::SpvStringLiteralForExtInst(_) => unreachable!(), + ConstKind::SpvStringLiteralForExtInst(_) => unreachable!(), } } }, @@ -1359,15 +1364,14 @@ impl Module { needs_ids_collector.visit_module(self); // Because `GlobalVar`s are given IDs by the `Const`s that point to them - // (i.e. `ConstCtor::PtrToGlobalVar`), any `GlobalVar`s in other positions + // (i.e. `ConstKind::PtrToGlobalVar`), any `GlobalVar`s in other positions // require extra care to ensure the ID-giving `Const` is visited. let global_var_to_id_giving_global = |gv| { let type_of_ptr_to_global_var = self.global_vars[gv].type_of_ptr_to; let ptr_to_global_var = cx.intern(ConstDef { attrs: AttrSet::default(), ty: type_of_ptr_to_global_var, - ctor: ConstCtor::PtrToGlobalVar(gv), - ctor_args: [].into_iter().collect(), + kind: ConstKind::PtrToGlobalVar(gv), }); Global::Const(ptr_to_global_var) }; diff --git a/src/spv/lower.rs b/src/spv/lower.rs index b0a8bad..68411f2 100644 --- a/src/spv/lower.rs +++ b/src/spv/lower.rs @@ -3,7 +3,7 @@ use crate::spv::{self, spec}; // FIXME(eddyb) import more to avoid `crate::` everywhere. use crate::{ - cfg, print, AddrSpace, Attr, AttrSet, Const, ConstCtor, ConstDef, Context, ControlNodeDef, + cfg, print, AddrSpace, Attr, AttrSet, Const, ConstDef, ConstKind, Context, ControlNodeDef, ControlNodeKind, ControlRegion, ControlRegionDef, ControlRegionInputDecl, DataInstDef, DataInstFormDef, DataInstKind, DeclDef, Diag, EntityDefs, EntityList, ExportKey, Exportee, Func, FuncDecl, FuncDefBody, FuncParam, FxIndexMap, GlobalVarDecl, GlobalVarDefBody, Import, @@ -581,7 +581,7 @@ impl Module { Seq::TypeConstOrGlobalVar } else if inst_category == spec::InstructionCategory::Const || opcode == wk.OpUndef { let id = inst.result_id.unwrap(); - let const_ctor_args = inst + let const_inputs = inst .ids .iter() .map(|&id| match id_defs.get(&id) { @@ -599,8 +599,9 @@ impl Module { let ct = cx.intern(ConstDef { attrs: mem::take(&mut attrs), ty: result_type.unwrap(), - ctor: ConstCtor::SpvInst(inst.without_ids), - ctor_args: const_ctor_args, + kind: ConstKind::SpvInst { + spv_inst_and_const_inputs: Rc::new((inst.without_ids, const_inputs)), + }, }); id_defs.insert(id, IdDef::Const(ct)); @@ -671,8 +672,7 @@ impl Module { let ptr_to_global_var = cx.intern(ConstDef { attrs: AttrSet::default(), ty: type_of_ptr_to_global_var, - ctor: ConstCtor::PtrToGlobalVar(global_var), - ctor_args: [].into_iter().collect(), + kind: ConstKind::PtrToGlobalVar(global_var), }); id_defs.insert(global_var_id, IdDef::Const(ptr_to_global_var)); @@ -1045,8 +1045,7 @@ impl Module { let ct = cx.intern(ConstDef { attrs: AttrSet::default(), ty, - ctor: ConstCtor::SpvStringLiteralForExtInst(*s), - ctor_args: [].into_iter().collect(), + kind: ConstKind::SpvStringLiteralForExtInst(*s), }); Ok(LocalIdDef::Value(Value::Const(ct))) } else { @@ -1497,8 +1496,8 @@ impl Module { .map(|export| match export { Export::Linkage { name, target_id } => { let exportee = match id_defs.get(&target_id) { - Some(id_def @ &IdDef::Const(ct)) => match cx[ct].ctor { - ConstCtor::PtrToGlobalVar(gv) => Ok(Exportee::GlobalVar(gv)), + Some(id_def @ &IdDef::Const(ct)) => match cx[ct].kind { + ConstKind::PtrToGlobalVar(gv) => Ok(Exportee::GlobalVar(gv)), _ => Err(id_def.descr(&cx)), }, Some(&IdDef::Func(func)) => Ok(Exportee::Func(func)), @@ -1532,8 +1531,8 @@ impl Module { let interface_global_vars = interface_ids .into_iter() .map(|id| match id_defs.get(&id) { - Some(id_def @ &IdDef::Const(ct)) => match cx[ct].ctor { - ConstCtor::PtrToGlobalVar(gv) => Ok(gv), + Some(id_def @ &IdDef::Const(ct)) => match cx[ct].kind { + ConstKind::PtrToGlobalVar(gv) => Ok(gv), _ => Err(id_def.descr(&cx)), }, Some(id_def) => Err(id_def.descr(&cx)), diff --git a/src/transform.rs b/src/transform.rs index 43aa478..dc273ed 100644 --- a/src/transform.rs +++ b/src/transform.rs @@ -3,7 +3,7 @@ use crate::func_at::FuncAtMut; use crate::qptr::{self, QPtrAttr, QPtrMemUsage, QPtrMemUsageKind, QPtrOp, QPtrUsage}; use crate::{ - cfg, spv, AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstCtor, ConstDef, ControlNode, + cfg, spv, AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstDef, ConstKind, ControlNode, ControlNodeDef, ControlNodeKind, ControlNodeOutputDecl, ControlRegion, ControlRegionDef, ControlRegionInputDecl, DataInst, DataInstDef, DataInstForm, DataInstFormDef, DataInstKind, DeclDef, EntityListIter, ExportKey, Exportee, Func, FuncDecl, FuncDefBody, FuncParam, @@ -447,28 +447,31 @@ impl InnerTransform for TypeDef { impl InnerTransform for ConstDef { fn inner_transform_with(&self, transformer: &mut impl Transformer) -> Transformed { - let Self { attrs, ty, ctor, ctor_args } = self; + let Self { attrs, ty, kind } = self; transform!({ attrs -> transformer.transform_attr_set_use(*attrs), ty -> transformer.transform_type_use(*ty), - ctor -> match ctor { - ConstCtor::PtrToGlobalVar(gv) => transform!({ + kind -> match kind { + ConstKind::PtrToGlobalVar(gv) => transform!({ gv -> transformer.transform_global_var_use(*gv), - } => ConstCtor::PtrToGlobalVar(gv)), - - ConstCtor::SpvInst(_) - | ConstCtor::SpvStringLiteralForExtInst(_) => Transformed::Unchanged + } => ConstKind::PtrToGlobalVar(gv)), + + ConstKind::SpvInst { spv_inst_and_const_inputs } => { + let (spv_inst, const_inputs) = &**spv_inst_and_const_inputs; + Transformed::map_iter( + const_inputs.iter(), + |&ct| transformer.transform_const_use(ct), + ).map(|new_iter| ConstKind::SpvInst { + spv_inst_and_const_inputs: Rc::new((spv_inst.clone(), new_iter.collect())), + }) + } + ConstKind::SpvStringLiteralForExtInst(_) => Transformed::Unchanged }, - ctor_args -> Transformed::map_iter( - ctor_args.iter(), - |&ct| transformer.transform_const_use(ct), - ).map(|new_iter| new_iter.collect()), } => Self { attrs, ty, - ctor, - ctor_args, + kind, }) } } diff --git a/src/visit.rs b/src/visit.rs index 56e45f5..962b654 100644 --- a/src/visit.rs +++ b/src/visit.rs @@ -3,7 +3,7 @@ use crate::func_at::FuncAt; use crate::qptr::{self, QPtrAttr, QPtrMemUsage, QPtrMemUsageKind, QPtrOp, QPtrUsage}; use crate::{ - cfg, spv, AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstCtor, ConstDef, ControlNode, + cfg, spv, AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstDef, ConstKind, ControlNode, ControlNodeDef, ControlNodeKind, ControlNodeOutputDecl, ControlRegion, ControlRegionDef, ControlRegionInputDecl, DataInstDef, DataInstForm, DataInstFormDef, DataInstKind, DeclDef, DiagMsgPart, EntityListIter, ExportKey, Exportee, Func, FuncDecl, FuncDefBody, FuncParam, @@ -328,16 +328,19 @@ impl InnerVisit for TypeDef { impl InnerVisit for ConstDef { fn inner_visit_with<'a>(&'a self, visitor: &mut impl Visitor<'a>) { - let Self { attrs, ty, ctor, ctor_args } = self; + let Self { attrs, ty, kind } = self; visitor.visit_attr_set_use(*attrs); visitor.visit_type_use(*ty); - match *ctor { - ConstCtor::PtrToGlobalVar(gv) => visitor.visit_global_var_use(gv), - ConstCtor::SpvInst(_) | ConstCtor::SpvStringLiteralForExtInst(_) => {} - } - for &ct in ctor_args { - visitor.visit_const_use(ct); + match kind { + &ConstKind::PtrToGlobalVar(gv) => visitor.visit_global_var_use(gv), + ConstKind::SpvInst { spv_inst_and_const_inputs } => { + let (_spv_inst, const_inputs) = &**spv_inst_and_const_inputs; + for &ct in const_inputs { + visitor.visit_const_use(ct); + } + } + ConstKind::SpvStringLiteralForExtInst(_) => {} } } } From cc14c56ca0fcbf8a5847e7c7298142ad5e558ac3 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 24 Oct 2023 18:59:02 +0300 Subject: [PATCH 3/4] Combine `TypeCtor` and "ctor args" into a single `TypeKind`. --- src/cfg.rs | 11 +- src/lib.rs | 23 +++- src/passes/qptr.rs | 2 +- src/print/mod.rs | 288 ++++++++++++++++++++++---------------------- src/qptr/analyze.rs | 6 +- src/qptr/layout.rs | 38 +++--- src/qptr/lift.rs | 55 ++++----- src/qptr/lower.rs | 29 ++--- src/qptr/mod.rs | 4 +- src/spv/lift.rs | 71 ++++++----- src/spv/lower.rs | 46 ++++--- src/transform.rs | 38 +++--- src/visit.rs | 21 ++-- 13 files changed, 321 insertions(+), 311 deletions(-) diff --git a/src/cfg.rs b/src/cfg.rs index 9746280..beb7c91 100644 --- a/src/cfg.rs +++ b/src/cfg.rs @@ -3,8 +3,8 @@ use crate::{ spv, AttrSet, Const, ConstDef, ConstKind, Context, ControlNode, ControlNodeDef, ControlNodeKind, ControlNodeOutputDecl, ControlRegion, ControlRegionDef, EntityList, - EntityOrientedDenseMap, FuncDefBody, FxIndexMap, FxIndexSet, SelectionKind, Type, TypeCtor, - TypeDef, Value, + EntityOrientedDenseMap, FuncDefBody, FxIndexMap, FxIndexSet, SelectionKind, Type, TypeKind, + Value, }; use itertools::Either; use smallvec::SmallVec; @@ -545,10 +545,9 @@ impl<'a> Structurizer<'a> { pub fn new(cx: &'a Context, func_def_body: &'a mut FuncDefBody) -> Self { // FIXME(eddyb) SPIR-T should have native booleans itself. let wk = &spv::spec::Spec::get().well_known; - let type_bool = cx.intern(TypeDef { - attrs: AttrSet::default(), - ctor: TypeCtor::SpvInst(wk.OpTypeBool.into()), - ctor_args: [].into_iter().collect(), + let type_bool = cx.intern(TypeKind::SpvInst { + spv_inst: wk.OpTypeBool.into(), + type_and_const_inputs: [].into_iter().collect(), }); let const_true = cx.intern(ConstDef { attrs: AttrSet::default(), diff --git a/src/lib.rs b/src/lib.rs index c87fd03..0189a81 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -458,13 +458,11 @@ pub use context::Type; #[derive(PartialEq, Eq, Hash)] pub struct TypeDef { pub attrs: AttrSet, - pub ctor: TypeCtor, - pub ctor_args: SmallVec<[TypeCtorArg; 2]>, + pub kind: TypeKind, } -/// [`Type`] "constructor": a [`TypeDef`] wiithout any [`TypeCtorArg`]s ([`Type`]s/[`Const`]s). #[derive(Clone, PartialEq, Eq, Hash)] -pub enum TypeCtor { +pub enum TypeKind { /// "Quasi-pointer", an untyped pointer-like abstract scalar that can represent /// both memory locations (in any address space) and other kinds of locations /// (e.g. SPIR-V `OpVariable`s in non-memory "storage classes"). @@ -481,15 +479,28 @@ pub enum TypeCtor { // separately in e.g. `ControlRegionInputDecl`, might be a better approach? QPtr, - SpvInst(spv::Inst), + SpvInst { + spv_inst: spv::Inst, + // FIXME(eddyb) find a better name. + type_and_const_inputs: SmallVec<[TypeOrConst; 2]>, + }, /// The type of a [`ConstKind::SpvStringLiteralForExtInst`] constant, i.e. /// a SPIR-V `OpString` with no actual type in SPIR-V. SpvStringLiteralForExtInst, } +// HACK(eddyb) this behaves like an implicit conversion for `cx.intern(...)`. +impl context::InternInCx for TypeKind { + fn intern_in_cx(self, cx: &Context) -> Type { + cx.intern(TypeDef { attrs: Default::default(), kind: self }) + } +} + +// HACK(eddyb) this is like `Either`, only used in `TypeKind::SpvInst`, +// and only because SPIR-V type definitions can references both types and consts. #[derive(Copy, Clone, PartialEq, Eq, Hash)] -pub enum TypeCtorArg { +pub enum TypeOrConst { Type(Type), Const(Const), } diff --git a/src/passes/qptr.rs b/src/passes/qptr.rs index 2ddc936..c4f49a4 100644 --- a/src/passes/qptr.rs +++ b/src/passes/qptr.rs @@ -1,4 +1,4 @@ -//! [`QPtr`](crate::TypeCtor::QPtr) transforms. +//! [`QPtr`](crate::TypeKind::QPtr) transforms. use crate::visit::{InnerVisit, Visitor}; use crate::{qptr, DataInstForm}; diff --git a/src/print/mod.rs b/src/print/mod.rs index d9a0878..c37eca0 100644 --- a/src/print/mod.rs +++ b/src/print/mod.rs @@ -29,8 +29,8 @@ use crate::{ ControlRegionDef, ControlRegionInputDecl, DataInst, DataInstDef, DataInstForm, DataInstFormDef, DataInstKind, DeclDef, Diag, DiagLevel, DiagMsgPart, EntityListIter, ExportKey, Exportee, Func, FuncDecl, FuncParam, FxIndexMap, FxIndexSet, GlobalVar, GlobalVarDecl, GlobalVarDefBody, - Import, Module, ModuleDebugInfo, ModuleDialect, OrdAssertEq, SelectionKind, Type, TypeCtor, - TypeCtorArg, TypeDef, Value, + Import, Module, ModuleDebugInfo, ModuleDialect, OrdAssertEq, SelectionKind, Type, TypeDef, + TypeKind, TypeOrConst, Value, }; use arrayvec::ArrayVec; use itertools::Either; @@ -539,11 +539,12 @@ impl<'a> Visitor<'a> for Plan<'a> { let pointee_type = { let wk = &spv::spec::Spec::get().well_known; - let type_of_ptr_to_def = &self.cx[gv_decl.type_of_ptr_to]; - match &type_of_ptr_to_def.ctor { - TypeCtor::SpvInst(inst) if inst.opcode == wk.OpTypePointer => { - match type_of_ptr_to_def.ctor_args[..] { - [TypeCtorArg::Type(ty)] => Some(ty), + match &self.cx[gv_decl.type_of_ptr_to].kind { + TypeKind::SpvInst { spv_inst, type_and_const_inputs } + if spv_inst.opcode == wk.OpTypePointer => + { + match type_and_const_inputs[..] { + [TypeOrConst::Type(ty)] => Some(ty), _ => unreachable!(), } } @@ -814,22 +815,25 @@ impl<'a> Printer<'a> { // FIXME(eddyb) remove the duplication between // here and `TypeDef`'s `Print` impl. - let has_compact_print = match &ty_def.ctor { - TypeCtor::SpvInst(inst) => [ - wk.OpTypeBool, - wk.OpTypeInt, - wk.OpTypeFloat, - wk.OpTypeVector, - ] - .contains(&inst.opcode), - - TypeCtor::QPtr | TypeCtor::SpvStringLiteralForExtInst => { + let has_compact_print_or_is_leaf = match &ty_def.kind { + TypeKind::SpvInst { spv_inst, type_and_const_inputs } => { + [ + wk.OpTypeBool, + wk.OpTypeInt, + wk.OpTypeFloat, + wk.OpTypeVector, + ] + .contains(&spv_inst.opcode) + || type_and_const_inputs.is_empty() + } + + TypeKind::QPtr | TypeKind::SpvStringLiteralForExtInst => { true } }; ty_def.attrs == AttrSet::default() - && (has_compact_print || ty_def.ctor_args.is_empty()) + && has_compact_print_or_is_leaf } CxInterned::Const(ct) => { let ct_def = &cx[ct]; @@ -2370,14 +2374,17 @@ impl Print for QPtrMemUsage { impl Print for TypeDef { type Output = AttrsAndDef; fn print(&self, printer: &Printer<'_>) -> AttrsAndDef { - let Self { attrs, ctor, ctor_args } = self; + let Self { attrs, kind } = self; let wk = &spv::spec::Spec::get().well_known; // FIXME(eddyb) should this be done by lowering SPIR-V types to SPIR-T? let kw = |kw| printer.declarative_keyword_style().apply(kw).into(); - #[allow(irrefutable_let_patterns)] - let compact_def = if let &TypeCtor::SpvInst(spv::Inst { opcode, ref imms }) = ctor { + let compact_def = if let &TypeKind::SpvInst { + spv_inst: spv::Inst { opcode, ref imms }, + ref type_and_const_inputs, + } = kind + { if opcode == wk.OpTypeBool { Some(kw("bool".into())) } else if opcode == wk.OpTypeInt { @@ -2397,8 +2404,8 @@ impl Print for TypeDef { Some(kw(format!("f{width}"))) } else if opcode == wk.OpTypeVector { - let (elem_ty, elem_count) = match (&imms[..], &ctor_args[..]) { - (&[spv::Imm::Short(_, elem_count)], &[TypeCtorArg::Type(elem_ty)]) => { + let (elem_ty, elem_count) = match (&imms[..], &type_and_const_inputs[..]) { + (&[spv::Imm::Short(_, elem_count)], &[TypeOrConst::Type(elem_ty)]) => { (elem_ty, elem_count) } _ => unreachable!(), @@ -2421,20 +2428,21 @@ impl Print for TypeDef { def_without_name: if let Some(def) = compact_def { def } else { - match *ctor { + match kind { // FIXME(eddyb) should this be shortened to `qtr`? - TypeCtor::QPtr => printer.declarative_keyword_style().apply("qptr").into(), - - TypeCtor::SpvInst(spv::Inst { opcode, ref imms }) => printer.pretty_spv_inst( - printer.spv_op_style(), - opcode, - imms, - ctor_args.iter().map(|&arg| match arg { - TypeCtorArg::Type(ty) => ty.print(printer), - TypeCtorArg::Const(ct) => ct.print(printer), - }), - ), - TypeCtor::SpvStringLiteralForExtInst => pretty::Fragment::new([ + TypeKind::QPtr => printer.declarative_keyword_style().apply("qptr").into(), + + TypeKind::SpvInst { spv_inst, type_and_const_inputs } => printer + .pretty_spv_inst( + printer.spv_op_style(), + spv_inst.opcode, + &spv_inst.imms, + type_and_const_inputs.iter().map(|&ty_or_ct| match ty_or_ct { + TypeOrConst::Type(ty) => ty.print(printer), + TypeOrConst::Const(ct) => ct.print(printer), + }), + ), + TypeKind::SpvStringLiteralForExtInst => pretty::Fragment::new([ printer.error_style().apply("type_of").into(), "(".into(), printer.pretty_spv_opcode(printer.spv_op_style(), wk.OpString), @@ -2484,8 +2492,11 @@ impl Print for ConstDef { if let ( Some(raw_bits), - &TypeCtor::SpvInst(spv::Inst { opcode: ty_opcode, imms: ref ty_imms }), - ) = (raw_bits, &printer.cx[*ty].ctor) + &TypeKind::SpvInst { + spv_inst: spv::Inst { opcode: ty_opcode, imms: ref ty_imms }, + .. + }, + ) = (raw_bits, &printer.cx[*ty].kind) { if ty_opcode == wk.OpTypeInt { let (width, signed) = match ty_imms[..] { @@ -2616,128 +2627,119 @@ impl Print for GlobalVarDecl { let wk = &spv::spec::Spec::get().well_known; - let type_ascription_suffix = { - // HACK(eddyb) get the pointee type from SPIR-V `OpTypePointer`, but - // ideally the `GlobalVarDecl` would hold that type itself. - let type_of_ptr_to_def = &printer.cx[*type_of_ptr_to]; - - match &type_of_ptr_to_def.ctor { - TypeCtor::QPtr if shape.is_some() => match shape.unwrap() { - qptr::shapes::GlobalVarShape::Handles { handle, fixed_count } => { - let handle = match handle { - qptr::shapes::Handle::Opaque(ty) => ty.print(printer), - qptr::shapes::Handle::Buffer(addr_space, buf) => { - pretty::Fragment::new([ - printer.declarative_keyword_style().apply("buffer").into(), - pretty::join_comma_sep( - "(", - [ - addr_space.print(printer), - pretty::Fragment::new([ - printer.pretty_named_argument_prefix("size"), - pretty::Fragment::new( - Some(buf.fixed_base.size) - .filter(|&base_size| { - base_size > 0 - || buf.dyn_unit_stride.is_none() - }) - .map(|base_size| { - printer - .numeric_literal_style() - .apply(base_size.to_string()) - .into() - }) - .into_iter() - .chain(buf.dyn_unit_stride.map(|stride| { - pretty::Fragment::new([ - "N × ".into(), - printer - .numeric_literal_style() - .apply(stride.to_string()), - ]) - })) - .intersperse_with(|| " + ".into()), - ), - ]), - pretty::Fragment::new([ - printer.pretty_named_argument_prefix("align"), - printer - .numeric_literal_style() - .apply(buf.fixed_base.align.to_string()) - .into(), - ]), - ], - ")", - ), - ]) - } - }; - - let handles = if fixed_count.map_or(0, |c| c.get()) == 1 { - handle - } else { - pretty::Fragment::new([ - "[".into(), - fixed_count - .map(|count| { - pretty::Fragment::new([ - printer - .numeric_literal_style() - .apply(count.to_string()), - " × ".into(), - ]) - }) - .unwrap_or_default(), - handle, - "]".into(), - ]) - }; - pretty::join_space(":", [handles]) - } - qptr::shapes::GlobalVarShape::UntypedData(mem_layout) => { - pretty::Fragment::new([ - " ".into(), - printer.declarative_keyword_style().apply("layout").into(), + // HACK(eddyb) get the pointee type from SPIR-V `OpTypePointer`, but + // ideally the `GlobalVarDecl` would hold that type itself. + let type_ascription_suffix = match &printer.cx[*type_of_ptr_to].kind { + TypeKind::QPtr if shape.is_some() => match shape.unwrap() { + qptr::shapes::GlobalVarShape::Handles { handle, fixed_count } => { + let handle = match handle { + qptr::shapes::Handle::Opaque(ty) => ty.print(printer), + qptr::shapes::Handle::Buffer(addr_space, buf) => pretty::Fragment::new([ + printer.declarative_keyword_style().apply("buffer").into(), pretty::join_comma_sep( "(", [ + addr_space.print(printer), pretty::Fragment::new([ printer.pretty_named_argument_prefix("size"), - printer - .numeric_literal_style() - .apply(mem_layout.size.to_string()) - .into(), + pretty::Fragment::new( + Some(buf.fixed_base.size) + .filter(|&base_size| { + base_size > 0 || buf.dyn_unit_stride.is_none() + }) + .map(|base_size| { + printer + .numeric_literal_style() + .apply(base_size.to_string()) + .into() + }) + .into_iter() + .chain(buf.dyn_unit_stride.map(|stride| { + pretty::Fragment::new([ + "N × ".into(), + printer + .numeric_literal_style() + .apply(stride.to_string()), + ]) + })) + .intersperse_with(|| " + ".into()), + ), ]), pretty::Fragment::new([ printer.pretty_named_argument_prefix("align"), printer .numeric_literal_style() - .apply(mem_layout.align.to_string()) + .apply(buf.fixed_base.align.to_string()) .into(), ]), ], ")", ), + ]), + }; + + let handles = if fixed_count.map_or(0, |c| c.get()) == 1 { + handle + } else { + pretty::Fragment::new([ + "[".into(), + fixed_count + .map(|count| { + pretty::Fragment::new([ + printer.numeric_literal_style().apply(count.to_string()), + " × ".into(), + ]) + }) + .unwrap_or_default(), + handle, + "]".into(), ]) - } - qptr::shapes::GlobalVarShape::TypedInterface(ty) => { - printer.pretty_type_ascription_suffix(ty) - } - }, - TypeCtor::SpvInst(inst) if inst.opcode == wk.OpTypePointer => { - match type_of_ptr_to_def.ctor_args[..] { - [TypeCtorArg::Type(ty)] => printer.pretty_type_ascription_suffix(ty), - _ => unreachable!(), - } + }; + pretty::join_space(":", [handles]) } - _ => pretty::Fragment::new([ - ": ".into(), - printer.error_style().apply("pointee_type_of").into(), - "(".into(), - type_of_ptr_to.print(printer), - ")".into(), + qptr::shapes::GlobalVarShape::UntypedData(mem_layout) => pretty::Fragment::new([ + " ".into(), + printer.declarative_keyword_style().apply("layout").into(), + pretty::join_comma_sep( + "(", + [ + pretty::Fragment::new([ + printer.pretty_named_argument_prefix("size"), + printer + .numeric_literal_style() + .apply(mem_layout.size.to_string()) + .into(), + ]), + pretty::Fragment::new([ + printer.pretty_named_argument_prefix("align"), + printer + .numeric_literal_style() + .apply(mem_layout.align.to_string()) + .into(), + ]), + ], + ")", + ), ]), + qptr::shapes::GlobalVarShape::TypedInterface(ty) => { + printer.pretty_type_ascription_suffix(ty) + } + }, + TypeKind::SpvInst { spv_inst, type_and_const_inputs } + if spv_inst.opcode == wk.OpTypePointer => + { + match type_and_const_inputs[..] { + [TypeOrConst::Type(ty)] => printer.pretty_type_ascription_suffix(ty), + _ => unreachable!(), + } } + _ => pretty::Fragment::new([ + ": ".into(), + printer.error_style().apply("pointee_type_of").into(), + "(".into(), + type_of_ptr_to.print(printer), + ")".into(), + ]), }; let addr_space_suffix = match addr_space { AddrSpace::Handles => pretty::Fragment::default(), @@ -3244,8 +3246,8 @@ impl Print for FuncAt<'_, DataInst> { // the default, and not meaningful *even if* the resulting // value is "used" in a kind of "untyped token" way. output_type_to_print = output_type_to_print.filter(|&ty| { - let is_void = match &printer.cx[ty].ctor { - TypeCtor::SpvInst(spv_inst) => spv_inst.opcode == wk.OpTypeVoid, + let is_void = match &printer.cx[ty].kind { + TypeKind::SpvInst { spv_inst, .. } => spv_inst.opcode == wk.OpTypeVoid, _ => false, }; !is_void diff --git a/src/qptr/analyze.rs b/src/qptr/analyze.rs index a9a4c7a..daaf139 100644 --- a/src/qptr/analyze.rs +++ b/src/qptr/analyze.rs @@ -1,4 +1,4 @@ -//! [`QPtr`](crate::TypeCtor::QPtr) usage analysis (for legalizing/lifting). +//! [`QPtr`](crate::TypeKind::QPtr) usage analysis (for legalizing/lifting). // HACK(eddyb) sharing layout code with other modules. use super::{layout::*, QPtrMemUsageKind}; @@ -9,7 +9,7 @@ use crate::visit::{InnerVisit, Visitor}; use crate::{ AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstKind, Context, ControlNode, ControlNodeKind, DataInst, DataInstForm, DataInstKind, DeclDef, Diag, EntityList, ExportKey, Exportee, Func, - FxIndexMap, GlobalVar, Module, OrdAssertEq, Type, TypeCtor, Value, + FxIndexMap, GlobalVar, Module, OrdAssertEq, Type, TypeKind, Value, }; use itertools::Either; use rustc_hash::FxHashMap; @@ -838,7 +838,7 @@ impl<'a> InferUsage<'a> { func: Func, ) -> FuncInferUsageResults { let cx = self.cx.clone(); - let is_qptr = |ty: Type| matches!(cx[ty].ctor, TypeCtor::QPtr); + let is_qptr = |ty: Type| matches!(cx[ty].kind, TypeKind::QPtr); let func_decl = &module.funcs[func]; let mut param_usages: SmallVec<[_; 2]> = diff --git a/src/qptr/layout.rs b/src/qptr/layout.rs index cfea017..00def11 100644 --- a/src/qptr/layout.rs +++ b/src/qptr/layout.rs @@ -2,7 +2,7 @@ use crate::qptr::shapes; use crate::{ - spv, AddrSpace, Attr, Const, ConstKind, Context, Diag, FxIndexMap, Type, TypeCtor, TypeCtorArg, + spv, AddrSpace, Attr, Const, ConstKind, Context, Diag, FxIndexMap, Type, TypeKind, TypeOrConst, }; use itertools::Either; use smallvec::SmallVec; @@ -206,15 +206,17 @@ impl<'a> LayoutCache<'a> { let wk = self.wk; let ty_def = &cx[ty]; - let spv_inst = match &ty_def.ctor { + let (spv_inst, type_and_const_inputs) = match &ty_def.kind { // FIXME(eddyb) treat `QPtr`s as scalars. - TypeCtor::QPtr => { + TypeKind::QPtr => { return Err(LayoutError(Diag::bug( ["`layout_of(qptr)` (already lowered?)".into()], ))); } - TypeCtor::SpvInst(spv_inst) => spv_inst, - TypeCtor::SpvStringLiteralForExtInst => { + TypeKind::SpvInst { spv_inst, type_and_const_inputs } => { + (spv_inst, type_and_const_inputs) + } + TypeKind::SpvStringLiteralForExtInst => { return Err(LayoutError(Diag::bug([ "`layout_of(type_of(OpString<\"...\">))`".into() ]))); @@ -368,8 +370,8 @@ impl<'a> LayoutCache<'a> { }; // NOTE(eddyb) `RowMajor` is disallowed on `OpTypeStruct` members below. array( - match ty_def.ctor_args[..] { - [TypeCtorArg::Type(elem_type)] => elem_type, + match type_and_const_inputs[..] { + [TypeOrConst::Type(elem_type)] => elem_type, _ => unreachable!(), }, ArrayParams { @@ -380,13 +382,12 @@ impl<'a> LayoutCache<'a> { }, )? } else if [wk.OpTypeArray, wk.OpTypeRuntimeArray].contains(&spv_inst.opcode) { - let len = ty_def - .ctor_args + let len = type_and_const_inputs .get(1) .map(|&len| { let len = match len { - TypeCtorArg::Const(len) => len, - TypeCtorArg::Type(_) => unreachable!(), + TypeOrConst::Const(len) => len, + TypeOrConst::Type(_) => unreachable!(), }; self.const_as_u32(len).ok_or_else(|| { LayoutError(Diag::bug( @@ -413,9 +414,9 @@ impl<'a> LayoutCache<'a> { } } array( - match ty_def.ctor_args[0] { - TypeCtorArg::Type(elem_type) => elem_type, - TypeCtorArg::Const(_) => unreachable!(), + match type_and_const_inputs[0] { + TypeOrConst::Type(elem_type) => elem_type, + TypeOrConst::Const(_) => unreachable!(), }, ArrayParams { fixed_len: len, @@ -425,12 +426,11 @@ impl<'a> LayoutCache<'a> { }, )? } else if spv_inst.opcode == wk.OpTypeStruct { - let field_layouts: SmallVec<[_; 4]> = ty_def - .ctor_args + let field_layouts: SmallVec<[_; 4]> = type_and_const_inputs .iter() - .map(|&arg| match arg { - TypeCtorArg::Type(field_type) => field_type, - TypeCtorArg::Const(_) => unreachable!(), + .map(|&ty_or_ct| match ty_or_ct { + TypeOrConst::Type(field_type) => field_type, + TypeOrConst::Const(_) => unreachable!(), }) .map(|field_type| match self.layout_of(field_type)? { TypeLayout::Handle(_) | TypeLayout::HandleArray(..) => { diff --git a/src/qptr/lift.rs b/src/qptr/lift.rs index d30497c..f962ded 100644 --- a/src/qptr/lift.rs +++ b/src/qptr/lift.rs @@ -1,4 +1,4 @@ -//! [`QPtr`](crate::TypeCtor::QPtr) lifting (e.g. to SPIR-V). +//! [`QPtr`](crate::TypeKind::QPtr) lifting (e.g. to SPIR-V). // HACK(eddyb) sharing layout code with other modules. use super::layout::*; @@ -10,7 +10,7 @@ use crate::{ spv, AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstDef, ConstKind, Context, ControlNode, ControlNodeKind, DataInst, DataInstDef, DataInstFormDef, DataInstKind, DeclDef, Diag, DiagLevel, EntityDefs, EntityOrientedDenseMap, Func, FuncDecl, FxIndexMap, GlobalVar, - GlobalVarDecl, Module, Type, TypeCtor, TypeCtorArg, TypeDef, Value, + GlobalVarDecl, Module, Type, TypeDef, TypeKind, TypeOrConst, Value, }; use smallvec::SmallVec; use std::cell::Cell; @@ -163,15 +163,16 @@ impl<'a> LiftToSpvPtrs<'a> { // // FIXME(eddyb) deduplicate with `qptr::lower`. fn as_spv_ptr_type(&self, ty: Type) -> Option<(AddrSpace, Type)> { - let ty_def = &self.cx[ty]; - match &ty_def.ctor { - TypeCtor::SpvInst(spv_inst) if spv_inst.opcode == self.wk.OpTypePointer => { + match &self.cx[ty].kind { + TypeKind::SpvInst { spv_inst, type_and_const_inputs } + if spv_inst.opcode == self.wk.OpTypePointer => + { let sc = match spv_inst.imms[..] { [spv::Imm::Short(_, sc)] => sc, _ => unreachable!(), }; - let pointee = match ty_def.ctor_args[..] { - [TypeCtorArg::Type(elem_type)] => elem_type, + let pointee = match type_and_const_inputs[..] { + [TypeOrConst::Type(elem_type)] => elem_type, _ => unreachable!(), }; Some((AddrSpace::SpvStorageClass(sc), pointee)) @@ -187,13 +188,12 @@ impl<'a> LiftToSpvPtrs<'a> { AddrSpace::Handles => unreachable!(), AddrSpace::SpvStorageClass(storage_class) => storage_class, }; - self.cx.intern(TypeDef { - attrs: AttrSet::default(), - ctor: TypeCtor::SpvInst(spv::Inst { + self.cx.intern(TypeKind::SpvInst { + spv_inst: spv::Inst { opcode: wk.OpTypePointer, imms: [spv::Imm::Short(wk.StorageClass, storage_class)].into_iter().collect(), - }), - ctor_args: [TypeCtorArg::Type(pointee_type)].into_iter().collect(), + }, + type_and_const_inputs: [TypeOrConst::Type(pointee_type)].into_iter().collect(), }) } @@ -287,11 +287,13 @@ impl<'a> LiftToSpvPtrs<'a> { Ok(self.cx.intern(TypeDef { attrs: stride_attrs.unwrap_or_default(), - ctor: TypeCtor::SpvInst(spv_opcode.into()), - ctor_args: [TypeCtorArg::Type(element_type)] - .into_iter() - .chain(fixed_len.map(|len| TypeCtorArg::Const(self.const_u32(len)))) - .collect(), + kind: TypeKind::SpvInst { + spv_inst: spv_opcode.into(), + type_and_const_inputs: [TypeOrConst::Type(element_type)] + .into_iter() + .chain(fixed_len.map(|len| TypeOrConst::Const(self.const_u32(len)))) + .collect(), + }, })) } @@ -304,7 +306,8 @@ impl<'a> LiftToSpvPtrs<'a> { let field_offsets_and_types = field_offsets_and_types.into_iter(); let mut attrs = AttrSetDef::default(); - let mut type_ctor_args = SmallVec::with_capacity(field_offsets_and_types.size_hint().0); + let mut type_and_const_inputs = + SmallVec::with_capacity(field_offsets_and_types.size_hint().0); for (i, field_offset_and_type) in field_offsets_and_types.enumerate() { let (offset, field_type) = field_offset_and_type?; attrs.attrs.insert(Attr::SpvAnnotation(spv::Inst { @@ -317,13 +320,12 @@ impl<'a> LiftToSpvPtrs<'a> { .into_iter() .collect(), })); - type_ctor_args.push(TypeCtorArg::Type(field_type)); + type_and_const_inputs.push(TypeOrConst::Type(field_type)); } attrs.attrs.extend(extra_attrs); Ok(self.cx.intern(TypeDef { attrs: self.cx.intern(attrs), - ctor: TypeCtor::SpvInst(wk.OpTypeStruct.into()), - ctor_args: type_ctor_args, + kind: TypeKind::SpvInst { spv_inst: wk.OpTypeStruct.into(), type_and_const_inputs }, })) } @@ -333,9 +335,8 @@ impl<'a> LiftToSpvPtrs<'a> { return cached; } let wk = self.wk; - let ty = self.cx.intern(TypeDef { - attrs: AttrSet::default(), - ctor: TypeCtor::SpvInst(spv::Inst { + let ty = self.cx.intern(TypeKind::SpvInst { + spv_inst: spv::Inst { opcode: wk.OpTypeInt, imms: [ spv::Imm::Short(wk.LiteralInteger, 32), @@ -343,8 +344,8 @@ impl<'a> LiftToSpvPtrs<'a> { ] .into_iter() .collect(), - }), - ctor_args: [].into_iter().collect(), + }, + type_and_const_inputs: [].into_iter().collect(), }); self.cached_u32_type.set(Some(ty)); ty @@ -1130,7 +1131,7 @@ impl Transformer for LiftToSpvPtrInstsInFunc<'_> { lifted = Err(LiftError(Diag::bug(["unimplemented qptr instruction".into()]))); } else if let Some(ty) = data_inst_form_def.output_type { - if matches!(self.lifter.cx[ty].ctor, TypeCtor::QPtr) { + if matches!(self.lifter.cx[ty].kind, TypeKind::QPtr) { lifted = Err(LiftError(Diag::bug([ "unimplemented qptr-producing instruction".into(), ]))); diff --git a/src/qptr/lower.rs b/src/qptr/lower.rs index d28bd14..512b685 100644 --- a/src/qptr/lower.rs +++ b/src/qptr/lower.rs @@ -1,4 +1,4 @@ -//! [`QPtr`](crate::TypeCtor::QPtr) lowering (e.g. from SPIR-V). +//! [`QPtr`](crate::TypeKind::QPtr) lowering (e.g. from SPIR-V). // HACK(eddyb) layout code used to be in this module. use super::layout::*; @@ -9,7 +9,7 @@ use crate::transform::{InnerInPlaceTransform, Transformed, Transformer}; use crate::{ spv, AddrSpace, AttrSet, AttrSetDef, Const, ConstDef, ConstKind, Context, ControlNode, ControlNodeKind, DataInst, DataInstDef, DataInstForm, DataInstFormDef, DataInstKind, Diag, - FuncDecl, GlobalVarDecl, OrdAssertEq, Type, TypeCtor, TypeCtorArg, TypeDef, Value, + FuncDecl, GlobalVarDecl, OrdAssertEq, Type, TypeKind, TypeOrConst, Value, }; use smallvec::SmallVec; use std::cell::Cell; @@ -153,15 +153,16 @@ impl<'a> LowerFromSpvPtrs<'a> { // `Block`-annotated type is a buffer or just interface nonsense. // (!!! may cause bad interactions with storage class inference `Generic` abuse) fn as_spv_ptr_type(&self, ty: Type) -> Option<(AddrSpace, Type)> { - let ty_def = &self.cx[ty]; - match &ty_def.ctor { - TypeCtor::SpvInst(spv_inst) if spv_inst.opcode == self.wk.OpTypePointer => { + match &self.cx[ty].kind { + TypeKind::SpvInst { spv_inst, type_and_const_inputs } + if spv_inst.opcode == self.wk.OpTypePointer => + { let sc = match spv_inst.imms[..] { [spv::Imm::Short(_, sc)] => sc, _ => unreachable!(), }; - let pointee = match ty_def.ctor_args[..] { - [TypeCtorArg::Type(elem_type)] => elem_type, + let pointee = match type_and_const_inputs[..] { + [TypeOrConst::Type(elem_type)] => elem_type, _ => unreachable!(), }; Some((AddrSpace::SpvStorageClass(sc), pointee)) @@ -189,11 +190,7 @@ impl<'a> LowerFromSpvPtrs<'a> { if let Some(cached) = self.cached_qptr_type.get() { return cached; } - let ty = self.cx.intern(TypeDef { - attrs: Default::default(), - ctor: TypeCtor::QPtr, - ctor_args: Default::default(), - }); + let ty = self.cx.intern(TypeKind::QPtr); self.cached_qptr_type.set(Some(ty)); ty } @@ -529,10 +526,10 @@ impl LowerFromSpvPtrInstsInFunc<'_> { // a `OpTypeRuntimeArray`, with the original type as the element type. let access_chain_base_layout = if [wk.OpPtrAccessChain, wk.OpInBoundsPtrAccessChain].contains(&spv_inst.opcode) { - self.lowerer.layout_of(cx.intern(TypeDef { - attrs: AttrSet::default(), - ctor: TypeCtor::SpvInst(wk.OpTypeRuntimeArray.into()), - ctor_args: [TypeCtorArg::Type(base_pointee_type)].into_iter().collect(), + self.lowerer.layout_of(cx.intern(TypeKind::SpvInst { + spv_inst: wk.OpTypeRuntimeArray.into(), + type_and_const_inputs: + [TypeOrConst::Type(base_pointee_type)].into_iter().collect(), }))? } else { self.lowerer.layout_of(base_pointee_type)? diff --git a/src/qptr/mod.rs b/src/qptr/mod.rs index c32ec2c..a9e9c97 100644 --- a/src/qptr/mod.rs +++ b/src/qptr/mod.rs @@ -1,6 +1,6 @@ -//! [`QPtr`](crate::TypeCtor::QPtr)-related type definitions and passes. +//! [`QPtr`](crate::TypeKind::QPtr)-related type definitions and passes. // -// FIXME(eddyb) consider `#[cfg(doc)] use crate::TypeCtor::QPtr;` for doc comments. +// FIXME(eddyb) consider `#[cfg(doc)] use crate::TypeKind::QPtr;` for doc comments. // FIXME(eddyb) PR description of https://github.com/EmbarkStudios/spirt/pull/24 // has more useful docs that could be copied here. diff --git a/src/spv/lift.rs b/src/spv/lift.rs index 7072e4b..4e5f91b 100644 --- a/src/spv/lift.rs +++ b/src/spv/lift.rs @@ -8,8 +8,8 @@ use crate::{ ControlNodeKind, ControlNodeOutputDecl, ControlRegion, ControlRegionInputDecl, DataInst, 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, + Import, Module, ModuleDebugInfo, ModuleDialect, SelectionKind, Type, TypeDef, TypeKind, + TypeOrConst, Value, }; use rustc_hash::FxHashMap; use smallvec::SmallVec; @@ -81,11 +81,13 @@ impl FuncDecl { cx.intern(TypeDef { attrs: AttrSet::default(), - ctor: TypeCtor::SpvInst(wk.OpTypeFunction.into()), - ctor_args: iter::once(self.ret_type) - .chain(self.params.iter().map(|param| param.ty)) - .map(TypeCtorArg::Type) - .collect(), + kind: TypeKind::SpvInst { + spv_inst: wk.OpTypeFunction.into(), + type_and_const_inputs: iter::once(self.ret_type) + .chain(self.params.iter().map(|param| param.ty)) + .map(TypeOrConst::Type) + .collect(), + }, }) } } @@ -119,17 +121,17 @@ impl Visitor<'_> for NeedsIdsCollector<'_> { return; } let ty_def = &self.cx[ty]; - match ty_def.ctor { + match ty_def.kind { // FIXME(eddyb) this should be a proper `Result`-based error instead, // and/or `spv::lift` should mutate the module for legalization. - TypeCtor::QPtr => { - unreachable!("`TypeCtor::QPtr` should be legalized away before lifting"); + TypeKind::QPtr => { + unreachable!("`TypeKind::QPtr` should be legalized away before lifting"); } - TypeCtor::SpvInst(_) => {} - TypeCtor::SpvStringLiteralForExtInst => { + TypeKind::SpvInst { .. } => {} + TypeKind::SpvStringLiteralForExtInst => { unreachable!( - "`TypeCtor::SpvStringLiteralForExtInst` should not be used \ + "`TypeKind::SpvStringLiteralForExtInst` should not be used \ as a type outside of `ConstKind::SpvStringLiteralForExtInst`" ); } @@ -160,8 +162,7 @@ impl Visitor<'_> for NeedsIdsCollector<'_> { self.cx[*ty] == TypeDef { attrs: AttrSet::default(), - ctor: TypeCtor::SpvStringLiteralForExtInst, - ctor_args: SmallVec::new(), + kind: TypeKind::SpvStringLiteralForExtInst, } ); @@ -1101,29 +1102,25 @@ impl LazyInst<'_, '_> { let (result_id, attrs, _) = self.result_id_attrs_and_import(module, ids); let inst = match self { Self::Global(global) => match global { - Global::Type(ty) => { - let ty_def = &cx[ty]; - match &ty_def.ctor { - TypeCtor::SpvInst(inst) => spv::InstWithIds { - without_ids: inst.clone(), - result_type_id: None, - result_id, - ids: ty_def - .ctor_args - .iter() - .map(|&arg| { - ids.globals[&match arg { - TypeCtorArg::Type(ty) => Global::Type(ty), - TypeCtorArg::Const(ct) => Global::Const(ct), - }] - }) - .collect(), - }, + Global::Type(ty) => match &cx[ty].kind { + TypeKind::SpvInst { spv_inst, type_and_const_inputs } => spv::InstWithIds { + without_ids: spv_inst.clone(), + result_type_id: None, + result_id, + ids: type_and_const_inputs + .iter() + .map(|&ty_or_ct| { + ids.globals[&match ty_or_ct { + TypeOrConst::Type(ty) => Global::Type(ty), + TypeOrConst::Const(ct) => Global::Const(ct), + }] + }) + .collect(), + }, - // Not inserted into `globals` while visiting. - TypeCtor::QPtr | TypeCtor::SpvStringLiteralForExtInst => unreachable!(), - } - } + // Not inserted into `globals` while visiting. + TypeKind::QPtr | TypeKind::SpvStringLiteralForExtInst => unreachable!(), + }, Global::Const(ct) => { let ct_def = &cx[ct]; match &ct_def.kind { diff --git a/src/spv/lower.rs b/src/spv/lower.rs index 68411f2..abf6453 100644 --- a/src/spv/lower.rs +++ b/src/spv/lower.rs @@ -7,7 +7,7 @@ use crate::{ ControlNodeKind, ControlRegion, ControlRegionDef, ControlRegionInputDecl, DataInstDef, DataInstFormDef, DataInstKind, DeclDef, Diag, EntityDefs, EntityList, ExportKey, Exportee, Func, FuncDecl, FuncDefBody, FuncParam, FxIndexMap, GlobalVarDecl, GlobalVarDefBody, Import, - InternedStr, Module, SelectionKind, Type, TypeCtor, TypeCtorArg, TypeDef, Value, + InternedStr, Module, SelectionKind, Type, TypeDef, TypeKind, TypeOrConst, Value, }; use rustc_hash::FxHashMap; use smallvec::SmallVec; @@ -546,8 +546,10 @@ impl Module { // serves as a first approximation for a "deferred error". let ty = cx.intern(TypeDef { attrs: mem::take(&mut attrs), - ctor: TypeCtor::SpvInst(spv::Inst { opcode, imms: [sc].into_iter().collect() }), - ctor_args: [].into_iter().collect(), + kind: TypeKind::SpvInst { + spv_inst: spv::Inst { opcode, imms: [sc].into_iter().collect() }, + type_and_const_inputs: [].into_iter().collect(), + }, }); id_defs.insert(id, IdDef::Type(ty)); @@ -555,12 +557,12 @@ impl Module { } else if inst_category == spec::InstructionCategory::Type { assert!(inst.result_type_id.is_none()); let id = inst.result_id.unwrap(); - let type_ctor_args = inst + let type_and_const_inputs = inst .ids .iter() .map(|&id| match id_defs.get(&id) { - Some(&IdDef::Type(ty)) => Ok(TypeCtorArg::Type(ty)), - Some(&IdDef::Const(ct)) => Ok(TypeCtorArg::Const(ct)), + Some(&IdDef::Type(ty)) => Ok(TypeOrConst::Type(ty)), + Some(&IdDef::Const(ct)) => Ok(TypeOrConst::Const(ct)), Some(id_def) => Err(id_def.descr(&cx)), None => Err(format!("a forward reference to %{id}")), }) @@ -573,8 +575,7 @@ impl Module { let ty = cx.intern(TypeDef { attrs: mem::take(&mut attrs), - ctor: TypeCtor::SpvInst(inst.without_ids), - ctor_args: type_ctor_args, + kind: TypeKind::SpvInst { spv_inst: inst.without_ids, type_and_const_inputs }, }); id_defs.insert(id, IdDef::Type(ty)); @@ -696,19 +697,19 @@ impl Module { let (func_type_ret_type, func_type_param_types) = match id_defs.get(&func_type_id) { - Some(&IdDef::Type(ty)) => { - let ty_def = &cx[ty]; - match &ty_def.ctor { - TypeCtor::SpvInst(inst) if inst.opcode == wk.OpTypeFunction => { - let mut types = ty_def.ctor_args.iter().map(|&arg| match arg { - TypeCtorArg::Type(ty) => ty, - TypeCtorArg::Const(_) => unreachable!(), + Some(&IdDef::Type(ty)) => match &cx[ty].kind { + TypeKind::SpvInst { spv_inst, type_and_const_inputs } + if spv_inst.opcode == wk.OpTypeFunction => + { + let mut types = + type_and_const_inputs.iter().map(|&ty_or_ct| match ty_or_ct { + TypeOrConst::Type(ty) => ty, + TypeOrConst::Const(_) => unreachable!(), }); - Some((types.next().unwrap(), types)) - } - _ => None, + Some((types.next().unwrap(), types)) } - } + _ => None, + }, _ => None, } .ok_or_else(|| { @@ -1037,14 +1038,9 @@ impl Module { // HACK(eddyb) intern `OpString`s as `Const`s on // the fly, as it's a less likely usage than the // `OpLine` one. - let ty = cx.intern(TypeDef { - attrs: AttrSet::default(), - ctor: TypeCtor::SpvStringLiteralForExtInst, - ctor_args: [].into_iter().collect(), - }); let ct = cx.intern(ConstDef { attrs: AttrSet::default(), - ty, + ty: cx.intern(TypeKind::SpvStringLiteralForExtInst), kind: ConstKind::SpvStringLiteralForExtInst(*s), }); Ok(LocalIdDef::Value(Value::Const(ct))) diff --git a/src/transform.rs b/src/transform.rs index dc273ed..6b97b8a 100644 --- a/src/transform.rs +++ b/src/transform.rs @@ -8,7 +8,7 @@ use crate::{ 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, + OrdAssertEq, SelectionKind, Type, TypeDef, TypeKind, TypeOrConst, Value, }; use std::cmp::Ordering; use std::rc::Rc; @@ -419,28 +419,32 @@ impl InnerTransform for QPtrMemUsageKind { impl InnerTransform for TypeDef { fn inner_transform_with(&self, transformer: &mut impl Transformer) -> Transformed { - let Self { attrs, ctor, ctor_args } = self; + let Self { attrs, kind } = self; transform!({ attrs -> transformer.transform_attr_set_use(*attrs), - ctor -> match ctor { - TypeCtor::QPtr - | TypeCtor::SpvInst(_) - | TypeCtor::SpvStringLiteralForExtInst => Transformed::Unchanged, + kind -> match kind { + TypeKind::QPtr | TypeKind::SpvStringLiteralForExtInst => Transformed::Unchanged, + + TypeKind::SpvInst { spv_inst, type_and_const_inputs } => Transformed::map_iter( + type_and_const_inputs.iter(), + |ty_or_ct| match *ty_or_ct { + TypeOrConst::Type(ty) => transform!({ + ty -> transformer.transform_type_use(ty), + } => TypeOrConst::Type(ty)), + + TypeOrConst::Const(ct) => transform!({ + ct -> transformer.transform_const_use(ct), + } => TypeOrConst::Const(ct)), + }, + ).map(|new_iter| TypeKind::SpvInst { + spv_inst: spv_inst.clone(), + type_and_const_inputs: new_iter.collect(), + }), }, - ctor_args -> Transformed::map_iter(ctor_args.iter(), |arg| match *arg { - TypeCtorArg::Type(ty) => transform!({ - ty -> transformer.transform_type_use(ty), - } => TypeCtorArg::Type(ty)), - - TypeCtorArg::Const(ct) => transform!({ - ct -> transformer.transform_const_use(ct), - } => TypeCtorArg::Const(ct)), - }).map(|new_iter| new_iter.collect()), } => Self { attrs, - ctor, - ctor_args, + kind, }) } } diff --git a/src/visit.rs b/src/visit.rs index 962b654..7bb837d 100644 --- a/src/visit.rs +++ b/src/visit.rs @@ -8,7 +8,7 @@ use crate::{ 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, + SelectionKind, Type, TypeDef, TypeKind, TypeOrConst, Value, }; // FIXME(eddyb) `Sized` bound shouldn't be needed but removing it requires @@ -311,16 +311,19 @@ impl InnerVisit for QPtrMemUsageKind { impl InnerVisit for TypeDef { fn inner_visit_with<'a>(&'a self, visitor: &mut impl Visitor<'a>) { - let Self { attrs, ctor, ctor_args } = self; + let Self { attrs, kind } = self; visitor.visit_attr_set_use(*attrs); - match ctor { - TypeCtor::QPtr | TypeCtor::SpvInst(_) | TypeCtor::SpvStringLiteralForExtInst => {} - } - for &arg in ctor_args { - match arg { - TypeCtorArg::Type(ty) => visitor.visit_type_use(ty), - TypeCtorArg::Const(ct) => visitor.visit_const_use(ct), + match kind { + TypeKind::QPtr | TypeKind::SpvStringLiteralForExtInst => {} + + TypeKind::SpvInst { spv_inst: _, type_and_const_inputs } => { + for &ty_or_ct in type_and_const_inputs { + match ty_or_ct { + TypeOrConst::Type(ty) => visitor.visit_type_use(ty), + TypeOrConst::Const(ct) => visitor.visit_const_use(ct), + } + } } } } From e9aa0d1aa26c824f19114a510d1c383cc48c8ef6 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Fri, 27 Oct 2023 19:53:47 +0300 Subject: [PATCH 4/4] Update CHANGELOG. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4546176..293e74f 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#51](https://github.com/EmbarkStudios/spirt/pull/51) combined `TypeCtor`/`ConstCtor` + and their respective "ctor args", into a single unified `TypeKind`/`ConstKind` - [PR#48](https://github.com/EmbarkStudios/spirt/pull/48) changed CFG structurization from "maximal loops" to "minimal loops" (computed using Tarjan's SCC algorithm), and added `OpLoopMerge` support on top (by extending a "minimal loop" as needed)