diff --git a/src/cf/structurize.rs b/src/cf/structurize.rs index 7d358120..d8ffa811 100644 --- a/src/cf/structurize.rs +++ b/src/cf/structurize.rs @@ -3,9 +3,8 @@ // FIXME(eddyb) consider moving docs to the module level? use crate::cf::SelectionKind; -use crate::cf::unstructured::{ - ControlInst, ControlInstKind, IncomingEdgeCount, LoopFinder, TraversalState, -}; +use crate::cf::unstructured::{ControlTarget, IncomingEdgeCount, LoopFinder, TraversalState}; +use crate::func_at::FuncAtMut; use crate::transform::{InnerInPlaceTransform as _, Transformed, Transformer}; use crate::{ AttrSet, Const, ConstDef, ConstKind, Context, DbgSrcLoc, EntityOrientedDenseMap, FuncDefBody, @@ -145,10 +144,10 @@ enum StructurizeRegionState { /// **Note**: `target` has a generic type `T` to reduce redundancy when it's /// already implied (e.g. by the key in [`DeferredEdgeBundleSet`]'s map). struct IncomingEdgeBundle { - /// Attributes from the original [`ControlInst`]s (likely debuginfo), kept - /// when merging only when exactly identical, which can naturally be the case - /// for debuginfo (e.g. for branches from inside `if`-`else`/`switch` to a - /// common merge point, just after the whole control-flow construct). + /// Attributes from the original `thunk`s (likely debuginfo), kept when + /// merging only when exactly identical, which can naturally be the case + /// for debuginfo (e.g. for branches from inside `if`-`else`/`switch` to + /// a common merge point, just after the whole control-flow construct). // // FIXME(eddyb) semantically filter these, maybe focus on debuginfo? attrs: AttrSet, @@ -253,12 +252,14 @@ struct LazyCondDyn { /// A target for one of the edge bundles in a [`DeferredEdgeBundleSet`], mostly /// separate from [`Region`] to allow expressing returns as well. +// +// FIXME(eddyb) consider reusing `ControlTarget` for this. #[derive(Copy, Clone, PartialEq, Eq, Hash)] enum DeferredTarget { Region(Region), /// Structured "return" out of the function (with `target_inputs` used for - /// the function body `output`s, i.e. inputs of [`ControlInstKind::Return`]). + /// the function body `output`s). Return, } @@ -587,7 +588,8 @@ impl<'a> Structurizer<'a> { .as_ref() .map(|cfg| { let loop_header_to_exit_targets = - LoopFinder::new(cfg).find_all_loops_starting_at(func_def_body.body); + LoopFinder::new(cx, func_def_body.at(()), cfg) + .find_all_loops_starting_at(func_def_body.body); let mut state = TraversalState { incoming_edge_counts: EntityOrientedDenseMap::new(), @@ -881,7 +883,7 @@ impl<'a> Structurizer<'a> { self.cx, NodeDef { // FIXME(eddyb) could it be possible to synthesize attrs - // from `ControlInst`s' attrs and/or `OpLoopMerge`'s? + // from thunks' attrs and/or `OpLoopMerge`'s? attrs: AttrSet::default(), kind: NodeKind::Loop { // TODO(eddyb) make this a regular body output (first one?). @@ -1040,54 +1042,118 @@ impl<'a> Structurizer<'a> { } } - let control_inst_on_exit = self - .func_def_body - .unstructured_cfg - .as_mut() - .unwrap() - .control_inst_on_exit_from - .remove(region) - .expect( - "cfg::Structurizer::structurize_region: missing \ - `ControlInst` (CFG wasn't unstructured in the first place?)", + // FIXME(eddyb) `ControlFlowGraph::edges_from_thunk_tailed_region` + // overlaps a lot with this, but is only traversing, not "stealing", + // so there might not be an easy way to deduplicate between the two. + // HACK(eddyb) only not a method because of its very limited usability + // (e.g. it modifies the function in such a way to guarantee that being + // called twice with the same arguments would result in panics). + // + // FIXME(eddyb) consider keeping `thunk`s in, initially, plumbing them + // in the same way that the "deferred edge" conditions and target inputs + // are today, but instead of generating `if`-`else` on conditions, using + // some kind of "`thunk` `switch`" node (handling only claimed regions), + // which would leave behind the subset of unhandled cases, merged with + // new potential destinations from within the handled cases, and doing + // this "unfolding" of the CFG would be the main point of structurization, + // with actual encodings for the remaining `thunk`s being implemented + // as a separate step (with optimizations such as integers replacing + // the "one-hot" `bool` condition encoding, reusing output slots with + // the same type - or even the same bit width - between targets, etc.). + fn steal_tail_thunk_from_region( + this: &mut Structurizer<'_>, + edge_source_region: Region, + thunk_binding_region: Region, + ) -> Result { + let thunk = + mem::take(&mut this.func_def_body.at_mut(thunk_binding_region).def().outputs) + .into_iter() + .exactly_one() + .ok() + .unwrap(); + + let thunk_node = match thunk { + Value::Var(thunk) => match this.func_def_body.at(thunk).decl().kind() { + VarKind::NodeOutput { node, output_idx: 0 } => node, + _ => unreachable!(), + }, + Value::Const(ct) => match this.cx[ct].kind { + ConstKind::Undef => return Err(DeferredEdgeBundleSet::Unreachable), + _ => unreachable!(), + }, + }; + + let thunk_node_def = mem::replace( + this.func_def_body.at_mut(thunk_node).def(), + // HACK(eddyb) there isn't a good "tombstone" to use here, + // but really the only thing we care about is no + // heap allocations remain around. + // FIXME(eddyb) come up with a better "nop" or add first-class + // tombstones (maybe with generational entity refs?). + NodeDef { + attrs: Default::default(), + kind: NodeKind::Select(SelectionKind::BoolCond), + inputs: Default::default(), + child_regions: Default::default(), + outputs: Default::default(), + }, ); - // Start with the concatenation of `region` and `control_inst_on_exit`, - // always appending `Node`s (including the children of entire - // `ClaimedRegion`s) to `region`'s definition itself. - let mut deferred_edges = { - let ControlInst { attrs, kind, inputs, targets, target_inputs } = control_inst_on_exit; + assert!( + Value::Var(thunk_node_def.outputs.into_iter().exactly_one().ok().unwrap()) == thunk + ); - let target_regions: SmallVec<[_; 8]> = targets - .iter() - .map(|&target| { - self.try_claim_edge_bundle(IncomingEdgeBundle { - attrs: if targets.len() == 1 { attrs } else { AttrSet::default() }, + let thunk_binding_def = &mut this.func_def_body.regions[thunk_binding_region]; + { + let thunk_binding_region_children = thunk_binding_def.children.iter(); + assert!(thunk_binding_region_children.last == Some(thunk_node)); + if edge_source_region != thunk_binding_region { + assert!( + thunk_binding_region_children.first == thunk_binding_region_children.last + ); + } + } + thunk_binding_def.children.remove(thunk_node, &mut this.func_def_body.nodes); + + match thunk_node_def.kind { + NodeKind::ThunkBind(target) => { + let target = match target { + ControlTarget::Region(target) => target, + ControlTarget::Return => { + return Err(DeferredEdgeBundleSet::Always { + target: DeferredTarget::Return, + edge_bundle: IncomingEdgeBundle { + attrs: thunk_node_def.attrs, + accumulated_count: IncomingEdgeCount::default(), + target: (), + target_inputs: thunk_node_def.inputs, + }, + }); + } + }; + this.try_claim_edge_bundle(IncomingEdgeBundle { + attrs: thunk_node_def.attrs, target, accumulated_count: IncomingEdgeCount::ONE, - target_inputs: target_inputs.get(&target).cloned().unwrap_or_default(), + target_inputs: thunk_node_def.inputs, }) .map_err(|edge_bundle| { // HACK(eddyb) special-case "shared `unreachable`" to // always inline it and avoid awkward "merges". // FIXME(eddyb) should this be in a separate CFG pass? // (i.e. is there a risk of other logic needing this?) - let target_is_trivial_unreachable = - match self.structurize_region_state.get(&edge_bundle.target) { - Some(StructurizeRegionState::Ready { - region_deferred_edges: DeferredEdgeBundleSet::Unreachable, - .. - }) => { - // FIXME(eddyb) DRY this "is empty region" check. - self.func_def_body - .at(edge_bundle.target) - .at_children() - .into_iter() - .next() - .is_none() - } - _ => false, - }; + let target_is_trivial_unreachable = match this + .structurize_region_state + .get(&edge_bundle.target) + { + Some(StructurizeRegionState::Ready { + region_deferred_edges: DeferredEdgeBundleSet::Unreachable, + .. + }) => { + this.func_def_body.at(edge_bundle.target).def().children.is_empty() + } + _ => false, + }; if target_is_trivial_unreachable { DeferredEdgeBundleSet::Unreachable } else { @@ -1097,88 +1163,39 @@ impl<'a> Structurizer<'a> { } } }) - }) - .collect(); - - match kind { - ControlInstKind::Unreachable => { - // FIXME(eddyb) this loses `attrs`. - let _ = attrs; - - assert_eq!((inputs.len(), target_regions.len()), (0, 0)); - - // FIXME(eddyb) this may result in lost optimizations over - // actually encoding it in `Node`/`Region` - // (e.g. a new `NodeKind`, or replacing region `outputs`), - // but it's simpler to handle it like this. - // - // NOTE(eddyb) actually, this encoding is lossless *during* - // structurization, and a divergent region can only end up as: - // - the function body, where it implies the function can - // never actually return: not fully structurized currently - // (but only for a silly reason, and is entirely fixable) - // - a `Select` case, where it implies that case never merges - // back into the `Select` node, and potentially that the - // case can never be taken: this is where a structured - // encoding can be introduced, by pruning unreachable - // cases, and potentially even introducing `assume`s - // - a `Loop` body is not actually possible when divergent - // (as there can be no backedge to form a cyclic CFG) - DeferredEdgeBundleSet::Unreachable } - - ControlInstKind::ExitInvocation(kind) => { - assert_eq!(target_regions.len(), 0); - - let node = self.func_def_body.nodes.define( - self.cx, - NodeDef { - attrs, - kind: NodeKind::ExitInvocation(kind), - inputs, - child_regions: [].into_iter().collect(), - outputs: [].into_iter().collect(), - } - .into(), - ); - self.func_def_body.regions[region] - .children - .insert_last(node, &mut self.func_def_body.nodes); - - DeferredEdgeBundleSet::Unreachable - } - - ControlInstKind::Return => { - assert_eq!(target_regions.len(), 0); - - DeferredEdgeBundleSet::Always { - target: DeferredTarget::Return, - edge_bundle: IncomingEdgeBundle { - attrs, - accumulated_count: IncomingEdgeCount::default(), - target: (), - target_inputs: inputs, - }, - } - } - - ControlInstKind::Branch => { - assert_eq!(inputs.len(), 0); - - self.append_maybe_claimed_region( - region, - target_regions.into_iter().exactly_one().ok().unwrap(), - ) - } - - ControlInstKind::SelectBranch(kind) => { - assert_eq!(inputs.len(), 1); - - let scrutinee = inputs[0]; - - self.structurize_select_into(region, attrs, kind, Ok(scrutinee), target_regions) + // FIXME(eddyb) try to reuse the existing node and regions, + // which housed the individual thunks for CFG edges, in the + // final structured control-flow. + NodeKind::Select(kind) => { + assert!(edge_source_region == thunk_binding_region); + + let scrutinee = thunk_node_def.inputs.into_iter().exactly_one().ok().unwrap(); + + let cases = thunk_node_def + .child_regions + .into_iter() + .map(|case| steal_tail_thunk_from_region(this, edge_source_region, case)) + .collect(); + + Err(this.structurize_select_into( + edge_source_region, + thunk_node_def.attrs, + kind, + Ok(scrutinee), + cases, + )) } + _ => unreachable!(), } + } + + // Start with the concatenation of `region` and its unstructured thunk, + // always appending `Node`s (including the children of entire + // `ClaimedRegion`s) to `region`'s definition itself. + let mut deferred_edges = { + let tail_thunk = steal_tail_thunk_from_region(self, region, region); + self.append_maybe_claimed_region(region, tail_thunk) }; // Try to resolve deferred edges that may have accumulated, and keep @@ -1805,8 +1822,8 @@ impl<'a> Structurizer<'a> { } /// When structurization is only partial, and there remain unclaimed regions, - /// they have to be reintegrated into the CFG, putting back [`ControlInst`]s - /// where `structurize_region` has taken them from. + /// they have to be reintegrated into the CFG, putting back `thunk`s where + /// `structurize_region` has taken them from. /// /// This function handles one region at a time to make it more manageable, /// despite it having a single call site (in a loop in `structurize_func`). @@ -1822,118 +1839,145 @@ impl<'a> Structurizer<'a> { after it takes `structurize_region_state`" ); + let cx = self.cx; + + let thunk_ty = cx.intern(TypeKind::Thunk); + let build_thunk = |func_at_region: FuncAtMut<'_, Region>, (target, target_inputs)| { + let region = func_at_region.position; + let func = func_at_region.at(()); + + let target = match target { + DeferredTarget::Region(target) => ControlTarget::Region(target), + DeferredTarget::Return => ControlTarget::Return, + }; + + let thunk_node = func.nodes.define( + cx, + NodeDef { + attrs: AttrSet::default(), + kind: NodeKind::ThunkBind(target), + inputs: target_inputs, + child_regions: [].into_iter().collect(), + outputs: [].into_iter().collect(), + } + .into(), + ); + func.regions[region].children.insert_last(thunk_node, func.nodes); + + let thunk_var = func.vars.define( + cx, + VarDecl { + attrs: AttrSet::default(), + ty: thunk_ty, + + def_parent: Either::Right(thunk_node), + def_idx: 0, + }, + ); + func.nodes[thunk_node].outputs.push(thunk_var); + + Value::Var(thunk_var) + }; + // Build a chain of conditional branches to apply deferred edges. let mut control_source = Some(region); loop { let taken_then; - (taken_then, deferred_edges) = - deferred_edges.split_out_matching(|deferred| match deferred.edge_bundle.target { - DeferredTarget::Region(target) => { - Ok((deferred.condition, (target, deferred.edge_bundle.target_inputs))) - } - DeferredTarget::Return => Err(deferred), - }); - let Some((condition, then_target_and_inputs)) = taken_then else { + (taken_then, deferred_edges) = deferred_edges.split_out_matching(|deferred| { + Ok(( + deferred.condition, + (deferred.edge_bundle.target, deferred.edge_bundle.target_inputs), + )) + }); + let Some((condition, then_edge)) = taken_then else { break; }; + let branch_source = control_source.take().unwrap(); - let else_target_and_inputs = match deferred_edges { + let else_edge = match deferred_edges { // At most one deferral left, so it can be used as the "else" // case, or the branch left unconditional in its absence. DeferredEdgeBundleSet::Unreachable => None, - DeferredEdgeBundleSet::Always { - target: DeferredTarget::Region(else_target), - edge_bundle, - } => { + DeferredEdgeBundleSet::Always { target: else_target, edge_bundle } => { deferred_edges = DeferredEdgeBundleSet::Unreachable; Some((else_target, edge_bundle.target_inputs)) } - // Either more branches, or a deferred return, are needed, so - // the "else" case must be a `Region` that itself can - // have a `ControlInst` attached to it later on. - _ => { + // More branches are needed, so the "else" case must be a `Region` + // that itself can have a `thunk` attached to it later on. + DeferredEdgeBundleSet::Choice { .. } => { let new_empty_region = - self.func_def_body.regions.define(self.cx, RegionDef::default()); + self.func_def_body.regions.define(cx, RegionDef::default()); control_source = Some(new_empty_region); - Some((new_empty_region, [].into_iter().collect())) + Some((DeferredTarget::Region(new_empty_region), [].into_iter().collect())) } }; - let condition = Some(condition) - .filter(|_| else_target_and_inputs.is_some()) - .map(|cond| self.materialize_lazy_cond(&cond)); - let branch_control_inst = ControlInst { - attrs: AttrSet::default(), - kind: if condition.is_some() { - ControlInstKind::SelectBranch(SelectionKind::BoolCond) - } else { - ControlInstKind::Branch - }, - inputs: condition.into_iter().collect(), - targets: [&then_target_and_inputs] - .into_iter() - .chain(&else_target_and_inputs) - .map(|&(target, _)| target) - .collect(), - target_inputs: [then_target_and_inputs] + let thunk = if let Some(else_edge) = else_edge { + let condition = self.materialize_lazy_cond(&condition); + + let cases = [then_edge, else_edge] .into_iter() - .chain(else_target_and_inputs) - .filter(|(_, inputs)| !inputs.is_empty()) - .collect(), + .map(|target_with_inputs| { + let case = self.func_def_body.regions.define(cx, RegionDef::default()); + let thunk = + build_thunk(self.func_def_body.at_mut(case), target_with_inputs); + self.func_def_body.regions[case].outputs.push(thunk); + case + }) + .collect(); + + let select_node = self.func_def_body.nodes.define( + cx, + NodeDef { + attrs: AttrSet::default(), + kind: NodeKind::Select(SelectionKind::BoolCond), + inputs: [condition].into_iter().collect(), + child_regions: cases, + outputs: [].into_iter().collect(), + } + .into(), + ); + self.func_def_body.regions[branch_source] + .children + .insert_last(select_node, &mut self.func_def_body.nodes); + + let select_thunk_var = self.func_def_body.vars.define( + cx, + VarDecl { + attrs: AttrSet::default(), + ty: thunk_ty, + + def_parent: Either::Right(select_node), + def_idx: 0, + }, + ); + self.func_def_body.nodes[select_node].outputs.push(select_thunk_var); + + Value::Var(select_thunk_var) + } else { + build_thunk(self.func_def_body.at_mut(branch_source), then_edge) }; - assert!( - self.func_def_body - .unstructured_cfg - .as_mut() - .unwrap() - .control_inst_on_exit_from - .insert(branch_source, branch_control_inst) - .is_none() - ); - } - let deferred_return = match deferred_edges { - DeferredEdgeBundleSet::Unreachable => None, - DeferredEdgeBundleSet::Always { target: DeferredTarget::Return, edge_bundle } => { - Some(edge_bundle.target_inputs) - } - _ => unreachable!(), - }; + self.func_def_body.regions[branch_source].outputs = [thunk].into_iter().collect(); + } let final_source = match control_source { Some(region) => region, - None => { - // The loop above handled all the targets, nothing left to do. - assert!(deferred_return.is_none()); - return; - } + // The loop above handled all the targets, nothing left to do. + None => return, }; - // Final deferral is either a `Return` (if needed), or an `Unreachable` - // (only when truly divergent, i.e. no `deferred_edges`/`deferred_return`). - let final_control_inst = { - let (kind, inputs) = match deferred_return { - Some(return_values) => (ControlInstKind::Return, return_values), - None => (ControlInstKind::Unreachable, [].into_iter().collect()), - }; - ControlInst { - attrs: AttrSet::default(), - kind, - inputs, - targets: [].into_iter().collect(), - target_inputs: FxIndexMap::default(), - } - }; - assert!( - self.func_def_body - .unstructured_cfg - .as_mut() - .unwrap() - .control_inst_on_exit_from - .insert(final_source, final_control_inst) - .is_none() - ); + // Final deferral is unreachable (only when truly divergent, + // i.e. no `deferred_edges`). + // FIXME(eddyb) this should probably be special-cased at the start of + // this function, instead of here at the end. + let final_thunk = Value::Const(cx.intern(ConstDef { + attrs: AttrSet::default(), + ty: thunk_ty, + kind: ConstKind::Undef, + })); + self.func_def_body.regions[final_source].outputs = [final_thunk].into_iter().collect(); } /// Create an undefined constant (as a placeholder where a value needs to be diff --git a/src/cf/unstructured.rs b/src/cf/unstructured.rs index af6a6254..fec3a3be 100644 --- a/src/cf/unstructured.rs +++ b/src/cf/unstructured.rs @@ -1,66 +1,91 @@ //! Unstructured control-flow graph (CFG) abstractions and utilities. +use crate::func_at::FuncAt; use crate::{ - AttrSet, EntityOrientedDenseMap, FuncDefBody, FxIndexMap, FxIndexSet, Region, Value, cf, + AttrSet, Const, ConstKind, Context, EntityOrientedDenseMap, FuncDefBody, FxIndexMap, + FxIndexSet, NodeKind, Region, Value, VarKind, }; -use itertools::Either; +use itertools::{Either, Itertools as _}; use smallvec::SmallVec; -/// The control-flow graph (CFG) of a function, as control-flow instructions -/// ([`ControlInst`]s) attached to [`Region`]s, as an "action on exit", i.e. -/// "terminator" (while intra-region control-flow is strictly structured). +/// The control-flow graph (CFG) of a function, as control-flow edges (`thunk`s) +/// attached to [`Region`]s, as an "action on exit" (similar to "terminator"s), +/// while intra-region control-flow is strictly structured. +// +// FIXME(eddyb) update docs above (or remove `ControlFlowGraph` entirely). +// +// FIXME(eddyb) treat "loop `break`s" (branches to the "loop merge" block) +// as Rust-like "`break` out of labeled block" for a block around the loop, +// e.g. `loop { ... break; ... }` -> `'label: { loop { ... break 'label; ... } }`, +// and encode the "labeled block" aspect using a node for "executing a CFG", +// with a `thunk` output type whenever the loop body has control-flow edges +// to the outside of the loop (and they're not to the "loop merge" block), +// such as `break`-ing out of an enclosing `switch`, or even `return`ing. +// +// FIXME(eddyb) the above approach could also be extended to encode other +// kinds of "merges" (`OpSelectionMerge`, or even the "continue" of a loop) +// as well (same "break out of labeled block" idea, really), and inside-out +// structurization should then preserve all possible SPIR-V merge hierarchies +// (even if structured SPIR-T lacks e.g. `switch` fallthru, it should always +// correctly reconverge, thanks to structurization never duplicating regions). +// +// FIXME(eddyb) in the end, it seems like there is a need for *both*: +// 1. `thunk` `switch`ing (a new `SelectionKind` variant?) +// - structurization (and similar transforms) can be seen as replacing +// `thunk` with boolean/integer target encodings, along the "target inputs", +// as plain values region can output, and `if`-`else`/`switch` for dispatch +// 2. `thunk` `loop`ing (new node or some kind of modified `NodeKind::Loop`?) +// - can be seen as taking the fixpoint of a `thunk -> thunk` region, +// which, in the fully unstructured CFG case, would consist of a single node +// (specifically a "`thunk` `switch`" dispatching all regions in the CFG, +// which makes it feel wasteful, but there has to be a way to distinguish +// between single-step and "until `break`-ing out") +// - maybe this can be done using the `thunk` `switch` node, with extra modes +// in e.g. its `SelectionKind` variant(s), such as "has a default case" and +// "`loop`-`switch` until `break" (although there is an unique hazard with +// reusing `NodeKind::Select` while having regions with different signatures, +// in that passes might assume matching signatures, and also any kind of +// looping control-flow has to account for the potential to execute any +// of the child regions, and therefore any side-effect, more than once etc.) #[derive(Clone, Default)] pub struct ControlFlowGraph { - pub control_inst_on_exit_from: EntityOrientedDenseMap, - // HACK(eddyb) this currently only comes from `OpLoopMerge`, and cannot be // inferred (because implies too strong of an ownership/uniqueness notion). + // + // FIXME(eddyb) this is the only reason for `ControlFlowGraph` to still + // exist in this state, and should be replaced with one of the ideas above. pub loop_merge_to_loop_header: FxIndexMap, } -#[derive(Clone)] -pub struct ControlInst { - pub attrs: AttrSet, - - pub kind: ControlInstKind, - - pub inputs: SmallVec<[Value; 2]>, - - // FIXME(eddyb) change the inline size of this to fit most instructions. - pub targets: SmallVec<[Region; 4]>, - - /// `target_inputs[region][input_idx]` is the [`Value`] that - /// `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") - /// cannot tell apart multiple edges with the same source and destination. - pub target_inputs: FxIndexMap>, -} - -#[derive(Clone)] -pub enum ControlInstKind { - /// Reaching this point in the control-flow is undefined behavior, e.g.: - /// * a `SelectBranch` case that's known to be impossible - /// * after a function call, where the function never returns - /// - /// Optimizations can take advantage of this information, to assume that any - /// necessary preconditions for reaching this point, are never met. - Unreachable, +#[derive(Copy, Clone, PartialEq, Eq, Hash)] +pub enum ControlTarget { + Region(Region), - /// Leave the current function, optionally returning a value. + /// Leave the current function (returning `target_inputs`, if any). + // + // FIXME(eddyb) now that this is used through `NodeKind::ThunkBind`, + // it should probably be more like `break` or some kind of "leave scope". + // FIXME(eddyb) one way to encode this unambiguously, even when dealing + // with e.g. multiple nested CFGs (such as the "execute a CFG" node idea), + // is to pick a region as the function-scoped "exit" (or "postdominator"), + // and in the worst-case it would only serve to output its own inputs + // (alternatively, non-trivial exits from "execute a CFG" can use `thunk` + // wrapping, so in the case of e.g. `return`ing from inside N nested loops, + // one wrap N `break`-like thunks, with each of the N levels being unwrapped + // by leaving the loops etc. - but that still technically has the ambiguity, + // and would result in the nested encoding showing up in the final structured + // result, so e.g. even if integers are used, N of them would be neeeded). Return, +} - /// Leave the current invocation, similar to returning from every function - /// call in the stack (up to and including the entry-point), but potentially - /// indicating a fatal error as well. - ExitInvocation(cf::ExitInvocationKind), - - /// Unconditional branch to a single target. - Branch, +// HACK(eddyb) this is only public so it can be shared with `spv::lift`. +#[derive(Copy, Clone, PartialEq, Eq, Hash)] +pub struct ControlEdge { + pub source_region: Region, - /// Branch to one of several targets, chosen by a single value input. - SelectBranch(cf::SelectionKind), + // HACK(eddyb) this is the minimal amount of information needed to look up + // the thunk, given access to the function (including `ControlFlowGraph`). + pub thunk_binding_region: Region, } impl ControlFlowGraph { @@ -90,6 +115,99 @@ impl ControlFlowGraph { ); post_order.into_iter().rev() } + + pub fn edges_from<'a>( + &'a self, + func_at_edge_source: FuncAt<'a, Region>, + ) -> impl DoubleEndedIterator + ExactSizeIterator + Clone + 'a { + self.edges_from_thunk_tailed_region(func_at_edge_source, func_at_edge_source.position) + } + + // FIXME(eddyb) properly distinguish the different failure cases possible, + // maybe even avoid panicking entirely? + fn edges_from_thunk_tailed_region<'a>( + &'a self, + func_at_edge_source: FuncAt<'a, Region>, + thunk_binding_region: Region, + ) -> impl DoubleEndedIterator + ExactSizeIterator + Clone + 'a { + let func = func_at_edge_source.at(()); + let edge_source_region = func_at_edge_source.position; + + let thunk = + func.at(thunk_binding_region).def().outputs.iter().copied().exactly_one().ok().unwrap(); + + let thunk_node = match thunk { + Value::Var(thunk) => match func.at(thunk).decl().kind() { + VarKind::NodeOutput { node, output_idx: 0 } => node, + _ => unreachable!(), + }, + Value::Const(_) => { + return Either::Left( + [ControlEdge { source_region: edge_source_region, thunk_binding_region }] + .into_iter(), + ); + } + }; + + let thunk_binding_region_children = func.at(thunk_binding_region).def().children.iter(); + assert!(thunk_binding_region_children.last == Some(thunk_node)); + if edge_source_region != thunk_binding_region { + assert!(thunk_binding_region_children.first == thunk_binding_region_children.last); + } + + let thunk_node_def = func.at(thunk_node).def(); + match thunk_node_def.kind { + NodeKind::ThunkBind(_) => Either::Left( + [ControlEdge { source_region: edge_source_region, thunk_binding_region }] + .into_iter(), + ), + NodeKind::Select(_) => { + assert!(edge_source_region == thunk_binding_region); + + Either::Right(thunk_node_def.child_regions.iter().map(move |&case| { + self.edges_from_thunk_tailed_region(func_at_edge_source, case) + .exactly_one() + .ok() + .unwrap() + })) + } + _ => unreachable!(), + } + } + + pub fn edge_attrs_target_and_inputs<'a>( + &self, + func_at_edge: FuncAt<'a, ControlEdge>, + ) -> (AttrSet, Result, &'a [Value]) { + let func = func_at_edge.at(()); + let edge = func_at_edge.position; + + let thunk = func + .at(edge.thunk_binding_region) + .def() + .outputs + .iter() + .copied() + .exactly_one() + .ok() + .unwrap(); + + match thunk { + Value::Var(thunk) => match func.at(thunk).decl().kind() { + VarKind::NodeOutput { node, output_idx: 0 } => { + let thunk_node_def = func.at(node).def(); + match thunk_node_def.kind { + NodeKind::ThunkBind(target) => { + (thunk_node_def.attrs, Ok(target), &thunk_node_def.inputs) + } + _ => unreachable!(), + } + } + _ => unreachable!(), + }, + Value::Const(ct) => (AttrSet::default(), Err(ct), &[]), + } + } } // HACK(eddyb) this only serves to disallow accessing `IncomingEdgeCount`'s private field. @@ -140,16 +258,18 @@ impl ControlFlowGraph { // Quick sanity check that this is the right CFG for `func_def_body`. assert!(std::ptr::eq(func_def_body.unstructured_cfg.as_ref().unwrap(), self)); - assert!(func_at_body.def().outputs.is_empty()); - self.traverse(func_def_body.body, state); + self.traverse(func_at_body, state); } fn traverse( &self, - region: Region, + func_at_region: FuncAt<'_, Region>, state: &mut TraversalState, ) { + let func = func_at_region.at(()); + let region = func_at_region.position; + // FIXME(eddyb) `EntityOrientedDenseMap` should have an `entry` API. if let Some(existing_count) = state.incoming_edge_counts.get_mut(region) { *existing_count += IncomingEdgeCount::ONE; @@ -159,23 +279,32 @@ impl ControlFlowGraph { (state.pre_order_visit)(region); - let control_inst = self - .control_inst_on_exit_from - .get(region) - .expect("cfg: missing `ControlInst`, despite having left structured control-flow"); + let targets = self.targets_of(func_at_region); - let targets = control_inst.targets.iter().copied(); + let targets = targets.filter_map(|target| match target { + Ok(ControlTarget::Region(target)) => Some(target), + Ok(ControlTarget::Return) | Err(_) => None, + }); let targets = if state.reverse_targets { Either::Left(targets.rev()) } else { Either::Right(targets) }; for target in targets { - self.traverse(target, state); + self.traverse(func.at(target), state); } (state.post_order_visit)(region); } + + fn targets_of<'a>( + &'a self, + func_at_edge_source: FuncAt<'a, Region>, + ) -> impl DoubleEndedIterator> + Clone + 'a { + let func = func_at_edge_source.at(()); + self.edges_from(func_at_edge_source) + .map(move |edge| self.edge_attrs_target_and_inputs(func.at(edge)).1) + } } /// Minimal loop analysis, based on Tarjan's SCC (strongly connected components) @@ -203,6 +332,8 @@ impl ControlFlowGraph { /// be misleading with explicit `break`s (moving user code from just before /// the `break` to after the loop), but is less impactful than "maximal loops" pub struct LoopFinder<'a> { + cx: &'a Context, + func: FuncAt<'a, ()>, cfg: &'a ControlFlowGraph, // FIXME(eddyb) this feels a bit inefficient (are many-exit loops rare?). @@ -266,9 +397,15 @@ impl std::ops::BitOrAssign for EventualCfgExits { } } +// HACK(eddyb) marker type for an `undef` thunk. +#[derive(Copy, Clone)] +struct Unreachable; + impl<'a> LoopFinder<'a> { - pub fn new(cfg: &'a ControlFlowGraph) -> Self { + pub fn new(cx: &'a Context, func: FuncAt<'a, ()>, cfg: &'a ControlFlowGraph) -> Self { Self { + cx, + func, cfg, loop_header_to_exit_targets: FxIndexMap::default(), scc_stack: vec![], @@ -312,7 +449,7 @@ impl<'a> LoopFinder<'a> { // `Complete` state until the loop header itself is complete, // and the monotonic nature of `EventualCfgExits` means that // the loop header will still get to see the complete picture. - (Some(scc_stack_idx), EventualCfgExits::default()) + (Some(scc_stack_idx), EventualCfgExits { may_return_from_func: false }) } SccState::Complete(eventual_cfg_exits) => (None, eventual_cfg_exits), }; @@ -321,22 +458,23 @@ impl<'a> LoopFinder<'a> { self.scc_stack.push(node); *state_entry = Some(SccState::Pending(scc_stack_idx)); - let control_inst = self - .cfg - .control_inst_on_exit_from - .get(node) - .expect("cfg: missing `ControlInst`, despite having left structured control-flow"); + let targets = self.targets_of(node); - let mut eventual_cfg_exits = EventualCfgExits::default(); + let mut eventual_cfg_exits = EventualCfgExits { + may_return_from_func: targets + .clone() + .any(|target| matches!(target, Ok(ControlTarget::Return))), + }; - if let ControlInstKind::Return = control_inst.kind { - eventual_cfg_exits.may_return_from_func = true; - } + let earliest_scc_root = targets + .flat_map(|target| { + let target = match target { + Ok(ControlTarget::Region(target)) => target, + Ok(ControlTarget::Return) | Err(Unreachable) => { + return None.into_iter().chain(None); + } + }; - let earliest_scc_root = control_inst - .targets - .iter() - .flat_map(|&target| { let (earliest_scc_root_of_target, eventual_cfg_exits_of_target) = self.find_earliest_scc_root_of(target); eventual_cfg_exits |= eventual_cfg_exits_of_target; @@ -390,7 +528,10 @@ impl<'a> LoopFinder<'a> { self.scc_stack[scc_start..] .iter() .flat_map(|&scc_node| { - self.cfg.control_inst_on_exit_from[scc_node].targets.iter().copied() + self.targets_of(scc_node).filter_map(|target| match target { + Ok(ControlTarget::Region(target)) => Some(target), + Ok(ControlTarget::Return) | Err(Unreachable) => None, + }) }) .filter(|&target| target_is_exit(target)) .collect(), @@ -426,4 +567,18 @@ impl<'a> LoopFinder<'a> { (earliest_scc_root, eventual_cfg_exits) } + + fn targets_of( + &self, + region: Region, + ) -> impl Iterator> + Clone + 'a { + let cx = self.cx; + + self.cfg.targets_of(self.func.at(region)).map(|edge| { + edge.map_err(|ct| match cx[ct].kind { + ConstKind::Undef => Unreachable, + _ => unreachable!(), + }) + }) + } } diff --git a/src/lib.rs b/src/lib.rs index 52c6ce1c..c5b3a120 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -552,6 +552,9 @@ pub enum TypeKind { // and kept separately in `VarDecl`, might be a better approach? QPtr, + // TODO(eddyb) reconsider name? add signature? etc. + Thunk, + SpvInst { spv_inst: spv::Inst, // FIXME(eddyb) find a better name. @@ -754,9 +757,8 @@ pub struct FuncDefBody { /// [`Node`], or function (the latter being a "structured return") /// * "divergent": execution gets stuck in the region (an infinite loop), /// or is aborted (e.g. `OpTerminateInvocation` from SPIR-V) -/// * "unstructured": [`Region`]s which connect to other [`Region`]s -/// using [`cfg::ControlInst`](crate::cfg::ControlInst)s (as described by a -/// [`cfg::ControlFlowGraph`](crate::cfg::ControlFlowGraph)) +/// * "unstructured": [`Region`]s which connect to other [`Region`]s using "`thunk`s" +/// (as described by [`cfg::ControlFlowGraph`](crate::cfg::ControlFlowGraph)) /// /// When a function's entire body can be described by a single [`Region`], /// that function is said to have (entirely) "structured control-flow". @@ -802,8 +804,7 @@ pub struct FuncDefBody { /// instead of in the merge (where phi nodes require special-casing, as /// their "uses" of all the "source" values would normally be illegal) /// * in unstructured control-flow, region `inputs` are additionally used for -/// representing phi nodes, as [`cfg::ControlInst`](crate::cfg::ControlInst)s -/// passing values to their target regions +/// representing phi nodes, as `thunk`s passing values to their target regions /// * all value uses across unstructured control-flow edges (i.e. not in the /// same region containing the value definition) *require* explicit passing, /// as unstructured control-flow [`Region`](crate::Region)s @@ -907,6 +908,9 @@ pub enum NodeKind { #[from] QPtr(qptr::QPtrOp), + // TODO(eddyb) document (maybe move into e.g. `cf::ThunkOp`?). + ThunkBind(cf::unstructured::ControlTarget), + // FIXME(eddyb) should this have `#[from]`? SpvInst(spv::Inst), SpvExtInst { diff --git a/src/mem/analyze.rs b/src/mem/analyze.rs index 6f54919b..762ff2a8 100644 --- a/src/mem/analyze.rs +++ b/src/mem/analyze.rs @@ -934,6 +934,7 @@ impl<'a> GatherAccesses<'a> { DataInstKind::FuncCall(_) | DataInstKind::Mem(_) | DataInstKind::QPtr(_) + | DataInstKind::ThunkBind(_) | DataInstKind::SpvInst(_) | DataInstKind::SpvExtInst { .. } => {} } @@ -1216,6 +1217,21 @@ impl<'a> GatherAccesses<'a> { ); } + DataInstKind::ThunkBind(_) => { + if data_inst_def + .inputs + .iter() + .any(|&v| is_qptr(func_def_body.at(v).type_of(&cx))) + { + accesses_or_err_attrs_to_attach.push(( + AttrTarget::Node(node), + Err(AnalysisError(Diag::bug([ + "unsupported `thunk.bind` with pointer inputs".into(), + ]))), + )); + } + } + DataInstKind::SpvInst(_) | DataInstKind::SpvExtInst { .. } => { let mut has_from_spv_ptr_output_attr = false; for attr in &cx[data_inst_def.attrs].attrs { diff --git a/src/mem/layout.rs b/src/mem/layout.rs index 7b0656af..9687afab 100644 --- a/src/mem/layout.rs +++ b/src/mem/layout.rs @@ -213,6 +213,9 @@ impl<'a> LayoutCache<'a> { ["`layout_of(qptr)` (already lowered?)".into()], ))); } + TypeKind::Thunk => { + return Err(LayoutError(Diag::bug(["`layout_of(thunk)`".into()]))); + } TypeKind::SpvInst { spv_inst, type_and_const_inputs } => { (spv_inst, type_and_const_inputs) } diff --git a/src/print/mod.rs b/src/print/mod.rs index 6124ceee..c7358129 100644 --- a/src/print/mod.rs +++ b/src/print/mod.rs @@ -613,20 +613,16 @@ impl<'a> Visitor<'a> for Plan<'a> { } } - fn visit_func_decl(&mut self, func_decl: &'a FuncDecl) { - if let DeclDef::Present(func_def_body) = &func_decl.def - && let Some(cfg) = &func_def_body.unstructured_cfg - { - for region in cfg.rev_post_order(func_def_body) { - if let Some(control_inst) = cfg.control_inst_on_exit_from.get(region) { - for &target in &control_inst.targets { - *self.use_counts.entry(Use::RegionLabel(target)).or_default() += 1; - } + fn visit_node_def(&mut self, func_at_node: FuncAt<'a, Node>) { + if let DataInstKind::ThunkBind(target) = func_at_node.def().kind { + match target { + cf::unstructured::ControlTarget::Region(target) => { + *self.use_counts.entry(Use::RegionLabel(target)).or_default() += 1; } + cf::unstructured::ControlTarget::Return => {} } } - - func_decl.inner_visit_with(self); + func_at_node.inner_visit_with(self); } fn visit_value_use(&mut self, v: &'a Value) { @@ -791,13 +787,8 @@ impl DbgScopeDefPlacer<'_> { None => self.scopes_used_in_region(func_def_body.at_body()), Some(cfg) => DbgScopeDefMap::merge_unordered( DbgScopeDefPlace::top_of(func_def_body.at_body()), - cfg.rev_post_order(func_def_body).map(|region| { - let mut scopes = self.scopes_used_in_region(func_def_body.at(region)); - if let Some(control_inst) = cfg.control_inst_on_exit_from.get(region) { - scopes.prepend_attrs(self.cx, control_inst.attrs); - } - scopes - }), + cfg.rev_post_order(func_def_body) + .map(|region| self.scopes_used_in_region(func_def_body.at(region))), ), } } @@ -1110,9 +1101,9 @@ impl<'a> Printer<'a> { || type_and_const_inputs.is_empty() } - TypeKind::QPtr | TypeKind::SpvStringLiteralForExtInst => { - true - } + TypeKind::QPtr + | TypeKind::Thunk + | TypeKind::SpvStringLiteralForExtInst => true, }; ty_def.attrs == AttrSet::default() @@ -3127,6 +3118,8 @@ impl Print for TypeDef { // FIXME(eddyb) should this be shortened to `qtr`? TypeKind::QPtr => printer.declarative_keyword_style().apply("qptr").into(), + TypeKind::Thunk => printer.imperative_keyword_style().apply("thunk").into(), + TypeKind::SpvInst { spv_inst, type_and_const_inputs } => printer .pretty_spv_inst( printer.spv_op_style(), @@ -3550,24 +3543,23 @@ impl Print for FuncDecl { pretty::Fragment::default() }; - // FIXME(eddyb) `:` as used here for C-like "label syntax" - // interferes (in theory) with `e: T` "type ascription syntax". pretty::Fragment::new([ pretty::Node::ForceLineSeparation.into(), label.print_as_def(printer), label_inputs, - ":".into(), - pretty::Node::ForceLineSeparation.into(), ]) + } else if region == def.body { + printer.imperative_keyword_style().apply("entry").into() } else { - pretty::Fragment::default() + printer.error_style().apply("/* undefined label */_").into() }; pretty::Fragment::new([ label_header, + " {".into(), pretty::Node::IndentedBlock(vec![def.at(region).print(printer)]) .into(), - cfg.control_inst_on_exit_from[region].print(printer), + "}".into(), ]) }) .intersperse({ @@ -3845,6 +3837,7 @@ impl Print for FuncAt<'_, Node> { DataInstKind::FuncCall(_) | DataInstKind::Mem(_) | DataInstKind::QPtr(_) + | DataInstKind::ThunkBind(_) | DataInstKind::SpvInst(_) | DataInstKind::SpvExtInst { .. } => { // FIXME(eddyb) `outputs_header` is wastefully built even in @@ -4030,6 +4023,29 @@ impl FuncAt<'_, DataInst> { ]) } + &DataInstKind::ThunkBind(target) => pretty::Fragment::new([ + printer + .demote_style_for_namespace_prefix(printer.declarative_keyword_style()) + .apply("thunk.") + .into(), + printer.imperative_keyword_style().apply("bind").into(), + pretty::join_comma_sep( + "(", + [ + match target { + cf::unstructured::ControlTarget::Region(target) => { + Use::RegionLabel(target).print(printer) + } + cf::unstructured::ControlTarget::Return => { + printer.imperative_keyword_style().apply("return").into() + } + }, + pretty::join_comma_sep("(", inputs.iter().map(|v| v.print(printer)), ")"), + ], + ")", + ), + ]), + DataInstKind::SpvInst(inst) => printer.pretty_spv_inst( printer.spv_op_style(), inst.opcode, @@ -4253,74 +4269,6 @@ impl FuncAt<'_, DataInst> { } } -impl Print for cf::unstructured::ControlInst { - type Output = pretty::Fragment; - fn print(&self, printer: &Printer<'_>) -> pretty::Fragment { - let Self { attrs, kind, inputs, targets, target_inputs } = self; - - let attrs = attrs.print(printer); - - let kw_style = printer.imperative_keyword_style(); - let kw = |kw| kw_style.apply(kw).into(); - - let mut targets = targets.iter().map(|&target_region| { - let mut target = pretty::Fragment::new([ - kw("branch"), - " ".into(), - Use::RegionLabel(target_region).print(printer), - ]); - if let Some(inputs) = target_inputs.get(&target_region) { - target = pretty::Fragment::new([ - target, - pretty::join_comma_sep("(", inputs.iter().map(|v| v.print(printer)), ")"), - ]); - } - target - }); - - let def = match kind { - cf::unstructured::ControlInstKind::Unreachable => { - // FIXME(eddyb) use `targets.is_empty()` when that is stabilized. - assert!(targets.len() == 0 && inputs.is_empty()); - kw("unreachable") - } - cf::unstructured::ControlInstKind::Return => { - // FIXME(eddyb) use `targets.is_empty()` when that is stabilized. - assert!(targets.len() == 0); - match inputs[..] { - [] => kw("return"), - [v] => pretty::Fragment::new([kw("return"), " ".into(), v.print(printer)]), - _ => unreachable!(), - } - } - cf::unstructured::ControlInstKind::ExitInvocation(cf::ExitInvocationKind::SpvInst( - spv::Inst { opcode, imms }, - )) => { - // FIXME(eddyb) use `targets.is_empty()` when that is stabilized. - assert!(targets.len() == 0); - printer.pretty_spv_inst( - kw_style, - *opcode, - imms, - inputs.iter().map(|v| v.print(printer)), - ) - } - - cf::unstructured::ControlInstKind::Branch => { - assert_eq!((targets.len(), inputs.len()), (1, 0)); - targets.next().unwrap() - } - - cf::unstructured::ControlInstKind::SelectBranch(kind) => { - assert_eq!(inputs.len(), 1); - kind.print_with_scrutinee_and_cases(printer, kw_style, inputs[0], targets) - } - }; - - pretty::Fragment::new([attrs, def]) - } -} - impl SelectionKind { fn print_with_scrutinee_and_cases( &self, diff --git a/src/qptr/lift.rs b/src/qptr/lift.rs index 890dbe57..48eba658 100644 --- a/src/qptr/lift.rs +++ b/src/qptr/lift.rs @@ -832,6 +832,17 @@ impl LiftToSpvPtrInstsInFunc<'_> { new_data_inst_def } + &DataInstKind::ThunkBind(_) => { + for &v in &data_inst_def.inputs { + if self.lifter.as_spv_ptr_type(type_of_val(v)).is_some() { + return Err(LiftError(Diag::bug([ + "unsupported `thunk.bind` with pointer inputs".into(), + ]))); + } + } + return Ok(Transformed::Unchanged); + } + DataInstKind::SpvInst(_) | DataInstKind::SpvExtInst { .. } => { let mut to_spv_ptr_input_adjustments = vec![]; let mut from_spv_ptr_output = None; diff --git a/src/qptr/lower.rs b/src/qptr/lower.rs index e2cddefb..01d893fb 100644 --- a/src/qptr/lower.rs +++ b/src/qptr/lower.rs @@ -637,7 +637,8 @@ impl LowerFromSpvPtrInstsInFunc<'_> { | NodeKind::ExitInvocation(_) | DataInstKind::FuncCall(_) | DataInstKind::Mem(_) - | DataInstKind::QPtr(_) => return, + | DataInstKind::QPtr(_) + | DataInstKind::ThunkBind(_) => return, DataInstKind::SpvInst(_) | DataInstKind::SpvExtInst { .. } => {} } diff --git a/src/spv/lift.rs b/src/spv/lift.rs index cb9ac3d3..06e32150 100644 --- a/src/spv/lift.rs +++ b/src/spv/lift.rs @@ -8,13 +8,12 @@ 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, OrdAssertEq, Region, Type, TypeDef, TypeKind, TypeOrConst, Value, Var, VarDecl, - VarKind, + NodeDef, NodeKind, OrdAssertEq, Region, Type, TypeDef, TypeKind, TypeOrConst, Value, Var, + VarDecl, VarKind, }; use itertools::Itertools; use rustc_hash::FxHashMap; use smallvec::SmallVec; -use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet}; use std::num::NonZeroU32; use std::path::Path; @@ -128,6 +127,11 @@ impl Visitor<'_> for NeedsIdsCollector<'_> { unreachable!("`TypeKind::QPtr` should be legalized away before lifting"); } + TypeKind::Thunk => { + // HACK(eddyb) unstructured control-flow uses thunks. + return; + } + TypeKind::SpvInst { .. } => {} TypeKind::SpvStringLiteralForExtInst => { unreachable!( @@ -146,6 +150,10 @@ impl Visitor<'_> for NeedsIdsCollector<'_> { } let ct_def = &self.cx[ct]; match ct_def.kind { + ConstKind::Undef if matches!(self.cx[ct_def.ty].kind, TypeKind::Thunk) => { + // HACK(eddyb) unstructured control-flow may use `undef` thunks. + } + ConstKind::Undef | ConstKind::PtrToGlobalVar(_) | ConstKind::PtrToFunc(_) @@ -219,6 +227,8 @@ impl Visitor<'_> for NeedsIdsCollector<'_> { match func_at_node.def().kind { NodeKind::Select(_) | NodeKind::Loop { .. } | NodeKind::ExitInvocation(_) => {} + DataInstKind::FuncCall(_) => {} + // FIXME(eddyb) this should be a proper `Result`-based error instead, // and/or `spv::lift` should mutate the module for legalization. DataInstKind::Mem(_) => { @@ -231,7 +241,7 @@ impl Visitor<'_> for NeedsIdsCollector<'_> { unreachable!("`DataInstKind::QPtr` should be legalized away before lifting"); } - DataInstKind::FuncCall(_) => {} + DataInstKind::ThunkBind(_) => {} DataInstKind::SpvInst(_) => {} DataInstKind::SpvExtInst { ext_set, .. } => { @@ -273,8 +283,8 @@ struct FuncLifting<'a> { /// 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`, -/// and the target [`Region`] itself has phis for its `inputs`. +/// Note that this is not used when an unstructured `thunk` carries input values +/// for its target [`Region`] (which would itself have phis for its `inputs`). enum RegionInputsSource { FuncParams, LoopHeaderPhis(Node), @@ -289,6 +299,12 @@ enum CfgPoint { NodeEntry(Node), NodeExit(Node), + + // HACK(eddyb) this is only needed to recover φ ("phi") semantics for + // unstructured CFG edges, while letting SPIR-T use "BB args" semantics + // (i.e. potentially different values for the same target `Region`). + // NOTE(eddyb) this is much more relied upon after all `thunk` refactors. + UnstructuredEdge(cf::unstructured::ControlEdge), } struct BlockLifting<'a> { @@ -310,7 +326,7 @@ struct Phi { default_value: Option, } -/// Similar to [`cf::unstructured::ControlInst`], except: +/// Similar to unstructured `thunk`s, except: /// * `targets` use [`CfgPoint`]s instead of [`Region`]s, to be able to /// reach any of the SPIR-V blocks being created during lifting /// * φ ("phi") values can be provided for targets regardless of "which side" of @@ -321,7 +337,7 @@ struct Phi { struct Terminator<'a> { attrs: AttrSet, - kind: Cow<'a, cf::unstructured::ControlInstKind>, + kind: TerminatorKind<'a>, // FIXME(eddyb) use `Cow` or something, but ideally the "owned" case always // has at most one input, so allocating a whole `Vec` for that seems unwise. @@ -335,6 +351,17 @@ struct Terminator<'a> { merge: Option>, } +enum TerminatorKind<'a> { + Unreachable, + Return, + Branch, + SelectBranch(&'a cf::SelectionKind), + + // HACK(eddyb) this is the only case the unstructured CFG doesn't represent + // using `thunk`s (as it has been moved to `NodeKind::ExitInvocation`). + ExitInvocation(&'a cf::ExitInvocationKind), +} + #[derive(Copy, Clone)] enum Merge { Selection(L), @@ -413,11 +440,32 @@ impl<'p> FuncAt<'_, CfgCursor<'p>> { /// chain within structured control-flow (i.e. no branching to child regions). fn unique_successor(self) -> Option> { let cursor = self.position; + + // HACK(eddyb) this skips past any tailing `thunk`-producing node, and + // goes straight to the `RegionExit` instead. + let filter_out_tail_thunk = |node| { + let node_def = &self.nodes[node]; + let is_last_node = node_def.next_in_list().is_none(); + let is_tail_thunk = is_last_node + && match node_def.kind { + NodeKind::ThunkBind(_) => true, + NodeKind::Select(_) => node_def.child_regions.iter().all(|&case| { + self.at(case).at_children().into_iter().exactly_one().is_ok_and( + |case_node| matches!(case_node.def().kind, NodeKind::ThunkBind(_)), + ) + }), + _ => false, + }; + (!is_tail_thunk).then_some(node) + }; + match cursor.point { // Entering a `Region` enters its first `Node` child, // or exits the region right away (if it has no children). CfgPoint::RegionEntry(region) => Some(CfgCursor { - point: match self.at(region).def().children.iter().first { + point: match (self.at(region).def().children.iter().first) + .and_then(filter_out_tail_thunk) + { Some(first_child) => CfgPoint::NodeEntry(first_child), None => CfgPoint::RegionExit(region), }, @@ -431,6 +479,10 @@ impl<'p> FuncAt<'_, CfgCursor<'p>> { CfgCursor { point: CfgPoint::NodeExit(parent_node), parent: parent.parent } } }), + CfgPoint::UnstructuredEdge { .. } => { + assert!(cursor.parent.is_none()); + None + } // Entering a `Node` depends entirely on the `NodeKind`. CfgPoint::NodeEntry(node) => match self.at(node).def().kind { @@ -441,6 +493,7 @@ impl<'p> FuncAt<'_, CfgCursor<'p>> { DataInstKind::FuncCall(_) | DataInstKind::Mem(_) | DataInstKind::QPtr(_) + | DataInstKind::ThunkBind(_) | DataInstKind::SpvInst(_) | DataInstKind::SpvExtInst { .. } => { Some(CfgCursor { point: CfgPoint::NodeExit(node), parent: cursor.parent }) @@ -449,7 +502,7 @@ impl<'p> FuncAt<'_, CfgCursor<'p>> { // Exiting a `Node` chains to a sibling/parent. CfgPoint::NodeExit(node) => { - Some(match self.nodes[node].next_in_list() { + Some(match self.nodes[node].next_in_list().and_then(filter_out_tail_thunk) { // Enter the next sibling in the `Region`, if one exists. Some(next_node) => { CfgCursor { point: CfgPoint::NodeEntry(next_node), parent: cursor.parent } @@ -583,7 +636,7 @@ impl<'a> FuncLifting<'a> { .collect::>()? } } - CfgPoint::RegionExit(_) => SmallVec::new(), + CfgPoint::RegionExit(_) | CfgPoint::UnstructuredEdge { .. } => SmallVec::new(), CfgPoint::NodeEntry(node) => { let node_def = func_def_body.at(node).def(); @@ -657,6 +710,8 @@ impl<'a> FuncLifting<'a> { | DataInstKind::QPtr(_) | DataInstKind::SpvInst(_) | DataInstKind::SpvExtInst { .. } => [node].into_iter().collect(), + + DataInstKind::ThunkBind(_) => unreachable!(), }, _ => SmallVec::new(), }; @@ -665,33 +720,60 @@ impl<'a> FuncLifting<'a> { let terminator = match (point, func_def_body.at(point_cursor).unique_successor()) { // Exiting a `Region` w/o a structured parent. (CfgPoint::RegionExit(region), None) => { - let unstructured_terminator = func_def_body - .unstructured_cfg - .as_ref() - .and_then(|cfg| cfg.control_inst_on_exit_from.get(region)); - if let Some(terminator) = unstructured_terminator { - let cf::unstructured::ControlInst { - attrs, + let unstructured_cfg_thunk = + func_def_body.unstructured_cfg.as_ref().map(|cfg| { + ( + cfg, + func_def_body + .at(region) + .def() + .outputs + .iter() + .copied() + .exactly_one() + .ok() + .unwrap(), + ) + }); + if let Some((cfg, thunk)) = unstructured_cfg_thunk { + // FIXME(eddyb) this partially overlaps with + // `ControlFlowGraph::edges_from_thunk_tailed_region`. + let thunk_node_def = match thunk { + Value::Var(thunk) => match func_def_body.at(thunk).decl().kind() { + VarKind::NodeOutput { node, output_idx: 0 } => { + Ok(func_def_body.at(node).def()) + } + _ => unreachable!(), + }, + Value::Const(ct) => Err(ct), + }; + let (kind, inputs) = match thunk_node_def { + Ok(NodeDef { kind: NodeKind::ThunkBind(_), .. }) | Err(_) => { + (TerminatorKind::Branch, [].into_iter().collect()) + } + Ok(NodeDef { kind: NodeKind::Select(kind), inputs, .. }) => { + ( + TerminatorKind::SelectBranch(kind), + // FIXME(eddyb) borrow these whenever possible. + inputs.clone(), + ) + } + Ok(_) => unreachable!(), + }; + + Terminator { + attrs: thunk_node_def.map(|def| def.attrs).unwrap_or_default(), kind, inputs, - targets, - target_inputs, - } = terminator; - Terminator { - attrs: *attrs, - kind: Cow::Borrowed(kind), - // FIXME(eddyb) borrow these whenever possible. - inputs: inputs.clone(), - targets: targets - .iter() - .map(|&target| CfgPoint::RegionEntry(target)) - .collect(), - target_phi_values: target_inputs - .iter() - .map(|(&target, target_inputs)| { - (CfgPoint::RegionEntry(target), &target_inputs[..]) - }) + // FIXME(eddyb) try limiting this to repeated target `Region`s + // which *also* pass different value inputs. + // NOTE(eddyb) this is much more relied upon, + // after all `thunk` refactors. + targets: cfg + .edges_from(func_def_body.at(region)) + .map(CfgPoint::UnstructuredEdge) .collect(), + target_phi_values: FxIndexMap::default(), merge: None, } } else { @@ -699,7 +781,7 @@ impl<'a> FuncLifting<'a> { assert!(region == func_def_body.body); Terminator { attrs: AttrSet::default(), - kind: Cow::Owned(cf::unstructured::ControlInstKind::Return), + kind: TerminatorKind::Return, inputs: func_def_body.at_body().def().outputs.clone(), targets: [].into_iter().collect(), target_phi_values: FxIndexMap::default(), @@ -707,6 +789,43 @@ impl<'a> FuncLifting<'a> { } } } + (CfgPoint::UnstructuredEdge(edge), None) => { + let cfg = func_def_body.unstructured_cfg.as_ref().unwrap(); + let (attrs, target, target_inputs) = + cfg.edge_attrs_target_and_inputs(func_def_body.at(edge)); + + let target = match target { + Ok(cf::unstructured::ControlTarget::Region(target)) => Ok(target), + Ok(cf::unstructured::ControlTarget::Return) => Err(TerminatorKind::Return), + Err(ct) => match cx[ct].kind { + ConstKind::Undef => Err(TerminatorKind::Unreachable), + _ => unreachable!(), + }, + }; + match target { + Ok(target) => Terminator { + attrs, + kind: TerminatorKind::Branch, + inputs: [].into_iter().collect(), + targets: [CfgPoint::RegionEntry(target)].into_iter().collect(), + target_phi_values: [(CfgPoint::RegionEntry(target), target_inputs)] + .into_iter() + .collect(), + merge: None, + }, + Err(terminator_kind) => { + Terminator { + attrs, + kind: terminator_kind, + // FIXME(eddyb) borrow these whenever possible. + inputs: target_inputs.iter().copied().collect(), + targets: [].into_iter().collect(), + target_phi_values: FxIndexMap::default(), + merge: None, + } + } + } + } // Entering a `Node` with child `Region`s (or diverging). (CfgPoint::NodeEntry(node), None) => { @@ -714,9 +833,7 @@ impl<'a> FuncLifting<'a> { match &node_def.kind { NodeKind::Select(kind) => Terminator { attrs: AttrSet::default(), - kind: Cow::Owned(cf::unstructured::ControlInstKind::SelectBranch( - kind.clone(), - )), + kind: TerminatorKind::SelectBranch(kind), inputs: [node_def.inputs[0]].into_iter().collect(), targets: node_def .child_regions @@ -731,7 +848,7 @@ impl<'a> FuncLifting<'a> { let body = node_def.child_regions[0]; Terminator { attrs: AttrSet::default(), - kind: Cow::Owned(cf::unstructured::ControlInstKind::Branch), + kind: TerminatorKind::Branch, inputs: [].into_iter().collect(), targets: [CfgPoint::RegionEntry(body)].into_iter().collect(), target_phi_values: FxIndexMap::default(), @@ -751,9 +868,7 @@ impl<'a> FuncLifting<'a> { NodeKind::ExitInvocation(kind) => Terminator { attrs: AttrSet::default(), - kind: Cow::Owned(cf::unstructured::ControlInstKind::ExitInvocation( - kind.clone(), - )), + kind: TerminatorKind::ExitInvocation(kind), inputs: node_def.inputs.clone(), targets: [].into_iter().collect(), target_phi_values: FxIndexMap::default(), @@ -763,6 +878,7 @@ impl<'a> FuncLifting<'a> { DataInstKind::FuncCall(_) | DataInstKind::Mem(_) | DataInstKind::QPtr(_) + | DataInstKind::ThunkBind(_) | DataInstKind::SpvInst(_) | DataInstKind::SpvExtInst { .. } => unreachable!(), } @@ -782,7 +898,7 @@ impl<'a> FuncLifting<'a> { match func_def_body.at(parent_node).def().kind { NodeKind::Select { .. } => Terminator { attrs: AttrSet::default(), - kind: Cow::Owned(cf::unstructured::ControlInstKind::Branch), + kind: TerminatorKind::Branch, inputs: [].into_iter().collect(), targets: [parent_exit].into_iter().collect(), target_phi_values: region_outputs @@ -813,7 +929,7 @@ impl<'a> FuncLifting<'a> { if is_infinite_loop { Terminator { attrs: AttrSet::default(), - kind: Cow::Owned(cf::unstructured::ControlInstKind::Branch), + kind: TerminatorKind::Branch, inputs: [].into_iter().collect(), targets: [backedge].into_iter().collect(), target_phi_values, @@ -830,11 +946,7 @@ impl<'a> FuncLifting<'a> { } Terminator { attrs: AttrSet::default(), - kind: Cow::Owned( - cf::unstructured::ControlInstKind::SelectBranch( - SelectionKind::BoolCond, - ), - ), + kind: TerminatorKind::SelectBranch(&SelectionKind::BoolCond), inputs: [repeat_condition].into_iter().collect(), targets: [backedge, parent_exit].into_iter().collect(), target_phi_values, @@ -847,6 +959,7 @@ impl<'a> FuncLifting<'a> { | DataInstKind::FuncCall(_) | DataInstKind::Mem(_) | DataInstKind::QPtr(_) + | DataInstKind::ThunkBind(_) | DataInstKind::SpvInst(_) | DataInstKind::SpvExtInst { .. } => unreachable!(), } @@ -863,7 +976,7 @@ impl<'a> FuncLifting<'a> { // `unique_predecessor` helper (just like `unique_successor`). (_, Some(succ_cursor)) => Terminator { attrs: AttrSet::default(), - kind: Cow::Owned(cf::unstructured::ControlInstKind::Branch), + kind: TerminatorKind::Branch, inputs: [].into_iter().collect(), targets: [succ_cursor.point].into_iter().collect(), target_phi_values: FxIndexMap::default(), @@ -885,6 +998,17 @@ impl<'a> FuncLifting<'a> { Some(cfg) => { for region in cfg.rev_post_order(func_def_body) { func_def_body.at(region).rev_post_order_try_for_each(&mut visit_cfg_point)?; + + // FIXME(eddyb) try limiting this to repeated target `Region`s + // which *also* pass different value inputs. + // NOTE(eddyb) this is much more relied upon, + // after all `thunk` refactors. + for edge in cfg.edges_from(func_def_body.at(region)) { + visit_cfg_point(CfgCursor { + point: CfgPoint::UnstructuredEdge(edge), + parent: None, + })?; + } } } } @@ -936,10 +1060,8 @@ impl<'a> FuncLifting<'a> { // SPIR-V allows their targets to just be the whole merge block // (the same one that `OpSelectionMerge` describes). let block = &blocks[block_idx]; - if let ( - cf::unstructured::ControlInstKind::SelectBranch(_), - Some(Merge::Selection(merge_point)), - ) = (&*block.terminator.kind, block.terminator.merge) + if let (TerminatorKind::SelectBranch(_), Some(Merge::Selection(merge_point))) = + (&block.terminator.kind, block.terminator.merge) { for target_idx in 0..block.terminator.targets.len() { let block = &blocks[block_idx]; @@ -966,7 +1088,7 @@ impl<'a> FuncLifting<'a> { (phis.is_empty() && insts.is_empty() && *attrs == AttrSet::default() - && matches!(**kind, cf::unstructured::ControlInstKind::Branch) + && matches!(kind, TerminatorKind::Branch) && inputs.is_empty() && targets.len() == 1 && target_phi_values.is_empty() @@ -989,7 +1111,7 @@ impl<'a> FuncLifting<'a> { &block.terminator; (*attrs == AttrSet::default() - && matches!(**kind, cf::unstructured::ControlInstKind::Branch) + && matches!(kind, TerminatorKind::Branch) && inputs.is_empty() && targets.len() == 1 && target_phi_values.is_empty() @@ -1015,7 +1137,7 @@ impl<'a> FuncLifting<'a> { new_terminator, Terminator { attrs: Default::default(), - kind: Cow::Owned(cf::unstructured::ControlInstKind::Unreachable), + kind: TerminatorKind::Unreachable, inputs: Default::default(), targets: Default::default(), target_phi_values: Default::default(), @@ -1246,7 +1368,9 @@ impl LazyInst<'_, '_> { }, // Not inserted into `globals` while visiting. - TypeKind::QPtr | TypeKind::SpvStringLiteralForExtInst => unreachable!(), + TypeKind::QPtr | TypeKind::Thunk | TypeKind::SpvStringLiteralForExtInst => { + unreachable!() + } }, Global::Const(ct) => { let ct_def = &cx[ct]; @@ -1377,7 +1501,7 @@ impl LazyInst<'_, '_> { unreachable!() } - DataInstKind::Mem(_) | DataInstKind::QPtr(_) => { + DataInstKind::Mem(_) | DataInstKind::QPtr(_) | DataInstKind::ThunkBind(_) => { // Disallowed while visiting. unreachable!() } @@ -1429,25 +1553,21 @@ impl LazyInst<'_, '_> { ids: [merge_label_id, continue_label_id].into_iter().collect(), }, Self::Terminator { parent_func, terminator } => { - let inst = match &*terminator.kind { - cf::unstructured::ControlInstKind::Unreachable => wk.OpUnreachable.into(), - cf::unstructured::ControlInstKind::Return => { + let inst = match terminator.kind { + TerminatorKind::Unreachable => wk.OpUnreachable.into(), + TerminatorKind::Return => { if terminator.inputs.is_empty() { wk.OpReturn.into() } else { wk.OpReturnValue.into() } } - cf::unstructured::ControlInstKind::ExitInvocation( - cf::ExitInvocationKind::SpvInst(inst), - ) - | cf::unstructured::ControlInstKind::SelectBranch(SelectionKind::SpvInst( - inst, - )) => inst.clone(), + TerminatorKind::ExitInvocation(cf::ExitInvocationKind::SpvInst(inst)) + | TerminatorKind::SelectBranch(SelectionKind::SpvInst(inst)) => inst.clone(), - cf::unstructured::ControlInstKind::Branch => wk.OpBranch.into(), + TerminatorKind::Branch => wk.OpBranch.into(), - cf::unstructured::ControlInstKind::SelectBranch(SelectionKind::BoolCond) => { + TerminatorKind::SelectBranch(SelectionKind::BoolCond) => { wk.OpBranchConditional.into() } }; diff --git a/src/spv/lower.rs b/src/spv/lower.rs index 9384ac7f..9111a098 100644 --- a/src/spv/lower.rs +++ b/src/spv/lower.rs @@ -3,13 +3,14 @@ use crate::cf::{self, SelectionKind}; use crate::spv::{self, spec}; // FIXME(eddyb) import more to avoid `crate::` everywhere. +use crate::func_at::FuncAtMut; 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, Region, - RegionDef, Type, TypeDef, TypeKind, TypeOrConst, Value, VarDecl, print, + FuncParam, FxIndexMap, GlobalVarDecl, GlobalVarDefBody, Import, InternedStr, Module, NodeDef, + NodeKind, Region, RegionDef, Type, TypeDef, TypeKind, TypeOrConst, Value, VarDecl, print, }; -use itertools::Either; +use itertools::{Either, Itertools as _}; use rustc_hash::FxHashMap; use smallvec::SmallVec; use std::collections::{BTreeMap, BTreeSet}; @@ -1465,7 +1466,7 @@ impl Module { // Split the operands into value inputs (e.g. a branch's // condition or an `OpSwitch`'s selector) and target blocks. let mut inputs = SmallVec::new(); - let mut targets = SmallVec::new(); + let mut targets = SmallVec::<[_; 4]>::new(); for &id in ids { match lookup_global_or_local_id_for_data_or_control_inst_input(id)? { LocalIdDef::Value(v) => { @@ -1484,45 +1485,146 @@ impl Module { } } - let kind = if opcode == wk.OpUnreachable { - assert!(targets.is_empty() && inputs.is_empty()); - cf::unstructured::ControlInstKind::Unreachable - } else if [wk.OpReturn, wk.OpReturnValue].contains(&opcode) { - assert!(targets.is_empty() && inputs.len() <= 1); - cf::unstructured::ControlInstKind::Return - } else if targets.is_empty() { - cf::unstructured::ControlInstKind::ExitInvocation( - cf::ExitInvocationKind::SpvInst(raw_inst.without_ids.clone()), + // FIXME(eddyb) collect targets in this form to + // begin with (instead of recombining them here). + let targets_with_inputs = targets.into_iter().map(|target| { + ( + cf::unstructured::ControlTarget::Region(target), + target_inputs.get(&target).cloned().unwrap_or_default(), ) - } else if opcode == wk.OpBranch { - assert_eq!((targets.len(), inputs.len()), (1, 0)); - cf::unstructured::ControlInstKind::Branch - } else if opcode == wk.OpBranchConditional { - assert_eq!((targets.len(), inputs.len()), (2, 1)); - cf::unstructured::ControlInstKind::SelectBranch(SelectionKind::BoolCond) + }); + + // FIXME(eddyb) cache this. + let thunk_ty = cx.intern(TypeKind::Thunk); + + let build_thunk = + |func_at_region: FuncAtMut<'_, Region>, (target, target_inputs)| { + let region = func_at_region.position; + let func = func_at_region.at(()); + + let thunk_node = func.nodes.define( + &cx, + NodeDef { + attrs, + kind: NodeKind::ThunkBind(target), + inputs: target_inputs, + child_regions: [].into_iter().collect(), + outputs: [].into_iter().collect(), + } + .into(), + ); + func.regions[region].children.insert_last(thunk_node, func.nodes); + + let thunk_var = func.vars.define( + &cx, + VarDecl { + attrs: AttrSet::default(), + ty: thunk_ty, + + def_parent: Either::Right(thunk_node), + def_idx: 0, + }, + ); + func.nodes[thunk_node].outputs.push(thunk_var); + + Value::Var(thunk_var) + }; + + let selection_kind = if opcode == wk.OpBranchConditional { + assert_eq!((targets_with_inputs.len(), inputs.len()), (2, 1)); + Some(SelectionKind::BoolCond) } else if opcode == wk.OpSwitch { - cf::unstructured::ControlInstKind::SelectBranch(SelectionKind::SpvInst( - raw_inst.without_ids.clone(), - )) + Some(SelectionKind::SpvInst(raw_inst.without_ids.clone())) } else { - return Err(invalid("unsupported control-flow instruction")); + None }; - func_def_body - .unstructured_cfg - .as_mut() - .unwrap() - .control_inst_on_exit_from - .insert( - current_block.region, - cf::unstructured::ControlInst { + let target_thunk = if let Some(selection_kind) = selection_kind { + let cases = targets_with_inputs + .map(|target_with_inputs| { + let case = func_def_body.regions.define(&cx, RegionDef::default()); + let thunk = + build_thunk(func_def_body.at_mut(case), target_with_inputs); + func_def_body.regions[case].outputs.push(thunk); + case + }) + .collect(); + + let select_node = func_def_body.nodes.define( + &cx, + NodeDef { attrs, - kind, - inputs, - targets, - target_inputs, + kind: NodeKind::Select(selection_kind), + inputs: mem::take(&mut inputs), + child_regions: cases, + outputs: [].into_iter().collect(), + } + .into(), + ); + func_def_body.regions[current_block.region] + .children + .insert_last(select_node, &mut func_def_body.nodes); + + let select_thunk_var = func_def_body.vars.define( + &cx, + VarDecl { + attrs: AttrSet::default(), + ty: thunk_ty, + + def_parent: Either::Right(select_node), + def_idx: 0, }, ); + func_def_body.nodes[select_node].outputs.push(select_thunk_var); + + Value::Var(select_thunk_var) + } else if [wk.OpReturn, wk.OpReturnValue].contains(&opcode) { + assert!(targets_with_inputs.len() == 0 && inputs.len() <= 1); + build_thunk( + func_def_body.at_mut(current_block.region), + (cf::unstructured::ControlTarget::Return, mem::take(&mut inputs)), + ) + } else if targets_with_inputs.len() == 0 { + if opcode != wk.OpUnreachable { + let node = func_def_body.nodes.define( + &cx, + NodeDef { + attrs, + kind: NodeKind::ExitInvocation( + cf::ExitInvocationKind::SpvInst( + raw_inst.without_ids.clone(), + ), + ), + inputs: mem::take(&mut inputs), + child_regions: [].into_iter().collect(), + outputs: [].into_iter().collect(), + } + .into(), + ); + func_def_body.regions[current_block.region] + .children + .insert_last(node, &mut func_def_body.nodes); + } + + // FIXME(eddyb) cache this. + Value::Const(cx.intern(ConstDef { + attrs: AttrSet::default(), + ty: thunk_ty, + kind: ConstKind::Undef, + })) + } else if opcode == wk.OpBranch { + build_thunk( + func_def_body.at_mut(current_block.region), + targets_with_inputs.exactly_one().ok().unwrap(), + ) + } else { + return Err(invalid("unsupported control-flow instruction")); + }; + + assert_eq!(inputs.len(), 0); + + func_def_body.regions[current_block.region].outputs = + [target_thunk].into_iter().collect(); } else if opcode == wk.OpPhi { if !current_block_region_def.children.is_empty() { return Err(invalid( diff --git a/src/transform.rs b/src/transform.rs index 0c76ace4..148669ad 100644 --- a/src/transform.rs +++ b/src/transform.rs @@ -438,7 +438,7 @@ impl InnerTransform for TypeDef { transform!({ attrs -> transformer.transform_attr_set_use(*attrs), kind -> match kind { - TypeKind::QPtr | TypeKind::SpvStringLiteralForExtInst => Transformed::Unchanged, + TypeKind::QPtr | TypeKind::Thunk | TypeKind::SpvStringLiteralForExtInst => Transformed::Unchanged, TypeKind::SpvInst { spv_inst, type_and_const_inputs } => Transformed::map_iter( type_and_const_inputs.iter(), @@ -580,11 +580,6 @@ impl InnerInPlaceTransform for FuncDefBody { for region in rpo { transformer.in_place_transform_region_def(self.at_mut(region)); - - let cfg = self.unstructured_cfg.as_mut().unwrap(); - if let Some(control_inst) = cfg.control_inst_on_exit_from.get_mut(region) { - control_inst.inner_in_place_transform_with(transformer); - } } } } @@ -641,6 +636,7 @@ impl InnerInPlaceTransform for FuncAtMut<'_, Node> { | QPtrOp::Offset(_) | QPtrOp::DynOffset { .. }, ) + | DataInstKind::ThunkBind(_) | DataInstKind::SpvInst(_) | DataInstKind::SpvExtInst { .. } => {} } @@ -679,33 +675,6 @@ impl InnerInPlaceTransform for VarDecl { } } -impl InnerInPlaceTransform for cf::unstructured::ControlInst { - fn inner_in_place_transform_with(&mut self, transformer: &mut impl Transformer) { - let Self { attrs, kind, inputs, targets: _, target_inputs } = self; - - transformer.transform_attr_set_use(*attrs).apply_to(attrs); - match kind { - cf::unstructured::ControlInstKind::Unreachable - | cf::unstructured::ControlInstKind::Return - | cf::unstructured::ControlInstKind::ExitInvocation(cf::ExitInvocationKind::SpvInst( - _, - )) - | cf::unstructured::ControlInstKind::Branch - | cf::unstructured::ControlInstKind::SelectBranch( - SelectionKind::BoolCond | SelectionKind::SpvInst(_), - ) => {} - } - for v in inputs { - transformer.transform_value_use(v).apply_to(v); - } - for inputs in target_inputs.values_mut() { - for v in inputs { - transformer.transform_value_use(v).apply_to(v); - } - } - } -} - impl InnerTransform for Value { fn inner_transform_with(&self, transformer: &mut impl Transformer) -> Transformed { match self { diff --git a/src/visit.rs b/src/visit.rs index bd63094e..b25d4cbc 100644 --- a/src/visit.rs +++ b/src/visit.rs @@ -321,7 +321,7 @@ impl InnerVisit for TypeDef { visitor.visit_attr_set_use(*attrs); match kind { - TypeKind::QPtr | TypeKind::SpvStringLiteralForExtInst => {} + TypeKind::QPtr | TypeKind::Thunk | TypeKind::SpvStringLiteralForExtInst => {} TypeKind::SpvInst { spv_inst: _, type_and_const_inputs } => { for &ty_or_ct in type_and_const_inputs { @@ -426,10 +426,6 @@ impl InnerVisit for FuncDefBody { Some(cfg) => { for region in cfg.rev_post_order(self) { visitor.visit_region_def(self.at(region)); - - if let Some(control_inst) = cfg.control_inst_on_exit_from.get(region) { - control_inst.inner_visit_with(visitor); - } } } } @@ -483,6 +479,7 @@ impl<'a> FuncAt<'a, Node> { | QPtrOp::Offset(_) | QPtrOp::DynOffset { .. }, ) + | DataInstKind::ThunkBind(_) | DataInstKind::SpvInst(_) | DataInstKind::SpvExtInst { .. } => {} } @@ -513,33 +510,6 @@ impl InnerVisit for VarDecl { } } -impl InnerVisit for cf::unstructured::ControlInst { - fn inner_visit_with<'a>(&'a self, visitor: &mut impl Visitor<'a>) { - let Self { attrs, kind, inputs, targets: _, target_inputs } = self; - - visitor.visit_attr_set_use(*attrs); - match kind { - cf::unstructured::ControlInstKind::Unreachable - | cf::unstructured::ControlInstKind::Return - | cf::unstructured::ControlInstKind::ExitInvocation(cf::ExitInvocationKind::SpvInst( - _, - )) - | cf::unstructured::ControlInstKind::Branch - | cf::unstructured::ControlInstKind::SelectBranch( - SelectionKind::BoolCond | SelectionKind::SpvInst(_), - ) => {} - } - for v in inputs { - visitor.visit_value_use(v); - } - for inputs in target_inputs.values() { - for v in inputs { - visitor.visit_value_use(v); - } - } - } -} - impl InnerVisit for Value { fn inner_visit_with<'a>(&'a self, visitor: &mut impl Visitor<'a>) { match self {