From 4e3cb25983552a6d18514516d0be5b08b03e77d5 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Fri, 13 Sep 2019 18:58:50 +0200 Subject: [PATCH] Use a sorted array for (Ebb, Inst) interval again (fixes #1084); --- cranelift/codegen/src/regalloc/coalescing.rs | 30 +- cranelift/codegen/src/regalloc/coloring.rs | 24 +- .../src/regalloc/live_value_tracker.rs | 2 +- cranelift/codegen/src/regalloc/liveness.rs | 29 +- cranelift/codegen/src/regalloc/liverange.rs | 342 +++++++++--------- cranelift/codegen/src/regalloc/spilling.rs | 6 +- cranelift/codegen/src/value_label.rs | 8 +- cranelift/codegen/src/verifier/cssa.rs | 7 +- cranelift/codegen/src/verifier/liveness.rs | 6 +- cranelift/codegen/src/verifier/locations.rs | 8 +- 10 files changed, 212 insertions(+), 250 deletions(-) diff --git a/cranelift/codegen/src/regalloc/coalescing.rs b/cranelift/codegen/src/regalloc/coalescing.rs index 8a1f1d81bb..b27a105d65 100644 --- a/cranelift/codegen/src/regalloc/coalescing.rs +++ b/cranelift/codegen/src/regalloc/coalescing.rs @@ -196,12 +196,7 @@ impl<'a> Context<'a> { // to be live at the use. for i in 0..num_params { let param = self.func.dfg.ebb_params(ebb)[i]; - if self.liveness[param].reaches_use( - pred_inst, - pred_ebb, - self.liveness.forest(), - &self.func.layout, - ) { + if self.liveness[param].reaches_use(pred_inst, pred_ebb, &self.func.layout) { self.isolate_param(ebb, param); } } @@ -255,7 +250,7 @@ impl<'a> Context<'a> { ); // The only other possibility is that `arg` is live-in to `ebb`. - lr.is_livein(ebb, self.liveness.forest(), &self.func.layout) + lr.is_livein(ebb, &self.func.layout) }; if interference { @@ -435,12 +430,7 @@ impl<'a> Context<'a> { // Check for interference between `parent` and `value`. Since `parent` dominates // `value`, we only have to check if it overlaps the definition. - if self.liveness[parent.value].overlaps_def( - node.def, - node.ebb, - self.liveness.forest(), - &self.func.layout, - ) { + if self.liveness[parent.value].overlaps_def(node.def, node.ebb, &self.func.layout) { // The two values are interfering, so they can't be in the same virtual register. debug!("-> interference: {} overlaps def of {}", parent, value); return false; @@ -626,12 +616,7 @@ impl<'a> Context<'a> { // Check if the parent value interferes with the virtual copy. let inst = node.def.unwrap_inst(); if node.set_id != parent.set_id - && self.liveness[parent.value].reaches_use( - inst, - node.ebb, - self.liveness.forest(), - &self.func.layout, - ) + && self.liveness[parent.value].reaches_use(inst, node.ebb, &self.func.layout) { debug!( " - interference: {} overlaps vcopy at {}:{}", @@ -655,12 +640,7 @@ impl<'a> Context<'a> { // Both node and parent are values, so check for interference. debug_assert!(node.is_value() && parent.is_value()); if node.set_id != parent.set_id - && self.liveness[parent.value].overlaps_def( - node.def, - node.ebb, - self.liveness.forest(), - &self.func.layout, - ) + && self.liveness[parent.value].overlaps_def(node.def, node.ebb, &self.func.layout) { // The two values are interfering. debug!(" - interference: {} overlaps def of {}", parent, node.value); diff --git a/cranelift/codegen/src/regalloc/coloring.rs b/cranelift/codegen/src/regalloc/coloring.rs index 23a546a479..f7fb50129f 100644 --- a/cranelift/codegen/src/regalloc/coloring.rs +++ b/cranelift/codegen/src/regalloc/coloring.rs @@ -54,7 +54,7 @@ use crate::regalloc::affinity::Affinity; use crate::regalloc::diversion::RegDiversions; use crate::regalloc::live_value_tracker::{LiveValue, LiveValueTracker}; use crate::regalloc::liveness::Liveness; -use crate::regalloc::liverange::{LiveRange, LiveRangeForest}; +use crate::regalloc::liverange::LiveRange; use crate::regalloc::register_set::RegisterSet; use crate::regalloc::solver::{Solver, SolverError}; use crate::timing; @@ -461,7 +461,7 @@ impl<'a> Context<'a> { "Can't handle EBB arguments: {}", self.cur.display_inst(inst) ); - self.undivert_regs(|lr, _, _| !lr.is_local()); + self.undivert_regs(|lr, _| !lr.is_local()); } } @@ -726,12 +726,7 @@ impl<'a> Context<'a> { // the new variable as killed or live-through. Always special-case the // pinned register as a through variable. let layout = &self.cur.func.layout; - if self.liveness[value].killed_at( - inst, - layout.pp_ebb(inst), - self.liveness.forest(), - layout, - ) { + if self.liveness[value].killed_at(inst, layout.pp_ebb(inst), layout) { self.solver.add_killed_var(value, op.regclass, cur_reg); } else { self.solver.add_through_var(value, op.regclass, cur_reg); @@ -760,7 +755,7 @@ impl<'a> Context<'a> { // // Values with a global live range that are not live in to `dest` could appear as branch // arguments, so they can't always be un-diverted. - self.undivert_regs(|lr, forest, layout| lr.is_livein(dest, forest, layout)); + self.undivert_regs(|lr, layout| lr.is_livein(dest, layout)); // Now handle the EBB arguments. let br_args = self.cur.func.dfg.inst_variable_args(inst); @@ -830,14 +825,14 @@ impl<'a> Context<'a> { /// are reallocated to their global register assignments. fn undivert_regs(&mut self, mut pred: Pred) where - Pred: FnMut(&LiveRange, &LiveRangeForest, &Layout) -> bool, + Pred: FnMut(&LiveRange, &Layout) -> bool, { for (&value, rdiv) in self.divert.iter() { let lr = self .liveness .get(value) .expect("Missing live range for diverted register"); - if pred(lr, self.liveness.forest(), &self.cur.func.layout) { + if pred(lr, &self.cur.func.layout) { if let Affinity::Reg(rci) = lr.affinity { let rc = self.reginfo.rc(rci); // Stack diversions should not be possible here. They only live transiently @@ -1086,20 +1081,19 @@ impl<'a> Context<'a> { let inst = self.cur.current_inst().expect("Not on an instruction"); let layout = &self.cur.func.layout; - let forest = self.liveness.forest(); match self.cur.func.dfg.analyze_branch(inst) { NotABranch => false, SingleDest(ebb, _) => { let lr = &self.liveness[value]; - lr.is_livein(ebb, forest, layout) + lr.is_livein(ebb, layout) } Table(jt, ebb) => { let lr = &self.liveness[value]; !lr.is_local() - && (ebb.map_or(false, |ebb| lr.is_livein(ebb, forest, layout)) + && (ebb.map_or(false, |ebb| lr.is_livein(ebb, layout)) || self.cur.func.jump_tables[jt] .iter() - .any(|ebb| lr.is_livein(*ebb, forest, layout))) + .any(|ebb| lr.is_livein(*ebb, layout))) } } } diff --git a/cranelift/codegen/src/regalloc/live_value_tracker.rs b/cranelift/codegen/src/regalloc/live_value_tracker.rs index adfe56c410..5e17bb3557 100644 --- a/cranelift/codegen/src/regalloc/live_value_tracker.rs +++ b/cranelift/codegen/src/regalloc/live_value_tracker.rs @@ -198,7 +198,7 @@ impl LiveValueTracker { .expect("Immediate dominator value has no live range"); // Check if this value is live-in here. - if let Some(endpoint) = lr.livein_local_end(ebb, liveness.forest(), layout) { + if let Some(endpoint) = lr.livein_local_end(ebb, layout) { self.live.push(value, endpoint, lr); } } diff --git a/cranelift/codegen/src/regalloc/liveness.rs b/cranelift/codegen/src/regalloc/liveness.rs index 1ace6bf6d1..419e35bc98 100644 --- a/cranelift/codegen/src/regalloc/liveness.rs +++ b/cranelift/codegen/src/regalloc/liveness.rs @@ -181,7 +181,7 @@ use crate::ir::dfg::ValueDef; use crate::ir::{Ebb, Function, Inst, Layout, ProgramPoint, Value}; use crate::isa::{EncInfo, OperandConstraint, TargetIsa}; use crate::regalloc::affinity::Affinity; -use crate::regalloc::liverange::{LiveRange, LiveRangeForest}; +use crate::regalloc::liverange::LiveRange; use crate::timing; use core::mem; use core::ops::Index; @@ -249,14 +249,13 @@ fn extend_to_use( worklist: &mut Vec, func: &Function, cfg: &ControlFlowGraph, - forest: &mut LiveRangeForest, ) { // This is our scratch working space, and we'll leave it empty when we return. debug_assert!(worklist.is_empty()); // Extend the range locally in `ebb`. // If there already was a live interval in that block, we're done. - if lr.extend_in_ebb(ebb, to, &func.layout, forest) { + if lr.extend_in_ebb(ebb, to, &func.layout) { worklist.push(ebb); } @@ -277,7 +276,7 @@ fn extend_to_use( inst: branch, } in cfg.pred_iter(livein) { - if lr.extend_in_ebb(pred, branch, &func.layout, forest) { + if lr.extend_in_ebb(pred, branch, &func.layout) { // This predecessor EBB also became live-in. We need to process it later. worklist.push(pred); } @@ -292,9 +291,6 @@ pub struct Liveness { /// The live ranges that have been computed so far. ranges: LiveRangeSet, - /// Memory pool for the live ranges. - forest: LiveRangeForest, - /// Working space for the `extend_to_use` algorithm. /// This vector is always empty, except for inside that function. /// It lives here to avoid repeated allocation of scratch memory. @@ -309,16 +305,10 @@ impl Liveness { pub fn new() -> Self { Self { ranges: LiveRangeSet::new(), - forest: LiveRangeForest::new(), worklist: Vec::new(), } } - /// Current forest storage. - pub fn forest(&self) -> &LiveRangeForest { - &self.forest - } - /// Current live ranges. pub fn ranges(&self) -> &LiveRangeSet { &self.ranges @@ -327,7 +317,6 @@ impl Liveness { /// Clear all data structures in this liveness analysis. pub fn clear(&mut self) { self.ranges.clear(); - self.forest.clear(); self.worklist.clear(); } @@ -376,7 +365,7 @@ impl Liveness { ) -> &mut Affinity { debug_assert_eq!(Some(ebb), layout.inst_ebb(user)); let lr = self.ranges.get_mut(value).expect("Value has no live range"); - let livein = lr.extend_in_ebb(ebb, user, layout, &mut self.forest); + let livein = lr.extend_in_ebb(ebb, user, layout); debug_assert!(!livein, "{} should already be live in {}", value, ebb); &mut lr.affinity } @@ -431,15 +420,7 @@ impl Liveness { let lr = get_or_create(&mut self.ranges, arg, isa, func, &encinfo); // Extend the live range to reach this use. - extend_to_use( - lr, - ebb, - inst, - &mut self.worklist, - func, - cfg, - &mut self.forest, - ); + extend_to_use(lr, ebb, inst, &mut self.worklist, func, cfg); // Apply operand constraint, ignoring any variable arguments after the fixed // operands described by `operand_constraints`. Variable arguments are either diff --git a/cranelift/codegen/src/regalloc/liverange.rs b/cranelift/codegen/src/regalloc/liverange.rs index fb8fcc5fff..1d30650fa1 100644 --- a/cranelift/codegen/src/regalloc/liverange.rs +++ b/cranelift/codegen/src/regalloc/liverange.rs @@ -63,11 +63,11 @@ //! //! ## Current representation //! -//! Our current implementation uses a B-tree map with the necessary interface for an efficient -//! implementation of coalescing, implemented as a generic data-structure bforest::Map. -//! -//! A `BTreeMap` could have been used for the live-in intervals, but it doesn't provide -//! the necessary API to make coalescing easy, nor does it optimize for our types' sizes. +//! Our current implementation uses a sorted array of compressed intervals, represented by their +//! boundaries (Ebb, Inst), sorted by Ebb. This is a simple data structure, enables coalescing of +//! intervals easily, and shows some nice performance behavior. See +//! https://github.com/CraneStation/cranelift/issues/1084 for benchmarks against using a +//! bforest::Map. //! //! ## EBB ordering //! @@ -107,13 +107,19 @@ //! It is more complicated to work with, though, so it is probably not worth it. The performance //! benefits of switching to a numerical EBB order only appears if the binary search is doing //! EBB-EBB comparisons. +//! +//! A `BTreeMap` could have been used for the live-in intervals, but it doesn't provide +//! the necessary API to make coalescing easy, nor does it optimize for our types' sizes. +//! +//! Even the specialized `bforest::Map` implementation is slower than a plain sorted +//! array, see https://github.com/CraneStation/cranelift/issues/1084 for details. -use crate::bforest; use crate::entity::SparseMapValue; use crate::ir::{Ebb, ExpandedProgramPoint, Inst, Layout, ProgramOrder, ProgramPoint, Value}; use crate::regalloc::affinity::Affinity; use core::cmp::Ordering; use core::marker::PhantomData; +use smallvec::SmallVec; /// Global live range of a single SSA value. /// @@ -144,6 +150,12 @@ use core::marker::PhantomData; /// branch and jump instructions. pub type LiveRange = GenericLiveRange; +// See comment of liveins below. +pub struct Interval { + begin: Ebb, + end: Inst, +} + /// Generic live range implementation. /// /// The intended generic parameter is `PO=Layout`, but tests are simpler with a mock order. @@ -167,29 +179,18 @@ pub struct GenericLiveRange { /// Additional live-in intervals sorted in program order. /// - /// This map is empty for most values which are only used in one EBB. + /// This vector is empty for most values which are only used in one EBB. /// - /// A map entry `ebb -> inst` means that the live range is live-in to `ebb`, continuing up to + /// An entry `ebb -> inst` means that the live range is live-in to `ebb`, continuing up to /// `inst` which may belong to a later EBB in the program order. /// /// The entries are non-overlapping, and none of them overlap the EBB where the value is /// defined. - liveins: bforest::Map, + liveins: SmallVec<[Interval; 2]>, po: PhantomData<*const PO>, } -/// Forest of B-trees used for storing live ranges. -pub type LiveRangeForest = bforest::MapForest; - -struct Cmp<'a, PO: ProgramOrder + 'a>(&'a PO); - -impl<'a, PO: ProgramOrder> bforest::Comparator for Cmp<'a, PO> { - fn cmp(&self, a: Ebb, b: Ebb) -> Ordering { - self.0.cmp(a, b) - } -} - /// A simple helper macro to make comparisons more natural to read. macro_rules! cmp { ($order:ident, $a:ident > $b:expr) => { @@ -216,11 +217,26 @@ impl GenericLiveRange { affinity, def_begin: def, def_end: def, - liveins: bforest::Map::new(), + liveins: SmallVec::new(), po: PhantomData, } } + /// Finds an entry in the compressed set of live-in intervals that contains `ebb`, or return + /// the position where to insert such a new entry. + fn lookup_entry_containing_ebb(&self, ebb: Ebb, order: &PO) -> Result { + self.liveins + .binary_search_by(|interval| order.cmp(interval.begin, ebb)) + .or_else(|n| { + // The previous interval's end might cover the searched ebb. + if n > 0 && cmp!(order, ebb <= self.liveins[n - 1].end) { + Ok(n - 1) + } else { + Err(n) + } + }) + } + /// Extend the local interval for `ebb` so it reaches `to` which must belong to `ebb`. /// Create a live-in interval if necessary. /// @@ -232,83 +248,101 @@ impl GenericLiveRange { /// /// The return value can be used to detect if we just learned that the value is live-in to /// `ebb`. This can trigger recursive extensions in `ebb`'s CFG predecessor blocks. - pub fn extend_in_ebb( - &mut self, - ebb: Ebb, - to: Inst, - order: &PO, - forest: &mut bforest::MapForest, - ) -> bool { + pub fn extend_in_ebb(&mut self, ebb: Ebb, inst: Inst, order: &PO) -> bool { // First check if we're extending the def interval. // - // We're assuming here that `to` never precedes `def_begin` in the same EBB, but we can't - // check it without a method for getting `to`'s EBB. - if cmp!(order, ebb <= self.def_end) && cmp!(order, to >= self.def_begin) { - let to_pp = to.into(); + // We're assuming here that `inst` never precedes `def_begin` in the same EBB, but we can't + // check it without a method for getting `inst`'s EBB. + if cmp!(order, ebb <= self.def_end) && cmp!(order, inst >= self.def_begin) { + let inst_pp = inst.into(); debug_assert_ne!( - to_pp, self.def_begin, + inst_pp, self.def_begin, "Can't use value in the defining instruction." ); - if cmp!(order, to > self.def_end) { - self.def_end = to_pp; + if cmp!(order, inst > self.def_end) { + self.def_end = inst_pp; } return false; } // Now check if we're extending any of the existing live-in intervals. - let cmp = Cmp(order); - let mut c = self.liveins.cursor(forest, &cmp); - let first_time_livein; - - if let Some(end) = c.goto(ebb) { - // There's an interval beginning at `ebb`. See if it extends. - first_time_livein = false; - if cmp!(order, end < to) { - *c.value_mut().unwrap() = to; - } else { - return first_time_livein; - } - } else if let Some((_, end)) = c.prev() { - // There's no interval beginning at `ebb`, but we could still be live-in at `ebb` with - // a coalesced interval that begins before and ends after. - if cmp!(order, end > ebb) { - // Yep, the previous interval overlaps `ebb`. - first_time_livein = false; - if cmp!(order, end < to) { - *c.value_mut().unwrap() = to; - } else { - return first_time_livein; + match self.lookup_entry_containing_ebb(ebb, order) { + Ok(n) => { + // We found one interval and might need to extend it. + if cmp!(order, inst <= self.liveins[n].end) { + // Both interval parts are already included in a compressed interval. + return false; } - } else { - first_time_livein = true; - // The current interval does not overlap `ebb`, but it may still be possible to - // coalesce with it. - if order.is_ebb_gap(end, ebb) { - *c.value_mut().unwrap() = to; - } else { - c.insert(ebb, to); + + // If the instruction at the end is the last instruction before the next block, + // coalesce the two intervals: + // [ival.begin; ival.end] + [next.begin; next.end] = [ival.begin; next.end] + if let Some(next) = &self.liveins.get(n + 1) { + if order.is_ebb_gap(inst, next.begin) { + // At this point we can choose to remove the current interval or the next + // one; remove the next one to avoid one memory move. + let next_end = next.end; + debug_assert!(cmp!(order, next_end > self.liveins[n].end)); + self.liveins[n].end = next_end; + self.liveins.remove(n + 1); + return false; + } } + + // We can't coalesce, just extend the interval. + self.liveins[n].end = inst; + false } - } else { - // There is no existing interval before `ebb`. - first_time_livein = true; - c.insert(ebb, to); - } - // Now `c` is left pointing at an interval that ends in `to`. - debug_assert_eq!(c.value(), Some(to)); + Err(n) => { + // No interval was found containing the current EBB: we need to insert a new one, + // unless there's a coalescing opportunity with the previous or next one. + let coalesce_next = self + .liveins + .get(n) + .filter(|next| order.is_ebb_gap(inst, next.begin)) + .is_some(); + let coalesce_prev = self + .liveins + .get(n.wrapping_sub(1)) + .filter(|prev| order.is_ebb_gap(prev.end, ebb)) + .is_some(); - // See if it can be coalesced with the following interval. - if let Some((next_ebb, next_end)) = c.next() { - if order.is_ebb_gap(to, next_ebb) { - // Remove this interval and extend the previous end point to `next_end`. - c.remove(); - c.prev(); - *c.value_mut().unwrap() = next_end; + match (coalesce_prev, coalesce_next) { + // The new interval is the missing hole between prev and next: we can merge + // them all together. + (true, true) => { + let prev_end = self.liveins[n - 1].end; + debug_assert!(cmp!(order, prev_end <= self.liveins[n].end)); + self.liveins[n - 1].end = self.liveins[n].end; + self.liveins.remove(n); + } + + // Coalesce only with the previous or next one. + (true, false) => { + debug_assert!(cmp!(order, inst >= self.liveins[n - 1].end)); + self.liveins[n - 1].end = inst; + } + (false, true) => { + debug_assert!(cmp!(order, ebb <= self.liveins[n].begin)); + self.liveins[n].begin = ebb; + } + + (false, false) => { + // No coalescing opportunity, we have to insert. + self.liveins.insert( + n, + Interval { + begin: ebb, + end: inst, + }, + ); + } + } + + true } } - - first_time_livein } /// Is this the live range of a dead value? @@ -359,43 +393,39 @@ impl GenericLiveRange { /// If the live range is live through all of `ebb`, the terminator of `ebb` is a correct /// answer, but it is also possible that an even later program point is returned. So don't /// depend on the returned `Inst` to belong to `ebb`. - pub fn livein_local_end(&self, ebb: Ebb, forest: &LiveRangeForest, order: &PO) -> Option { - let cmp = Cmp(order); - self.liveins - .get_or_less(ebb, forest, &cmp) - .and_then(|(_, inst)| { - // We have an entry that ends at `inst`. - if cmp!(order, inst > ebb) { - Some(inst) + pub fn livein_local_end(&self, ebb: Ebb, order: &PO) -> Option { + self.lookup_entry_containing_ebb(ebb, order) + .and_then(|i| { + let inst = self.liveins[i].end; + if cmp!(order, ebb < inst) { + Ok(inst) } else { - None + // Can be any error type, really, since it's discarded by ok(). + Err(i) } }) + .ok() } /// Is this value live-in to `ebb`? /// /// An EBB argument is not considered to be live in. - pub fn is_livein(&self, ebb: Ebb, forest: &LiveRangeForest, order: &PO) -> bool { - self.livein_local_end(ebb, forest, order).is_some() + pub fn is_livein(&self, ebb: Ebb, order: &PO) -> bool { + self.livein_local_end(ebb, order).is_some() } /// Get all the live-in intervals. /// /// Note that the intervals are stored in a compressed form so each entry may span multiple /// EBBs where the value is live in. - pub fn liveins<'a>(&'a self, forest: &'a LiveRangeForest) -> bforest::MapIter<'a, Ebb, Inst> { - self.liveins.iter(forest) + pub fn liveins<'a>(&'a self) -> impl Iterator + 'a { + self.liveins + .iter() + .map(|interval| (interval.begin, interval.end)) } /// Check if this live range overlaps a definition in `ebb`. - pub fn overlaps_def( - &self, - def: ExpandedProgramPoint, - ebb: Ebb, - forest: &LiveRangeForest, - order: &PO, - ) -> bool { + pub fn overlaps_def(&self, def: ExpandedProgramPoint, ebb: Ebb, order: &PO) -> bool { // Two defs at the same program point always overlap, even if one is dead. if def == self.def_begin.into() { return true; @@ -407,30 +437,29 @@ impl GenericLiveRange { } // Check for an overlap with a live-in range. - match self.livein_local_end(ebb, forest, order) { + match self.livein_local_end(ebb, order) { Some(inst) => cmp!(order, def < inst), None => false, } } /// Check if this live range reaches a use at `user` in `ebb`. - pub fn reaches_use(&self, user: Inst, ebb: Ebb, forest: &LiveRangeForest, order: &PO) -> bool { + pub fn reaches_use(&self, user: Inst, ebb: Ebb, order: &PO) -> bool { // Check for an overlap with the local range. if cmp!(order, user > self.def_begin) && cmp!(order, user <= self.def_end) { return true; } // Check for an overlap with a live-in range. - match self.livein_local_end(ebb, forest, order) { + match self.livein_local_end(ebb, order) { Some(inst) => cmp!(order, user <= inst), None => false, } } /// Check if this live range is killed at `user` in `ebb`. - pub fn killed_at(&self, user: Inst, ebb: Ebb, forest: &LiveRangeForest, order: &PO) -> bool { - self.def_local_end() == user.into() - || self.livein_local_end(ebb, forest, order) == Some(user) + pub fn killed_at(&self, user: Inst, ebb: Ebb, order: &PO) -> bool { + self.def_local_end() == user.into() || self.livein_local_end(ebb, order) == Some(user) } } @@ -443,8 +472,7 @@ impl SparseMapValue for GenericLiveRange { #[cfg(test)] mod tests { - use super::GenericLiveRange; - use crate::bforest; + use super::{GenericLiveRange, Interval}; use crate::entity::EntityRef; use crate::ir::{Ebb, Inst, Value}; use crate::ir::{ExpandedProgramPoint, ProgramOrder}; @@ -496,11 +524,7 @@ mod tests { } // Validate the live range invariants. - fn validate( - &self, - lr: &GenericLiveRange, - forest: &bforest::MapForest, - ) { + fn validate(&self, lr: &GenericLiveRange) { // The def interval must cover a single EBB. let def_ebb = self.pp_ebb(lr.def_begin); assert_eq!(def_ebb, self.pp_ebb(lr.def_end)); @@ -516,7 +540,10 @@ mod tests { // Check the live-in intervals. let mut prev_end = None; - for (begin, end) in lr.liveins.iter(forest) { + for Interval { begin, end } in lr.liveins.iter() { + let begin = *begin; + let end = *end; + assert_eq!(self.cmp(begin, end), Ordering::Less); if let Some(e) = prev_end { assert_eq!(self.cmp(e, begin), Ordering::Less); @@ -545,18 +572,17 @@ mod tests { let i2 = Inst::new(2); let e2 = Ebb::new(2); let lr = GenericLiveRange::new(v0, i1.into(), Default::default()); - let forest = &bforest::MapForest::new(); assert!(lr.is_dead()); assert!(lr.is_local()); assert_eq!(lr.def(), i1.into()); assert_eq!(lr.def_local_end(), i1.into()); - assert_eq!(lr.livein_local_end(e2, forest, PO), None); - PO.validate(&lr, forest); + assert_eq!(lr.livein_local_end(e2, PO), None); + PO.validate(&lr); // A dead live range overlaps its own def program point. - assert!(lr.overlaps_def(i1.into(), e0, forest, PO)); - assert!(!lr.overlaps_def(i2.into(), e0, forest, PO)); - assert!(!lr.overlaps_def(e0.into(), e0, forest, PO)); + assert!(lr.overlaps_def(i1.into(), e0, PO)); + assert!(!lr.overlaps_def(i2.into(), e0, PO)); + assert!(!lr.overlaps_def(e0.into(), e0, PO)); } #[test] @@ -564,14 +590,13 @@ mod tests { let v0 = Value::new(0); let e2 = Ebb::new(2); let lr = GenericLiveRange::new(v0, e2.into(), Default::default()); - let forest = &bforest::MapForest::new(); assert!(lr.is_dead()); assert!(lr.is_local()); assert_eq!(lr.def(), e2.into()); assert_eq!(lr.def_local_end(), e2.into()); // The def interval of an EBB argument does not count as live-in. - assert_eq!(lr.livein_local_end(e2, forest, PO), None); - PO.validate(&lr, forest); + assert_eq!(lr.livein_local_end(e2, PO), None); + PO.validate(&lr); } #[test] @@ -582,18 +607,17 @@ mod tests { let i12 = Inst::new(12); let i13 = Inst::new(13); let mut lr = GenericLiveRange::new(v0, i11.into(), Default::default()); - let forest = &mut bforest::MapForest::new(); - assert_eq!(lr.extend_in_ebb(e10, i13, PO, forest), false); - PO.validate(&lr, forest); + assert_eq!(lr.extend_in_ebb(e10, i13, PO), false); + PO.validate(&lr); assert!(!lr.is_dead()); assert!(lr.is_local()); assert_eq!(lr.def(), i11.into()); assert_eq!(lr.def_local_end(), i13.into()); // Extending to an already covered inst should not change anything. - assert_eq!(lr.extend_in_ebb(e10, i12, PO, forest), false); - PO.validate(&lr, forest); + assert_eq!(lr.extend_in_ebb(e10, i12, PO), false); + PO.validate(&lr); assert_eq!(lr.def(), i11.into()); assert_eq!(lr.def_local_end(), i13.into()); } @@ -606,26 +630,25 @@ mod tests { let i12 = Inst::new(12); let i13 = Inst::new(13); let mut lr = GenericLiveRange::new(v0, e10.into(), Default::default()); - let forest = &mut bforest::MapForest::new(); // Extending a dead EBB argument in its own block should not indicate that a live-in // interval was created. - assert_eq!(lr.extend_in_ebb(e10, i12, PO, forest), false); - PO.validate(&lr, forest); + assert_eq!(lr.extend_in_ebb(e10, i12, PO), false); + PO.validate(&lr); assert!(!lr.is_dead()); assert!(lr.is_local()); assert_eq!(lr.def(), e10.into()); assert_eq!(lr.def_local_end(), i12.into()); // Extending to an already covered inst should not change anything. - assert_eq!(lr.extend_in_ebb(e10, i11, PO, forest), false); - PO.validate(&lr, forest); + assert_eq!(lr.extend_in_ebb(e10, i11, PO), false); + PO.validate(&lr); assert_eq!(lr.def(), e10.into()); assert_eq!(lr.def_local_end(), i12.into()); // Extending further. - assert_eq!(lr.extend_in_ebb(e10, i13, PO, forest), false); - PO.validate(&lr, forest); + assert_eq!(lr.extend_in_ebb(e10, i13, PO), false); + PO.validate(&lr); assert_eq!(lr.def(), e10.into()); assert_eq!(lr.def_local_end(), i13.into()); } @@ -641,23 +664,22 @@ mod tests { let i22 = Inst::new(22); let i23 = Inst::new(23); let mut lr = GenericLiveRange::new(v0, i11.into(), Default::default()); - let forest = &mut bforest::MapForest::new(); - assert_eq!(lr.extend_in_ebb(e10, i12, PO, forest), false); + assert_eq!(lr.extend_in_ebb(e10, i12, PO), false); // Adding a live-in interval. - assert_eq!(lr.extend_in_ebb(e20, i22, PO, forest), true); - PO.validate(&lr, forest); - assert_eq!(lr.livein_local_end(e20, forest, PO), Some(i22)); + assert_eq!(lr.extend_in_ebb(e20, i22, PO), true); + PO.validate(&lr); + assert_eq!(lr.livein_local_end(e20, PO), Some(i22)); // Non-extending the live-in. - assert_eq!(lr.extend_in_ebb(e20, i21, PO, forest), false); - assert_eq!(lr.livein_local_end(e20, forest, PO), Some(i22)); + assert_eq!(lr.extend_in_ebb(e20, i21, PO), false); + assert_eq!(lr.livein_local_end(e20, PO), Some(i22)); // Extending the existing live-in. - assert_eq!(lr.extend_in_ebb(e20, i23, PO, forest), false); - PO.validate(&lr, forest); - assert_eq!(lr.livein_local_end(e20, forest, PO), Some(i23)); + assert_eq!(lr.extend_in_ebb(e20, i23, PO), false); + PO.validate(&lr); + assert_eq!(lr.livein_local_end(e20, PO), Some(i23)); } #[test] @@ -671,32 +693,28 @@ mod tests { let e40 = Ebb::new(40); let i41 = Inst::new(41); let mut lr = GenericLiveRange::new(v0, i11.into(), Default::default()); - let forest = &mut bforest::MapForest::new(); - assert_eq!(lr.extend_in_ebb(e30, i31, PO, forest), true); - assert_eq!(lr.liveins(forest).collect::>(), [(e30, i31)]); + assert_eq!(lr.extend_in_ebb(e30, i31, PO,), true); + assert_eq!(lr.liveins().collect::>(), [(e30, i31)]); // Coalesce to previous - assert_eq!(lr.extend_in_ebb(e40, i41, PO, forest), true); - assert_eq!(lr.liveins(forest).collect::>(), [(e30, i41)]); + assert_eq!(lr.extend_in_ebb(e40, i41, PO,), true); + assert_eq!(lr.liveins().collect::>(), [(e30, i41)]); // Coalesce to next - assert_eq!(lr.extend_in_ebb(e20, i21, PO, forest), true); - assert_eq!(lr.liveins(forest).collect::>(), [(e20, i41)]); + assert_eq!(lr.extend_in_ebb(e20, i21, PO,), true); + assert_eq!(lr.liveins().collect::>(), [(e20, i41)]); let mut lr = GenericLiveRange::new(v0, i11.into(), Default::default()); - assert_eq!(lr.extend_in_ebb(e40, i41, PO, forest), true); - assert_eq!(lr.liveins(forest).collect::>(), [(e40, i41)]); + assert_eq!(lr.extend_in_ebb(e40, i41, PO,), true); + assert_eq!(lr.liveins().collect::>(), [(e40, i41)]); - assert_eq!(lr.extend_in_ebb(e20, i21, PO, forest), true); - assert_eq!( - lr.liveins(forest).collect::>(), - [(e20, i21), (e40, i41)] - ); + assert_eq!(lr.extend_in_ebb(e20, i21, PO,), true); + assert_eq!(lr.liveins().collect::>(), [(e20, i21), (e40, i41)]); // Coalesce to previous and next - assert_eq!(lr.extend_in_ebb(e30, i31, PO, forest), true); - assert_eq!(lr.liveins(forest).collect::>(), [(e20, i41)]); + assert_eq!(lr.extend_in_ebb(e30, i31, PO,), true); + assert_eq!(lr.liveins().collect::>(), [(e20, i41)]); } } diff --git a/cranelift/codegen/src/regalloc/spilling.rs b/cranelift/codegen/src/regalloc/spilling.rs index 0012ad01d8..ace368d798 100644 --- a/cranelift/codegen/src/regalloc/spilling.rs +++ b/cranelift/codegen/src/regalloc/spilling.rs @@ -324,13 +324,11 @@ impl<'a> Context<'a> { ConstraintKind::FixedReg(_) => reguse.fixed = true, ConstraintKind::Tied(_) => { // A tied operand must kill the used value. - reguse.tied = - !lr.killed_at(inst, ebb, self.liveness.forest(), &self.cur.func.layout); + reguse.tied = !lr.killed_at(inst, ebb, &self.cur.func.layout); } ConstraintKind::FixedTied(_) => { reguse.fixed = true; - reguse.tied = - !lr.killed_at(inst, ebb, self.liveness.forest(), &self.cur.func.layout); + reguse.tied = !lr.killed_at(inst, ebb, &self.cur.func.layout); } ConstraintKind::Reg => {} } diff --git a/cranelift/codegen/src/value_label.rs b/cranelift/codegen/src/value_label.rs index b6ad71faf8..3d5802f12e 100644 --- a/cranelift/codegen/src/value_label.rs +++ b/cranelift/codegen/src/value_label.rs @@ -98,7 +98,6 @@ where let encinfo = isa.encoding_info(); let values_locations = &func.locations; let liveness_ranges = regalloc.liveness().ranges(); - let liveness_forest = regalloc.liveness().forest(); let mut ranges = HashMap::new(); let mut add_range = |label, range: (u32, u32), loc: ValueLoc| { @@ -127,10 +126,7 @@ where // Remove killed values. tracked_values.retain(|(x, label, start_offset, last_loc)| { let range = liveness_ranges.get(*x); - if range - .expect("value") - .killed_at(inst, ebb, &liveness_forest, &func.layout) - { + if range.expect("value").killed_at(inst, ebb, &func.layout) { add_range(*label, (*start_offset, end_offset), *last_loc); return false; } @@ -177,7 +173,7 @@ where // Ignore dead/inactive Values. let range = liveness_ranges.get(*v); match range { - Some(r) => r.reaches_use(inst, ebb, &liveness_forest, &func.layout), + Some(r) => r.reaches_use(inst, ebb, &func.layout), None => false, } }); diff --git a/cranelift/codegen/src/verifier/cssa.rs b/cranelift/codegen/src/verifier/cssa.rs index 3878f096d1..85c6a638c2 100644 --- a/cranelift/codegen/src/verifier/cssa.rs +++ b/cranelift/codegen/src/verifier/cssa.rs @@ -118,12 +118,7 @@ impl<'a> CssaVerifier<'a> { if self.preorder.dominates(prev_ebb, def_ebb) && self.domtree.dominates(prev_def, def, &self.func.layout) { - if self.liveness[prev_val].overlaps_def( - def, - def_ebb, - self.liveness.forest(), - &self.func.layout, - ) { + if self.liveness[prev_val].overlaps_def(def, def_ebb, &self.func.layout) { return fatal!( errors, val, diff --git a/cranelift/codegen/src/verifier/liveness.rs b/cranelift/codegen/src/verifier/liveness.rs index 37b59c154d..218bb60fdc 100644 --- a/cranelift/codegen/src/verifier/liveness.rs +++ b/cranelift/codegen/src/verifier/liveness.rs @@ -107,7 +107,7 @@ impl<'a> LivenessVerifier<'a> { }; debug_assert!(self.func.layout.inst_ebb(inst).unwrap() == ebb); - if !lr.reaches_use(inst, ebb, self.liveness.forest(), &self.func.layout) { + if !lr.reaches_use(inst, ebb, &self.func.layout) { return fatal!(errors, inst, "{} is not live at this use", val); } @@ -179,7 +179,7 @@ impl<'a> LivenessVerifier<'a> { } // Now check the live-in intervals against the CFG. - for (mut ebb, end) in lr.liveins(self.liveness.forest()) { + for (mut ebb, end) in lr.liveins() { if !l.is_ebb_inserted(ebb) { return fatal!( errors, @@ -207,7 +207,7 @@ impl<'a> LivenessVerifier<'a> { loop { // If `val` is live-in at `ebb`, it must be live at all the predecessors. for BasicBlock { inst: pred, ebb } in self.cfg.pred_iter(ebb) { - if !lr.reaches_use(pred, ebb, self.liveness.forest(), &self.func.layout) { + if !lr.reaches_use(pred, ebb, &self.func.layout) { return fatal!( errors, pred, diff --git a/cranelift/codegen/src/verifier/locations.rs b/cranelift/codegen/src/verifier/locations.rs index d32bb7e4bd..6a32cec0f6 100644 --- a/cranelift/codegen/src/verifier/locations.rs +++ b/cranelift/codegen/src/verifier/locations.rs @@ -334,10 +334,10 @@ impl<'a> LocationVerifier<'a> { let lr = &liveness[value]; if is_after_branch && unique_predecessor { // Forward diversions based on the targeted branch. - if !lr.is_livein(ebb, liveness.forest(), &self.func.layout) { + if !lr.is_livein(ebb, &self.func.layout) { val_to_remove.push(value) } - } else if lr.is_livein(ebb, liveness.forest(), &self.func.layout) { + } else if lr.is_livein(ebb, &self.func.layout) { return fatal!( errors, inst, @@ -359,7 +359,7 @@ impl<'a> LocationVerifier<'a> { for (&value, d) in divert.iter() { let lr = &liveness[value]; if let Some(ebb) = ebb { - if lr.is_livein(ebb, liveness.forest(), &self.func.layout) { + if lr.is_livein(ebb, &self.func.layout) { return fatal!( errors, inst, @@ -371,7 +371,7 @@ impl<'a> LocationVerifier<'a> { } } for ebb in self.func.jump_tables[jt].iter() { - if lr.is_livein(*ebb, liveness.forest(), &self.func.layout) { + if lr.is_livein(*ebb, &self.func.layout) { return fatal!( errors, inst,