diff --git a/src/checker.rs b/src/checker.rs index 323c66b..3ccb6df 100644 --- a/src/checker.rs +++ b/src/checker.rs @@ -3,7 +3,7 @@ * regalloc.rs project * (https://github.com/bytecodealliance/regalloc.rs). regalloc.rs is * also licensed under Apache-2.0 with the LLVM exception, as the rest - * of regalloc2's non-Ion-derived code is. + * of regalloc2 is. */ //! Checker: verifies that spills/reloads/moves retain equivalent @@ -17,55 +17,75 @@ //! conceptually generates a symbolic value "Vn" when storing to (or //! modifying) a virtual register. //! +//! A symbolic value is logically a *set of virtual registers*, +//! representing all virtual registers equal to the value in the given +//! storage slot at a given program point. This representation (as +//! opposed to tracking just one virtual register) is necessary +//! because the regalloc may implement moves in the source program +//! (via move instructions or blockparam assignments on edges) in +//! "intelligent" ways, taking advantage of values that are already in +//! the right place, so we need to know *all* names for a value. +//! //! These symbolic values are precise but partial: in other words, if //! a physical register is described as containing a virtual register //! at a program point, it must actually contain the value of this -//! register (modulo any analysis bugs); but it may resolve to -//! `Conflicts` even in cases where one *could* statically prove that -//! it contains a certain register, because the analysis is not +//! register (modulo any analysis bugs); but it may describe fewer +//! virtual registers even in cases where one *could* statically prove +//! that it contains a certain register, because the analysis is not //! perfectly path-sensitive or value-sensitive. However, all //! assignments *produced by our register allocator* should be -//! analyzed fully precisely. +//! analyzed fully precisely. (This last point is important and bears +//! repeating: we only need to verify the programs that we produce, +//! not arbitrary programs.) //! //! Operand constraints (fixed register, register, any) are also checked //! at each operand. //! -//! The dataflow analysis state at each program point is: +//! ## Formal Definition //! -//! - map of: Allocation -> lattice value (top > Vn symbols (unordered) > bottom) +//! The analysis lattice is: +//! +//! ```plain +//! Top (V) +//! | +//! 𝒫(V) // the Powerset of the set of virtual regs +//! | +//! Bottom ( ∅ ) // the empty set +//! ``` +//! +//! and the lattice ordering relation is the subset relation: S ≤ U +//! iff S ⊆ U. The lattice meet-function is intersection. +//! +//! The dataflow analysis state at each program point (each point +//! before or after an instruction) is: +//! +//! - map of: Allocation -> lattice value //! //! And the transfer functions for instructions are (where `A` is the -//! above map from allocated physical registers to symbolic values): +//! above map from allocated physical registers to lattice values): //! //! - `Edit::Move` inserted by RA: [ alloc_d := alloc_s ] //! -//! A[alloc_d] := A[alloc_s] -//! -//! - phi-node [ V_i := phi block_j:V_j, block_k:V_k, ... ] -//! with allocations [ A_i := phi block_j:A_j, block_k:A_k, ... ] -//! (N.B.: phi-nodes are not semantically present in the final -//! machine code, but we include their allocations so that this -//! checker can work) -//! -//! A[A_i] := meet(A[A_j], A[A_k], ...) +//! A' = A[alloc_d → A[alloc_s]] //! //! - statement in pre-regalloc function [ V_i := op V_j, V_k, ... ] //! with allocated form [ A_i := op A_j, A_k, ... ] //! -//! A[A_i] := `V_i` +//! A' = { A_k → A[A_k] \ { V_i } for k ≠ i } ∪ +//! { A_i -> { V_i } } //! //! In other words, a statement, even after allocation, generates //! a symbol that corresponds to its original virtual-register -//! def. +//! def. Simultaneously, that same virtual register symbol is removed +//! from all other allocs: they no longer carry the current value. //! -//! (N.B.: moves in pre-regalloc function fall into this last case -//! -- they are "just another operation" and generate a new -//! symbol) +//! - Parallel moves or blockparam-assignments in original program +//! [ V_d1 := V_s1, V_d2 := V_s2, ... ] //! -//! At control-flow join points, the symbols meet using a very simple -//! lattice meet-function: two different symbols in the same -//! allocation meet to "conflicted"; otherwise, the symbol meets with -//! itself to produce itself (reflexivity). +//! A' = { A_k → subst(A[A_k]) for all k } +//! where subst(S) removes symbols for overwritten virtual +//! registers (V_d1 .. V_dn) and then adds V_di whenever +//! V_si appeared prior to the removals. //! //! To check correctness, we first find the dataflow fixpoint with the //! above lattice and transfer/meet functions. Then, at each op, we @@ -80,8 +100,9 @@ use crate::{ Allocation, AllocationKind, Block, Edit, Function, Inst, InstOrEdit, InstPosition, Operand, OperandConstraint, OperandKind, OperandPos, Output, PReg, RegClass, VReg, }; - -use std::collections::{HashMap, HashSet, VecDeque}; +use fxhash::{FxHashMap, FxHashSet}; +use smallvec::{smallvec, SmallVec}; +use std::collections::VecDeque; use std::default::Default; use std::hash::Hash; use std::result::Result; @@ -109,11 +130,11 @@ pub enum CheckerError { op: Operand, alloc: Allocation, }, - IncorrectValueInAllocation { + IncorrectValuesInAllocation { inst: Inst, op: Operand, alloc: Allocation, - actual: VReg, + actual: FxHashSet, }, ConstraintViolated { inst: Inst, @@ -145,55 +166,64 @@ pub enum CheckerError { inst: Inst, alloc: Allocation, }, - NonRefValueInStackmap { + NonRefValuesInStackmap { inst: Inst, alloc: Allocation, - vreg: VReg, + vregs: FxHashSet, }, } /// Abstract state for an allocation. /// -/// Forms a lattice with \top (`Unknown`), \bot (`Conflicted`), and a -/// number of mutually unordered value-points in between, one per real -/// or virtual register. Any two different registers meet to \bot. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -enum CheckerValue { - /// "top" value: this storage slot has no known value. - Unknown, - /// "bottom" value: this storage slot has a conflicted value. - Conflicted, - /// Reg: this storage slot has a value that originated as a def - /// into the given virtual register. - /// - /// The boolean flag indicates whether the value is - /// reference-typed. - Reg(VReg, bool), +/// Equivalent to a set of virtual register names, with the +/// universe-set as top and empty set as bottom lattice element. The +/// meet-function is thus set intersection. +#[derive(Clone, Debug, PartialEq, Eq)] +struct CheckerValue { + /// This value is the "universe set". + universe: bool, + /// The VRegs that this value is equal to. + vregs: FxHashSet, } impl Default for CheckerValue { fn default() -> CheckerValue { - CheckerValue::Unknown + CheckerValue { + universe: true, + vregs: FxHashSet::default(), + } } } impl CheckerValue { - /// Meet function of the abstract-interpretation value lattice. - fn meet(&self, other: &CheckerValue) -> CheckerValue { - match (self, other) { - (&CheckerValue::Unknown, _) => *other, - (_, &CheckerValue::Unknown) => *self, - (&CheckerValue::Conflicted, _) => *self, - (_, &CheckerValue::Conflicted) => *other, - (&CheckerValue::Reg(r1, ref1), &CheckerValue::Reg(r2, ref2)) - if r1 == r2 && ref1 == ref2 => - { - CheckerValue::Reg(r1, ref1) - } - _ => { - trace!("{:?} and {:?} meet to Conflicted", self, other); - CheckerValue::Conflicted - } + /// Meet function of the abstract-interpretation value + /// lattice. Returns a boolean value indicating whether `self` was + /// changed. + fn meet_with(&mut self, other: &CheckerValue) { + if other.universe { + // Nothing. + } else if self.universe { + *self = other.clone(); + } else { + self.vregs.retain(|vreg| other.vregs.contains(vreg)); + } + } + + fn from_reg(reg: VReg) -> CheckerValue { + CheckerValue { + universe: false, + vregs: std::iter::once(reg).collect(), + } + } + + fn remove_vreg(&mut self, reg: VReg) { + self.vregs.remove(®); + } + + fn empty() -> CheckerValue { + CheckerValue { + universe: false, + vregs: FxHashSet::default(), } } } @@ -201,36 +231,46 @@ impl CheckerValue { /// State that steps through program points as we scan over the instruction stream. #[derive(Clone, Debug, PartialEq, Eq)] struct CheckerState { - allocations: HashMap, + top: bool, + allocations: FxHashMap, } impl Default for CheckerState { fn default() -> CheckerState { CheckerState { - allocations: HashMap::new(), + top: true, + allocations: FxHashMap::default(), } } } impl std::fmt::Display for CheckerValue { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - match self { - CheckerValue::Unknown => write!(f, "?"), - CheckerValue::Conflicted => write!(f, "!"), - CheckerValue::Reg(r, false) => write!(f, "{}", r), - CheckerValue::Reg(r, true) => write!(f, "{}/ref", r), + if self.universe { + write!(f, "top") + } else { + write!(f, "{{ ")?; + for vreg in &self.vregs { + write!(f, "{} ", vreg)?; + } + write!(f, "}}")?; + Ok(()) } } } +/// Meet function for analysis value: meet individual values at +/// matching allocations, and intersect keys (remove key-value pairs +/// only on one side). Returns boolean flag indicating whether `into` +/// changed. fn merge_map( - into: &mut HashMap, - from: &HashMap, + into: &mut FxHashMap, + from: &FxHashMap, ) { - for (k, v) in from { - let into_v = into.entry(*k).or_insert(Default::default()); - let merged = into_v.meet(v); - *into_v = merged; + into.retain(|k, _| from.contains_key(k)); + for (k, into_v) in into.iter_mut() { + let from_v = from.get(k).unwrap(); + into_v.meet_with(from_v); } } @@ -242,7 +282,14 @@ impl CheckerState { /// Merge this checker state with another at a CFG join-point. fn meet_with(&mut self, other: &CheckerState) { - merge_map(&mut self.allocations, &other.allocations); + if other.top { + // Nothing. + } else if self.top { + *self = other.clone(); + } else { + self.top = false; + merge_map(&mut self.allocations, &other.allocations); + } } fn check_val( @@ -250,29 +297,24 @@ impl CheckerState { inst: Inst, op: Operand, alloc: Allocation, - val: CheckerValue, + val: &CheckerValue, allocs: &[Allocation], ) -> Result<(), CheckerError> { if alloc == Allocation::none() { return Err(CheckerError::MissingAllocation { inst, op }); } - match val { - CheckerValue::Unknown => { - return Err(CheckerError::UnknownValueInAllocation { inst, op, alloc }); - } - CheckerValue::Conflicted => { - return Err(CheckerError::ConflictedValueInAllocation { inst, op, alloc }); - } - CheckerValue::Reg(r, _) if r != op.vreg() => { - return Err(CheckerError::IncorrectValueInAllocation { - inst, - op, - alloc, - actual: r, - }); - } - _ => {} + if val.universe { + return Err(CheckerError::UnknownValueInAllocation { inst, op, alloc }); + } + + if !val.vregs.contains(&op.vreg()) { + return Err(CheckerError::IncorrectValuesInAllocation { + inst, + op, + alloc, + actual: val.vregs.clone(), + }); } self.check_constraint(inst, op, alloc, allocs)?; @@ -283,7 +325,13 @@ impl CheckerState { /// Check an instruction against this state. This must be called /// twice: once with `InstPosition::Before`, and once with /// `InstPosition::After` (after updating state with defs). - fn check(&self, pos: InstPosition, checkinst: &CheckerInst) -> Result<(), CheckerError> { + fn check<'a, F: Function>( + &self, + pos: InstPosition, + checkinst: &CheckerInst, + checker: &Checker<'a, F>, + ) -> Result<(), CheckerError> { + let default_val = Default::default(); match checkinst { &CheckerInst::Op { inst, @@ -317,11 +365,7 @@ impl CheckerState { continue; } - let val = self - .allocations - .get(alloc) - .cloned() - .unwrap_or(Default::default()); + let val = self.allocations.get(alloc).unwrap_or(&default_val); trace!( "checker: checkinst {:?}: op {:?}, alloc {:?}, checker value {:?}", checkinst, @@ -334,11 +378,7 @@ impl CheckerState { } &CheckerInst::Safepoint { inst, ref allocs } => { for &alloc in allocs { - let val = self - .allocations - .get(&alloc) - .cloned() - .unwrap_or(Default::default()); + let val = self.allocations.get(&alloc).unwrap_or(&default_val); trace!( "checker: checkinst {:?}: safepoint slot {}, checker value {:?}", checkinst, @@ -346,32 +386,35 @@ impl CheckerState { val ); - match val { - CheckerValue::Unknown => {} - CheckerValue::Conflicted => { - return Err(CheckerError::ConflictedValueInStackmap { inst, alloc }); - } - CheckerValue::Reg(vreg, false) => { - return Err(CheckerError::NonRefValueInStackmap { inst, alloc, vreg }); - } - CheckerValue::Reg(_, true) => {} + let reffy = val + .vregs + .iter() + .any(|vreg| checker.reftyped_vregs.contains(vreg)); + if !reffy { + return Err(CheckerError::NonRefValuesInStackmap { + inst, + alloc, + vregs: val.vregs.clone(), + }); } } } - _ => {} + &CheckerInst::ParallelMove { .. } | &CheckerInst::Move { .. } => { + // This doesn't need verification; we just update + // according to the move semantics in the step + // function below. + } } Ok(()) } /// Update according to instruction. fn update<'a, F: Function>(&mut self, checkinst: &CheckerInst, checker: &Checker<'a, F>) { + self.top = false; + let default_val = Default::default(); match checkinst { &CheckerInst::Move { into, from } => { - let val = self - .allocations - .get(&from) - .cloned() - .unwrap_or(Default::default()); + let val = self.allocations.get(&from).unwrap_or(&default_val).clone(); trace!( "checker: checkinst {:?} updating: move {:?} -> {:?} val {:?}", checkinst, @@ -381,35 +424,83 @@ impl CheckerState { ); self.allocations.insert(into, val); } + &CheckerInst::ParallelMove { ref moves } => { + // First, build map of actions for each vreg in an + // alloc. If an alloc has a reg V_i before a parallel + // move, then for each use of V_i as a source (V_i -> + // V_j), we might add a new V_j wherever V_i appears; + // and if V_i is used as a dest (at most once), then + // it must be removed first from allocs' vreg sets. + let mut additions: FxHashMap> = FxHashMap::default(); + let mut deletions: FxHashSet = FxHashSet::default(); + + for &(dest, src) in moves { + deletions.insert(dest); + additions + .entry(src) + .or_insert_with(|| smallvec![]) + .push(dest); + } + + // Now process each allocation's set of vreg labels, + // first deleting those labels that were updated by + // this parallel move, then adding back labels + // redefined by the move. + for value in self.allocations.values_mut() { + if value.universe { + continue; + } + let mut insertions: SmallVec<[VReg; 2]> = smallvec![]; + for &vreg in &value.vregs { + if let Some(additions) = additions.get(&vreg) { + insertions.extend(additions.iter().cloned()); + } + } + for &d in &deletions { + value.vregs.remove(&d); + } + value.vregs.extend(insertions); + } + } &CheckerInst::Op { ref operands, ref allocs, ref clobbers, .. } => { + // For each def, (i) update alloc to reflect defined + // vreg (and only that vreg), and (ii) update all + // other allocs in the checker state by removing this + // vreg, if defined (other defs are now stale). for (op, alloc) in operands.iter().zip(allocs.iter()) { if op.kind() != OperandKind::Def { continue; } - let reftyped = checker.reftyped_vregs.contains(&op.vreg()); self.allocations - .insert(*alloc, CheckerValue::Reg(op.vreg(), reftyped)); + .insert(*alloc, CheckerValue::from_reg(op.vreg())); + for (other_alloc, other_value) in &mut self.allocations { + if *alloc != *other_alloc { + other_value.remove_vreg(op.vreg()); + } + } } for clobber in clobbers { self.allocations.remove(&Allocation::reg(*clobber)); } } - &CheckerInst::DefAlloc { alloc, vreg } => { - let reftyped = checker.reftyped_vregs.contains(&vreg); - self.allocations - .insert(alloc, CheckerValue::Reg(vreg, reftyped)); - } &CheckerInst::Safepoint { ref allocs, .. } => { for (alloc, value) in &mut self.allocations { - if let CheckerValue::Reg(_, true) = *value { - if !allocs.contains(&alloc) { - *value = CheckerValue::Conflicted; - } + if alloc.is_reg() { + continue; + } + if !allocs.contains(&alloc) { + // Remove all reftyped vregs as labels. + let new_vregs = value + .vregs + .difference(&checker.reftyped_vregs) + .cloned() + .collect(); + value.vregs = new_vregs; } } } @@ -475,6 +566,15 @@ pub(crate) enum CheckerInst { /// spillslots). Move { into: Allocation, from: Allocation }, + /// A parallel move in the original program. Simultaneously moves + /// from all source vregs to all corresponding dest vregs, + /// permitting overlap in the src and dest sets and doing all + /// reads before any writes. + ParallelMove { + /// Vector of (dest, src) moves. + moves: Vec<(VReg, VReg)>, + }, + /// A regular instruction with fixed use and def slots. Contains /// both the original operands (as given to the regalloc) and the /// allocation results. @@ -485,11 +585,6 @@ pub(crate) enum CheckerInst { clobbers: Vec, }, - /// Define an allocation's contents. Like BlockParams but for one - /// allocation. Used sometimes when moves are elided but ownership - /// of a value is logically transferred to a new vreg. - DefAlloc { alloc: Allocation, vreg: VReg }, - /// A safepoint, with the given Allocations specified as containing /// reftyped values. All other reftyped values become invalid. Safepoint { inst: Inst, allocs: Vec }, @@ -498,9 +593,10 @@ pub(crate) enum CheckerInst { #[derive(Debug)] pub struct Checker<'a, F: Function> { f: &'a F, - bb_in: HashMap, - bb_insts: HashMap>, - reftyped_vregs: HashSet, + bb_in: FxHashMap, + bb_insts: FxHashMap>, + edge_insts: FxHashMap<(Block, Block), Vec>, + reftyped_vregs: FxHashSet, } impl<'a, F: Function> Checker<'a, F> { @@ -509,14 +605,18 @@ impl<'a, F: Function> Checker<'a, F> { /// methods to add abstract instructions to each BB before /// invoking `run()` to check for errors. pub fn new(f: &'a F) -> Checker<'a, F> { - let mut bb_in = HashMap::new(); - let mut bb_insts = HashMap::new(); - let mut reftyped_vregs = HashSet::new(); + let mut bb_in = FxHashMap::default(); + let mut bb_insts = FxHashMap::default(); + let mut edge_insts = FxHashMap::default(); + let mut reftyped_vregs = FxHashSet::default(); for block in 0..f.num_blocks() { let block = Block::new(block); bb_in.insert(block, Default::default()); bb_insts.insert(block, vec![]); + for &succ in f.block_succs(block) { + edge_insts.insert((block, succ), vec![]); + } } for &vreg in f.reftype_vregs() { @@ -527,6 +627,7 @@ impl<'a, F: Function> Checker<'a, F> { f, bb_in, bb_insts, + edge_insts, reftyped_vregs, } } @@ -536,7 +637,7 @@ impl<'a, F: Function> Checker<'a, F> { pub fn prepare(&mut self, out: &Output) { trace!("checker: out = {:?}", out); // Preprocess safepoint stack-maps into per-inst vecs. - let mut safepoint_slots: HashMap> = HashMap::new(); + let mut safepoint_slots: FxHashMap> = FxHashMap::default(); for &(progpoint, slot) in &out.safepoint_slots { safepoint_slots .entry(progpoint.inst()) @@ -565,7 +666,7 @@ impl<'a, F: Function> Checker<'a, F> { &mut self, block: Block, inst: Inst, - safepoint_slots: &mut HashMap>, + safepoint_slots: &mut FxHashMap>, out: &Output, ) { // If this is a safepoint, then check the spillslots at this point. @@ -576,10 +677,9 @@ impl<'a, F: Function> Checker<'a, F> { self.bb_insts.get_mut(&block).unwrap().push(checkinst); } - // Skip if this is a branch: the blockparams do not - // exist in post-regalloc code, and the edge-moves - // have to be inserted before the branch rather than - // after. + // Skip normal checks if this is a branch: the blockparams do + // not exist in post-regalloc code, and the edge-moves have to + // be inserted before the branch rather than after. if !self.f.is_branch(inst) { // Instruction itself. let operands: Vec<_> = self.f.inst_operands(inst).iter().cloned().collect(); @@ -594,6 +694,22 @@ impl<'a, F: Function> Checker<'a, F> { trace!("checker: adding inst {:?}", checkinst); self.bb_insts.get_mut(&block).unwrap().push(checkinst); } + // Instead, if this is a branch, emit a ParallelMove on each + // outgoing edge as necessary to handle blockparams. + else { + for (i, &succ) in self.f.block_succs(block).iter().enumerate() { + let args = self.f.branch_blockparams(block, inst, i); + let params = self.f.block_params(succ); + assert_eq!(args.len(), params.len()); + if args.len() > 0 { + let moves = params.iter().cloned().zip(args.iter().cloned()).collect(); + self.edge_insts + .get_mut(&(block, succ)) + .unwrap() + .push(CheckerInst::ParallelMove { moves }); + } + } + } } fn handle_edit(&mut self, block: Block, edit: &Edit) { @@ -605,19 +721,13 @@ impl<'a, F: Function> Checker<'a, F> { .unwrap() .push(CheckerInst::Move { into: to, from }); } - &Edit::DefAlloc { alloc, vreg } => { - self.bb_insts - .get_mut(&block) - .unwrap() - .push(CheckerInst::DefAlloc { alloc, vreg }); - } } } /// Perform the dataflow analysis to compute checker state at each BB entry. fn analyze(&mut self) { let mut queue = VecDeque::new(); - let mut queue_set = HashSet::new(); + let mut queue_set = FxHashSet::default(); for block in 0..self.f.num_blocks() { let block = Block::new(block); queue.push_back(block); @@ -635,10 +745,29 @@ impl<'a, F: Function> Checker<'a, F> { } for &succ in self.f.block_succs(block) { - let cur_succ_in = self.bb_in.get(&succ).unwrap(); let mut new_state = state.clone(); + for edge_inst in self.edge_insts.get(&(block, succ)).unwrap() { + new_state.update(edge_inst, self); + trace!( + "analyze: succ {:?}: inst {:?} -> state {:?}", + succ, + edge_inst, + new_state + ); + } + + let cur_succ_in = self.bb_in.get(&succ).unwrap(); + trace!( + "meeting state {:?} for block {} with state {:?} for block {}", + new_state, + block.index(), + cur_succ_in, + succ.index() + ); new_state.meet_with(cur_succ_in); let changed = &new_state != cur_succ_in; + trace!(" -> {:?}, changed {}", new_state, changed); + if changed { trace!( "analyze: block {} state changed from {:?} to {:?}; pushing onto queue", @@ -664,12 +793,12 @@ impl<'a, F: Function> Checker<'a, F> { for (block, input) in &self.bb_in { let mut state = input.clone(); for inst in self.bb_insts.get(block).unwrap() { - if let Err(e) = state.check(InstPosition::Before, inst) { + if let Err(e) = state.check(InstPosition::Before, inst, self) { trace!("Checker error: {:?}", e); errors.push(e); } state.update(inst, self); - if let Err(e) = state.check(InstPosition::After, inst) { + if let Err(e) = state.check(InstPosition::After, inst, self) { trace!("Checker error: {:?}", e); errors.push(e); } @@ -725,9 +854,6 @@ impl<'a, F: Function> Checker<'a, F> { &CheckerInst::Move { from, into } => { trace!(" {} -> {}", from, into); } - &CheckerInst::DefAlloc { alloc, vreg } => { - trace!(" defalloc: {}:{}", vreg, alloc); - } &CheckerInst::Safepoint { ref allocs, .. } => { let mut slotargs = vec![]; for &slot in allocs { @@ -735,10 +861,32 @@ impl<'a, F: Function> Checker<'a, F> { } trace!(" safepoint: {}", slotargs.join(", ")); } + &CheckerInst::ParallelMove { .. } => { + panic!("unexpected parallel_move in body (non-edge)") + } } state.update(inst, &self); print_state(&state); } + + for &succ in self.f.block_succs(bb) { + trace!(" succ {:?}:", succ); + let mut state = state.clone(); + for edge_inst in self.edge_insts.get(&(bb, succ)).unwrap() { + match edge_inst { + &CheckerInst::ParallelMove { ref moves } => { + let moves = moves + .iter() + .map(|(dest, src)| format!("{} -> {}", src, dest)) + .collect::>(); + trace!(" parallel_move {}", moves.join(", ")); + } + _ => panic!("unexpected edge_inst: not a parallel move"), + } + state.update(edge_inst, &self); + print_state(&state); + } + } } result diff --git a/src/ion/moves.rs b/src/ion/moves.rs index 0002cb5..c5964d0 100644 --- a/src/ion/moves.rs +++ b/src/ion/moves.rs @@ -1017,60 +1017,29 @@ impl<'a, F: Function> Env<'a, F> { } if self.allocation_is_stack(src) && self.allocation_is_stack(dst) { if !scratch_used_yet { - self.add_move_edit( - pos_prio, - src, - Allocation::reg(scratch), - to_vreg, - ); - self.add_move_edit( - pos_prio, - Allocation::reg(scratch), - dst, - to_vreg, - ); + self.add_move_edit(pos_prio, src, Allocation::reg(scratch)); + self.add_move_edit(pos_prio, Allocation::reg(scratch), dst); } else { debug_assert!(extra_slot.is_some()); self.add_move_edit( pos_prio, Allocation::reg(scratch), extra_slot.unwrap(), - None, - ); - self.add_move_edit( - pos_prio, - src, - Allocation::reg(scratch), - to_vreg, - ); - self.add_move_edit( - pos_prio, - Allocation::reg(scratch), - dst, - to_vreg, ); + self.add_move_edit(pos_prio, src, Allocation::reg(scratch)); + self.add_move_edit(pos_prio, Allocation::reg(scratch), dst); self.add_move_edit( pos_prio, extra_slot.unwrap(), Allocation::reg(scratch), - None, ); } } else { - self.add_move_edit(pos_prio, src, dst, to_vreg); + self.add_move_edit(pos_prio, src, dst); } } else { trace!(" -> redundant move elided"); } - #[cfg(feature = "checker")] - if let Some((alloc, vreg)) = action.def_alloc { - trace!( - " -> converted to DefAlloc: alloc {} vreg {}", - alloc, - vreg - ); - self.edits.push((pos_prio, Edit::DefAlloc { alloc, vreg })); - } } } @@ -1086,48 +1055,6 @@ impl<'a, F: Function> Env<'a, F> { ); let action = redundant_moves.process_move(m.from_alloc, m.to_alloc, m.to_vreg); debug_assert!(action.elide); - if let Some((alloc, vreg)) = action.def_alloc { - trace!(" -> DefAlloc: alloc {} vreg {}", alloc, vreg); - self.edits.push((pos_prio, Edit::DefAlloc { alloc, vreg })); - } - } - } - - #[cfg(feature = "checker")] - { - // Add edits to describe blockparam locations too. This is - // required by the checker. This comes after any edge-moves. - use crate::ion::data_structures::u64_key; - self.blockparam_allocs - .sort_unstable_by_key(|&(block, idx, _, _)| u64_key(block.raw_u32(), idx)); - self.stats.blockparam_allocs_count = self.blockparam_allocs.len(); - let mut i = 0; - while i < self.blockparam_allocs.len() { - let start = i; - let block = self.blockparam_allocs[i].0; - while i < self.blockparam_allocs.len() && self.blockparam_allocs[i].0 == block { - i += 1; - } - let params = &self.blockparam_allocs[start..i]; - let vregs = params - .iter() - .map(|(_, _, vreg_idx, _)| self.vreg_regs[vreg_idx.index()]) - .collect::>(); - let allocs = params - .iter() - .map(|(_, _, _, alloc)| *alloc) - .collect::>(); - debug_assert_eq!(vregs.len(), self.func.block_params(block).len()); - debug_assert_eq!(allocs.len(), self.func.block_params(block).len()); - for (vreg, alloc) in vregs.into_iter().zip(allocs.into_iter()) { - self.edits.push(( - PosWithPrio { - pos: self.cfginfo.block_entry[block.index()], - prio: InsertMovePrio::BlockParam as u32, - }, - Edit::DefAlloc { alloc, vreg }, - )); - } } } @@ -1146,38 +1073,17 @@ impl<'a, F: Function> Env<'a, F> { &Edit::Move { from, to } => { self.annotate(pos_prio.pos, format!("move {} -> {})", from, to)); } - &Edit::DefAlloc { alloc, vreg } => { - let s = format!("defalloc {:?} := {:?}", alloc, vreg); - self.annotate(pos_prio.pos, s); - } } } } } - pub fn add_move_edit( - &mut self, - pos_prio: PosWithPrio, - from: Allocation, - to: Allocation, - _to_vreg: Option, - ) { + pub fn add_move_edit(&mut self, pos_prio: PosWithPrio, from: Allocation, to: Allocation) { if from != to { if from.is_reg() && to.is_reg() { debug_assert_eq!(from.as_reg().unwrap().class(), to.as_reg().unwrap().class()); } self.edits.push((pos_prio, Edit::Move { from, to })); } - - #[cfg(feature = "checker")] - if let Some(to_vreg) = _to_vreg { - self.edits.push(( - pos_prio, - Edit::DefAlloc { - alloc: to, - vreg: to_vreg, - }, - )); - } } } diff --git a/src/ion/redundant_moves.rs b/src/ion/redundant_moves.rs index 5a0f192..41ed255 100644 --- a/src/ion/redundant_moves.rs +++ b/src/ion/redundant_moves.rs @@ -18,8 +18,6 @@ pub struct RedundantMoveEliminator { #[derive(Copy, Clone, Debug)] pub struct RedundantMoveAction { pub elide: bool, - #[cfg(feature = "checker")] - pub def_alloc: Option<(Allocation, VReg)>, } impl RedundantMoveEliminator { @@ -57,11 +55,7 @@ impl RedundantMoveEliminator { self.clear_alloc(to); self.allocs .insert(to, RedundantMoveState::Orig(to_vreg.unwrap())); - return RedundantMoveAction { - elide: true, - #[cfg(feature = "checker")] - def_alloc: Some((to, to_vreg.unwrap())), - }; + return RedundantMoveAction { elide: true }; } let src_vreg = match from_state { @@ -86,13 +80,6 @@ impl RedundantMoveEliminator { }; trace!(" -> elide {}", elide); - let def_alloc = if dst_vreg != existing_dst_vreg && dst_vreg.is_some() { - Some((to, dst_vreg.unwrap())) - } else { - None - }; - trace!(" -> def_alloc {:?}", def_alloc); - // Invalidate all existing copies of `to` if `to` actually changed value. if !elide { self.clear_alloc(to); @@ -113,11 +100,7 @@ impl RedundantMoveEliminator { .push(to); } - RedundantMoveAction { - elide, - #[cfg(feature = "checker")] - def_alloc, - } + RedundantMoveAction { elide } } pub fn clear(&mut self) { diff --git a/src/lib.rs b/src/lib.rs index 800dd4f..6a1fd66 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1119,13 +1119,6 @@ pub enum Edit { /// are the same if the vreg changes; this allows proper metadata /// tracking even when moves are elided. Move { from: Allocation, to: Allocation }, - - /// Define a particular Allocation to contain a particular VReg. Useful - /// for the checker. - /// - /// `DefAlloc` edits are only emitted when the `"checker"` Cargo feature is - /// enabled. - DefAlloc { alloc: Allocation, vreg: VReg }, } /// Wrapper around either an original instruction or an inserted edit.