From 7782749763d287ee9c881c30dd9af98a32d071fe Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 9 Sep 2025 11:42:12 +0300 Subject: [PATCH 1/4] spv/lower: don't assign IDs ahead of time. --- src/spv/lower.rs | 213 ++++++++++++++++++++++------------------------- 1 file changed, 98 insertions(+), 115 deletions(-) diff --git a/src/spv/lower.rs b/src/spv/lower.rs index bbed5e1e..04de27fe 100644 --- a/src/spv/lower.rs +++ b/src/spv/lower.rs @@ -921,12 +921,6 @@ impl Module { let func_decl = &mut module.funcs[func]; - #[derive(Copy, Clone)] - enum LocalIdDef { - Value(Value), - BlockLabel(Region), - } - #[derive(PartialEq, Eq, Hash)] struct PhiKey { source_block_id: spv::Id, @@ -943,9 +937,7 @@ impl Module { cfgssa_inter_block_uses: FxIndexMap, } - // Index IDs declared within the function, first. - let mut local_id_defs = FxIndexMap::default(); - // `OpPhi`s are also collected here, to assign them per-edge. + // Gather `OpLabel`s and `OpPhi`s early (so they can be random-accessed). let mut phi_to_values = FxIndexMap::>::default(); // FIXME(eddyb) wouldn't `EntityOrientedDenseMap` make more sense? let mut block_details = FxIndexMap::::default(); @@ -968,7 +960,6 @@ impl Module { }) }; { - let mut next_param_idx = 0u32; for raw_inst in &raw_insts { let IntraFuncInst { without_ids: spv::Inst { opcode, ref imms }, @@ -977,95 +968,61 @@ impl Module { .. } = *raw_inst; - if let Some(id) = result_id { - let local_id_def = if opcode == wk.OpFunctionParameter { - let idx = next_param_idx; - next_param_idx = idx.checked_add(1).unwrap(); + if opcode == wk.OpFunctionParameter { + continue; + } - let body = match &func_decl.def { - // `LocalIdDef`s not needed for declarations. - DeclDef::Imported(_) => continue, + let is_entry_block = !has_blocks; + has_blocks = true; - DeclDef::Present(def) => def.body, - }; - LocalIdDef::Value(Value::RegionInput { region: body, input_idx: idx }) + let func_def_body = match &mut func_decl.def { + // Error will be emitted later, below. + DeclDef::Imported(_) => continue, + DeclDef::Present(def) => def, + }; + + if opcode == wk.OpLabel { + let block = if is_entry_block { + // A `Region` was defined earlier, + // to be able to create the `FuncDefBody`. + func_def_body.body } else { - let is_entry_block = !has_blocks; - has_blocks = true; + func_def_body.regions.define(&cx, RegionDef::default()) + }; + block_details.insert( + block, + BlockDetails { + label_id: result_id.unwrap(), + phi_count: 0, + cfgssa_inter_block_uses: Default::default(), + }, + ); + } else if opcode == wk.OpPhi { + let (_, block_details) = match block_details.last_mut() { + Some(entry) => entry, + // Error will be emitted later, below. + None => continue, + }; - let func_def_body = match &mut func_decl.def { - // Error will be emitted later, below. - DeclDef::Imported(_) => continue, - DeclDef::Present(def) => def, - }; + let phi_idx = block_details.phi_count; + block_details.phi_count = phi_idx.checked_add(1).unwrap(); + let phi_idx = u32::try_from(phi_idx).unwrap(); - if opcode == wk.OpLabel { - let block = if is_entry_block { - // A `Region` was defined earlier, - // to be able to create the `FuncDefBody`. - func_def_body.body - } else { - func_def_body.regions.define(&cx, RegionDef::default()) - }; - block_details.insert( - block, - BlockDetails { - label_id: id, - phi_count: 0, - cfgssa_inter_block_uses: Default::default(), - }, - ); - LocalIdDef::BlockLabel(block) - } else if opcode == wk.OpPhi { - let (¤t_block, block_details) = match block_details.last_mut() - { - Some(entry) => entry, - // Error will be emitted later, below. - None => continue, - }; - - let phi_idx = block_details.phi_count; - block_details.phi_count = phi_idx.checked_add(1).unwrap(); - let phi_idx = u32::try_from(phi_idx).unwrap(); - - assert!(imms.is_empty()); - // FIXME(eddyb) use `array_chunks` when that's stable. - for value_and_source_block_id in raw_inst.ids.chunks(2) { - let &[value_id, source_block_id]: &[_; 2] = - value_and_source_block_id.try_into().unwrap(); - - phi_to_values - .entry(PhiKey { - source_block_id, - target_block_id: block_details.label_id, - target_phi_idx: phi_idx, - }) - .or_default() - .push(value_id); - } + assert!(imms.is_empty()); + // FIXME(eddyb) use `array_chunks` when that's stable. + for value_and_source_block_id in raw_inst.ids.chunks(2) { + let &[value_id, source_block_id]: &[_; 2] = + value_and_source_block_id.try_into().unwrap(); - LocalIdDef::Value(Value::RegionInput { - region: current_block, - input_idx: phi_idx, + phi_to_values + .entry(PhiKey { + source_block_id, + target_block_id: block_details.label_id, + target_phi_idx: phi_idx, }) - } else { - // HACK(eddyb) can't get a `DataInst` without - // defining it (as a dummy) first. - let inst = func_def_body.nodes.define( - &cx, - DataInstDef { - attrs: AttrSet::default(), - kind: DataInstKind::SpvInst(wk.OpNop.into()), - inputs: [].into_iter().collect(), - child_regions: [].into_iter().collect(), - outputs: [].into_iter().collect(), - } - .into(), - ); - LocalIdDef::Value(Value::NodeOutput { node: inst, output_idx: 0 }) - } - }; - local_id_defs.insert(id, local_id_def); + .or_default() + .push(value_id); + } } if let Some(def_map) = &mut cfgssa_def_map @@ -1121,6 +1078,21 @@ impl Module { None }; + #[derive(Copy, Clone)] + enum LocalIdDef { + Value(Value), + BlockLabel(Region), + } + + let mut local_id_defs = FxIndexMap::default(); + + // Labels can be forward-referenced, so always have them present. + local_id_defs.extend( + block_details + .iter() + .map(|(®ion, details)| (details.label_id, LocalIdDef::BlockLabel(region))), + ); + // HACK(eddyb) an entire separate traversal is required to find // all inter-block uses, before any blocks get lowered to SPIR-T. let mut cfgssa_use_accumulator = cfgssa_def_map @@ -1260,6 +1232,9 @@ impl Module { "unsupported use of {} outside `OpExtInst`", id_def.descr(&cx), ))), + // FIXME(eddyb) scan the rest of the function for any + // instructions returning this ID, to report an invalid + // forward reference (use before def). None | Some(IdDef::FuncForwardRef(_)) => local_id_defs .get(&id) .copied() @@ -1279,11 +1254,17 @@ impl Module { let ty = result_type.unwrap(); params.push(FuncParam { attrs, ty }); if let Some(func_def_body) = &mut func_def_body { - func_def_body - .at_mut_body() - .def() - .inputs - .push(RegionInputDecl { attrs, ty }); + let body_inputs = &mut func_def_body.at_mut_body().def().inputs; + let input_idx = u32::try_from(body_inputs.len()).unwrap(); + body_inputs.push(RegionInputDecl { attrs, ty }); + + local_id_defs.insert( + result_id.unwrap(), + LocalIdDef::Value(Value::RegionInput { + region: func_def_body.body, + input_idx, + }), + ); } continue; } @@ -1297,8 +1278,7 @@ impl Module { return Err(invalid("block lacks terminator instruction")); } - // A `Region` (using an empty `Block` `Node` - // as its sole child) was defined earlier, + // An empty `Region` was defined earlier, // to be able to have an entry in `local_id_defs`. let region = match local_id_defs[&result_id.unwrap()] { LocalIdDef::BlockLabel(region) => region, @@ -1341,10 +1321,11 @@ impl Module { current_block.details.cfgssa_inter_block_uses.iter().map( |(&used_id, &ty)| { let input_idx = - current_block_region_def.inputs.len().try_into().unwrap(); + u32::try_from(current_block_region_def.inputs.len()).unwrap(); current_block_region_def .inputs .push(RegionInputDecl { attrs: AttrSet::default(), ty }); + ( used_id, LocalIdDef::Value(Value::RegionInput { @@ -1515,9 +1496,18 @@ impl Module { )); } - current_block_region_def - .inputs - .push(RegionInputDecl { attrs, ty: result_type.unwrap() }); + let ty = result_type.unwrap(); + + let input_idx = u32::try_from(current_block_region_def.inputs.len()).unwrap(); + current_block_region_def.inputs.push(RegionInputDecl { attrs, ty }); + + local_id_defs.insert( + result_id.unwrap(), + LocalIdDef::Value(Value::RegionInput { + region: current_block.region, + input_idx, + }), + ); } else if [wk.OpSelectionMerge, wk.OpLoopMerge].contains(&opcode) { let is_second_to_last_in_block = lookahead_raw_inst(2) .is_none_or(|next_raw_inst| next_raw_inst.without_ids.opcode == wk.OpLabel); @@ -1643,19 +1633,12 @@ impl Module { }) .collect(), }; - let inst = match result_id { - Some(id) => match local_id_defs[&id] { - LocalIdDef::Value(Value::NodeOutput { node: inst, .. }) => { - // A dummy was defined earlier, to be able to - // have an entry in `local_id_defs`. - func_def_body.nodes[inst] = data_inst_def.into(); - inst - } - _ => unreachable!(), - }, - None => func_def_body.nodes.define(&cx, data_inst_def.into()), - }; + let inst = func_def_body.nodes.define(&cx, data_inst_def.into()); + if let Some(result_id) = result_id { + let output = Value::NodeOutput { node: inst, output_idx: 0 }; + local_id_defs.insert(result_id, LocalIdDef::Value(output)); + } current_block_region_def.children.insert_last(inst, &mut func_def_body.nodes); } From c7c932625c9969c0c72acf5bc2602dcca336699f Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 9 Sep 2025 11:42:12 +0300 Subject: [PATCH 2/4] WIP: refactor non-`Const` `Value` variants into `VarKind`. --- src/cf/structurize.rs | 51 ++++++++++++++++++++++-------------------- src/cf/unstructured.rs | 2 +- src/func_at.rs | 15 ++++++++++--- src/lib.rs | 32 ++++++++++++++------------ src/mem/analyze.rs | 39 ++++++++++++++++++++------------ src/print/mod.rs | 31 +++++++++++++++---------- src/qptr/lift.rs | 22 ++++++++++-------- src/qptr/lower.rs | 6 ++--- src/spv/lift.rs | 11 +++++---- src/spv/lower.rs | 16 ++++++------- src/transform.rs | 11 ++++++++- src/visit.rs | 12 ++++++++-- 12 files changed, 151 insertions(+), 97 deletions(-) diff --git a/src/cf/structurize.rs b/src/cf/structurize.rs index d566595f..64c43384 100644 --- a/src/cf/structurize.rs +++ b/src/cf/structurize.rs @@ -10,7 +10,7 @@ use crate::transform::{InnerInPlaceTransform as _, Transformer}; use crate::{ AttrSet, Const, ConstDef, ConstKind, Context, DbgSrcLoc, EntityOrientedDenseMap, FuncDefBody, FxIndexMap, FxIndexSet, Node, NodeDef, NodeKind, NodeOutputDecl, Region, RegionDef, Type, - TypeKind, Value, spv, + TypeKind, Value, VarKind, spv, }; use itertools::{Either, Itertools}; use smallvec::SmallVec; @@ -72,20 +72,20 @@ pub struct Structurizer<'a> { structurize_region_state: FxIndexMap, /// Accumulated rewrites (caused by e.g. `target_inputs`s, but not only), - /// i.e.: `Value::RegionInput { region, input_idx }` must be + /// i.e.: `VarKind::RegionInput { region, input_idx }` must be /// rewritten based on `region_input_rewrites[region]`, as either /// the original `region` wasn't reused, or its inputs were renumbered. region_input_rewrites: EntityOrientedDenseMap, } -/// How all `Value::RegionInput { region, input_idx }` for a `region` +/// How all `VarKind::RegionInput { region, input_idx }` for a `region` /// must be rewritten (see also `region_input_rewrites` docs). enum RegionInputRewrites { /// Complete replacement with another value (which can take any form), as /// `region` wasn't kept in its original form in the final structured IR. /// /// **Note**: such replacement can be chained, i.e. a replacement value can - /// be `Value::RegionInput { region: other_region, .. }`, and then + /// be `VarKind::RegionInput { region: other_region, .. }`, and then /// `other_region` itself may have its inputs written. ReplaceWith(SmallVec<[Value; 2]>), @@ -119,7 +119,7 @@ impl RegionInputRewrites { ReplaceValueWith(move |v| { let mut new_v = v; - while let Value::RegionInput { region, input_idx } = new_v { + while let Value::Var(VarKind::RegionInput { region, input_idx }) = new_v { match rewrites.get(region) { // NOTE(eddyb) this needs to be able to apply multiple replacements, // due to the input potentially having redundantly chained `OpPhi`s. @@ -135,7 +135,7 @@ impl RegionInputRewrites { renumbering_and_replacements, )) => match renumbering_and_replacements[input_idx as usize] { Ok(new_idx) => { - new_v = Value::RegionInput { region, input_idx: new_idx }; + new_v = Value::Var(VarKind::RegionInput { region, input_idx: new_idx }); break; } Err(replacement) => new_v = replacement, @@ -201,7 +201,7 @@ struct IncomingEdgeBundle { target: T, accumulated_count: IncomingEdgeCount, - /// The [`Value`]s that `Value::RegionInput { region, .. }` will get + /// The [`Value`]s that `VarKind::RegionInput { region, .. }` will get /// on entry into `region`, through this "edge bundle". target_inputs: SmallVec<[Value; 2]>, } @@ -268,7 +268,7 @@ impl DeferredEdgeBundle { /// A recipe for computing a control-flow-sensitive (boolean) condition [`Value`], /// potentially requiring merging through an arbitrary number of `Select`s -/// (via per-case outputs and [`Value::NodeOutput`], for each `Select`). +/// (via per-case outputs and [`VarKind::NodeOutput`], for each `Select`). /// /// This should largely be equivalent to eagerly generating all region outputs /// that might be needed, and then removing the unused ones, but this way we @@ -573,11 +573,11 @@ struct ClaimedRegion { // perspective of being "inside" `structured_body` (wrt hermeticity). structured_body: Region, - /// The [`Value`]s that `Value::RegionInput { region: structured_body, .. }` + /// The [`Value`]s that `VarKind::RegionInput { region: structured_body, .. }` /// will get on entry into `structured_body`, when this region ends up /// merged into a larger region, or as a child of a new [`Node`]. // - // FIXME(eddyb) don't replace `Value::RegionInput { region: structured_body, .. }` + // FIXME(eddyb) don't replace `VarKind::RegionInput { region: structured_body, .. }` // with `region_inputs` when `structured_body` ends up a `Node` child, // but instead make all `Region`s entirely hermetic wrt inputs. structured_body_inputs: SmallVec<[Value; 2]>, @@ -768,7 +768,7 @@ impl<'a> Structurizer<'a> { // while structurizing (i.e. `region_input_rewrites`). // // FIXME(eddyb) obsolete this by fully taking advantage of hermeticity, - // and only replacing `Value::RegionInput { region, .. }` within + // and only replacing `VarKind::RegionInput { region, .. }` within // `region`'s children, shallowly, whenever `region` gets claimed. self.func_def_body.inner_in_place_transform_with(&mut RegionInputRewrites::rewrite_all( &self.region_input_rewrites, @@ -880,21 +880,24 @@ impl<'a> Structurizer<'a> { let original_idx = u32::try_from(original_idx).unwrap(); if backedge_value - == (Value::RegionInput { region: body, input_idx: original_idx }) + == Value::Var(VarKind::RegionInput { + region: body, + input_idx: original_idx, + }) { // FIXME(eddyb) does this have to be general purpose, // or could this be handled as `None` with a single // `wrapper_region` per `RegionInputRewrites`? - Err(Value::RegionInput { + Err(Value::Var(VarKind::RegionInput { region: wrapper_region, input_idx: original_idx, - }) + })) } else { let renumbered_idx = u32::try_from(body_def.inputs.len()).unwrap(); - initial_inputs.push(Value::RegionInput { + initial_inputs.push(Value::Var(VarKind::RegionInput { region: wrapper_region, input_idx: original_idx, - }); + })); body_def.inputs.push(original_input_decls[original_idx as usize]); body_def.outputs.push(backedge_value); Ok(renumbered_idx) @@ -1329,7 +1332,7 @@ impl<'a> Structurizer<'a> { }; // Support lazily defining the `Select` node, as soon as it's necessary - // (i.e. to plumb per-case dataflow through `Value::NodeOutput`s), + // (i.e. to plumb per-case dataflow through `VarKind::NodeOutput`s), // but also if any of the cases actually have non-empty regions, which // is checked after the special-cases (which return w/o a `Select` at all). // @@ -1419,7 +1422,7 @@ impl<'a> Structurizer<'a> { // only get applied after structurization fully completes, but here it's // very useful to have the fully resolved values across all `cases`' // incoming/outgoing edges (note, however, that within outgoing edges, - // i.e. `case_deferred_edges`' `target_inputs`, `Value::RegionInput` + // i.e. `case_deferred_edges`' `target_inputs`, `VarKind::RegionInput` // are not resolved using the contents of `case_structured_body_inputs`, // which is kept hermetic until just before `structurize_select` returns). for case in &mut cases { @@ -1496,12 +1499,12 @@ impl<'a> Structurizer<'a> { .. }) => match v { Value::Const(_) => Ok(v), - Value::RegionInput { region, input_idx } + Value::Var(VarKind::RegionInput { region, input_idx }) if region == *structured_body => { Ok(structured_body_inputs[input_idx as usize]) } - _ => Err(()), + Value::Var(_) => Err(()), }, // `case` has no region of its own, so everything @@ -1544,10 +1547,10 @@ impl<'a> Structurizer<'a> { assert_eq!(outputs.len(), output_idx); outputs.push(v); } - Value::NodeOutput { + Value::Var(VarKind::NodeOutput { node: select_node, output_idx: output_idx.try_into().unwrap(), - } + }) }) .collect(); @@ -1596,7 +1599,7 @@ impl<'a> Structurizer<'a> { // Only as the very last step, can per-case `region_inputs` be added to // `region_input_rewrites`. // - // FIXME(eddyb) don't replace `Value::RegionInput { region, .. }` + // FIXME(eddyb) don't replace `VarKind::RegionInput { region, .. }` // with `region_inputs` when the `region` ends up a `Node` child, // but instead make all `Region`s entirely hermetic wrt inputs. #[allow(clippy::manual_flatten)] @@ -1678,7 +1681,7 @@ impl<'a> Structurizer<'a> { assert_eq!(outputs.len(), output_decls.len()); } - Value::NodeOutput { node, output_idx } + Value::Var(VarKind::NodeOutput { node, output_idx }) } } } diff --git a/src/cf/unstructured.rs b/src/cf/unstructured.rs index de67e27c..af6a6254 100644 --- a/src/cf/unstructured.rs +++ b/src/cf/unstructured.rs @@ -30,7 +30,7 @@ pub struct ControlInst { pub targets: SmallVec<[Region; 4]>, /// `target_inputs[region][input_idx]` is the [`Value`] that - /// `Value::RegionInput { region, input_idx }` will get on entry, + /// `VarKind::RegionInput { region, input_idx }` will get on entry, /// where `region` must be appear at least once in `targets` - this is a /// separate map instead of being part of `targets` because it reflects the /// limitations of φ ("phi") nodes, which (unlike "basic block arguments") diff --git a/src/func_at.rs b/src/func_at.rs index 4d506fb2..a45d16ba 100644 --- a/src/func_at.rs +++ b/src/func_at.rs @@ -13,7 +13,7 @@ use crate::{ Context, EntityDefs, EntityList, EntityListIter, FuncDefBody, Node, NodeDef, Region, RegionDef, - Type, Value, + Type, Value, Var, VarKind, }; /// Immutable traversal (i.e. visiting) helper for intra-function entities. @@ -81,10 +81,19 @@ impl FuncAt<'_, Value> { pub fn type_of(self, cx: &Context) -> Type { match self.position { Value::Const(ct) => cx[ct].ty, - Value::RegionInput { region, input_idx } => { + Value::Var(var) => self.at(var).type_of(), + } + } +} + +impl FuncAt<'_, Var> { + /// Return the [`Type`] of this [`Var`]. + pub fn type_of(self) -> Type { + match self.position { + VarKind::RegionInput { region, input_idx } => { self.at(region).def().inputs[input_idx as usize].ty } - Value::NodeOutput { node, output_idx } => { + VarKind::NodeOutput { node, output_idx } => { self.at(node).def().outputs[output_idx as usize].ty } } diff --git a/src/lib.rs b/src/lib.rs index a73ad782..e41100e1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -706,7 +706,7 @@ pub struct FuncDefBody { /// The [`Region`] representing the whole body of the function. /// /// Function parameters are provided via `body.inputs`, i.e. they can be - /// only accessed with `Value::RegionInputs { region: body, idx }`. + /// only accessed with `VarKind::RegionInput { region: body, idx }`. /// /// When `unstructured_cfg` is `None`, this includes the structured return /// of the function, with `body.outputs` as the returned values. @@ -814,7 +814,7 @@ pub use context::Region; #[derive(Clone, Default)] pub struct RegionDef { /// Inputs to this [`Region`]: - /// * accessed using [`Value::RegionInput`] + /// * accessed using [`VarKind::RegionInput`] /// * values provided by the parent: /// * when this is the function body: the function's parameters pub inputs: SmallVec<[RegionInputDecl; 2]>, @@ -824,11 +824,11 @@ pub struct RegionDef { /// Output values from this [`Region`], provided to the parent: /// * when this is the function body: these are the structured return values /// * when this is a `Select` case: these are the values for the parent - /// [`Node`]'s outputs (accessed using [`Value::NodeOutput`]) + /// [`Node`]'s outputs (accessed using [`VarKind::NodeOutput`]) /// * when this is a `Loop` body: these are the values to be used for the /// next loop iteration's body `inputs` - /// * **not** accessible through [`Value::NodeOutput`] on the `Loop`, - /// as it's both confusing regarding [`Value::RegionInput`], and + /// * **not** accessible through [`VarKind::NodeOutput`] on the `Loop`, + /// as it's both confusing regarding [`VarKind::RegionInput`], and /// also there's nothing stopping body-defined values from directly being /// used outside the loop (once that changes, this aspect can be flipped) pub outputs: SmallVec<[Value; 2]>, @@ -863,7 +863,7 @@ pub struct NodeDef { pub child_regions: SmallVec<[Region; 2]>, /// Outputs from this [`Node`]: - /// * accessed using [`Value::NodeOutput`] + /// * accessed using [`VarKind::NodeOutput`] /// * values provided by `region.outputs`, where `region` is the executed /// child [`Region`]: /// * when this is a `Select`: the case that was chosen @@ -942,15 +942,22 @@ pub type DataInstKind = NodeKind; #[derive(Copy, Clone, PartialEq, Eq, Hash)] pub enum Value { Const(Const), + Var(Var), +} + +type Var = VarKind; +// FIXME(eddyb) document the fact that "variable" is used here in a sense +// more like e.g. math/lambda calculus/SSA/Rust immutable variables, +// and *not* some sort of "mutable slot" (like e.g. wasm local variables), +// also mention `GlobalVar`/`mem::MemOp::FuncLocalVar`. +#[derive(Copy, Clone, PartialEq, Eq, Hash)] +pub enum VarKind { /// One of the inputs to a [`Region`]: /// * declared by `region.inputs[input_idx]` /// * value provided by the parent of the `region`: /// * when `region` is the function body: `input_idx`th function parameter - RegionInput { - region: Region, - input_idx: u32, - }, + RegionInput { region: Region, input_idx: u32 }, /// One of the outputs produced by a [`Node`]: /// * declared by `node.outputs[output_idx]` @@ -958,8 +965,5 @@ pub enum Value { /// executed child [`Region`] (of `node`): /// * when `node` is a `Select`: the case that was chosen // TODO(eddyb) include former `DataInst`s in above docs. - NodeOutput { - node: Node, - output_idx: u32, - }, + NodeOutput { node: Node, output_idx: u32 }, } diff --git a/src/mem/analyze.rs b/src/mem/analyze.rs index 83f74d9c..eba4f889 100644 --- a/src/mem/analyze.rs +++ b/src/mem/analyze.rs @@ -9,7 +9,7 @@ use crate::visit::{InnerVisit, Visitor}; use crate::{ AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstKind, Context, DataInstKind, DeclDef, Diag, ExportKey, Exportee, Func, FxIndexMap, GlobalVar, Module, Node, NodeKind, OrdAssertEq, Type, - TypeKind, Value, + TypeKind, Value, VarKind, }; use itertools::{Either, Itertools as _}; use rustc_hash::FxHashMap; @@ -776,11 +776,11 @@ impl<'a> GatherAccesses<'a> { for (v, accesses) in accesses_or_err_attrs_to_attach { let attrs = match v { Value::Const(_) => unreachable!(), - Value::RegionInput { region, input_idx } => { + Value::Var(VarKind::RegionInput { region, input_idx }) => { &mut func_def_body.at_mut(region).def().inputs[input_idx as usize] .attrs } - Value::NodeOutput { node, output_idx } => { + Value::Var(VarKind::NodeOutput { node, output_idx }) => { let node_def = func_def_body.at_mut(node).def(); // HACK(eddyb) `NodeOutput { output_idx: !0, .. }` @@ -886,16 +886,18 @@ impl<'a> GatherAccesses<'a> { // FIXME(eddyb) may be relevant? _ => unreachable!(), }, - Value::RegionInput { region, input_idx } if region == func_def_body.body => { + Value::Var(VarKind::RegionInput { region, input_idx }) + if region == func_def_body.body => + { &mut param_accesses[input_idx as usize] } - Value::RegionInput { .. } => { + Value::Var(VarKind::RegionInput { .. }) => { // FIXME(eddyb) don't throw away `new_accesses`. accesses_or_err_attrs_to_attach .push((ptr, Err(AnalysisError(Diag::bug(["unsupported φ".into()]))))); return; } - Value::NodeOutput { node: ptr_node, output_idx } => { + Value::Var(VarKind::NodeOutput { node: ptr_node, output_idx }) => { let i = output_idx as usize; let slots = node_to_per_output_accesses.entry(ptr_node).or_default(); if i >= slots.len() { @@ -917,7 +919,10 @@ impl<'a> GatherAccesses<'a> { match &node_def.kind { NodeKind::Select(_) | NodeKind::Loop { .. } | NodeKind::ExitInvocation { .. } => { for (i, accesses) in per_output_accesses.iter().enumerate() { - let output = Value::NodeOutput { node, output_idx: i.try_into().unwrap() }; + let output = Value::Var(VarKind::NodeOutput { + node, + output_idx: i.try_into().unwrap(), + }); if let Some(_accesses) = accesses { // FIXME(eddyb) don't throw away `accesses`. accesses_or_err_attrs_to_attach.push(( @@ -961,7 +966,7 @@ impl<'a> GatherAccesses<'a> { } FuncGatherAccessesState::InProgress => { accesses_or_err_attrs_to_attach.push(( - Value::NodeOutput { node, output_idx: 0 }, + Value::Var(VarKind::NodeOutput { node, output_idx: 0 }), Err(AnalysisError(Diag::bug( ["unsupported recursive call".into()], ))), @@ -973,15 +978,19 @@ impl<'a> GatherAccesses<'a> { .is_some_and(|o| is_qptr(o.ty)) && let Some(accesses) = output_accesses { - accesses_or_err_attrs_to_attach - .push((Value::NodeOutput { node, output_idx: 0 }, accesses)); + accesses_or_err_attrs_to_attach.push(( + Value::Var(VarKind::NodeOutput { node, output_idx: 0 }), + accesses, + )); } } DataInstKind::Mem(MemOp::FuncLocalVar(_)) => { if let Some(accesses) = output_accesses { - accesses_or_err_attrs_to_attach - .push((Value::NodeOutput { node, output_idx: 0 }, accesses)); + accesses_or_err_attrs_to_attach.push(( + Value::Var(VarKind::NodeOutput { node, output_idx: 0 }), + accesses, + )); } } DataInstKind::QPtr(QPtrOp::HandleArrayIndex) => { @@ -1291,8 +1300,10 @@ impl<'a> GatherAccesses<'a> { if has_from_spv_ptr_output_attr { // FIXME(eddyb) merge with `FromSpvPtrOutput`'s `pointee`. if let Some(accesses) = output_accesses { - accesses_or_err_attrs_to_attach - .push((Value::NodeOutput { node, output_idx: 0 }, accesses)); + accesses_or_err_attrs_to_attach.push(( + Value::Var(VarKind::NodeOutput { node, output_idx: 0 }), + accesses, + )); } } } diff --git a/src/print/mod.rs b/src/print/mod.rs index e5d968ab..53e034e8 100644 --- a/src/print/mod.rs +++ b/src/print/mod.rs @@ -31,7 +31,7 @@ use crate::{ EntityOrientedDenseMap, ExportKey, Exportee, Func, FuncDecl, FuncDefBody, FuncParam, FxIndexMap, FxIndexSet, GlobalVar, GlobalVarDecl, GlobalVarDefBody, Import, InternedStr, Module, ModuleDebugInfo, ModuleDialect, Node, NodeDef, NodeKind, NodeOutputDecl, OrdAssertEq, - Region, RegionDef, RegionInputDecl, Type, TypeDef, TypeKind, TypeOrConst, Value, spv, + Region, RegionDef, RegionInputDecl, Type, TypeDef, TypeKind, TypeOrConst, Value, VarKind, spv, }; use arrayvec::ArrayVec; use itertools::Either; @@ -269,8 +269,12 @@ impl From for Use { fn from(value: Value) -> Self { match value { Value::Const(ct) => Use::CxInterned(CxInterned::Const(ct)), - Value::RegionInput { region, input_idx } => Use::RegionInput { region, input_idx }, - Value::NodeOutput { node, output_idx } => Use::NodeOutput { node, output_idx }, + Value::Var(VarKind::RegionInput { region, input_idx }) => { + Use::RegionInput { region, input_idx } + } + Value::Var(VarKind::NodeOutput { node, output_idx }) => { + Use::NodeOutput { node, output_idx } + } } } } @@ -644,7 +648,7 @@ impl<'a> Visitor<'a> for Plan<'a> { fn visit_value_use(&mut self, v: &'a Value) { match *v { Value::Const(_) => {} - _ => *self.use_counts.entry(Use::from(*v)).or_default() += 1, + Value::Var(_) => *self.use_counts.entry(Use::from(*v)).or_default() += 1, } v.inner_visit_with(self); } @@ -3530,10 +3534,10 @@ impl Print for FuncDecl { params.iter().enumerate().map(|(i, param)| { let param_name = match def { DeclDef::Imported(_) => "_".into(), - DeclDef::Present(def) => Value::RegionInput { + DeclDef::Present(def) => Value::Var(VarKind::RegionInput { region: def.body, input_idx: i.try_into().unwrap(), - } + }) .print_as_def(printer), }; param.print(printer).insert_name_before_def(param_name) @@ -3566,10 +3570,10 @@ impl Print for FuncDecl { "(", inputs.iter().enumerate().map(|(input_idx, input)| { input.print(printer).insert_name_before_def( - Value::RegionInput { + Value::Var(VarKind::RegionInput { region, input_idx: input_idx.try_into().unwrap(), - } + }) .print_as_def(printer), ) }), @@ -3715,8 +3719,11 @@ impl Print for FuncAt<'_, Node> { let outputs_header = if !outputs.is_empty() { let mut outputs = outputs.iter().enumerate().map(|(output_idx, output)| { output.print(printer).insert_name_before_def( - Value::NodeOutput { node, output_idx: output_idx.try_into().unwrap() } - .print_as_def(printer), + Value::Var(VarKind::NodeOutput { + node, + output_idx: output_idx.try_into().unwrap(), + }) + .print_as_def(printer), ) }); let outputs_lhs = if outputs.len() == 1 { @@ -3771,10 +3778,10 @@ impl Print for FuncAt<'_, Node> { inputs.iter().enumerate().map(|(input_idx, input)| { ( input, - Value::RegionInput { + Value::Var(VarKind::RegionInput { region: body, input_idx: input_idx.try_into().unwrap(), - }, + }), ) }); ( diff --git a/src/qptr/lift.rs b/src/qptr/lift.rs index 40ad4a61..21f113ed 100644 --- a/src/qptr/lift.rs +++ b/src/qptr/lift.rs @@ -8,7 +8,7 @@ use crate::{ AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstDef, ConstKind, Context, DataInst, DataInstDef, DataInstKind, DeclDef, Diag, DiagLevel, EntityDefs, EntityOrientedDenseMap, Func, FuncDecl, FxIndexMap, GlobalVar, GlobalVarDecl, Module, Node, NodeKind, NodeOutputDecl, Region, - Type, TypeDef, TypeKind, TypeOrConst, Value, spv, + Type, TypeDef, TypeKind, TypeOrConst, Value, VarKind, spv, }; use smallvec::SmallVec; use std::cell::Cell; @@ -434,7 +434,7 @@ impl LiftToSpvPtrInstsInFunc<'_> { // FIXME(eddyb) maybe all this data should be packaged up together in a // type with fields like those of `DeferredPtrNoop` (or even more). let type_of_val_as_spv_ptr_with_layout = |v: Value| { - if let Value::NodeOutput { node: v_data_inst, output_idx: 0 } = v + if let Value::Var(VarKind::NodeOutput { node: v_data_inst, output_idx: 0 }) = v && let Some(ptr_noop) = self.deferred_ptr_noops.get(&v_data_inst) { return Ok(( @@ -805,8 +805,10 @@ impl LiftToSpvPtrInstsInFunc<'_> { func.nodes, ); - new_data_inst_def.inputs[input_idx] = - Value::NodeOutput { node: access_chain_data_inst, output_idx: 0 }; + new_data_inst_def.inputs[input_idx] = Value::Var(VarKind::NodeOutput { + node: access_chain_data_inst, + output_idx: 0, + }); } new_data_inst_def @@ -879,8 +881,10 @@ impl LiftToSpvPtrInstsInFunc<'_> { func.nodes, ); - new_data_inst_def.inputs[input_idx] = - Value::NodeOutput { node: access_chain_data_inst, output_idx: 0 }; + new_data_inst_def.inputs[input_idx] = Value::Var(VarKind::NodeOutput { + node: access_chain_data_inst, + output_idx: 0, + }); } if let Some((addr_space, pointee_type)) = from_spv_ptr_output { @@ -1023,7 +1027,7 @@ impl LiftToSpvPtrInstsInFunc<'_> { for v in values { // FIXME(eddyb) the loop could theoretically be avoided, but that'd // make tracking use counts harder. - while let Value::NodeOutput { node: inst, output_idx: 0 } = *v { + while let Value::Var(VarKind::NodeOutput { node: inst, output_idx: 0 }) = *v { match self.deferred_ptr_noops.get(&inst) { Some(ptr_noop) => { *v = ptr_noop.output_pointer; @@ -1038,7 +1042,7 @@ impl LiftToSpvPtrInstsInFunc<'_> { // encoded as `Option` for (dense) map entry reasons. fn add_value_uses(&mut self, values: &[Value]) { for &v in values { - if let Value::NodeOutput { node: inst, .. } = v { + if let Value::Var(VarKind::NodeOutput { node: inst, .. }) = v { let count = self.data_inst_use_counts.entry(inst); *count = Some( NonZeroU32::new(count.map_or(0, |c| c.get()).checked_add(1).unwrap()).unwrap(), @@ -1048,7 +1052,7 @@ impl LiftToSpvPtrInstsInFunc<'_> { } fn remove_value_uses(&mut self, values: &[Value]) { for &v in values { - if let Value::NodeOutput { node: inst, .. } = v { + if let Value::Var(VarKind::NodeOutput { node: inst, .. }) = v { let count = self.data_inst_use_counts.entry(inst); *count = NonZeroU32::new(count.unwrap().get() - 1); } diff --git a/src/qptr/lower.rs b/src/qptr/lower.rs index c5b27aa7..9d771e07 100644 --- a/src/qptr/lower.rs +++ b/src/qptr/lower.rs @@ -7,7 +7,7 @@ use crate::transform::{InnerInPlaceTransform, Transformed, Transformer}; use crate::{ AddrSpace, AttrSet, AttrSetDef, Const, ConstDef, ConstKind, Context, DataInst, DataInstDef, DataInstKind, Diag, FuncDecl, GlobalVarDecl, Node, NodeKind, NodeOutputDecl, OrdAssertEq, - Region, Type, TypeKind, TypeOrConst, Value, spv, + Region, Type, TypeKind, TypeOrConst, Value, VarKind, spv, }; use itertools::Itertools as _; use smallvec::SmallVec; @@ -284,7 +284,7 @@ impl LowerFromSpvPtrInstsInFunc<'_> { let const_idx_as_i32 = |idx| match idx { // FIXME(eddyb) figure out the signedness semantics here. Value::Const(idx) => self.lowerer.const_as_u32(idx).map(|idx_u32| idx_u32 as i32), - _ => None, + Value::Var(_) => None, }; let mut steps: SmallVec<[QPtrChainStep; 4]> = SmallVec::new(); @@ -576,7 +576,7 @@ impl LowerFromSpvPtrInstsInFunc<'_> { func.nodes, ); - ptr = Value::NodeOutput { node: step_data_inst, output_idx: 0 }; + ptr = Value::Var(VarKind::NodeOutput { node: step_data_inst, output_idx: 0 }); } final_step.into_data_inst_kind_and_inputs(ptr) } else if spv_inst.opcode == wk.OpBitcast { diff --git a/src/spv/lift.rs b/src/spv/lift.rs index b3728949..ca3f5fb5 100644 --- a/src/spv/lift.rs +++ b/src/spv/lift.rs @@ -9,7 +9,7 @@ use crate::{ DataInstKind, DbgSrcLoc, DeclDef, ExportKey, Exportee, Func, FuncDecl, FuncParam, FxIndexMap, FxIndexSet, GlobalVar, GlobalVarDefBody, Import, Module, ModuleDebugInfo, ModuleDialect, Node, NodeKind, NodeOutputDecl, OrdAssertEq, Region, RegionInputDecl, Type, TypeDef, TypeKind, - TypeOrConst, Value, + TypeOrConst, Value, VarKind, }; use itertools::Itertools; use rustc_hash::FxHashMap; @@ -263,7 +263,7 @@ struct FuncLifting<'a> { blocks: FxIndexMap>, } -/// What determines the values for [`Value::RegionInput`]s, for a specific +/// What determines the values for [`VarKind::RegionInput`]s, for a specific /// region (effectively the subset of "region parents" that support inputs). /// /// Note that this is not used when a [`cf::unstructured::ControlInst`] has `target_inputs`, @@ -793,8 +793,7 @@ impl<'a> FuncLifting<'a> { } _ => false, }, - - _ => false, + Value::Var(_) => false, }; if is_infinite_loop { Terminator { @@ -1171,7 +1170,7 @@ impl LazyInst<'_, '_> { _ => ids.globals[&Global::Const(ct)], }, - Value::RegionInput { region, input_idx } => { + Value::Var(VarKind::RegionInput { region, input_idx }) => { let input_idx = usize::try_from(input_idx).unwrap(); match parent_func.region_inputs_source.get(®ion) { Some(RegionInputsSource::FuncParams) => parent_func.param_ids[input_idx], @@ -1184,7 +1183,7 @@ impl LazyInst<'_, '_> { } } } - Value::NodeOutput { node, output_idx } => { + Value::Var(VarKind::NodeOutput { node, output_idx }) => { if let Some(&data_inst_output_id) = parent_func.data_inst_output_ids.get(&node) { // HACK(eddyb) multi-output instructions don't exist pre-disaggregate. assert_eq!(output_idx, 0); diff --git a/src/spv/lower.rs b/src/spv/lower.rs index 04de27fe..195fb7a7 100644 --- a/src/spv/lower.rs +++ b/src/spv/lower.rs @@ -8,7 +8,7 @@ use crate::{ DbgSrcLoc, DeclDef, Diag, EntityDefs, ExportKey, Exportee, Func, FuncDecl, FuncDefBody, FuncParam, FxIndexMap, GlobalVarDecl, GlobalVarDefBody, Import, InternedStr, Module, NodeOutputDecl, Region, RegionDef, RegionInputDecl, Type, TypeDef, TypeKind, TypeOrConst, - Value, print, + Value, VarKind, print, }; use rustc_hash::FxHashMap; use smallvec::SmallVec; @@ -1260,10 +1260,10 @@ impl Module { local_id_defs.insert( result_id.unwrap(), - LocalIdDef::Value(Value::RegionInput { + LocalIdDef::Value(Value::Var(VarKind::RegionInput { region: func_def_body.body, input_idx, - }), + })), ); } continue; @@ -1328,10 +1328,10 @@ impl Module { ( used_id, - LocalIdDef::Value(Value::RegionInput { + LocalIdDef::Value(Value::Var(VarKind::RegionInput { region: current_block.region, input_idx, - }), + })), ) }, ), @@ -1503,10 +1503,10 @@ impl Module { local_id_defs.insert( result_id.unwrap(), - LocalIdDef::Value(Value::RegionInput { + LocalIdDef::Value(Value::Var(VarKind::RegionInput { region: current_block.region, input_idx, - }), + })), ); } else if [wk.OpSelectionMerge, wk.OpLoopMerge].contains(&opcode) { let is_second_to_last_in_block = lookahead_raw_inst(2) @@ -1636,7 +1636,7 @@ impl Module { let inst = func_def_body.nodes.define(&cx, data_inst_def.into()); if let Some(result_id) = result_id { - let output = Value::NodeOutput { node: inst, output_idx: 0 }; + let output = Value::Var(VarKind::NodeOutput { node: inst, output_idx: 0 }); local_id_defs.insert(result_id, LocalIdDef::Value(output)); } diff --git a/src/transform.rs b/src/transform.rs index 238fddef..c6b427af 100644 --- a/src/transform.rs +++ b/src/transform.rs @@ -9,7 +9,7 @@ use crate::{ DeclDef, EntityListIter, ExportKey, Exportee, Func, FuncDecl, FuncDefBody, FuncParam, GlobalVar, GlobalVarDecl, GlobalVarDefBody, Import, Module, ModuleDebugInfo, ModuleDialect, Node, NodeDef, NodeKind, NodeOutputDecl, OrdAssertEq, Region, RegionDef, RegionInputDecl, Type, - TypeDef, TypeKind, TypeOrConst, Value, spv, + TypeDef, TypeKind, TypeOrConst, Value, VarKind, spv, }; use std::cmp::Ordering; use std::rc::Rc; @@ -721,7 +721,16 @@ impl InnerTransform for Value { Self::Const(ct) => transform!({ ct -> transformer.transform_const_use(*ct), } => Self::Const(ct)), + Self::Var(var) => transform!({ + var -> var.inner_transform_with(transformer), + } => Self::Var(var)), + } + } +} +impl InnerTransform for VarKind { + fn inner_transform_with(&self, _transformer: &mut impl Transformer) -> Transformed { + match self { Self::RegionInput { region: _, input_idx: _ } | Self::NodeOutput { node: _, output_idx: _ } => Transformed::Unchanged, } diff --git a/src/visit.rs b/src/visit.rs index 77e45c69..30a8c28a 100644 --- a/src/visit.rs +++ b/src/visit.rs @@ -9,7 +9,7 @@ use crate::{ DeclDef, DiagMsgPart, EntityListIter, ExportKey, Exportee, Func, FuncDecl, FuncDefBody, FuncParam, GlobalVar, GlobalVarDecl, GlobalVarDefBody, Import, Module, ModuleDebugInfo, ModuleDialect, Node, NodeDef, NodeKind, NodeOutputDecl, OrdAssertEq, Region, RegionDef, - RegionInputDecl, Type, TypeDef, TypeKind, TypeOrConst, Value, spv, + RegionInputDecl, Type, TypeDef, TypeKind, TypeOrConst, Value, VarKind, spv, }; // FIXME(eddyb) `Sized` bound shouldn't be needed but removing it requires @@ -547,8 +547,16 @@ impl InnerVisit for cf::unstructured::ControlInst { impl InnerVisit for Value { fn inner_visit_with<'a>(&'a self, visitor: &mut impl Visitor<'a>) { + match self { + &Self::Const(ct) => visitor.visit_const_use(ct), + Self::Var(var) => var.inner_visit_with(visitor), + } + } +} + +impl InnerVisit for VarKind { + fn inner_visit_with<'a>(&'a self, _visitor: &mut impl Visitor<'a>) { match *self { - Self::Const(ct) => visitor.visit_const_use(ct), Self::RegionInput { region: _, input_idx: _ } | Self::NodeOutput { node: _, output_idx: _ } => {} } From bf00c5d7980ee4ff14b45c35259b66e129c2cd7c Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 9 Sep 2025 11:42:12 +0300 Subject: [PATCH 3/4] WIP: use `VarKind` instead of `Value` where appropriate. --- src/mem/analyze.rs | 54 +++++++++++++++++++++--------------------- src/print/mod.rs | 58 +++++++++++++++------------------------------- src/qptr/lift.rs | 38 +++++++++++++++++++----------- 3 files changed, 72 insertions(+), 78 deletions(-) diff --git a/src/mem/analyze.rs b/src/mem/analyze.rs index eba4f889..479d03d8 100644 --- a/src/mem/analyze.rs +++ b/src/mem/analyze.rs @@ -653,9 +653,15 @@ impl MemTypeLayout { } } +#[derive(Copy, Clone)] +enum AttrTarget { + Var(VarKind), + Node(Node), +} + struct FuncGatherAccessesResults { param_accesses: SmallVec<[Option>; 2]>, - accesses_or_err_attrs_to_attach: Vec<(Value, Result)>, + accesses_or_err_attrs_to_attach: Vec<(AttrTarget, Result)>, } #[derive(Clone)] @@ -775,22 +781,18 @@ impl<'a> GatherAccesses<'a> { for (v, accesses) in accesses_or_err_attrs_to_attach { let attrs = match v { - Value::Const(_) => unreachable!(), - Value::Var(VarKind::RegionInput { region, input_idx }) => { + AttrTarget::Var(VarKind::RegionInput { region, input_idx }) => { &mut func_def_body.at_mut(region).def().inputs[input_idx as usize] .attrs } - Value::Var(VarKind::NodeOutput { node, output_idx }) => { - let node_def = func_def_body.at_mut(node).def(); - - // HACK(eddyb) `NodeOutput { output_idx: !0, .. }` - // may be used to attach errors to a whole `Node`. - if output_idx == !0 { - assert!(accesses.is_err()); - &mut node_def.attrs - } else { - &mut node_def.outputs[output_idx as usize].attrs - } + AttrTarget::Var(VarKind::NodeOutput { node, output_idx }) => { + &mut func_def_body.at_mut(node).def().outputs[output_idx as usize] + .attrs + } + AttrTarget::Node(node) => { + assert!(accesses.is_err()); + + &mut func_def_body.at_mut(node).def().attrs } }; match accesses { @@ -891,10 +893,12 @@ impl<'a> GatherAccesses<'a> { { &mut param_accesses[input_idx as usize] } - Value::Var(VarKind::RegionInput { .. }) => { + Value::Var(ptr @ VarKind::RegionInput { .. }) => { // FIXME(eddyb) don't throw away `new_accesses`. - accesses_or_err_attrs_to_attach - .push((ptr, Err(AnalysisError(Diag::bug(["unsupported φ".into()]))))); + accesses_or_err_attrs_to_attach.push(( + AttrTarget::Var(ptr), + Err(AnalysisError(Diag::bug(["unsupported φ".into()]))), + )); return; } Value::Var(VarKind::NodeOutput { node: ptr_node, output_idx }) => { @@ -919,14 +923,12 @@ impl<'a> GatherAccesses<'a> { match &node_def.kind { NodeKind::Select(_) | NodeKind::Loop { .. } | NodeKind::ExitInvocation { .. } => { for (i, accesses) in per_output_accesses.iter().enumerate() { - let output = Value::Var(VarKind::NodeOutput { - node, - output_idx: i.try_into().unwrap(), - }); + let output = + VarKind::NodeOutput { node, output_idx: i.try_into().unwrap() }; if let Some(_accesses) = accesses { // FIXME(eddyb) don't throw away `accesses`. accesses_or_err_attrs_to_attach.push(( - output, + AttrTarget::Var(output), Err(AnalysisError(Diag::bug(["unsupported φ".into()]))), )); } @@ -966,7 +968,7 @@ impl<'a> GatherAccesses<'a> { } FuncGatherAccessesState::InProgress => { accesses_or_err_attrs_to_attach.push(( - Value::Var(VarKind::NodeOutput { node, output_idx: 0 }), + AttrTarget::Node(node), Err(AnalysisError(Diag::bug( ["unsupported recursive call".into()], ))), @@ -979,7 +981,7 @@ impl<'a> GatherAccesses<'a> { && let Some(accesses) = output_accesses { accesses_or_err_attrs_to_attach.push(( - Value::Var(VarKind::NodeOutput { node, output_idx: 0 }), + AttrTarget::Var(VarKind::NodeOutput { node, output_idx: 0 }), accesses, )); } @@ -988,7 +990,7 @@ impl<'a> GatherAccesses<'a> { DataInstKind::Mem(MemOp::FuncLocalVar(_)) => { if let Some(accesses) = output_accesses { accesses_or_err_attrs_to_attach.push(( - Value::Var(VarKind::NodeOutput { node, output_idx: 0 }), + AttrTarget::Var(VarKind::NodeOutput { node, output_idx: 0 }), accesses, )); } @@ -1301,7 +1303,7 @@ impl<'a> GatherAccesses<'a> { // FIXME(eddyb) merge with `FromSpvPtrOutput`'s `pointee`. if let Some(accesses) = output_accesses { accesses_or_err_attrs_to_attach.push(( - Value::Var(VarKind::NodeOutput { node, output_idx: 0 }), + AttrTarget::Var(VarKind::NodeOutput { node, output_idx: 0 }), accesses, )); } diff --git a/src/print/mod.rs b/src/print/mod.rs index 53e034e8..bf697383 100644 --- a/src/print/mod.rs +++ b/src/print/mod.rs @@ -248,16 +248,7 @@ enum Use { RegionLabel(Region), - // FIXME(eddyb) these are `Value`'s variants except `Const`, maybe `Use` - // should just use `Value` and assert it's never `Const`? - RegionInput { - region: Region, - input_idx: u32, - }, - NodeOutput { - node: Node, - output_idx: u32, - }, + Var(VarKind), // NOTE(eddyb) these overlap somewhat with other cases, but they're always // generated, even when there is no "use", for `multiversion` alignment. @@ -269,12 +260,7 @@ impl From for Use { fn from(value: Value) -> Self { match value { Value::Const(ct) => Use::CxInterned(CxInterned::Const(ct)), - Value::Var(VarKind::RegionInput { region, input_idx }) => { - Use::RegionInput { region, input_idx } - } - Value::Var(VarKind::NodeOutput { node, output_idx }) => { - Use::NodeOutput { node, output_idx } - } + Value::Var(v) => Use::Var(v), } } } @@ -292,9 +278,7 @@ impl Use { Self::DbgScope { .. } => ("", "d"), Self::RegionLabel(_) => ("label", "L"), - - Self::RegionInput { .. } | Self::NodeOutput { .. } => ("", "v"), - + Self::Var(_) => ("", "v"), Self::AlignmentAnchorForRegion(_) | Self::AlignmentAnchorForNode(_) => { ("", Self::ANCHOR_ALIGNMENT_NAME_PREFIX) } @@ -1062,9 +1046,7 @@ impl<'a> Printer<'a> { .iter() .map(|(&use_kind, &use_count)| { // HACK(eddyb) these are assigned later. - if let Use::RegionLabel(_) | Use::RegionInput { .. } | Use::NodeOutput { .. } = - use_kind - { + if let Use::RegionLabel(_) | Use::Var(_) = use_kind { return (use_kind, UseStyle::Inline); } @@ -1096,8 +1078,7 @@ impl<'a> Printer<'a> { } Use::DbgScope { .. } | Use::RegionLabel(_) - | Use::RegionInput { .. } - | Use::NodeOutput { .. } + | Use::Var(_) | Use::AlignmentAnchorForRegion(_) | Use::AlignmentAnchorForNode(_) => unreachable!(), } @@ -1169,8 +1150,7 @@ impl<'a> Printer<'a> { Use::DbgScope { .. } | Use::RegionLabel(_) - | Use::RegionInput { .. } - | Use::NodeOutput { .. } + | Use::Var(_) | Use::AlignmentAnchorForRegion(_) | Use::AlignmentAnchorForNode(_) => { unreachable!() @@ -1193,8 +1173,7 @@ impl<'a> Printer<'a> { ) | Use::DbgScope { .. } | Use::RegionLabel(_) - | Use::RegionInput { .. } - | Use::NodeOutput { .. } + | Use::Var(_) | Use::AlignmentAnchorForRegion(_) | Use::AlignmentAnchorForNode(_) => { unreachable!() @@ -1477,7 +1456,10 @@ impl<'a> Printer<'a> { for (i, input_decl) in inputs.iter().enumerate() { define( - Use::RegionInput { region, input_idx: i.try_into().unwrap() }, + Use::Var(VarKind::RegionInput { + region, + input_idx: i.try_into().unwrap(), + }), Some(input_decl.attrs), ); } @@ -1497,7 +1479,10 @@ impl<'a> Printer<'a> { for (i, output_decl) in outputs.iter().enumerate() { define( - Use::NodeOutput { node, output_idx: i.try_into().unwrap() }, + Use::Var(VarKind::NodeOutput { + node, + output_idx: i.try_into().unwrap(), + }), Some(output_decl.attrs), ); } @@ -1523,9 +1508,7 @@ impl<'a> Printer<'a> { (&mut region_label_counter, use_styles.get_mut(&use_kind)) } - Use::RegionInput { .. } | Use::NodeOutput { .. } => { - (&mut value_counter, use_styles.get_mut(&use_kind)) - } + Use::Var(_) => (&mut value_counter, use_styles.get_mut(&use_kind)), Use::AlignmentAnchorForRegion(_) | Use::AlignmentAnchorForNode(_) => ( &mut alignment_anchor_counter, @@ -2147,10 +2130,7 @@ impl Use { )) .into(), - Self::DbgScope { .. } - | Self::RegionLabel(_) - | Self::RegionInput { .. } - | Self::NodeOutput { .. } => "_".into(), + Self::DbgScope { .. } | Self::RegionLabel(_) | Self::Var(_) => "_".into(), Self::AlignmentAnchorForRegion(_) | Self::AlignmentAnchorForNode(_) => { unreachable!() @@ -3901,8 +3881,8 @@ impl FuncAt<'_, DataInst> { None }; - let mut output_use_to_print_as_lhs = - output_type.map(|_| Use::NodeOutput { node: self.position, output_idx: 0 }); + let mut output_use_to_print_as_lhs = output_type + .map(|_| Use::Var(VarKind::NodeOutput { node: self.position, output_idx: 0 })); let mut output_type_to_print = output_type; diff --git a/src/qptr/lift.rs b/src/qptr/lift.rs index 21f113ed..90bd1002 100644 --- a/src/qptr/lift.rs +++ b/src/qptr/lift.rs @@ -6,10 +6,11 @@ use crate::qptr::{QPtrAttr, QPtrOp}; use crate::transform::{InnerInPlaceTransform, InnerTransform, Transformed, Transformer}; use crate::{ AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstDef, ConstKind, Context, DataInst, - DataInstDef, DataInstKind, DeclDef, Diag, DiagLevel, EntityDefs, EntityOrientedDenseMap, Func, - FuncDecl, FxIndexMap, GlobalVar, GlobalVarDecl, Module, Node, NodeKind, NodeOutputDecl, Region, - Type, TypeDef, TypeKind, TypeOrConst, Value, VarKind, spv, + DataInstDef, DataInstKind, DeclDef, Diag, DiagLevel, EntityDefs, Func, FuncDecl, FxIndexMap, + GlobalVar, GlobalVarDecl, Module, Node, NodeKind, NodeOutputDecl, Region, Type, TypeDef, + TypeKind, TypeOrConst, Value, VarKind, spv, }; +use rustc_hash::FxHashMap; use smallvec::SmallVec; use std::cell::Cell; use std::mem; @@ -69,7 +70,7 @@ impl<'a> LiftToSpvPtrs<'a> { parent_region: None, deferred_ptr_noops: Default::default(), - data_inst_use_counts: Default::default(), + var_use_counts: Default::default(), func_has_mem_analysis_bug_diags: false, } @@ -396,11 +397,11 @@ struct LiftToSpvPtrInstsInFunc<'a> { /// `OpTypeRuntimeArray`s (which cannot be nested in non-`Block` structs). /// /// The `QPtrOp` itself is only removed after the entire function is lifted, - /// (using `data_inst_use_counts` to determine whether they're truly unused). + /// (using `var_use_counts` to determine whether they're truly unused). deferred_ptr_noops: FxIndexMap, - // FIXME(eddyb) consider removing this and just do a full second traversal. - data_inst_use_counts: EntityOrientedDenseMap, + // TODO(eddyb) switch this back to `EntityOrientedDenseMap` (removing `Option`). + var_use_counts: FxHashMap>, // HACK(eddyb) this is used to avoid noise when `mem::analyze` failed. func_has_mem_analysis_bug_diags: bool, @@ -1020,7 +1021,7 @@ impl LiftToSpvPtrInstsInFunc<'_> { /// Apply rewrites implied by `deferred_ptr_noops` to `values`. /// - /// This **does not** update `data_inst_use_counts` - in order to do that, + /// This **does not** update `var_use_counts` - in order to do that, /// you must call `self.remove_value_uses(values)` beforehand, and then also /// call `self.after_value_uses(values)` afterwards. fn resolve_deferred_ptr_noop_uses(&self, values: &mut [Value]) { @@ -1042,8 +1043,8 @@ impl LiftToSpvPtrInstsInFunc<'_> { // encoded as `Option` for (dense) map entry reasons. fn add_value_uses(&mut self, values: &[Value]) { for &v in values { - if let Value::Var(VarKind::NodeOutput { node: inst, .. }) = v { - let count = self.data_inst_use_counts.entry(inst); + if let Value::Var(v) = v { + let count = self.var_use_counts.entry(v).or_default(); *count = Some( NonZeroU32::new(count.map_or(0, |c| c.get()).checked_add(1).unwrap()).unwrap(), ); @@ -1052,8 +1053,8 @@ impl LiftToSpvPtrInstsInFunc<'_> { } fn remove_value_uses(&mut self, values: &[Value]) { for &v in values { - if let Value::Var(VarKind::NodeOutput { node: inst, .. }) = v { - let count = self.data_inst_use_counts.entry(inst); + if let Value::Var(v) = v { + let count = self.var_use_counts.entry(v).or_default(); *count = NonZeroU32::new(count.unwrap().get() - 1); } } @@ -1151,7 +1152,18 @@ impl Transformer for LiftToSpvPtrInstsInFunc<'_> { // NOTE(eddyb) reverse order is important, as each removal can reduce // use counts of an earlier definition, allowing further removal. for (inst, ptr_noop) in deferred_ptr_noops.into_iter().rev() { - if self.data_inst_use_counts.get(inst).is_none() { + let is_used = func_def_body + .at(inst) + .def() + .outputs + .iter() + .enumerate() + .map(|(i, _)| VarKind::NodeOutput { + node: inst, + output_idx: i.try_into().unwrap(), + }) + .any(|v| self.var_use_counts.get(&v).is_some_and(|count| count.is_some())); + if !is_used { // HACK(eddyb) can't really use helpers like `FuncAtMut::def`, // due to the need to borrow `regions` and `nodes` // at the same time - perhaps some kind of `FuncAtMut` position From c19d7bad4df85257ee6d896acbe043b4f00d689e Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 9 Sep 2025 11:42:12 +0300 Subject: [PATCH 4/4] WIP: add `Var` entity, `VarDecl` pointing back at region/node input/output slot. --- src/cf/structurize.rs | 301 +++++++++++++++++++----------------------- src/context.rs | 1 + src/func_at.rs | 56 +++++--- src/lib.rs | 72 ++++++---- src/mem/analyze.rs | 86 ++++++------ src/print/mod.rs | 189 ++++++++++++-------------- src/qptr/lift.rs | 190 ++++++++++++++------------ src/qptr/lower.rs | 39 +++--- src/spv/lift.rs | 75 +++++++---- src/spv/lower.rs | 126 ++++++++++-------- src/transform.rs | 63 +++------ src/visit.rs | 40 ++---- 12 files changed, 632 insertions(+), 606 deletions(-) diff --git a/src/cf/structurize.rs b/src/cf/structurize.rs index 64c43384..2d4de1fb 100644 --- a/src/cf/structurize.rs +++ b/src/cf/structurize.rs @@ -6,11 +6,11 @@ use crate::cf::SelectionKind; use crate::cf::unstructured::{ ControlFlowGraph, ControlInst, ControlInstKind, IncomingEdgeCount, LoopFinder, TraversalState, }; -use crate::transform::{InnerInPlaceTransform as _, Transformer}; +use crate::transform::{InnerInPlaceTransform as _, Transformed, Transformer}; use crate::{ AttrSet, Const, ConstDef, ConstKind, Context, DbgSrcLoc, EntityOrientedDenseMap, FuncDefBody, - FxIndexMap, FxIndexSet, Node, NodeDef, NodeKind, NodeOutputDecl, Region, RegionDef, Type, - TypeKind, Value, VarKind, spv, + FxIndexMap, FxIndexSet, Node, NodeDef, NodeKind, Region, RegionDef, Type, TypeKind, Value, Var, + VarDecl, VarKind, spv, }; use itertools::{Either, Itertools}; use smallvec::SmallVec; @@ -71,80 +71,33 @@ pub struct Structurizer<'a> { // FIXME(eddyb) use `EntityOrientedDenseMap` (which lacks iteration by design). structurize_region_state: FxIndexMap, - /// Accumulated rewrites (caused by e.g. `target_inputs`s, but not only), - /// i.e.: `VarKind::RegionInput { region, input_idx }` must be - /// rewritten based on `region_input_rewrites[region]`, as either - /// the original `region` wasn't reused, or its inputs were renumbered. - region_input_rewrites: EntityOrientedDenseMap, + // FIXME(eddyb) perhaps come up with a centralized abstraction for this + // (in theory `VarDecl`s could indicate aliases, but that's a tradeoff). + var_replacements: EntityOrientedDenseMap, } -/// How all `VarKind::RegionInput { region, input_idx }` for a `region` -/// must be rewritten (see also `region_input_rewrites` docs). -enum RegionInputRewrites { - /// Complete replacement with another value (which can take any form), as - /// `region` wasn't kept in its original form in the final structured IR. - /// - /// **Note**: such replacement can be chained, i.e. a replacement value can - /// be `VarKind::RegionInput { region: other_region, .. }`, and then - /// `other_region` itself may have its inputs written. - ReplaceWith(SmallVec<[Value; 2]>), - - /// The value may remain an input of the same `region`, only changing its - /// `input_idx` (e.g. if indices need compaction after removing some inputs), - /// or get replaced anyway, depending on the `Result` for `input_idx`. - /// - /// **Note**: renumbering can only be the last rewrite step of a value, - /// as `region` must've been chosen to be kept in the final structured IR, - /// but the `Err` cases are transitive just like `ReplaceWith`. - // - // FIXME(eddyb) this is a bit silly, maybe try to rely more on hermeticity - // to get rid of this? - RenumberOrReplaceWith(SmallVec<[Result; 2]>), -} +// FIXME(eddyb) maybe this should be provided by `transform`. +struct VarReplacer<'a>(&'a EntityOrientedDenseMap); +impl Transformer for VarReplacer<'_> { + fn transform_value_use(&mut self, v: &Value) -> Transformed { + let mut new_v = *v; -impl RegionInputRewrites { - // HACK(eddyb) this is here because it depends on a field of `Structurizer` - // and borrowing issues ensue if it's made a method of `Structurizer`. - fn rewrite_all( - rewrites: &EntityOrientedDenseMap, - ) -> impl crate::transform::Transformer + '_ { - // FIXME(eddyb) maybe this should be provided by `transform`. - use crate::transform::*; - struct ReplaceValueWith(F); - impl Option> Transformer for ReplaceValueWith { - fn transform_value_use(&mut self, v: &Value) -> Transformed { - self.0(*v).map_or(Transformed::Unchanged, Transformed::Changed) - } + // NOTE(eddyb) this needs to be able to apply multiple replacements, + // due to the input potentially having redundantly chained `OpPhi`s. + // + // FIXME(eddyb) union-find-style "path compression" could record the + // final value inside `self.0` while replacements are being made, + // (e.g. using the type `EntityOrientedDenseMap>`?) + // to avoid going through a chain more than once (and some of these + // replacements could also be applied early). + while let Value::Var(var) = new_v { + new_v = match self.0.get(var) { + Some(&v) => v, + None => break, + }; } - ReplaceValueWith(move |v| { - let mut new_v = v; - while let Value::Var(VarKind::RegionInput { region, input_idx }) = new_v { - match rewrites.get(region) { - // NOTE(eddyb) this needs to be able to apply multiple replacements, - // due to the input potentially having redundantly chained `OpPhi`s. - // - // FIXME(eddyb) union-find-style "path compression" could record the - // final value inside `rewrites` while replacements are being made, - // to avoid going through a chain more than once (and some of these - // replacements could also be applied early). - Some(RegionInputRewrites::ReplaceWith(replacements)) => { - new_v = replacements[input_idx as usize]; - } - Some(RegionInputRewrites::RenumberOrReplaceWith( - renumbering_and_replacements, - )) => match renumbering_and_replacements[input_idx as usize] { - Ok(new_idx) => { - new_v = Value::Var(VarKind::RegionInput { region, input_idx: new_idx }); - break; - } - Err(replacement) => new_v = replacement, - }, - None => break, - } - } - (v != new_v).then_some(new_v) - }) + (*v != new_v).then_some(new_v).map_or(Transformed::Unchanged, Transformed::Changed) } } @@ -578,7 +531,7 @@ struct ClaimedRegion { /// merged into a larger region, or as a child of a new [`Node`]. // // FIXME(eddyb) don't replace `VarKind::RegionInput { region: structured_body, .. }` - // with `region_inputs` when `structured_body` ends up a `Node` child, + // with `structured_body_inputs` when `structured_body` ends up a `Node` child, // but instead make all `Region`s entirely hermetic wrt inputs. structured_body_inputs: SmallVec<[Value; 2]>, @@ -668,7 +621,7 @@ impl<'a> Structurizer<'a> { incoming_edge_counts_including_loop_exits, structurize_region_state: FxIndexMap::default(), - region_input_rewrites: EntityOrientedDenseMap::new(), + var_replacements: EntityOrientedDenseMap::new(), } } @@ -764,15 +717,13 @@ impl<'a> Structurizer<'a> { } } - // The last step of structurization is applying rewrites accumulated - // while structurizing (i.e. `region_input_rewrites`). + // The last step of structurization is applying replacements accumulated + // while structurizing (i.e. `var_replacements`). // // FIXME(eddyb) obsolete this by fully taking advantage of hermeticity, // and only replacing `VarKind::RegionInput { region, .. }` within // `region`'s children, shallowly, whenever `region` gets claimed. - self.func_def_body.inner_in_place_transform_with(&mut RegionInputRewrites::rewrite_all( - &self.region_input_rewrites, - )); + self.func_def_body.inner_in_place_transform_with(&mut VarReplacer(&self.var_replacements)); } fn try_claim_edge_bundle( @@ -857,8 +808,8 @@ impl<'a> Structurizer<'a> { // FIXME(eddyb) `Loop` `Node`s should be changed to be hermetic // and have the loop state be output from the whole node itself, // for any outside uses of values defined within the loop body. - let body_def = self.func_def_body.at_mut(body).def(); - let original_input_decls = mem::take(&mut body_def.inputs); + let body_def = &mut self.func_def_body.regions[body]; + let original_body_input_vars = mem::take(&mut body_def.inputs); assert!(body_def.outputs.is_empty()); // HACK(eddyb) some dataflow through the loop body is redundant, @@ -868,45 +819,54 @@ impl<'a> Structurizer<'a> { // feasible to move `body`'s children into a new region without // wasting it completely (i.e. can't swap with `wrapper_region`). let mut initial_inputs = SmallVec::<[_; 2]>::new(); - let body_input_rewrites = RegionInputRewrites::RenumberOrReplaceWith( - backedge - .target_inputs - .into_iter() - .enumerate() - .map(|(original_idx, mut backedge_value)| { - RegionInputRewrites::rewrite_all(&self.region_input_rewrites) - .transform_value_use(&backedge_value) - .apply_to(&mut backedge_value); - - let original_idx = u32::try_from(original_idx).unwrap(); - if backedge_value - == Value::Var(VarKind::RegionInput { - region: body, - input_idx: original_idx, - }) - { - // FIXME(eddyb) does this have to be general purpose, - // or could this be handled as `None` with a single - // `wrapper_region` per `RegionInputRewrites`? - Err(Value::Var(VarKind::RegionInput { - region: wrapper_region, - input_idx: original_idx, - })) - } else { - let renumbered_idx = u32::try_from(body_def.inputs.len()).unwrap(); - initial_inputs.push(Value::Var(VarKind::RegionInput { - region: wrapper_region, - input_idx: original_idx, - })); - body_def.inputs.push(original_input_decls[original_idx as usize]); - body_def.outputs.push(backedge_value); - Ok(renumbered_idx) - } - }) - .collect(), - ); - self.region_input_rewrites.insert(body, body_input_rewrites); + // FIXME(eddyb) optimize this (also, maybe it's worth introducing + // a high-level `retain` for `body_def.inputs`, also updating decls). + for (original_body_input_var, mut backedge_value) in + original_body_input_vars.into_iter().zip_eq(backedge.target_inputs) + { + VarReplacer(&self.var_replacements) + .transform_value_use(&backedge_value) + .apply_to(&mut backedge_value); + + // FIXME(eddyb) this fully duplicates attributes, could be messy? + let input_var_decl = self.func_def_body.vars[original_body_input_var].clone(); + + let wrapper_region_input_vars = + &mut self.func_def_body.regions[wrapper_region].inputs; + let wrapper_region_input_decl = VarDecl { + def_parent: Either::Left(wrapper_region), + def_idx: wrapper_region_input_vars.len().try_into().unwrap(), + ..input_var_decl.clone() + }; + + if backedge_value == Value::Var(original_body_input_var) { + // Move this redundant input to `wrapper_region`, instead of + // allocating a new `Var`, to avoid wasting the now-unused + // `original_body_input_var` (which would've also needed a + // `var_replacements` entry, mapping it to the new `Var`). + self.func_def_body.vars[original_body_input_var] = wrapper_region_input_decl; + wrapper_region_input_vars.push(original_body_input_var); + } else { + let wrapper_region_input_var = + self.func_def_body.vars.define(self.cx, wrapper_region_input_decl); + wrapper_region_input_vars.push(wrapper_region_input_var); + + initial_inputs.push(Value::Var(wrapper_region_input_var)); + + let body_def = &mut self.func_def_body.regions[body]; + self.func_def_body.vars[original_body_input_var] = VarDecl { + def_parent: Either::Left(body), + def_idx: body_def.inputs.len().try_into().unwrap(), + ..input_var_decl + }; + body_def.inputs.push(original_body_input_var); + + body_def.outputs.push(backedge_value); + } + } + + let body_def = &mut self.func_def_body.regions[body]; assert_eq!(initial_inputs.len(), body_def.inputs.len()); assert_eq!(body_def.outputs.len(), body_def.inputs.len()); @@ -925,9 +885,9 @@ impl<'a> Structurizer<'a> { .into(), ); - let wrapper_region_def = &mut self.func_def_body.regions[wrapper_region]; - wrapper_region_def.inputs = original_input_decls; - wrapper_region_def.children.insert_last(loop_node, &mut self.func_def_body.nodes); + self.func_def_body.regions[wrapper_region] + .children + .insert_last(loop_node, &mut self.func_def_body.nodes); // HACK(eddyb) we've treated loop exits as extra "false edges", so // here they have to be added to the loop (potentially unlocking @@ -1417,7 +1377,7 @@ impl<'a> Structurizer<'a> { } } - // FIXME(eddyb) `region_input_rewrites` mappings, generated + // FIXME(eddyb) `var_replacements` (`Var` -> `Value`) mappings, generated // for every `ClaimedRegion` that has been merged into a larger region, // only get applied after structurization fully completes, but here it's // very useful to have the fully resolved values across all `cases`' @@ -1438,9 +1398,7 @@ impl<'a> Structurizer<'a> { .flat_map(|(_, edge_bundle)| &mut edge_bundle.target_inputs), ); for v in all_values { - RegionInputRewrites::rewrite_all(&self.region_input_rewrites) - .transform_value_use(v) - .apply_to(v); + VarReplacer(&self.var_replacements).transform_value_use(v).apply_to(v); } } @@ -1499,12 +1457,14 @@ impl<'a> Structurizer<'a> { .. }) => match v { Value::Const(_) => Ok(v), - Value::Var(VarKind::RegionInput { region, input_idx }) - if region == *structured_body => - { - Ok(structured_body_inputs[input_idx as usize]) - } - Value::Var(_) => Err(()), + Value::Var(v) => match self.func_def_body.at(v).decl().kind() { + VarKind::RegionInput { region, input_idx } + if region == *structured_body => + { + Ok(structured_body_inputs[input_idx as usize]) + } + _ => Err(()), + }, }, // `case` has no region of its own, so everything @@ -1520,7 +1480,10 @@ impl<'a> Structurizer<'a> { let ty = match target { DeferredTarget::Region(target) => { - self.func_def_body.at(target).def().inputs[target_input_idx].ty + self.func_def_body + .at(self.func_def_body.at(target).def().inputs[target_input_idx]) + .decl() + .ty } // HACK(eddyb) in the absence of `FuncDecl`, infer the // type from each returned value (and require they match). @@ -1535,9 +1498,18 @@ impl<'a> Structurizer<'a> { }; let select_node = get_or_define_select_node(self, &cases); - let output_decls = &mut self.func_def_body.at_mut(select_node).def().outputs; - let output_idx = output_decls.len(); - output_decls.push(NodeOutputDecl { attrs: AttrSet::default(), ty }); + let output_vars = &mut self.func_def_body.nodes[select_node].outputs; + let output_idx = output_vars.len(); + let output_var = self.func_def_body.vars.define( + self.cx, + VarDecl { + attrs: AttrSet::default(), + ty, + def_parent: Either::Right(select_node), + def_idx: output_idx.try_into().unwrap(), + }, + ); + output_vars.push(output_var); for (case_idx, v) in per_case_target_input.enumerate() { let v = v.unwrap_or_else(|| Value::Const(self.const_undef(ty))); @@ -1547,10 +1519,7 @@ impl<'a> Structurizer<'a> { assert_eq!(outputs.len(), output_idx); outputs.push(v); } - Value::Var(VarKind::NodeOutput { - node: select_node, - output_idx: output_idx.try_into().unwrap(), - }) + Value::Var(output_var) }) .collect(); @@ -1596,22 +1565,20 @@ impl<'a> Structurizer<'a> { }); let deferred_edges = deferred_edges.collect(); - // Only as the very last step, can per-case `region_inputs` be added to - // `region_input_rewrites`. + // Only as the very last step, can per-case `structured_body_inputs` be added to + // `var_replacements`. // // FIXME(eddyb) don't replace `VarKind::RegionInput { region, .. }` - // with `region_inputs` when the `region` ends up a `Node` child, + // with `structured_body_inputs` when the `region` ends up a `Node` child, // but instead make all `Region`s entirely hermetic wrt inputs. #[allow(clippy::manual_flatten)] for case in cases { - if let Ok(ClaimedRegion { structured_body, structured_body_inputs, .. }) = case - && !structured_body_inputs.is_empty() - { - self.region_input_rewrites.insert( - structured_body, - RegionInputRewrites::ReplaceWith(structured_body_inputs), - ); - self.func_def_body.at_mut(structured_body).def().inputs.clear(); + if let Ok(ClaimedRegion { structured_body, structured_body_inputs, .. }) = case { + let input_vars = + mem::take(&mut self.func_def_body.at_mut(structured_body).def().inputs); + for (input_var, v) in input_vars.into_iter().zip_eq(structured_body_inputs) { + self.var_replacements.insert(input_var, v); + } } } @@ -1645,7 +1612,7 @@ impl<'a> Structurizer<'a> { .map(|cond| self.materialize_lazy_cond(cond)) .collect(); - let NodeDef { attrs: _, kind, inputs, child_regions, outputs: output_decls } = + let NodeDef { attrs: _, kind, inputs, child_regions, outputs: output_vars } = &mut *self.func_def_body.nodes[node]; let cases = match kind { NodeKind::Select(kind) => { @@ -1672,16 +1639,24 @@ impl<'a> Structurizer<'a> { _ => unreachable!(), }; - let output_idx = u32::try_from(output_decls.len()).unwrap(); - output_decls.push(NodeOutputDecl { attrs: AttrSet::default(), ty: self.type_bool }); + let output_var = self.func_def_body.vars.define( + self.cx, + VarDecl { + attrs: AttrSet::default(), + ty: self.type_bool, + def_parent: Either::Right(node), + def_idx: output_vars.len().try_into().unwrap(), + }, + ); + output_vars.push(output_var); for (&case, cond) in cases.iter().zip_eq(per_case_conds) { let RegionDef { outputs, .. } = &mut self.func_def_body.regions[case]; outputs.push(cond); - assert_eq!(outputs.len(), output_decls.len()); + assert_eq!(outputs.len(), output_vars.len()); } - Value::Var(VarKind::NodeOutput { node, output_idx }) + Value::Var(output_var) } } } @@ -1698,14 +1673,14 @@ impl<'a> Structurizer<'a> { ) -> DeferredEdgeBundleSet { match maybe_claimed_region { Ok(ClaimedRegion { structured_body, structured_body_inputs, deferred_edges }) => { - if !structured_body_inputs.is_empty() { - self.region_input_rewrites.insert( - structured_body, - RegionInputRewrites::ReplaceWith(structured_body_inputs), - ); + let structured_body_def = self.func_def_body.at_mut(structured_body).def(); + + let input_vars = mem::take(&mut structured_body_def.inputs); + for (input_var, v) in input_vars.into_iter().zip_eq(structured_body_inputs) { + self.var_replacements.insert(input_var, v); } - let new_children = - mem::take(&mut self.func_def_body.at_mut(structured_body).def().children); + + let new_children = mem::take(&mut structured_body_def.children); self.func_def_body.regions[parent_region] .children .append(new_children, &mut self.func_def_body.nodes); diff --git a/src/context.rs b/src/context.rs index c20e65f0..1e1a28dd 100644 --- a/src/context.rs +++ b/src/context.rs @@ -957,4 +957,5 @@ entities! { Func => chunk_size(0x1_0000) crate::FuncDecl, Region => chunk_size(0x1000) crate::RegionDef, Node => chunk_size(0x1000) EntityListNode, + Var => chunk_size(0x1000) crate::VarDecl, } diff --git a/src/func_at.rs b/src/func_at.rs index a45d16ba..6cf1b9de 100644 --- a/src/func_at.rs +++ b/src/func_at.rs @@ -13,7 +13,7 @@ use crate::{ Context, EntityDefs, EntityList, EntityListIter, FuncDefBody, Node, NodeDef, Region, RegionDef, - Type, Value, Var, VarKind, + Type, Value, Var, VarDecl, }; /// Immutable traversal (i.e. visiting) helper for intra-function entities. @@ -24,6 +24,7 @@ use crate::{ pub struct FuncAt<'a, P: Copy> { pub regions: &'a EntityDefs, pub nodes: &'a EntityDefs, + pub vars: &'a EntityDefs, pub position: P, } @@ -31,7 +32,7 @@ pub struct FuncAt<'a, P: Copy> { impl<'a, P: Copy> FuncAt<'a, P> { /// Reposition to `new_position`. pub fn at(self, new_position: P2) -> FuncAt<'a, P2> { - FuncAt { regions: self.regions, nodes: self.nodes, position: new_position } + FuncAt { regions: self.regions, nodes: self.nodes, vars: self.vars, position: new_position } } } @@ -76,6 +77,12 @@ impl<'a> FuncAt<'a, Node> { } } +impl<'a> FuncAt<'a, Var> { + pub fn decl(self) -> &'a VarDecl { + &self.vars[self.position] + } +} + impl FuncAt<'_, Value> { /// Return the [`Type`] of this [`Value`] ([`Context`] used for [`Value::Const`]). pub fn type_of(self, cx: &Context) -> Type { @@ -88,15 +95,10 @@ impl FuncAt<'_, Value> { impl FuncAt<'_, Var> { /// Return the [`Type`] of this [`Var`]. + // + // FIXME(eddyb) is this really necessary? if so, should it have this name? pub fn type_of(self) -> Type { - match self.position { - VarKind::RegionInput { region, input_idx } => { - self.at(region).def().inputs[input_idx as usize].ty - } - VarKind::NodeOutput { node, output_idx } => { - self.at(node).def().outputs[output_idx as usize].ty - } - } + self.decl().ty } } @@ -107,6 +109,7 @@ impl FuncAt<'_, Var> { pub struct FuncAtMut<'a, P: Copy> { pub regions: &'a mut EntityDefs, pub nodes: &'a mut EntityDefs, + pub vars: &'a mut EntityDefs, pub position: P, } @@ -114,20 +117,30 @@ pub struct FuncAtMut<'a, P: Copy> { impl<'a, P: Copy> FuncAtMut<'a, P> { /// Emulate a "reborrow", which is automatic only for `&mut` types. pub fn reborrow(&mut self) -> FuncAtMut<'_, P> { - FuncAtMut { regions: self.regions, nodes: self.nodes, position: self.position } + FuncAtMut { + regions: self.regions, + nodes: self.nodes, + vars: self.vars, + position: self.position, + } } /// Reposition to `new_position`. pub fn at(self, new_position: P2) -> FuncAtMut<'a, P2> { - FuncAtMut { regions: self.regions, nodes: self.nodes, position: new_position } + FuncAtMut { + regions: self.regions, + nodes: self.nodes, + vars: self.vars, + position: new_position, + } } /// Demote to a `FuncAt`, with the same `position`. // // FIXME(eddyb) maybe find a better name for this? pub fn freeze(self) -> FuncAt<'a, P> { - let FuncAtMut { regions, nodes, position } = self; - FuncAt { regions, nodes, position } + let FuncAtMut { regions, nodes, vars, position } = self; + FuncAt { regions, nodes, vars, position } } } @@ -165,15 +178,26 @@ impl<'a> FuncAtMut<'a, Node> { } } +impl<'a> FuncAtMut<'a, Var> { + pub fn decl(self) -> &'a mut VarDecl { + &mut self.vars[self.position] + } +} + impl FuncDefBody { /// Start immutably traversing the function at `position`. pub fn at(&self, position: P) -> FuncAt<'_, P> { - FuncAt { regions: &self.regions, nodes: &self.nodes, position } + FuncAt { regions: &self.regions, nodes: &self.nodes, vars: &self.vars, position } } /// Start mutably traversing the function at `position`. pub fn at_mut(&mut self, position: P) -> FuncAtMut<'_, P> { - FuncAtMut { regions: &mut self.regions, nodes: &mut self.nodes, position } + FuncAtMut { + regions: &mut self.regions, + nodes: &mut self.nodes, + vars: &mut self.vars, + position, + } } /// Shorthand for `func_def_body.at(func_def_body.body)`. diff --git a/src/lib.rs b/src/lib.rs index e41100e1..e92afb90 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -548,8 +548,8 @@ pub enum TypeKind { /// (e.g. "points to variable `x`" or "accessed at offset `y`") can be found /// attached as `Attr`s on those `Value`s (see [`Attr::QPtr`]). // - // FIXME(eddyb) a "refinement system" that's orthogonal from types, and kept - // separately in e.g. `RegionInputDecl`, might be a better approach? + // FIXME(eddyb) a "refinement system" that's orthogonal from types, + // and kept separately in `VarDecl`, might be a better approach? QPtr, SpvInst { @@ -702,6 +702,7 @@ pub struct FuncParam { pub struct FuncDefBody { pub regions: EntityDefs, pub nodes: EntityDefs, + pub vars: EntityDefs, /// The [`Region`] representing the whole body of the function. /// @@ -817,7 +818,7 @@ pub struct RegionDef { /// * accessed using [`VarKind::RegionInput`] /// * values provided by the parent: /// * when this is the function body: the function's parameters - pub inputs: SmallVec<[RegionInputDecl; 2]>, + pub inputs: SmallVec<[Var; 2]>, pub children: EntityList, @@ -834,13 +835,6 @@ pub struct RegionDef { pub outputs: SmallVec<[Value; 2]>, } -#[derive(Copy, Clone)] -pub struct RegionInputDecl { - pub attrs: AttrSet, - - pub ty: Type, -} - /// Entity handle for a [`NodeDef`](crate::NodeDef) /// (a control-flow operator or leaf). /// @@ -868,14 +862,7 @@ pub struct NodeDef { /// child [`Region`]: /// * when this is a `Select`: the case that was chosen // TODO(eddyb) include former `DataInst`s in above docs. - pub outputs: SmallVec<[NodeOutputDecl; 2]>, -} - -#[derive(Copy, Clone)] -pub struct NodeOutputDecl { - pub attrs: AttrSet, - - pub ty: Type, + pub outputs: SmallVec<[Var; 2]>, } #[derive(Clone, PartialEq, Eq, Hash, derive_more::From)] @@ -939,19 +926,42 @@ pub type DataInst = Node; pub type DataInstDef = NodeDef; pub type DataInstKind = NodeKind; -#[derive(Copy, Clone, PartialEq, Eq, Hash)] -pub enum Value { - Const(Const), - Var(Var), -} - -type Var = VarKind; - +// FIXME(eddyb) should this be above region/node? // FIXME(eddyb) document the fact that "variable" is used here in a sense // more like e.g. math/lambda calculus/SSA/Rust immutable variables, // and *not* some sort of "mutable slot" (like e.g. wasm local variables), // also mention `GlobalVar`/`mem::MemOp::FuncLocalVar`. -#[derive(Copy, Clone, PartialEq, Eq, Hash)] +pub use context::Var; + +/// Declaration for a [`Var`]: a [`Region`] input or [`Node`] output. +#[derive(Clone)] +pub struct VarDecl { + pub attrs: AttrSet, + + pub ty: Type, + + // FIXME(eddyb) add a `context::PackedEither` using the sign of s/u32/i32/ + // interned/entity, and use it to be more compact than `Either`. + pub def_parent: itertools::Either, + pub def_idx: u32, +} + +impl VarDecl { + // FIXME(eddyb) `VarKind` maybe should've been `VarDef`, but the `Def` suffix + // can get confusing, and `VarKind` was picked early in the refactor. + // FIXME(eddyb) document that the indices returned are only valid while the + // `Var`s (`RegionDef` `inputs` or `NodeDef` `outputs`) remain unchanged. + pub fn kind(&self) -> VarKind { + self.def_parent.either( + |region| VarKind::RegionInput { region, input_idx: self.def_idx }, + |node| VarKind::NodeOutput { node, output_idx: self.def_idx }, + ) + } +} + +// FIXME(eddyb) consider using `usize` (still packed as `u32`) for indices. +// FIXME(eddyb) document that the indices contained are only valid while the +// `Var`s (`RegionDef` `inputs` or `NodeDef` `outputs`) remain unchanged. pub enum VarKind { /// One of the inputs to a [`Region`]: /// * declared by `region.inputs[input_idx]` @@ -967,3 +977,11 @@ pub enum VarKind { // TODO(eddyb) include former `DataInst`s in above docs. NodeOutput { node: Node, output_idx: u32 }, } + +// FIXME(eddyb) add a `context::PackedEither` using the sign of s/u32/i32/ +// interned/entity, and use it to be more compact than `Either`. +#[derive(Copy, Clone, PartialEq, Eq, Hash)] +pub enum Value { + Const(Const), + Var(Var), +} diff --git a/src/mem/analyze.rs b/src/mem/analyze.rs index 479d03d8..6f54919b 100644 --- a/src/mem/analyze.rs +++ b/src/mem/analyze.rs @@ -9,7 +9,7 @@ use crate::visit::{InnerVisit, Visitor}; use crate::{ AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstKind, Context, DataInstKind, DeclDef, Diag, ExportKey, Exportee, Func, FxIndexMap, GlobalVar, Module, Node, NodeKind, OrdAssertEq, Type, - TypeKind, Value, VarKind, + TypeKind, Value, Var, VarKind, }; use itertools::{Either, Itertools as _}; use rustc_hash::FxHashMap; @@ -655,7 +655,7 @@ impl MemTypeLayout { #[derive(Copy, Clone)] enum AttrTarget { - Var(VarKind), + Var(Var), Node(Node), } @@ -781,14 +781,7 @@ impl<'a> GatherAccesses<'a> { for (v, accesses) in accesses_or_err_attrs_to_attach { let attrs = match v { - AttrTarget::Var(VarKind::RegionInput { region, input_idx }) => { - &mut func_def_body.at_mut(region).def().inputs[input_idx as usize] - .attrs - } - AttrTarget::Var(VarKind::NodeOutput { node, output_idx }) => { - &mut func_def_body.at_mut(node).def().outputs[output_idx as usize] - .attrs - } + AttrTarget::Var(v) => &mut func_def_body.at_mut(v).decl().attrs, AttrTarget::Node(node) => { assert!(accesses.is_err()); @@ -888,27 +881,29 @@ impl<'a> GatherAccesses<'a> { // FIXME(eddyb) may be relevant? _ => unreachable!(), }, - Value::Var(VarKind::RegionInput { region, input_idx }) - if region == func_def_body.body => - { - &mut param_accesses[input_idx as usize] - } - Value::Var(ptr @ VarKind::RegionInput { .. }) => { - // FIXME(eddyb) don't throw away `new_accesses`. - accesses_or_err_attrs_to_attach.push(( - AttrTarget::Var(ptr), - Err(AnalysisError(Diag::bug(["unsupported φ".into()]))), - )); - return; - } - Value::Var(VarKind::NodeOutput { node: ptr_node, output_idx }) => { - let i = output_idx as usize; - let slots = node_to_per_output_accesses.entry(ptr_node).or_default(); - if i >= slots.len() { - slots.extend((slots.len()..=i).map(|_| None)); + Value::Var(ptr) => match func_def_body.at(ptr).decl().kind() { + VarKind::RegionInput { region, input_idx } + if region == func_def_body.body => + { + &mut param_accesses[input_idx as usize] } - &mut slots[i] - } + VarKind::RegionInput { .. } => { + // FIXME(eddyb) don't throw away `new_accesses`. + accesses_or_err_attrs_to_attach.push(( + AttrTarget::Var(ptr), + Err(AnalysisError(Diag::bug(["unsupported φ".into()]))), + )); + return; + } + VarKind::NodeOutput { node: ptr_node, output_idx } => { + let i = output_idx as usize; + let slots = node_to_per_output_accesses.entry(ptr_node).or_default(); + if i >= slots.len() { + slots.extend((slots.len()..=i).map(|_| None)); + } + &mut slots[i] + } + }, }; *slot = Some(match slot.take() { Some(old) => old.and_then(|old| { @@ -922,13 +917,12 @@ impl<'a> GatherAccesses<'a> { match &node_def.kind { NodeKind::Select(_) | NodeKind::Loop { .. } | NodeKind::ExitInvocation { .. } => { - for (i, accesses) in per_output_accesses.iter().enumerate() { - let output = - VarKind::NodeOutput { node, output_idx: i.try_into().unwrap() }; + for (&output_var, accesses) in node_def.outputs.iter().zip(&per_output_accesses) + { if let Some(_accesses) = accesses { // FIXME(eddyb) don't throw away `accesses`. accesses_or_err_attrs_to_attach.push(( - AttrTarget::Var(output), + AttrTarget::Var(output_var), Err(AnalysisError(Diag::bug(["unsupported φ".into()]))), )); } @@ -977,22 +971,18 @@ impl<'a> GatherAccesses<'a> { }; // HACK(eddyb) multi-output instructions don't exist pre-disaggregate. if (data_inst_def.outputs.iter().at_most_one().ok().unwrap()) - .is_some_and(|o| is_qptr(o.ty)) + .is_some_and(|&o| is_qptr(func_def_body.at(o).decl().ty)) && let Some(accesses) = output_accesses { - accesses_or_err_attrs_to_attach.push(( - AttrTarget::Var(VarKind::NodeOutput { node, output_idx: 0 }), - accesses, - )); + accesses_or_err_attrs_to_attach + .push((AttrTarget::Var(data_inst_def.outputs[0]), accesses)); } } DataInstKind::Mem(MemOp::FuncLocalVar(_)) => { if let Some(accesses) = output_accesses { - accesses_or_err_attrs_to_attach.push(( - AttrTarget::Var(VarKind::NodeOutput { node, output_idx: 0 }), - accesses, - )); + accesses_or_err_attrs_to_attach + .push((AttrTarget::Var(data_inst_def.outputs[0]), accesses)); } } DataInstKind::QPtr(QPtrOp::HandleArrayIndex) => { @@ -1180,7 +1170,9 @@ impl<'a> GatherAccesses<'a> { // HACK(eddyb) `_` will match multiple variants soon. #[allow(clippy::match_wildcard_for_single_variants)] let (op_name, access_type) = match op { - MemOp::Load => ("Load", data_inst_def.outputs[0].ty), + MemOp::Load => { + ("Load", func_def_body.at(data_inst_def.outputs[0]).decl().ty) + } MemOp::Store => { ("Store", func_def_body.at(data_inst_def.inputs[1]).type_of(&cx)) } @@ -1302,10 +1294,8 @@ impl<'a> GatherAccesses<'a> { if has_from_spv_ptr_output_attr { // FIXME(eddyb) merge with `FromSpvPtrOutput`'s `pointee`. if let Some(accesses) = output_accesses { - accesses_or_err_attrs_to_attach.push(( - AttrTarget::Var(VarKind::NodeOutput { node, output_idx: 0 }), - accesses, - )); + accesses_or_err_attrs_to_attach + .push((AttrTarget::Var(data_inst_def.outputs[0]), accesses)); } } } diff --git a/src/print/mod.rs b/src/print/mod.rs index bf697383..c18e6c6c 100644 --- a/src/print/mod.rs +++ b/src/print/mod.rs @@ -30,8 +30,8 @@ use crate::{ DataInstDef, DataInstKind, DbgSrcLoc, DeclDef, Diag, DiagLevel, DiagMsgPart, EntityOrientedDenseMap, ExportKey, Exportee, Func, FuncDecl, FuncDefBody, FuncParam, FxIndexMap, FxIndexSet, GlobalVar, GlobalVarDecl, GlobalVarDefBody, Import, InternedStr, - Module, ModuleDebugInfo, ModuleDialect, Node, NodeDef, NodeKind, NodeOutputDecl, OrdAssertEq, - Region, RegionDef, RegionInputDecl, Type, TypeDef, TypeKind, TypeOrConst, Value, VarKind, spv, + Module, ModuleDebugInfo, ModuleDialect, Node, NodeDef, NodeKind, OrdAssertEq, Region, + RegionDef, Type, TypeDef, TypeKind, TypeOrConst, Value, Var, VarDecl, spv, }; use arrayvec::ArrayVec; use itertools::Either; @@ -248,7 +248,7 @@ enum Use { RegionLabel(Region), - Var(VarKind), + Var(Var), // NOTE(eddyb) these overlap somewhat with other cases, but they're always // generated, even when there is no "use", for `multiversion` alignment. @@ -1454,13 +1454,10 @@ impl<'a> Printer<'a> { let RegionDef { inputs, children: _, outputs: _ } = func_def_body.at(region).def(); - for (i, input_decl) in inputs.iter().enumerate() { + for &input_var in inputs { define( - Use::Var(VarKind::RegionInput { - region, - input_idx: i.try_into().unwrap(), - }), - Some(input_decl.attrs), + Use::Var(input_var), + Some(func_def_body.at(input_var).decl().attrs), ); } } else { @@ -1477,13 +1474,10 @@ impl<'a> Printer<'a> { define(Use::AlignmentAnchorForNode(node), Some(*attrs)); - for (i, output_decl) in outputs.iter().enumerate() { + for &output_var in outputs { define( - Use::Var(VarKind::NodeOutput { - node, - output_idx: i.try_into().unwrap(), - }), - Some(output_decl.attrs), + Use::Var(output_var), + Some(func_def_body.at(output_var).decl().attrs), ); } } @@ -3508,22 +3502,18 @@ impl Print for FuncDecl { fn print(&self, printer: &Printer<'_>) -> AttrsAndDef { let Self { attrs, ret_type, params, def } = self; - let sig = pretty::Fragment::new([ - pretty::join_comma_sep( - "(", - params.iter().enumerate().map(|(i, param)| { - let param_name = match def { - DeclDef::Imported(_) => "_".into(), - DeclDef::Present(def) => Value::Var(VarKind::RegionInput { - region: def.body, - input_idx: i.try_into().unwrap(), - }) - .print_as_def(printer), - }; - param.print(printer).insert_name_before_def(param_name) - }), - ")", + let params = match def { + DeclDef::Imported(_) => Either::Left( + params.iter().map(|param| param.print(printer).insert_name_before_def("_")), ), + DeclDef::Present(def) => { + // FIXME(eddyb) this actually ignores the `FuncParam`s entirely. + Either::Right(def.at(Either::Left(def.body)).print_var_defs(printer)) + } + }; + + let sig = pretty::Fragment::new([ + pretty::join_comma_sep("(", params, ")"), " -> ".into(), ret_type.print(printer), ]); @@ -3548,15 +3538,7 @@ impl Print for FuncDecl { let label_inputs = if !inputs.is_empty() { pretty::join_comma_sep( "(", - inputs.iter().enumerate().map(|(input_idx, input)| { - input.print(printer).insert_name_before_def( - Value::Var(VarKind::RegionInput { - region, - input_idx: input_idx.try_into().unwrap(), - }) - .print_as_def(printer), - ) - }), + def.at(Either::Left(region)).print_var_defs(printer), ")", ) } else { @@ -3612,6 +3594,61 @@ impl Print for FuncParam { } } +impl<'a> FuncAt<'a, Either> { + fn print_var_defs( + &self, + printer: &'a Printer<'_>, + ) -> impl ExactSizeIterator + 'a { + let def_parent = self.position; + let func = self.at(()); + let vars = def_parent + .either(|region| &func.at(region).def().inputs, |node| &func.at(node).def().outputs); + vars.iter().map(move |&var| func.at(var).print_with_def_parent(printer, def_parent)) + } +} + +impl FuncAt<'_, Var> { + fn print_with_def_parent( + &self, + printer: &Printer<'_>, + expected_def_parent: Either, + ) -> pretty::Fragment { + let VarDecl { attrs, ty, def_parent, def_idx } = *self.decl(); + let var = self.position; + + let mut name = Use::Var(var).print_as_def(printer); + + let valid_attachment_or_err = if def_parent != expected_def_parent { + Err("/* BUG (attached elsewhere) */".into()) + } else { + let vars = def_parent.either( + |region| &self.at(region).def().inputs, + |node| &self.at(node).def().outputs, + ); + if vars.get(def_idx as usize) != Some(&var) { + let msg: Cow<'_, str> = if let Some(i) = vars.iter().position(|&v| v == var) { + format!("/* BUG (wrong position, attached at {i}) */").into() + } else { + "/* BUG (missing, half-detached?) */".into() + }; + Err(msg) + } else { + Ok(()) + } + }; + if let Err(msg) = valid_attachment_or_err { + name = + pretty::Fragment::new([printer.error_style().apply(msg).into(), " ".into(), name]); + } + + AttrsAndDef { + attrs: attrs.print(printer), + def_without_name: printer.pretty_type_ascription_suffix(ty), + } + .insert_name_before_def(name) + } +} + impl Print for FuncAt<'_, Region> { type Output = pretty::Fragment; fn print(&self, printer: &Printer<'_>) -> pretty::Fragment { @@ -3691,21 +3728,12 @@ impl Print for FuncAt<'_, Region> { impl Print for FuncAt<'_, Node> { type Output = pretty::Fragment; fn print(&self, printer: &Printer<'_>) -> pretty::Fragment { - let node = self.position; let NodeDef { attrs, kind, inputs, child_regions, outputs } = self.def(); let attrs = attrs.print(printer); let outputs_header = if !outputs.is_empty() { - let mut outputs = outputs.iter().enumerate().map(|(output_idx, output)| { - output.print(printer).insert_name_before_def( - Value::Var(VarKind::NodeOutput { - node, - output_idx: output_idx.try_into().unwrap(), - }) - .print_as_def(printer), - ) - }); + let mut outputs = self.at(Either::Right(self.position)).print_var_defs(printer); let outputs_lhs = if outputs.len() == 1 { outputs.next().unwrap() } else { @@ -3754,35 +3782,25 @@ impl Print for FuncAt<'_, Node> { // `body`, which start at `0`/`false`, and are replaced with // `v3`/`v4` after each iteration. let (inputs_header, body_suffix) = if !inputs.is_empty() { - let input_decls_and_uses = - inputs.iter().enumerate().map(|(input_idx, input)| { - ( - input, - Value::Var(VarKind::RegionInput { - region: body, - input_idx: input_idx.try_into().unwrap(), - }), - ) - }); ( pretty::join_comma_sep( "(", - input_decls_and_uses.clone().zip(initial_inputs).map( - |((input_decl, input_use), initial)| { + self.at(Either::Left(body)) + .print_var_defs(printer) + .zip(initial_inputs) + .map(|(lhs, initial)| { pretty::Fragment::new([ - input_decl.print(printer).insert_name_before_def( - input_use.print_as_def(printer), - ), + lhs, " <- ".into(), initial.print(printer), ]) - }, - ), + }), ")", ), pretty::Fragment::new([" -> ".into(), { - let mut input_dests = - input_decls_and_uses.map(|(_, input_use)| input_use.print(printer)); + let mut input_dests = inputs + .iter() + .map(|&input_var| Value::Var(input_var).print(printer)); if input_dests.len() == 1 { input_dests.next().unwrap() } else { @@ -3841,30 +3859,6 @@ impl Print for FuncAt<'_, Node> { } } -impl Print for RegionInputDecl { - type Output = AttrsAndDef; - fn print(&self, printer: &Printer<'_>) -> AttrsAndDef { - let Self { attrs, ty } = *self; - - AttrsAndDef { - attrs: attrs.print(printer), - def_without_name: printer.pretty_type_ascription_suffix(ty), - } - } -} - -impl Print for NodeOutputDecl { - type Output = AttrsAndDef; - fn print(&self, printer: &Printer<'_>) -> AttrsAndDef { - let Self { attrs, ty } = *self; - - AttrsAndDef { - attrs: attrs.print(printer), - def_without_name: printer.pretty_type_ascription_suffix(ty), - } - } -} - impl FuncAt<'_, DataInst> { fn print_data_inst(&self, printer: &Printer<'_>) -> pretty::Fragment { let DataInstDef { attrs, kind, inputs, child_regions, outputs } = self.def(); @@ -3876,13 +3870,12 @@ impl FuncAt<'_, DataInst> { // HACK(eddyb) multi-output instructions don't exist pre-disaggregate. let output_type = if !outputs.is_empty() { assert_eq!(outputs.len(), 1); - Some(outputs[0].ty) + Some(self.at(outputs[0]).decl().ty) } else { None }; - let mut output_use_to_print_as_lhs = output_type - .map(|_| Use::Var(VarKind::NodeOutput { node: self.position, output_idx: 0 })); + let mut output_use_to_print_as_lhs = output_type.map(|_| Use::Var(outputs[0])); let mut output_type_to_print = output_type; @@ -4384,12 +4377,6 @@ impl SelectionKind { } } -impl Value { - fn print_as_def(&self, printer: &Printer<'_>) -> pretty::Fragment { - Use::from(*self).print_as_def(printer) - } -} - impl Print for Value { type Output = pretty::Fragment; fn print(&self, printer: &Printer<'_>) -> pretty::Fragment { diff --git a/src/qptr/lift.rs b/src/qptr/lift.rs index 90bd1002..890dbe57 100644 --- a/src/qptr/lift.rs +++ b/src/qptr/lift.rs @@ -6,11 +6,11 @@ use crate::qptr::{QPtrAttr, QPtrOp}; use crate::transform::{InnerInPlaceTransform, InnerTransform, Transformed, Transformer}; use crate::{ AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstDef, ConstKind, Context, DataInst, - DataInstDef, DataInstKind, DeclDef, Diag, DiagLevel, EntityDefs, Func, FuncDecl, FxIndexMap, - GlobalVar, GlobalVarDecl, Module, Node, NodeKind, NodeOutputDecl, Region, Type, TypeDef, - TypeKind, TypeOrConst, Value, VarKind, spv, + DataInstDef, DataInstKind, DeclDef, Diag, DiagLevel, EntityDefs, EntityOrientedDenseMap, Func, + FuncDecl, FxIndexMap, GlobalVar, GlobalVarDecl, Module, Node, NodeKind, Region, Type, TypeDef, + TypeKind, TypeOrConst, Value, Var, VarDecl, spv, }; -use rustc_hash::FxHashMap; +use itertools::Either; use smallvec::SmallVec; use std::cell::Cell; use std::mem; @@ -398,10 +398,9 @@ struct LiftToSpvPtrInstsInFunc<'a> { /// /// The `QPtrOp` itself is only removed after the entire function is lifted, /// (using `var_use_counts` to determine whether they're truly unused). - deferred_ptr_noops: FxIndexMap, + deferred_ptr_noops: FxIndexMap, - // TODO(eddyb) switch this back to `EntityOrientedDenseMap` (removing `Option`). - var_use_counts: FxHashMap>, + var_use_counts: EntityOrientedDenseMap, // HACK(eddyb) this is used to avoid noise when `mem::analyze` failed. func_has_mem_analysis_bug_diags: bool, @@ -435,8 +434,8 @@ impl LiftToSpvPtrInstsInFunc<'_> { // FIXME(eddyb) maybe all this data should be packaged up together in a // type with fields like those of `DeferredPtrNoop` (or even more). let type_of_val_as_spv_ptr_with_layout = |v: Value| { - if let Value::Var(VarKind::NodeOutput { node: v_data_inst, output_idx: 0 }) = v - && let Some(ptr_noop) = self.deferred_ptr_noops.get(&v_data_inst) + if let Value::Var(var) = v + && let Some(ptr_noop) = self.deferred_ptr_noops.get(&var) { return Ok(( ptr_noop.output_pointer_addr_space, @@ -468,8 +467,9 @@ impl LiftToSpvPtrInstsInFunc<'_> { } DataInstKind::Mem(MemOp::FuncLocalVar(_mem_layout)) => { - let mem_accesses = - self.lifter.find_mem_accesses_attr(data_inst_def.outputs[0].attrs)?; + let mem_accesses = self + .lifter + .find_mem_accesses_attr(func.at(data_inst_def.outputs[0]).decl().attrs)?; // FIXME(eddyb) validate against `mem_layout`! let pointee_type = self.lifter.pointee_type_for_accesses(mem_accesses)?; @@ -479,9 +479,9 @@ impl LiftToSpvPtrInstsInFunc<'_> { opcode: wk.OpVariable, imms: [spv::Imm::Short(wk.StorageClass, wk.Function)].into_iter().collect(), }); - data_inst_def.outputs[0].attrs = - self.lifter.strip_mem_accesses_attr(data_inst_def.outputs[0].attrs); - data_inst_def.outputs[0].ty = + let output_decl = func_at_data_inst.reborrow().at(data_inst_def.outputs[0]).decl(); + output_decl.attrs = self.lifter.strip_mem_accesses_attr(output_decl.attrs); + output_decl.ty = self.lifter.spv_ptr_type(AddrSpace::SpvStorageClass(wk.Function), pointee_type); data_inst_def } @@ -507,7 +507,8 @@ impl LiftToSpvPtrInstsInFunc<'_> { let mut data_inst_def = data_inst_def.clone(); data_inst_def.kind = DataInstKind::SpvInst(wk.OpAccessChain.into()); - data_inst_def.outputs[0].ty = self.lifter.spv_ptr_type(addr_space, handle_type); + let output_decl = func_at_data_inst.reborrow().at(data_inst_def.outputs[0]).decl(); + output_decl.ty = self.lifter.spv_ptr_type(addr_space, handle_type); data_inst_def } DataInstKind::QPtr(QPtrOp::BufferData) => { @@ -520,7 +521,7 @@ impl LiftToSpvPtrInstsInFunc<'_> { }; self.deferred_ptr_noops.insert( - data_inst, + data_inst_def.outputs[0], DeferredPtrNoop { output_pointer: buf_ptr, output_pointer_addr_space: addr_space, @@ -531,8 +532,12 @@ impl LiftToSpvPtrInstsInFunc<'_> { // FIXME(eddyb) avoid the repeated call to `type_of_val`, // maybe don't even replace the `QPtrOp::BufferData` instruction? - let mut data_inst_def = data_inst_def.clone(); - data_inst_def.outputs[0].ty = type_of_val(buf_ptr); + let data_inst_def = data_inst_def.clone(); + + let new_output_ty = type_of_val(buf_ptr); + let output_decl = func_at_data_inst.reborrow().at(data_inst_def.outputs[0]).decl(); + output_decl.ty = new_output_ty; + data_inst_def } &DataInstKind::QPtr(QPtrOp::BufferDynLen { fixed_base_size, dyn_unit_stride }) => { @@ -653,7 +658,7 @@ impl LiftToSpvPtrInstsInFunc<'_> { let mut data_inst_def = data_inst_def.clone(); if access_chain_inputs.len() == 1 { self.deferred_ptr_noops.insert( - data_inst, + data_inst_def.outputs[0], DeferredPtrNoop { output_pointer: base_ptr, output_pointer_addr_space: addr_space, @@ -665,12 +670,16 @@ impl LiftToSpvPtrInstsInFunc<'_> { // FIXME(eddyb) avoid the repeated call to `type_of_val`, // maybe don't even replace the `QPtrOp::Offset` instruction? data_inst_def.kind = QPtrOp::Offset(0).into(); - data_inst_def.outputs[0].ty = type_of_val(base_ptr); + let new_output_ty = type_of_val(base_ptr); + let output_decl = + func_at_data_inst.reborrow().at(data_inst_def.outputs[0]).decl(); + output_decl.ty = new_output_ty; } else { data_inst_def.kind = DataInstKind::SpvInst(wk.OpAccessChain.into()); data_inst_def.inputs = access_chain_inputs; - data_inst_def.outputs[0].ty = - self.lifter.spv_ptr_type(addr_space, layout.original_type); + let output_decl = + func_at_data_inst.reborrow().at(data_inst_def.outputs[0]).decl(); + output_decl.ty = self.lifter.spv_ptr_type(addr_space, layout.original_type); } data_inst_def } @@ -748,15 +757,15 @@ impl LiftToSpvPtrInstsInFunc<'_> { let mut data_inst_def = data_inst_def.clone(); data_inst_def.kind = DataInstKind::SpvInst(wk.OpAccessChain.into()); data_inst_def.inputs = access_chain_inputs; - data_inst_def.outputs[0].ty = - self.lifter.spv_ptr_type(addr_space, layout.original_type); + let output_decl = func_at_data_inst.reborrow().at(data_inst_def.outputs[0]).decl(); + output_decl.ty = self.lifter.spv_ptr_type(addr_space, layout.original_type); data_inst_def } DataInstKind::Mem(op @ (MemOp::Load | MemOp::Store)) => { // HACK(eddyb) `_` will match multiple variants soon. #[allow(clippy::match_wildcard_for_single_variants)] let (spv_opcode, access_type) = match op { - MemOp::Load => (wk.OpLoad, data_inst_def.outputs[0].ty), + MemOp::Load => (wk.OpLoad, func.at(data_inst_def.outputs[0]).decl().ty), MemOp::Store => (wk.OpStore, type_of_val(data_inst_def.inputs[1])), _ => unreachable!(), }; @@ -766,14 +775,15 @@ impl LiftToSpvPtrInstsInFunc<'_> { let input_idx = 0; let ptr = data_inst_def.inputs[input_idx]; let (addr_space, pointee_layout) = type_of_val_as_spv_ptr_with_layout(ptr)?; - self.maybe_adjust_pointer_for_access( - ptr, - addr_space, - pointee_layout, - access_type, - )? - .map(|access_chain_data_inst_def| (input_idx, access_chain_data_inst_def)) - .into_iter() + self.maybe_adjust_pointer_for_access(ptr, pointee_layout, access_type)? + .map(|access_chain_data_inst_def| { + ( + input_idx, + self.lifter.spv_ptr_type(addr_space, access_type), + access_chain_data_inst_def, + ) + }) + .into_iter() }; let mut new_data_inst_def = DataInstDef { @@ -782,15 +792,27 @@ impl LiftToSpvPtrInstsInFunc<'_> { }; // FIXME(eddyb) written in a more general style for future deduplication. - for (input_idx, mut access_chain_data_inst_def) in maybe_ajustment { + for (input_idx, access_chain_output_ptr_type, mut access_chain_data_inst_def) in + maybe_ajustment + { // HACK(eddyb) account for `deferred_ptr_noops` interactions. self.resolve_deferred_ptr_noop_uses(&mut access_chain_data_inst_def.inputs); self.add_value_uses(&access_chain_data_inst_def.inputs); - let access_chain_data_inst = func_at_data_inst - .reborrow() - .nodes - .define(cx, access_chain_data_inst_def.into()); + let func = func_at_data_inst.reborrow().at(()); + + let access_chain_data_inst = + func.nodes.define(cx, access_chain_data_inst_def.into()); + let access_chain_output_var = func.vars.define( + cx, + VarDecl { + attrs: Default::default(), + ty: access_chain_output_ptr_type, + def_parent: Either::Right(access_chain_data_inst), + def_idx: 0, + }, + ); + func.nodes[access_chain_data_inst].outputs.push(access_chain_output_var); // HACK(eddyb) can't really use helpers like `FuncAtMut::def`, // due to the need to borrow `regions` and `nodes` @@ -798,18 +820,13 @@ impl LiftToSpvPtrInstsInFunc<'_> { // types for "where a list is in a parent entity" could be used // to make this more ergonomic, although the potential need for // an actual list entity of its own, should be considered. - let data_inst = func_at_data_inst.position; - let func = func_at_data_inst.reborrow().at(()); func.regions[self.parent_region.unwrap()].children.insert_before( access_chain_data_inst, data_inst, func.nodes, ); - new_data_inst_def.inputs[input_idx] = Value::Var(VarKind::NodeOutput { - node: access_chain_data_inst, - output_idx: 0, - }); + new_data_inst_def.inputs[input_idx] = Value::Var(access_chain_output_var); } new_data_inst_def @@ -834,13 +851,16 @@ impl LiftToSpvPtrInstsInFunc<'_> { if let Some(access_chain_data_inst_def) = self .maybe_adjust_pointer_for_access( input_ptr, - input_ptr_addr_space, input_pointee_layout, expected_pointee_type, )? { - to_spv_ptr_input_adjustments - .push((input_idx, access_chain_data_inst_def)); + to_spv_ptr_input_adjustments.push(( + input_idx, + self.lifter + .spv_ptr_type(input_ptr_addr_space, expected_pointee_type), + access_chain_data_inst_def, + )); } } Attr::QPtr(QPtrAttr::FromSpvPtrOutput { addr_space, pointee }) => { @@ -857,16 +877,28 @@ impl LiftToSpvPtrInstsInFunc<'_> { let mut new_data_inst_def = data_inst_def.clone(); + let func = func_at_data_inst.reborrow().at(()); + // FIXME(eddyb) deduplicate with `Load`/`Store`. - for (input_idx, mut access_chain_data_inst_def) in to_spv_ptr_input_adjustments { + for (input_idx, access_chain_output_ptr_type, mut access_chain_data_inst_def) in + to_spv_ptr_input_adjustments + { // HACK(eddyb) account for `deferred_ptr_noops` interactions. self.resolve_deferred_ptr_noop_uses(&mut access_chain_data_inst_def.inputs); self.add_value_uses(&access_chain_data_inst_def.inputs); - let access_chain_data_inst = func_at_data_inst - .reborrow() - .nodes - .define(cx, access_chain_data_inst_def.into()); + let access_chain_data_inst = + func.nodes.define(cx, access_chain_data_inst_def.into()); + let access_chain_output_var = func.vars.define( + cx, + VarDecl { + attrs: Default::default(), + ty: access_chain_output_ptr_type, + def_parent: Either::Right(access_chain_data_inst), + def_idx: 0, + }, + ); + func.nodes[access_chain_data_inst].outputs.push(access_chain_output_var); // HACK(eddyb) can't really use helpers like `FuncAtMut::def`, // due to the need to borrow `regions` and `nodes` @@ -874,22 +906,17 @@ impl LiftToSpvPtrInstsInFunc<'_> { // types for "where a list is in a parent entity" could be used // to make this more ergonomic, although the potential need for // an actual list entity of its own, should be considered. - let data_inst = func_at_data_inst.position; - let func = func_at_data_inst.reborrow().at(()); func.regions[self.parent_region.unwrap()].children.insert_before( access_chain_data_inst, data_inst, func.nodes, ); - new_data_inst_def.inputs[input_idx] = Value::Var(VarKind::NodeOutput { - node: access_chain_data_inst, - output_idx: 0, - }); + new_data_inst_def.inputs[input_idx] = Value::Var(access_chain_output_var); } if let Some((addr_space, pointee_type)) = from_spv_ptr_output { - new_data_inst_def.outputs[0].ty = + func.vars[new_data_inst_def.outputs[0]].ty = self.lifter.spv_ptr_type(addr_space, pointee_type); } @@ -902,12 +929,14 @@ impl LiftToSpvPtrInstsInFunc<'_> { /// If necessary, construct an `OpAccessChain` instruction to turn `ptr` /// (pointing to a type with `pointee_layout`) into a pointer to `access_type` /// (which can then be used with e.g. `OpLoad`/`OpStore`). + /// + /// **Note**: the returned instruction has empty `outputs`, the caller must + /// add one (of type `self.lifter.spv_ptr_type(addr_space, access_type)`). // // FIXME(eddyb) customize errors, to tell apart Load/Store/ToSpvPtrInput. fn maybe_adjust_pointer_for_access( &self, ptr: Value, - addr_space: AddrSpace, mut pointee_layout: TypeLayout, access_type: Type, ) -> Result, LiftError> { @@ -1007,12 +1036,7 @@ impl LiftToSpvPtrInstsInFunc<'_> { kind: DataInstKind::SpvInst(wk.OpAccessChain.into()), inputs: access_chain_inputs, child_regions: [].into_iter().collect(), - outputs: [NodeOutputDecl { - attrs: Default::default(), - ty: self.lifter.spv_ptr_type(addr_space, access_type), - }] - .into_iter() - .collect(), + outputs: [].into_iter().collect(), }) } else { None @@ -1028,8 +1052,8 @@ impl LiftToSpvPtrInstsInFunc<'_> { for v in values { // FIXME(eddyb) the loop could theoretically be avoided, but that'd // make tracking use counts harder. - while let Value::Var(VarKind::NodeOutput { node: inst, output_idx: 0 }) = *v { - match self.deferred_ptr_noops.get(&inst) { + while let Value::Var(var) = *v { + match self.deferred_ptr_noops.get(&var) { Some(ptr_noop) => { *v = ptr_noop.output_pointer; } @@ -1044,7 +1068,7 @@ impl LiftToSpvPtrInstsInFunc<'_> { fn add_value_uses(&mut self, values: &[Value]) { for &v in values { if let Value::Var(v) = v { - let count = self.var_use_counts.entry(v).or_default(); + let count = self.var_use_counts.entry(v); *count = Some( NonZeroU32::new(count.map_or(0, |c| c.get()).checked_add(1).unwrap()).unwrap(), ); @@ -1054,7 +1078,7 @@ impl LiftToSpvPtrInstsInFunc<'_> { fn remove_value_uses(&mut self, values: &[Value]) { for &v in values { if let Value::Var(v) = v { - let count = self.var_use_counts.entry(v).or_default(); + let count = self.var_use_counts.entry(v); *count = NonZeroU32::new(count.unwrap().get() - 1); } } @@ -1095,12 +1119,16 @@ impl Transformer for LiftToSpvPtrInstsInFunc<'_> { let mut lifted = self.try_lift_data_inst_def(func_at_node.reborrow()); if let Ok(Transformed::Unchanged) = lifted { - let data_inst_def = func_at_node.reborrow().def(); + let func_at_node = func_at_node.reborrow().freeze(); + let data_inst_def = func_at_node.def(); if let DataInstKind::QPtr(_) = data_inst_def.kind { lifted = Err(LiftError(Diag::bug(["unimplemented qptr instruction".into()]))); } else { - for output in &data_inst_def.outputs { - if matches!(self.lifter.cx[output.ty].kind, TypeKind::QPtr) { + for &output_var in &data_inst_def.outputs { + if matches!( + self.lifter.cx[func_at_node.at(output_var).decl().ty].kind, + TypeKind::QPtr + ) { lifted = Err(LiftError(Diag::bug([ "unimplemented qptr-producing instruction".into(), ]))); @@ -1151,19 +1179,11 @@ impl Transformer for LiftToSpvPtrInstsInFunc<'_> { let deferred_ptr_noops = mem::take(&mut self.deferred_ptr_noops); // NOTE(eddyb) reverse order is important, as each removal can reduce // use counts of an earlier definition, allowing further removal. - for (inst, ptr_noop) in deferred_ptr_noops.into_iter().rev() { - let is_used = func_def_body - .at(inst) - .def() - .outputs - .iter() - .enumerate() - .map(|(i, _)| VarKind::NodeOutput { - node: inst, - output_idx: i.try_into().unwrap(), - }) - .any(|v| self.var_use_counts.get(&v).is_some_and(|count| count.is_some())); + for (output_var, ptr_noop) in deferred_ptr_noops.into_iter().rev() { + let is_used = self.var_use_counts.get(output_var).is_some(); if !is_used { + let inst = func_def_body.at(output_var).decl().def_parent.right().unwrap(); + // HACK(eddyb) can't really use helpers like `FuncAtMut::def`, // due to the need to borrow `regions` and `nodes` // at the same time - perhaps some kind of `FuncAtMut` position diff --git a/src/qptr/lower.rs b/src/qptr/lower.rs index 9d771e07..e2cddefb 100644 --- a/src/qptr/lower.rs +++ b/src/qptr/lower.rs @@ -6,10 +6,10 @@ use crate::qptr::{QPtrAttr, QPtrOp}; use crate::transform::{InnerInPlaceTransform, Transformed, Transformer}; use crate::{ AddrSpace, AttrSet, AttrSetDef, Const, ConstDef, ConstKind, Context, DataInst, DataInstDef, - DataInstKind, Diag, FuncDecl, GlobalVarDecl, Node, NodeKind, NodeOutputDecl, OrdAssertEq, - Region, Type, TypeKind, TypeOrConst, Value, VarKind, spv, + DataInstKind, Diag, FuncDecl, GlobalVarDecl, Node, NodeKind, OrdAssertEq, Region, Type, + TypeKind, TypeOrConst, Value, VarDecl, spv, }; -use itertools::Itertools as _; +use itertools::{Either, Itertools as _}; use smallvec::SmallVec; use std::cell::Cell; use std::num::NonZeroU32; @@ -420,7 +420,7 @@ impl LowerFromSpvPtrInstsInFunc<'_> { let replacement_kind_and_inputs = if spv_inst.opcode == wk.OpVariable { assert!(data_inst_def.inputs.len() <= 1); let (_, var_data_type) = - self.lowerer.as_spv_ptr_type(outputs[0].ty).ok_or_else(|| { + self.lowerer.as_spv_ptr_type(func.at(outputs[0]).decl().ty).ok_or_else(|| { LowerError(Diag::bug(["output type not an `OpTypePointer`".into()])) })?; match self.lowerer.layout_of(var_data_type)? { @@ -545,23 +545,30 @@ impl LowerFromSpvPtrInstsInFunc<'_> { let mut ptr = base_ptr; for step in steps { + let func = func_at_data_inst.reborrow().at(()); + let (kind, inputs) = step.into_data_inst_kind_and_inputs(ptr); - let step_data_inst = func_at_data_inst.reborrow().nodes.define( + let step_data_inst = func.nodes.define( cx, DataInstDef { attrs: Default::default(), kind, inputs, child_regions: [].into_iter().collect(), - outputs: [NodeOutputDecl { - attrs: Default::default(), - ty: self.lowerer.qptr_type(), - }] - .into_iter() - .collect(), + outputs: [].into_iter().collect(), } .into(), ); + let step_output_var = func.vars.define( + cx, + VarDecl { + attrs: Default::default(), + ty: self.lowerer.qptr_type(), + def_parent: Either::Right(step_data_inst), + def_idx: 0, + }, + ); + func.nodes[step_data_inst].outputs.push(step_output_var); // HACK(eddyb) can't really use helpers like `FuncAtMut::def`, // due to the need to borrow `regions` and `nodes` @@ -569,21 +576,20 @@ impl LowerFromSpvPtrInstsInFunc<'_> { // types for "where a list is in a parent entity" could be used // to make this more ergonomic, although the potential need for // an actual list entity of its own, should be considered. - let func = func_at_data_inst.reborrow().at(()); func.regions[self.parent_region.unwrap()].children.insert_before( step_data_inst, data_inst, func.nodes, ); - ptr = Value::Var(VarKind::NodeOutput { node: step_data_inst, output_idx: 0 }); + ptr = Value::Var(step_output_var); } final_step.into_data_inst_kind_and_inputs(ptr) } else if spv_inst.opcode == wk.OpBitcast { let input = data_inst_def.inputs[0]; // Pointer-to-pointer casts are noops on `qptr`. if self.lowerer.as_spv_ptr_type(func.at(input).type_of(cx)).is_some() - && self.lowerer.as_spv_ptr_type(outputs[0].ty).is_some() + && self.lowerer.as_spv_ptr_type(func.at(outputs[0]).decl().ty).is_some() { // HACK(eddyb) noop cases should not use any `DataInst`s at all, // but that would require the ability to replace all uses of a `Value`. @@ -651,8 +657,9 @@ impl LowerFromSpvPtrInstsInFunc<'_> { } } // HACK(eddyb) multi-output instructions don't exist pre-disaggregate. - if let Some(output) = data_inst_def.outputs.iter().at_most_one().ok().unwrap() - && let Some((addr_space, pointee)) = self.lowerer.as_spv_ptr_type(output.ty) + if let Some(&output_var) = data_inst_def.outputs.iter().at_most_one().ok().unwrap() + && let Some((addr_space, pointee)) = + self.lowerer.as_spv_ptr_type(func.at(output_var).decl().ty) { old_and_new_attrs.get_or_insert_with(get_old_attrs).attrs.insert( QPtrAttr::FromSpvPtrOutput { diff --git a/src/spv/lift.rs b/src/spv/lift.rs index ca3f5fb5..38a73c17 100644 --- a/src/spv/lift.rs +++ b/src/spv/lift.rs @@ -8,8 +8,8 @@ use crate::{ AddrSpace, Attr, AttrSet, Const, ConstDef, ConstKind, Context, DataInst, DataInstDef, DataInstKind, DbgSrcLoc, DeclDef, ExportKey, Exportee, Func, FuncDecl, FuncParam, FxIndexMap, FxIndexSet, GlobalVar, GlobalVarDefBody, Import, Module, ModuleDebugInfo, ModuleDialect, Node, - NodeKind, NodeOutputDecl, OrdAssertEq, Region, RegionInputDecl, Type, TypeDef, TypeKind, - TypeOrConst, Value, VarKind, + NodeKind, OrdAssertEq, Region, Type, TypeDef, TypeKind, TypeOrConst, Value, Var, VarDecl, + VarKind, }; use itertools::Itertools; use rustc_hash::FxHashMap; @@ -251,6 +251,10 @@ struct AllocatedIds<'a> { // FIXME(eddyb) should this use ID ranges instead of `SmallVec<[spv::Id; 4]>`? struct FuncLifting<'a> { + // HACK(eddyb) temporary workaround before it's clear how to map everything + // to use the new `Var` abstraction effectively. + vars: Option<&'a crate::EntityDefs>, + func_id: spv::Id, param_ids: SmallVec<[spv::Id; 4]>, @@ -530,6 +534,8 @@ impl<'a> FuncLifting<'a> { let func_def_body = match &func_decl.def { DeclDef::Imported(_) => { return Ok(Self { + vars: None, + func_id, param_ids, region_inputs_source: Default::default(), @@ -560,7 +566,8 @@ impl<'a> FuncLifting<'a> { .def() .inputs .iter() - .map(|&RegionInputDecl { attrs, ty }| { + .map(|&input_var| { + let &VarDecl { attrs, ty, .. } = func_def_body.at(input_var).decl(); Ok(Phi { attrs, ty, @@ -593,15 +600,17 @@ impl<'a> FuncLifting<'a> { loop_body_inputs .iter() - .enumerate() - .map(|(i, &RegionInputDecl { attrs, ty })| { + .zip_eq(&node_def.inputs) + .map(|(&input_var, &initial_value)| { + let &VarDecl { attrs, ty, .. } = + func_def_body.at(input_var).decl(); Ok(Phi { attrs, ty, result_id: alloc_id()?, cases: FxIndexMap::default(), - default_value: Some(node_def.inputs[i]), + default_value: Some(initial_value), }) }) .collect::>()? @@ -615,7 +624,9 @@ impl<'a> FuncLifting<'a> { NodeKind::Select(_) => node_def .outputs .iter() - .map(|&NodeOutputDecl { attrs, ty }| { + .map(|&output_var| { + let &VarDecl { attrs, ty, .. } = + func_def_body.at(output_var).decl(); Ok(Phi { attrs, ty, @@ -1053,6 +1064,8 @@ impl<'a> FuncLifting<'a> { .filter(|&inst| !func_def_body.at(inst).def().outputs.is_empty()); Ok(Self { + vars: Some(&func_def_body.vars), + func_id, param_ids, region_inputs_source, @@ -1170,30 +1183,34 @@ impl LazyInst<'_, '_> { _ => ids.globals[&Global::Const(ct)], }, - Value::Var(VarKind::RegionInput { region, input_idx }) => { - let input_idx = usize::try_from(input_idx).unwrap(); - match parent_func.region_inputs_source.get(®ion) { - Some(RegionInputsSource::FuncParams) => parent_func.param_ids[input_idx], - Some(&RegionInputsSource::LoopHeaderPhis(loop_node)) => { - parent_func.blocks[&CfgPoint::NodeEntry(loop_node)].phis[input_idx] - .result_id - } - None => { - parent_func.blocks[&CfgPoint::RegionEntry(region)].phis[input_idx].result_id + Value::Var(v) => match parent_func.vars.unwrap()[v].kind() { + VarKind::RegionInput { region, input_idx } => { + let input_idx = usize::try_from(input_idx).unwrap(); + match parent_func.region_inputs_source.get(®ion) { + Some(RegionInputsSource::FuncParams) => parent_func.param_ids[input_idx], + Some(&RegionInputsSource::LoopHeaderPhis(loop_node)) => { + parent_func.blocks[&CfgPoint::NodeEntry(loop_node)].phis[input_idx] + .result_id + } + None => { + parent_func.blocks[&CfgPoint::RegionEntry(region)].phis[input_idx] + .result_id + } } } - } - Value::Var(VarKind::NodeOutput { node, output_idx }) => { - if let Some(&data_inst_output_id) = parent_func.data_inst_output_ids.get(&node) { - // HACK(eddyb) multi-output instructions don't exist pre-disaggregate. - assert_eq!(output_idx, 0); - data_inst_output_id - } else { - parent_func.blocks[&CfgPoint::NodeExit(node)].phis - [usize::try_from(output_idx).unwrap()] - .result_id + VarKind::NodeOutput { node, output_idx } => { + if let Some(&data_inst_output_id) = parent_func.data_inst_output_ids.get(&node) + { + // HACK(eddyb) multi-output instructions don't exist pre-disaggregate. + assert_eq!(output_idx, 0); + data_inst_output_id + } else { + parent_func.blocks[&CfgPoint::NodeExit(node)].phis + [usize::try_from(output_idx).unwrap()] + .result_id + } } - } + }, }; let (result_id, attrs, _) = self.result_id_attrs_and_import(module, ids); @@ -1358,7 +1375,7 @@ impl LazyInst<'_, '_> { without_ids: inst, // HACK(eddyb) multi-output instructions don't exist pre-disaggregate. result_type_id: (data_inst_def.outputs.iter().at_most_one().ok().unwrap()) - .map(|o| ids.globals[&Global::Type(o.ty)]), + .map(|&o| ids.globals[&Global::Type(parent_func.vars.unwrap()[o].ty)]), result_id, ids: extra_initial_id_operand .into_iter() diff --git a/src/spv/lower.rs b/src/spv/lower.rs index 195fb7a7..ed0804b1 100644 --- a/src/spv/lower.rs +++ b/src/spv/lower.rs @@ -6,10 +6,10 @@ use crate::spv::{self, spec}; use crate::{ AddrSpace, Attr, AttrSet, Const, ConstDef, ConstKind, Context, DataInstDef, DataInstKind, DbgSrcLoc, DeclDef, Diag, EntityDefs, ExportKey, Exportee, Func, FuncDecl, FuncDefBody, - FuncParam, FxIndexMap, GlobalVarDecl, GlobalVarDefBody, Import, InternedStr, Module, - NodeOutputDecl, Region, RegionDef, RegionInputDecl, Type, TypeDef, TypeKind, TypeOrConst, - Value, VarKind, print, + FuncParam, FxIndexMap, GlobalVarDecl, GlobalVarDefBody, Import, InternedStr, Module, Region, + RegionDef, Type, TypeDef, TypeKind, TypeOrConst, Value, VarDecl, print, }; +use itertools::Either; use rustc_hash::FxHashMap; use smallvec::SmallVec; use std::collections::{BTreeMap, BTreeSet}; @@ -817,6 +817,7 @@ impl Module { DeclDef::Present(FuncDefBody { regions, nodes: Default::default(), + vars: Default::default(), body, unstructured_cfg: Some(cf::unstructured::ControlFlowGraph::default()), }) @@ -959,6 +960,8 @@ impl Module { def_map }) }; + // FIXME(eddyb) remove the now-unnecessary indentation. + // FIXME(eddyb) rethink this first part as "allocating `Var`s". { for raw_inst in &raw_insts { let IntraFuncInst { @@ -1253,19 +1256,25 @@ impl Module { let ty = result_type.unwrap(); params.push(FuncParam { attrs, ty }); + if let Some(func_def_body) = &mut func_def_body { - let body_inputs = &mut func_def_body.at_mut_body().def().inputs; - let input_idx = u32::try_from(body_inputs.len()).unwrap(); - body_inputs.push(RegionInputDecl { attrs, ty }); - - local_id_defs.insert( - result_id.unwrap(), - LocalIdDef::Value(Value::Var(VarKind::RegionInput { - region: func_def_body.body, - input_idx, - })), + let body_inputs = &mut func_def_body.regions[func_def_body.body].inputs; + let input_var = func_def_body.vars.define( + &cx, + VarDecl { + attrs, + ty: result_type.unwrap(), + + def_parent: Either::Left(func_def_body.body), + def_idx: body_inputs.len().try_into().unwrap(), + }, ); + body_inputs.push(input_var); + + local_id_defs + .insert(result_id.unwrap(), LocalIdDef::Value(Value::Var(input_var))); } + continue; } let func_def_body = func_def_body.as_deref_mut().unwrap(); @@ -1320,19 +1329,23 @@ impl Module { current_block.shadowed_local_id_defs.extend( current_block.details.cfgssa_inter_block_uses.iter().map( |(&used_id, &ty)| { - let input_idx = - u32::try_from(current_block_region_def.inputs.len()).unwrap(); - current_block_region_def - .inputs - .push(RegionInputDecl { attrs: AttrSet::default(), ty }); - - ( - used_id, - LocalIdDef::Value(Value::Var(VarKind::RegionInput { - region: current_block.region, - input_idx, - })), - ) + let input_var = func_def_body.vars.define( + &cx, + VarDecl { + attrs: AttrSet::default(), + ty, + + def_parent: Either::Left(current_block.region), + def_idx: current_block_region_def + .inputs + .len() + .try_into() + .unwrap(), + }, + ); + current_block_region_def.inputs.push(input_var); + + (used_id, LocalIdDef::Value(Value::Var(input_var))) }, ), ); @@ -1498,16 +1511,20 @@ impl Module { let ty = result_type.unwrap(); - let input_idx = u32::try_from(current_block_region_def.inputs.len()).unwrap(); - current_block_region_def.inputs.push(RegionInputDecl { attrs, ty }); + let input_var = func_def_body.vars.define( + &cx, + VarDecl { + attrs, + ty, - local_id_defs.insert( - result_id.unwrap(), - LocalIdDef::Value(Value::Var(VarKind::RegionInput { - region: current_block.region, - input_idx, - })), + def_parent: Either::Left(current_block.region), + def_idx: current_block_region_def.inputs.len().try_into().unwrap(), + }, ); + current_block_region_def.inputs.push(input_var); + + local_id_defs + .insert(result_id.unwrap(), LocalIdDef::Value(Value::Var(input_var))); } else if [wk.OpSelectionMerge, wk.OpLoopMerge].contains(&opcode) { let is_second_to_last_in_block = lookahead_raw_inst(2) .is_none_or(|next_raw_inst| next_raw_inst.without_ids.opcode == wk.OpLabel); @@ -1616,28 +1633,33 @@ impl Module { }) .collect::>()?, child_regions: [].into_iter().collect(), - outputs: result_id - .map(|_| { - result_type.ok_or_else(|| { - invalid( - "expected value-producing instruction, \ - with a result type", - ) - }) - }) - .transpose()? - .into_iter() - .map(|ty| { - // FIXME(eddyb) split attrs between output and inst. - NodeOutputDecl { attrs: AttrSet::default(), ty } - }) - .collect(), + outputs: [].into_iter().collect(), }; let inst = func_def_body.nodes.define(&cx, data_inst_def.into()); if let Some(result_id) = result_id { - let output = Value::Var(VarKind::NodeOutput { node: inst, output_idx: 0 }); - local_id_defs.insert(result_id, LocalIdDef::Value(output)); + let ty = result_type.ok_or_else(|| { + invalid( + "expected value-producing instruction, \ + with a result type", + ) + })?; + + let outputs = &mut func_def_body.nodes[inst].outputs; + let output_var = func_def_body.vars.define( + &cx, + VarDecl { + // FIXME(eddyb) split attrs between output and inst. + attrs: AttrSet::default(), + ty, + + def_parent: Either::Right(inst), + def_idx: outputs.len().try_into().unwrap(), + }, + ); + outputs.push(output_var); + + local_id_defs.insert(result_id, LocalIdDef::Value(Value::Var(output_var))); } current_block_region_def.children.insert_last(inst, &mut func_def_body.nodes); diff --git a/src/transform.rs b/src/transform.rs index c6b427af..4cb38c04 100644 --- a/src/transform.rs +++ b/src/transform.rs @@ -8,8 +8,8 @@ use crate::{ AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstDef, ConstKind, DataInstKind, DbgSrcLoc, DeclDef, EntityListIter, ExportKey, Exportee, Func, FuncDecl, FuncDefBody, FuncParam, GlobalVar, GlobalVarDecl, GlobalVarDefBody, Import, Module, ModuleDebugInfo, ModuleDialect, - Node, NodeDef, NodeKind, NodeOutputDecl, OrdAssertEq, Region, RegionDef, RegionInputDecl, Type, - TypeDef, TypeKind, TypeOrConst, Value, VarKind, spv, + Node, NodeDef, NodeKind, OrdAssertEq, Region, RegionDef, Type, TypeDef, TypeKind, TypeOrConst, + Value, Var, VarDecl, spv, }; use std::cmp::Ordering; use std::rc::Rc; @@ -199,6 +199,9 @@ pub trait Transformer: Sized { fn in_place_transform_node_def(&mut self, mut func_at_node: FuncAtMut<'_, Node>) { func_at_node.inner_in_place_transform_with(self); } + fn in_place_transform_var_decl(&mut self, func_at_var: FuncAtMut<'_, Var>) { + func_at_var.decl().inner_in_place_transform_with(self); + } } /// Trait implemented on "transformable" types, to further "elaborate" a type by @@ -591,8 +594,11 @@ impl InnerInPlaceTransform for FuncAtMut<'_, Region> { // HACK(eddyb) handle the fields of `Region` separately, to // allow reborrowing `FuncAtMut` (for recursing into `Node`s). let RegionDef { inputs, children: _, outputs: _ } = self.reborrow().def(); - for input in inputs { - input.inner_transform_with(transformer).apply_to(input); + + // FIXME(eddyb) dedup with the future `FuncAtMut` solution. + for input_idx in 0..inputs.len() { + let input = self.reborrow().def().inputs[input_idx]; + transformer.in_place_transform_var_decl(self.reborrow().at(input)); } self.reborrow().at_children().into_iter().inner_in_place_transform_with(transformer); @@ -604,20 +610,6 @@ impl InnerInPlaceTransform for FuncAtMut<'_, Region> { } } -impl InnerTransform for RegionInputDecl { - fn inner_transform_with(&self, transformer: &mut impl Transformer) -> Transformed { - let Self { attrs, ty } = self; - - transform!({ - attrs -> transformer.transform_attr_set_use(*attrs), - ty -> transformer.transform_type_use(*ty), - } => Self { - attrs, - ty, - }) - } -} - impl InnerInPlaceTransform for FuncAtMut<'_, EntityListIter> { fn inner_in_place_transform_with(&mut self, transformer: &mut impl Transformer) { let mut iter = self.reborrow(); @@ -668,23 +660,20 @@ impl InnerInPlaceTransform for FuncAtMut<'_, Node> { transformer.transform_value_use(repeat_condition).apply_to(repeat_condition); } - for output in outputs { - output.inner_transform_with(transformer).apply_to(output); + // FIXME(eddyb) dedup with the future `FuncAtMut` solution. + for output_idx in 0..outputs.len() { + let output = self.reborrow().def().outputs[output_idx]; + transformer.in_place_transform_var_decl(self.reborrow().at(output)); } } } -impl InnerTransform for NodeOutputDecl { - fn inner_transform_with(&self, transformer: &mut impl Transformer) -> Transformed { - let Self { attrs, ty } = self; +impl InnerInPlaceTransform for VarDecl { + fn inner_in_place_transform_with(&mut self, transformer: &mut impl Transformer) { + let Self { attrs, ty, def_parent: _, def_idx: _ } = self; - transform!({ - attrs -> transformer.transform_attr_set_use(*attrs), - ty -> transformer.transform_type_use(*ty), - } => Self { - attrs, - ty, - }) + transformer.transform_attr_set_use(*attrs).apply_to(attrs); + transformer.transform_type_use(*ty).apply_to(ty); } } @@ -721,18 +710,8 @@ impl InnerTransform for Value { Self::Const(ct) => transform!({ ct -> transformer.transform_const_use(*ct), } => Self::Const(ct)), - Self::Var(var) => transform!({ - var -> var.inner_transform_with(transformer), - } => Self::Var(var)), - } - } -} - -impl InnerTransform for VarKind { - fn inner_transform_with(&self, _transformer: &mut impl Transformer) -> Transformed { - match self { - Self::RegionInput { region: _, input_idx: _ } - | Self::NodeOutput { node: _, output_idx: _ } => Transformed::Unchanged, + // FIXME(eddyb) maybe there should be a `transform_var_use`? + Self::Var(_) => Transformed::Unchanged, } } } diff --git a/src/visit.rs b/src/visit.rs index 30a8c28a..b48fcbf2 100644 --- a/src/visit.rs +++ b/src/visit.rs @@ -8,8 +8,8 @@ use crate::{ AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstDef, ConstKind, DataInstKind, DbgSrcLoc, DeclDef, DiagMsgPart, EntityListIter, ExportKey, Exportee, Func, FuncDecl, FuncDefBody, FuncParam, GlobalVar, GlobalVarDecl, GlobalVarDefBody, Import, Module, ModuleDebugInfo, - ModuleDialect, Node, NodeDef, NodeKind, NodeOutputDecl, OrdAssertEq, Region, RegionDef, - RegionInputDecl, Type, TypeDef, TypeKind, TypeOrConst, Value, VarKind, spv, + ModuleDialect, Node, NodeDef, NodeKind, OrdAssertEq, Region, RegionDef, Type, TypeDef, + TypeKind, TypeOrConst, Value, Var, VarDecl, spv, }; // FIXME(eddyb) `Sized` bound shouldn't be needed but removing it requires @@ -65,6 +65,9 @@ pub trait Visitor<'a>: Sized { fn visit_node_def(&mut self, func_at_node: FuncAt<'a, Node>) { func_at_node.inner_visit_with(self); } + fn visit_var_decl(&mut self, func_at_var: FuncAt<'a, Var>) { + func_at_var.decl().inner_visit_with(self); + } fn visit_value_use(&mut self, v: &'a Value) { v.inner_visit_with(self); } @@ -438,8 +441,8 @@ impl<'a> FuncAt<'a, Region> { pub fn inner_visit_with(self, visitor: &mut impl Visitor<'a>) { let RegionDef { inputs, children, outputs } = self.def(); - for input in inputs { - input.inner_visit_with(visitor); + for &input in inputs { + visitor.visit_var_decl(self.at(input)); } self.at(*children).into_iter().inner_visit_with(visitor); for v in outputs { @@ -448,15 +451,6 @@ impl<'a> FuncAt<'a, Region> { } } -impl InnerVisit for RegionInputDecl { - fn inner_visit_with<'a>(&'a self, visitor: &mut impl Visitor<'a>) { - let Self { attrs, ty } = *self; - - visitor.visit_attr_set_use(attrs); - visitor.visit_type_use(ty); - } -} - // FIXME(eddyb) this can't implement `InnerVisit` because of the `&'a self` // requirement, whereas this has `'a` in `self: FuncAt<'a, ...>`. impl<'a> FuncAt<'a, EntityListIter> { @@ -503,15 +497,15 @@ impl<'a> FuncAt<'a, Node> { visitor.visit_value_use(repeat_condition); } - for output in outputs { - output.inner_visit_with(visitor); + for &output in outputs { + visitor.visit_var_decl(self.at(output)); } } } -impl InnerVisit for NodeOutputDecl { +impl InnerVisit for VarDecl { fn inner_visit_with<'a>(&'a self, visitor: &mut impl Visitor<'a>) { - let Self { attrs, ty } = *self; + let Self { attrs, ty, def_parent: _, def_idx: _ } = *self; visitor.visit_attr_set_use(attrs); visitor.visit_type_use(ty); @@ -549,16 +543,8 @@ impl InnerVisit for Value { fn inner_visit_with<'a>(&'a self, visitor: &mut impl Visitor<'a>) { match self { &Self::Const(ct) => visitor.visit_const_use(ct), - Self::Var(var) => var.inner_visit_with(visitor), - } - } -} - -impl InnerVisit for VarKind { - fn inner_visit_with<'a>(&'a self, _visitor: &mut impl Visitor<'a>) { - match *self { - Self::RegionInput { region: _, input_idx: _ } - | Self::NodeOutput { node: _, output_idx: _ } => {} + // FIXME(eddyb) maybe there should be a `visit_var_use`? + Self::Var(_) => {} } } }