From 77e6a9e0d78a6163bde0167085e1995aea7b08c6 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Sun, 28 Nov 2021 17:52:50 +0000 Subject: [PATCH 1/7] Add support for fixed stack slots This works by allowing a PReg to be marked as being a stack location instead of a physical register. --- src/checker.rs | 2 +- src/ion/data_structures.rs | 1 + src/ion/liveranges.rs | 4 ++++ src/ion/merge.rs | 6 +++--- src/ion/moves.rs | 18 ++++++++++++++---- src/ion/process.rs | 7 +++++-- src/ion/requirement.rs | 33 ++++++++++++++++++--------------- src/lib.rs | 8 ++++++++ 8 files changed, 54 insertions(+), 25 deletions(-) diff --git a/src/checker.rs b/src/checker.rs index bf5c098..55f4860 100644 --- a/src/checker.rs +++ b/src/checker.rs @@ -78,7 +78,7 @@ use crate::{ Allocation, AllocationKind, Block, Edit, Function, Inst, InstPosition, Operand, - OperandConstraint, OperandKind, OperandPos, Output, PReg, ProgPoint, VReg, + OperandConstraint, OperandKind, OperandPos, Output, PReg, ProgPoint, RegClass, VReg, }; use std::collections::{HashMap, HashSet, VecDeque}; diff --git a/src/ion/data_structures.rs b/src/ion/data_structures.rs index 748d846..526a57b 100644 --- a/src/ion/data_structures.rs +++ b/src/ion/data_structures.rs @@ -266,6 +266,7 @@ pub struct VRegData { pub struct PRegData { pub reg: PReg, pub allocations: LiveRangeSet, + pub is_stack: bool, } #[derive(Clone, Debug)] diff --git a/src/ion/liveranges.rs b/src/ion/liveranges.rs index de100c9..758349a 100644 --- a/src/ion/liveranges.rs +++ b/src/ion/liveranges.rs @@ -102,6 +102,7 @@ impl<'a, F: Function> Env<'a, F> { PRegData { reg: PReg::invalid(), allocations: LiveRangeSet::new(), + is_stack: false, }, ); for i in 0..=PReg::MAX { @@ -110,6 +111,9 @@ impl<'a, F: Function> Env<'a, F> { let preg_float = PReg::new(i, RegClass::Float); self.pregs[preg_float.index()].reg = preg_float; } + for &preg in &self.env.fixed_stack_slots { + self.pregs[preg.index()].is_stack = true; + } // Create VRegs from the vreg count. for idx in 0..self.func.num_vregs() { // We'll fill in the real details when we see the def. diff --git a/src/ion/merge.rs b/src/ion/merge.rs index 2f3611b..dfdbe7d 100644 --- a/src/ion/merge.rs +++ b/src/ion/merge.rs @@ -99,9 +99,9 @@ impl<'a, F: Function> Env<'a, F> { || self.bundles[to.index()].cached_stack() || self.bundles[to.index()].cached_fixed() { - let req = self - .compute_requirement(from) - .merge(self.compute_requirement(to)); + let req_from = self.compute_requirement(from); + let req_to = self.compute_requirement(to); + let req = self.merge_requirement(req_from, req_to); if req == Requirement::Conflict { log::trace!(" -> conflicting requirements; aborting merge"); return false; diff --git a/src/ion/moves.rs b/src/ion/moves.rs index 2b562f7..c7b8489 100644 --- a/src/ion/moves.rs +++ b/src/ion/moves.rs @@ -35,6 +35,16 @@ impl<'a, F: Function> Env<'a, F> { pos == self.cfginfo.block_exit[block.index()] } + fn allocation_is_stack(&self, alloc: Allocation) -> bool { + if alloc.is_stack() { + true + } else if let Some(preg) = alloc.as_reg() { + self.pregs[preg.index()].is_stack + } else { + false + } + } + pub fn insert_move( &mut self, pos: ProgPoint, @@ -968,9 +978,9 @@ impl<'a, F: Function> Env<'a, F> { let scratch_used = resolved.iter().any(|&(src, dst, _)| { src == Allocation::reg(scratch) || dst == Allocation::reg(scratch) }); - let stack_stack_move = resolved - .iter() - .any(|&(src, dst, _)| src.is_stack() && dst.is_stack()); + let stack_stack_move = resolved.iter().any(|&(src, dst, _)| { + self.allocation_is_stack(src) && self.allocation_is_stack(dst) + }); let extra_slot = if scratch_used && stack_stack_move { if self.extra_spillslot[regclass as u8 as usize].is_none() { let slot = self.allocate_spillslot(regclass); @@ -989,7 +999,7 @@ impl<'a, F: Function> Env<'a, F> { if dst == Allocation::reg(scratch) { scratch_used_yet = true; } - if src.is_stack() && dst.is_stack() { + if self.allocation_is_stack(src) && self.allocation_is_stack(dst) { if !scratch_used_yet { self.add_edit( pos, diff --git a/src/ion/process.rs b/src/ion/process.rs index 123ae30..9c29156 100644 --- a/src/ion/process.rs +++ b/src/ion/process.rs @@ -374,7 +374,7 @@ impl<'a, F: Function> Env<'a, F> { for entry in &self.bundles[bundle.index()].ranges { for u in &self.ranges[entry.index.index()].uses { let this_req = Requirement::from_operand(u.operand); - req = req.merge(this_req); + req = self.merge_requirement(req, this_req); if req == Requirement::Conflict { return u.pos; } @@ -754,11 +754,14 @@ impl<'a, F: Function> Env<'a, F> { let class = self.spillsets[self.bundles[bundle.index()].spillset.index()].class; let req = self.compute_requirement(bundle); // Grab a hint from either the queue or our spillset, if any. - let hint_reg = if reg_hint != PReg::invalid() { + let mut hint_reg = if reg_hint != PReg::invalid() { reg_hint } else { self.spillsets[self.bundles[bundle.index()].spillset.index()].reg_hint }; + if self.pregs[hint_reg.index()].is_stack { + hint_reg = PReg::invalid(); + } log::trace!("process_bundle: bundle {:?} hint {:?}", bundle, hint_reg,); if let Requirement::Conflict = req { diff --git a/src/ion/requirement.rs b/src/ion/requirement.rs index 8aeb824..e5c9c90 100644 --- a/src/ion/requirement.rs +++ b/src/ion/requirement.rs @@ -25,20 +25,6 @@ pub enum Requirement { Conflict, } impl Requirement { - #[inline(always)] - pub fn merge(self, other: Requirement) -> Requirement { - match (self, other) { - (Requirement::Unknown, other) | (other, Requirement::Unknown) => other, - (Requirement::Conflict, _) | (_, Requirement::Conflict) => Requirement::Conflict, - (other, Requirement::Any) | (Requirement::Any, other) => other, - (Requirement::Stack, Requirement::Stack) => self, - (Requirement::Register, Requirement::Fixed(preg)) - | (Requirement::Fixed(preg), Requirement::Register) => Requirement::Fixed(preg), - (Requirement::Register, Requirement::Register) => self, - (Requirement::Fixed(a), Requirement::Fixed(b)) if a == b => self, - _ => Requirement::Conflict, - } - } #[inline(always)] pub fn from_operand(op: Operand) -> Requirement { match op.constraint() { @@ -51,6 +37,23 @@ impl Requirement { } impl<'a, F: Function> Env<'a, F> { + #[inline(always)] + pub fn merge_requirement(&self, a: Requirement, b: Requirement) -> Requirement { + match (a, b) { + (Requirement::Unknown, other) | (other, Requirement::Unknown) => other, + (Requirement::Conflict, _) | (_, Requirement::Conflict) => Requirement::Conflict, + (other, Requirement::Any) | (Requirement::Any, other) => other, + (Requirement::Stack, Requirement::Stack) => Requirement::Stack, + (Requirement::Register, Requirement::Register) => Requirement::Register, + (Requirement::Register, Requirement::Fixed(preg)) + | (Requirement::Fixed(preg), Requirement::Register) if !self.pregs[preg.index()].is_stack => Requirement::Fixed(preg), + (Requirement::Stack, Requirement::Fixed(preg)) + | (Requirement::Fixed(preg), Requirement::Stack) if self.pregs[preg.index()].is_stack => Requirement::Fixed(preg), + (Requirement::Fixed(a), Requirement::Fixed(b)) if a == b => Requirement::Fixed(a), + _ => Requirement::Conflict, + } + } + pub fn compute_requirement(&self, bundle: LiveBundleIndex) -> Requirement { let mut req = Requirement::Unknown; log::trace!("compute_requirement: {:?}", bundle); @@ -59,7 +62,7 @@ impl<'a, F: Function> Env<'a, F> { for u in &self.ranges[entry.index.index()].uses { log::trace!(" -> use {:?}", u); let r = Requirement::from_operand(u.operand); - req = req.merge(r); + req = self.merge_requirement(req, r); log::trace!(" -> req {:?}", req); } } diff --git a/src/lib.rs b/src/lib.rs index 6cc4354..8f21599 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1154,6 +1154,14 @@ pub struct MachineEnv { /// the emission of two machine-code instructions, this lowering /// can use the scratch register between them. pub scratch_by_class: [PReg; 2], + + /// Some `PReg`s can be designated as locations on the stack rather than + /// actual registers. These can be used to tell the register allocator about + /// pre-defined stack slots used for function arguments and return values. + /// + /// `PReg`s in this list cannot be used as a scratch register or as an + /// allocatable regsiter. + pub fixed_stack_slots: Vec, } /// The output of the register allocator. From 4f8e115115819ab4e4b6353d87e058029ab1f6aa Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Sun, 28 Nov 2021 17:52:50 +0000 Subject: [PATCH 2/7] Refactor requirement computation --- src/ion/merge.rs | 7 +++-- src/ion/process.rs | 28 ++---------------- src/ion/requirement.rs | 64 +++++++++++++++++++++++++----------------- 3 files changed, 44 insertions(+), 55 deletions(-) diff --git a/src/ion/merge.rs b/src/ion/merge.rs index dfdbe7d..b32a1b0 100644 --- a/src/ion/merge.rs +++ b/src/ion/merge.rs @@ -99,9 +99,10 @@ impl<'a, F: Function> Env<'a, F> { || self.bundles[to.index()].cached_stack() || self.bundles[to.index()].cached_fixed() { - let req_from = self.compute_requirement(from); - let req_to = self.compute_requirement(to); - let req = self.merge_requirement(req_from, req_to); + let req = self + .compute_requirement(from) + .0 + .merge(self.compute_requirement(to).0); if req == Requirement::Conflict { log::trace!(" -> conflicting requirements; aborting merge"); return false; diff --git a/src/ion/process.rs b/src/ion/process.rs index 9c29156..0afe894 100644 --- a/src/ion/process.rs +++ b/src/ion/process.rs @@ -367,29 +367,6 @@ impl<'a, F: Function> Env<'a, F> { } } - pub fn find_conflict_split_point(&self, bundle: LiveBundleIndex) -> ProgPoint { - // Find the first use whose requirement causes the merge up to - // this point to go to Conflict. - let mut req = Requirement::Unknown; - for entry in &self.bundles[bundle.index()].ranges { - for u in &self.ranges[entry.index.index()].uses { - let this_req = Requirement::from_operand(u.operand); - req = self.merge_requirement(req, this_req); - if req == Requirement::Conflict { - return u.pos; - } - } - } - - // Fallback: start of bundle. - self.bundles[bundle.index()] - .ranges - .first() - .unwrap() - .range - .from - } - pub fn get_or_create_spill_bundle( &mut self, bundle: LiveBundleIndex, @@ -752,7 +729,7 @@ impl<'a, F: Function> Env<'a, F> { reg_hint: PReg, ) -> Result<(), RegAllocError> { let class = self.spillsets[self.bundles[bundle.index()].spillset.index()].class; - let req = self.compute_requirement(bundle); + let (req, split_point) = self.compute_requirement(bundle); // Grab a hint from either the queue or our spillset, if any. let mut hint_reg = if reg_hint != PReg::invalid() { reg_hint @@ -772,7 +749,6 @@ impl<'a, F: Function> Env<'a, F> { !self.minimal_bundle(bundle), "Minimal bundle with conflict!" ); - let split_point = self.find_conflict_split_point(bundle); self.split_and_requeue_bundle( bundle, /* split_at_point = */ split_point, @@ -809,7 +785,7 @@ impl<'a, F: Function> Env<'a, F> { debug_assert!(attempts < 100 * self.func.num_insts()); let fixed_preg = match req { - Requirement::Fixed(preg) => Some(preg), + Requirement::FixedReg(preg) | Requirement::FixedStack(preg) => Some(preg), Requirement::Register => None, Requirement::Stack => { // If we must be on the stack, mark our spillset diff --git a/src/ion/requirement.rs b/src/ion/requirement.rs index e5c9c90..6e7920b 100644 --- a/src/ion/requirement.rs +++ b/src/ion/requirement.rs @@ -13,12 +13,13 @@ //! Requirements computation. use super::{Env, LiveBundleIndex}; -use crate::{Function, Operand, OperandConstraint, PReg}; +use crate::{Function, Operand, OperandConstraint, PReg, ProgPoint}; #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Requirement { Unknown, - Fixed(PReg), + FixedReg(PReg), + FixedStack(PReg), Register, Stack, Any, @@ -26,47 +27,58 @@ pub enum Requirement { } impl Requirement { #[inline(always)] - pub fn from_operand(op: Operand) -> Requirement { - match op.constraint() { - OperandConstraint::FixedReg(preg) => Requirement::Fixed(preg), - OperandConstraint::Reg | OperandConstraint::Reuse(_) => Requirement::Register, - OperandConstraint::Stack => Requirement::Stack, - _ => Requirement::Any, + pub fn merge(self, other: Requirement) -> Requirement { + match (self, other) { + (Requirement::Unknown, other) | (other, Requirement::Unknown) => other, + (Requirement::Conflict, _) | (_, Requirement::Conflict) => Requirement::Conflict, + (other, Requirement::Any) | (Requirement::Any, other) => other, + (Requirement::Register, Requirement::Register) => self, + (Requirement::Stack, Requirement::Stack) => self, + (Requirement::Register, Requirement::FixedReg(preg)) + | (Requirement::FixedReg(preg), Requirement::Register) => Requirement::FixedReg(preg), + (Requirement::Stack, Requirement::FixedStack(preg)) + | (Requirement::FixedStack(preg), Requirement::Stack) => Requirement::FixedStack(preg), + (Requirement::FixedReg(a), Requirement::FixedReg(b)) if a == b => self, + (Requirement::FixedStack(a), Requirement::FixedStack(b)) if a == b => self, + _ => Requirement::Conflict, } } } impl<'a, F: Function> Env<'a, F> { #[inline(always)] - pub fn merge_requirement(&self, a: Requirement, b: Requirement) -> Requirement { - match (a, b) { - (Requirement::Unknown, other) | (other, Requirement::Unknown) => other, - (Requirement::Conflict, _) | (_, Requirement::Conflict) => Requirement::Conflict, - (other, Requirement::Any) | (Requirement::Any, other) => other, - (Requirement::Stack, Requirement::Stack) => Requirement::Stack, - (Requirement::Register, Requirement::Register) => Requirement::Register, - (Requirement::Register, Requirement::Fixed(preg)) - | (Requirement::Fixed(preg), Requirement::Register) if !self.pregs[preg.index()].is_stack => Requirement::Fixed(preg), - (Requirement::Stack, Requirement::Fixed(preg)) - | (Requirement::Fixed(preg), Requirement::Stack) if self.pregs[preg.index()].is_stack => Requirement::Fixed(preg), - (Requirement::Fixed(a), Requirement::Fixed(b)) if a == b => Requirement::Fixed(a), - _ => Requirement::Conflict, + pub fn requirement_from_operand(&self, op: Operand) -> Requirement { + match op.constraint() { + OperandConstraint::FixedReg(preg) => { + if self.pregs[preg.index()].is_stack { + Requirement::FixedStack(preg) + } else { + Requirement::FixedReg(preg) + } + } + OperandConstraint::Reg | OperandConstraint::Reuse(_) => Requirement::Register, + OperandConstraint::Stack => Requirement::Stack, + _ => Requirement::Any, } } - pub fn compute_requirement(&self, bundle: LiveBundleIndex) -> Requirement { + pub fn compute_requirement(&self, bundle: LiveBundleIndex) -> (Requirement, ProgPoint) { let mut req = Requirement::Unknown; log::trace!("compute_requirement: {:?}", bundle); - for entry in &self.bundles[bundle.index()].ranges { + let ranges = &self.bundles[bundle.index()].ranges; + for entry in ranges { log::trace!(" -> LR {:?}", entry.index); for u in &self.ranges[entry.index.index()].uses { log::trace!(" -> use {:?}", u); - let r = Requirement::from_operand(u.operand); - req = self.merge_requirement(req, r); + let r = self.requirement_from_operand(u.operand); + req = req.merge(r); log::trace!(" -> req {:?}", req); + if req == Requirement::Conflict { + return (req, u.pos); + } } } log::trace!(" -> final: {:?}", req); - req + (req, ranges.first().unwrap().range.from) } } From 707aacd8187d9e701c612aacb06e9e40a8d14151 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Sun, 28 Nov 2021 17:52:50 +0000 Subject: [PATCH 3/7] Split up functions in liverange.rs This helps with profiling even if they are inlined since perf with DWARF callgraph profiling can attribute execution time to inlined functions. --- src/ion/liveranges.rs | 32 ++++++++++++++++++-------------- src/ion/mod.rs | 2 ++ 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/ion/liveranges.rs b/src/ion/liveranges.rs index 758349a..a8e3561 100644 --- a/src/ion/liveranges.rs +++ b/src/ion/liveranges.rs @@ -392,6 +392,10 @@ impl<'a, F: Function> Env<'a, F> { return Err(RegAllocError::EntryLivein); } + Ok(()) + } + + pub fn build_liveranges(&mut self) { for &vreg in self.func.reftype_vregs() { self.safepoints_per_vreg.insert(vreg.vreg(), HashSet::new()); } @@ -1142,6 +1146,20 @@ impl<'a, F: Function> Env<'a, F> { } } + self.blockparam_ins.sort_unstable(); + self.blockparam_outs.sort_unstable(); + self.prog_move_srcs.sort_unstable_by_key(|(pos, _)| *pos); + self.prog_move_dsts.sort_unstable_by_key(|(pos, _)| *pos); + + log::trace!("prog_move_srcs = {:?}", self.prog_move_srcs); + log::trace!("prog_move_dsts = {:?}", self.prog_move_dsts); + + self.stats.initial_liverange_count = self.ranges.len(); + self.stats.blockparam_ins_count = self.blockparam_ins.len(); + self.stats.blockparam_outs_count = self.blockparam_outs.len(); + } + + pub fn fixup_multi_fixed_vregs(&mut self) { // Do a fixed-reg cleanup pass: if there are any LiveRanges with // multiple uses (or defs) at the same ProgPoint and there is // more than one FixedReg constraint at that ProgPoint, we @@ -1239,19 +1257,5 @@ impl<'a, F: Function> Env<'a, F> { seen_fixed_for_vreg.clear(); } } - - self.blockparam_ins.sort_unstable(); - self.blockparam_outs.sort_unstable(); - self.prog_move_srcs.sort_unstable_by_key(|(pos, _)| *pos); - self.prog_move_dsts.sort_unstable_by_key(|(pos, _)| *pos); - - log::trace!("prog_move_srcs = {:?}", self.prog_move_srcs); - log::trace!("prog_move_dsts = {:?}", self.prog_move_dsts); - - self.stats.initial_liverange_count = self.ranges.len(); - self.stats.blockparam_ins_count = self.blockparam_ins.len(); - self.stats.blockparam_outs_count = self.blockparam_outs.len(); - - Ok(()) } } diff --git a/src/ion/mod.rs b/src/ion/mod.rs index aed56dd..5903f58 100644 --- a/src/ion/mod.rs +++ b/src/ion/mod.rs @@ -92,6 +92,8 @@ impl<'a, F: Function> Env<'a, F> { pub(crate) fn init(&mut self) -> Result<(), RegAllocError> { self.create_pregs_and_vregs(); self.compute_liveness()?; + self.build_liveranges(); + self.fixup_multi_fixed_vregs(); self.merge_vreg_bundles(); self.queue_bundles(); if log::log_enabled!(log::Level::Trace) { From 8f435243e0506f033ad8eb81ad8b23eea2d4242d Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Sun, 28 Nov 2021 17:52:50 +0000 Subject: [PATCH 4/7] Properly handle fixed stack slots during multi-fixed-reg fixup --- src/ion/data_structures.rs | 4 +- src/ion/liveranges.rs | 159 ++++++++++++++++---------- src/ion/moves.rs | 16 +-- src/lib.rs | 1 + src/util.rs | 224 +++++++++++++++++++++++++++++++++++++ 5 files changed, 335 insertions(+), 69 deletions(-) create mode 100644 src/util.rs diff --git a/src/ion/data_structures.rs b/src/ion/data_structures.rs index 526a57b..23b6594 100644 --- a/src/ion/data_structures.rs +++ b/src/ion/data_structures.rs @@ -331,8 +331,8 @@ pub struct Env<'a, F: Function> { // will insert a copy from wherever the VReg's primary allocation // was to the approprate PReg. // - // (progpoint, copy-from-preg, copy-to-preg, to-slot) - pub multi_fixed_reg_fixups: Vec<(ProgPoint, PRegIndex, PRegIndex, VRegIndex, usize)>, + // (progpoint, from-slot, copy-to-preg, vreg, to-slot) + pub multi_fixed_reg_fixups: Vec<(ProgPoint, u8, PRegIndex, VRegIndex, u8)>, pub inserted_moves: Vec, diff --git a/src/ion/liveranges.rs b/src/ion/liveranges.rs index a8e3561..b73f13f 100644 --- a/src/ion/liveranges.rs +++ b/src/ion/liveranges.rs @@ -18,6 +18,7 @@ use super::{ SpillSetIndex, Use, VRegData, VRegIndex, SLOT_NONE, }; use crate::indexset::IndexSet; +use crate::util::SliceGroupBy; use crate::{ Allocation, Block, Function, Inst, InstPosition, Operand, OperandConstraint, OperandKind, OperandPos, PReg, ProgPoint, RegAllocError, VReg, @@ -1169,8 +1170,6 @@ impl<'a, F: Function> Env<'a, F> { // have to split the multiple uses at the same progpoint into // different bundles, which breaks invariants related to // disjoint ranges and bundles). - let mut seen_fixed_for_vreg: SmallVec<[VReg; 16]> = smallvec![]; - let mut first_preg: SmallVec<[PRegIndex; 16]> = smallvec![]; let mut extra_clobbers: SmallVec<[(PReg, Inst); 8]> = smallvec![]; for vreg in 0..self.vregs.len() { for range_idx in 0..self.vregs[vreg].ranges.len() { @@ -1181,67 +1180,109 @@ impl<'a, F: Function> Env<'a, F> { VRegIndex::new(vreg), range, ); - let mut last_point = None; - let mut fixup_multi_fixed_vregs = |pos: ProgPoint, - slot: usize, - op: &mut Operand, - fixups: &mut Vec<( - ProgPoint, - PRegIndex, - PRegIndex, - VRegIndex, - usize, - )>| { - if last_point.is_some() && Some(pos) != last_point { - seen_fixed_for_vreg.clear(); - first_preg.clear(); - } - last_point = Some(pos); - if let OperandConstraint::FixedReg(preg) = op.constraint() { - let vreg_idx = VRegIndex::new(op.vreg().vreg()); - let preg_idx = PRegIndex::new(preg.index()); - log::trace!( - "at pos {:?}, vreg {:?} has fixed constraint to preg {:?}", - pos, - vreg_idx, - preg_idx - ); - if let Some(idx) = seen_fixed_for_vreg.iter().position(|r| *r == op.vreg()) - { - let orig_preg = first_preg[idx]; - if orig_preg != preg_idx { - log::trace!(" -> duplicate; switching to constraint Reg"); - fixups.push((pos, orig_preg, preg_idx, vreg_idx, slot)); - *op = Operand::new( - op.vreg(), - OperandConstraint::Reg, - op.kind(), - op.pos(), - ); - log::trace!( - " -> extra clobber {} at inst{}", - preg, - pos.inst().index() - ); - extra_clobbers.push((preg, pos.inst())); + // Find groups of uses that occur in at the same program point. + for uses in self.ranges[range.index()] + .uses + .group_by_mut_(|a, b| a.pos == b.pos) + { + if uses.len() < 2 { + continue; + } + + // Search for conflicting constraints in the uses. + let mut requires_reg = false; + let mut num_fixed_reg = 0; + let mut num_fixed_stack = 0; + let mut first_reg_slot = None; + let mut first_stack_slot = None; + for u in uses.iter() { + match u.operand.constraint() { + OperandConstraint::Any => { + first_reg_slot.get_or_insert(u.slot); + first_stack_slot.get_or_insert(u.slot); } - } else { - seen_fixed_for_vreg.push(op.vreg()); - first_preg.push(preg_idx); + OperandConstraint::Reg | OperandConstraint::Reuse(_) => { + first_reg_slot.get_or_insert(u.slot); + requires_reg = true; + } + OperandConstraint::FixedReg(preg) => { + if self.pregs[preg.index()].is_stack { + num_fixed_stack += 1; + first_stack_slot.get_or_insert(u.slot); + } else { + requires_reg = true; + num_fixed_reg += 1; + first_reg_slot.get_or_insert(u.slot); + } + } + // Maybe this could be supported in this future... + OperandConstraint::Stack => panic!( + "multiple uses of vreg with a Stack constraint are not supported" + ), } } - }; - for u in &mut self.ranges[range.index()].uses { - let pos = u.pos; - let slot = u.slot as usize; - fixup_multi_fixed_vregs( - pos, - slot, - &mut u.operand, - &mut self.multi_fixed_reg_fixups, - ); + // Fast path if there are no conflicts. + if num_fixed_reg + num_fixed_stack <= 1 + && !(requires_reg && num_fixed_stack != 0) + { + continue; + } + + // We pick one constraint (in order: FixedReg, Reg, FixedStack) + // and then rewrite any incompatible constraints to Any. + // This allows register allocation to succeed and we will + // later insert moves to satisfy the rewritten constraints. + let source_slot = if requires_reg { + first_reg_slot.unwrap() + } else { + first_stack_slot.unwrap() + }; + let mut first_preg = None; + for u in uses.iter_mut() { + if let OperandConstraint::FixedReg(preg) = u.operand.constraint() { + let vreg_idx = VRegIndex::new(u.operand.vreg().vreg()); + let preg_idx = PRegIndex::new(preg.index()); + log::trace!( + "at pos {:?}, vreg {:?} has fixed constraint to preg {:?}", + u.pos, + vreg_idx, + preg_idx + ); + + // FixedStack is incompatible if there are any + // Reg/FixedReg constraints. FixedReg is + // incompatible if there already is a different + // FixedReg constraint. + if !(requires_reg && self.pregs[preg.index()].is_stack) + && *first_preg.get_or_insert(preg) == preg + { + continue; + } + + log::trace!(" -> duplicate; switching to constraint Reg"); + self.multi_fixed_reg_fixups.push(( + u.pos, + source_slot, + preg_idx, + vreg_idx, + u.slot, + )); + u.operand = Operand::new( + u.operand.vreg(), + OperandConstraint::Any, + u.operand.kind(), + u.operand.pos(), + ); + log::trace!( + " -> extra clobber {} at inst{}", + preg, + u.pos.inst().index() + ); + extra_clobbers.push((preg, u.pos.inst())); + } + } } for &(clobber, inst) in &extra_clobbers { @@ -1253,8 +1294,6 @@ impl<'a, F: Function> Env<'a, F> { } extra_clobbers.clear(); - first_preg.clear(); - seen_fixed_for_vreg.clear(); } } } diff --git a/src/ion/moves.rs b/src/ion/moves.rs index c7b8489..ea27827 100644 --- a/src/ion/moves.rs +++ b/src/ion/moves.rs @@ -695,26 +695,28 @@ impl<'a, F: Function> Env<'a, F> { } // Handle multi-fixed-reg constraints by copying. - for (progpoint, from_preg, to_preg, to_vreg, slot) in + for (progpoint, from_slot, to_preg, to_vreg, to_slot) in std::mem::replace(&mut self.multi_fixed_reg_fixups, vec![]) { + let from_alloc = self.get_alloc(progpoint.inst(), from_slot as usize); + let to_alloc = Allocation::reg(self.pregs[to_preg.index()].reg); log::trace!( - "multi-fixed-move constraint at {:?} from p{} to p{} for v{}", + "multi-fixed-move constraint at {:?} from {} to {} for v{}", progpoint, - from_preg.index(), - to_preg.index(), + from_alloc, + to_alloc, to_vreg.index(), ); self.insert_move( progpoint, InsertMovePrio::MultiFixedReg, - Allocation::reg(self.pregs[from_preg.index()].reg), - Allocation::reg(self.pregs[to_preg.index()].reg), + from_alloc, + to_alloc, Some(self.vreg_regs[to_vreg.index()]), ); self.set_alloc( progpoint.inst(), - slot, + to_slot as usize, Allocation::reg(self.pregs[to_preg.index()].reg), ); } diff --git a/src/lib.rs b/src/lib.rs index 8f21599..63a4393 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,6 +19,7 @@ pub(crate) mod ion; pub(crate) mod moves; pub(crate) mod postorder; pub(crate) mod ssa; +pub(crate) mod util; #[macro_use] mod index; diff --git a/src/util.rs b/src/util.rs new file mode 100644 index 0000000..55c4a21 --- /dev/null +++ b/src/util.rs @@ -0,0 +1,224 @@ +// This file contains a port of the unstable GroupBy slice iterator from the +// Rust standard library. The methods have a trailing underscore to avoid +// naming conflicts with the standard library. + +use std::fmt; +use std::iter::FusedIterator; +use std::mem; + +pub trait SliceGroupBy { + /// Returns an iterator over the slice producing non-overlapping runs + /// of elements using the predicate to separate them. + /// + /// The predicate is called on two elements following themselves, + /// it means the predicate is called on `slice[0]` and `slice[1]` + /// then on `slice[1]` and `slice[2]` and so on. + fn group_by_(&self, pred: F) -> GroupBy<'_, T, F> + where + F: FnMut(&T, &T) -> bool; + + /// Returns an iterator over the slice producing non-overlapping mutable + /// runs of elements using the predicate to separate them. + /// + /// The predicate is called on two elements following themselves, + /// it means the predicate is called on `slice[0]` and `slice[1]` + /// then on `slice[1]` and `slice[2]` and so on. + fn group_by_mut_(&mut self, pred: F) -> GroupByMut<'_, T, F> + where + F: FnMut(&T, &T) -> bool; +} + +impl SliceGroupBy for [T] { + fn group_by_(&self, pred: F) -> GroupBy<'_, T, F> + where + F: FnMut(&T, &T) -> bool, + { + GroupBy::new(self, pred) + } + + fn group_by_mut_(&mut self, pred: F) -> GroupByMut<'_, T, F> + where + F: FnMut(&T, &T) -> bool, + { + GroupByMut::new(self, pred) + } +} + +/// An iterator over slice in (non-overlapping) chunks separated by a predicate. +pub struct GroupBy<'a, T: 'a, P> { + slice: &'a [T], + predicate: P, +} + +impl<'a, T: 'a, P> GroupBy<'a, T, P> { + pub(super) fn new(slice: &'a [T], predicate: P) -> Self { + GroupBy { slice, predicate } + } +} + +impl<'a, T: 'a, P> Iterator for GroupBy<'a, T, P> +where + P: FnMut(&T, &T) -> bool, +{ + type Item = &'a [T]; + + #[inline] + fn next(&mut self) -> Option { + if self.slice.is_empty() { + None + } else { + let mut len = 1; + let mut iter = self.slice.windows(2); + while let Some([l, r]) = iter.next() { + if (self.predicate)(l, r) { + len += 1 + } else { + break; + } + } + let (head, tail) = self.slice.split_at(len); + self.slice = tail; + Some(head) + } + } + + #[inline] + fn size_hint(&self) -> (usize, Option) { + if self.slice.is_empty() { + (0, Some(0)) + } else { + (1, Some(self.slice.len())) + } + } + + #[inline] + fn last(mut self) -> Option { + self.next_back() + } +} + +impl<'a, T: 'a, P> DoubleEndedIterator for GroupBy<'a, T, P> +where + P: FnMut(&T, &T) -> bool, +{ + #[inline] + fn next_back(&mut self) -> Option { + if self.slice.is_empty() { + None + } else { + let mut len = 1; + let mut iter = self.slice.windows(2); + while let Some([l, r]) = iter.next_back() { + if (self.predicate)(l, r) { + len += 1 + } else { + break; + } + } + let (head, tail) = self.slice.split_at(self.slice.len() - len); + self.slice = head; + Some(tail) + } + } +} + +impl<'a, T: 'a, P> FusedIterator for GroupBy<'a, T, P> where P: FnMut(&T, &T) -> bool {} + +impl<'a, T: 'a + fmt::Debug, P> fmt::Debug for GroupBy<'a, T, P> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("GroupBy") + .field("slice", &self.slice) + .finish() + } +} + +/// An iterator over slice in (non-overlapping) mutable chunks separated +/// by a predicate. +pub struct GroupByMut<'a, T: 'a, P> { + slice: &'a mut [T], + predicate: P, +} + +impl<'a, T: 'a, P> GroupByMut<'a, T, P> { + pub(super) fn new(slice: &'a mut [T], predicate: P) -> Self { + GroupByMut { slice, predicate } + } +} + +impl<'a, T: 'a, P> Iterator for GroupByMut<'a, T, P> +where + P: FnMut(&T, &T) -> bool, +{ + type Item = &'a mut [T]; + + #[inline] + fn next(&mut self) -> Option { + if self.slice.is_empty() { + None + } else { + let mut len = 1; + let mut iter = self.slice.windows(2); + while let Some([l, r]) = iter.next() { + if (self.predicate)(l, r) { + len += 1 + } else { + break; + } + } + let slice = mem::take(&mut self.slice); + let (head, tail) = slice.split_at_mut(len); + self.slice = tail; + Some(head) + } + } + + #[inline] + fn size_hint(&self) -> (usize, Option) { + if self.slice.is_empty() { + (0, Some(0)) + } else { + (1, Some(self.slice.len())) + } + } + + #[inline] + fn last(mut self) -> Option { + self.next_back() + } +} + +impl<'a, T: 'a, P> DoubleEndedIterator for GroupByMut<'a, T, P> +where + P: FnMut(&T, &T) -> bool, +{ + #[inline] + fn next_back(&mut self) -> Option { + if self.slice.is_empty() { + None + } else { + let mut len = 1; + let mut iter = self.slice.windows(2); + while let Some([l, r]) = iter.next_back() { + if (self.predicate)(l, r) { + len += 1 + } else { + break; + } + } + let slice = mem::take(&mut self.slice); + let (head, tail) = slice.split_at_mut(slice.len() - len); + self.slice = head; + Some(tail) + } + } +} + +impl<'a, T: 'a, P> FusedIterator for GroupByMut<'a, T, P> where P: FnMut(&T, &T) -> bool {} + +impl<'a, T: 'a + fmt::Debug, P> fmt::Debug for GroupByMut<'a, T, P> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("GroupByMut") + .field("slice", &self.slice) + .finish() + } +} From 870e4729e145fa3831f8d38a5cfb4c93d2555862 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Sun, 28 Nov 2021 17:52:50 +0000 Subject: [PATCH 5/7] Add fixed stack slots to the fuzzer --- src/checker.rs | 14 ++++++++++++-- src/fuzzing/func.rs | 17 ++++++++++------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/checker.rs b/src/checker.rs index 55f4860..d66610a 100644 --- a/src/checker.rs +++ b/src/checker.rs @@ -426,12 +426,22 @@ impl CheckerState { match op.constraint() { OperandConstraint::Any => {} OperandConstraint::Reg => { - if alloc.kind() != AllocationKind::Reg { - return Err(CheckerError::AllocationIsNotReg { inst, op, alloc }); + // Reject pregs that represent a fixed stack slot. + if let Some(preg) = alloc.as_reg() { + if preg.class() == RegClass::Int && (0..32).contains(&preg.hw_enc()) { + return Ok(()); + } } + return Err(CheckerError::AllocationIsNotReg { inst, op, alloc }); } OperandConstraint::Stack => { if alloc.kind() != AllocationKind::Stack { + // Accept pregs that represent a fixed stack slot. + if let Some(preg) = alloc.as_reg() { + if preg.class() == RegClass::Int && (32..63).contains(&preg.hw_enc()) { + return Ok(()); + } + } return Err(CheckerError::AllocationIsNotStack { inst, op, alloc }); } } diff --git a/src/fuzzing/func.rs b/src/fuzzing/func.rs index e5174cb..cbc9717 100644 --- a/src/fuzzing/func.rs +++ b/src/fuzzing/func.rs @@ -465,7 +465,7 @@ impl Func { let mut fixed = vec![]; for _ in 0..u.int_in_range(0..=operands.len() - 1)? { // Pick an operand and make it a fixed reg. - let fixed_reg = PReg::new(u.int_in_range(0..=30)?, RegClass::Int); + let fixed_reg = PReg::new(u.int_in_range(0..=62)?, RegClass::Int); if fixed.contains(&fixed_reg) { break; } @@ -602,15 +602,18 @@ impl std::fmt::Debug for Func { } pub fn machine_env() -> MachineEnv { - // Reg 31 is the scratch reg. - let regs: Vec = (0..31).map(|i| PReg::new(i, RegClass::Int)).collect(); - let preferred_regs_by_class: [Vec; 2] = [regs.iter().cloned().take(24).collect(), vec![]]; - let non_preferred_regs_by_class: [Vec; 2] = - [regs.iter().cloned().skip(24).collect(), vec![]]; - let scratch_by_class: [PReg; 2] = [PReg::new(31, RegClass::Int), PReg::new(0, RegClass::Float)]; + // Reg 63 is the scratch reg. + fn regs(r: std::ops::Range) -> Vec { + r.map(|i| PReg::new(i, RegClass::Int)).collect() + } + let preferred_regs_by_class: [Vec; 2] = [regs(0..24), vec![]]; + let non_preferred_regs_by_class: [Vec; 2] = [regs(24..32), vec![]]; + let scratch_by_class: [PReg; 2] = [PReg::new(63, RegClass::Int), PReg::new(0, RegClass::Float)]; + let fixed_stack_slots = regs(32..63); MachineEnv { preferred_regs_by_class, non_preferred_regs_by_class, scratch_by_class, + fixed_stack_slots, } } From 38ffc479c2ca4c621945ce02e3d4e0b0543b7a4b Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Sun, 28 Nov 2021 18:49:47 +0000 Subject: [PATCH 6/7] Simplify the internal representation of PReg --- src/ion/liveranges.rs | 2 +- src/lib.rs | 30 +++++++++++++----------------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/ion/liveranges.rs b/src/ion/liveranges.rs index b73f13f..3d18326 100644 --- a/src/ion/liveranges.rs +++ b/src/ion/liveranges.rs @@ -99,7 +99,7 @@ impl<'a, F: Function> Env<'a, F> { pub fn create_pregs_and_vregs(&mut self) { // Create PRegs from the env. self.pregs.resize( - PReg::MAX_INDEX, + PReg::NUM_INDEX, PRegData { reg: PReg::invalid(), allocations: LiveRangeSet::new(), diff --git a/src/lib.rs b/src/lib.rs index 63a4393..f3c695f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -65,14 +65,13 @@ pub enum RegClass { /// integer registers and indices 64..=127 are the 64 float registers. #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct PReg { - hw_enc: u8, - class: RegClass, + bits: u8, } impl PReg { pub const MAX_BITS: usize = 6; pub const MAX: usize = (1 << Self::MAX_BITS) - 1; - pub const MAX_INDEX: usize = 1 << (Self::MAX_BITS + 1); // including RegClass bit + pub const NUM_INDEX: usize = 1 << (Self::MAX_BITS + 1); // including RegClass bit /// Create a new PReg. The `hw_enc` range is 6 bits. #[inline(always)] @@ -86,22 +85,24 @@ impl PReg { let _ = HW_ENC_MUST_BE_IN_BOUNDS[hw_enc]; PReg { - hw_enc: hw_enc as u8, - class, + bits: ((class as u8) << Self::MAX_BITS) | (hw_enc as u8), } } /// The physical register number, as encoded by the ISA for the particular register class. #[inline(always)] pub fn hw_enc(self) -> usize { - let hw_enc = self.hw_enc as usize; - hw_enc + self.bits as usize & Self::MAX } /// The register class. #[inline(always)] pub fn class(self) -> RegClass { - self.class + if self.bits & (1 << Self::MAX_BITS) == 0 { + RegClass::Int + } else { + RegClass::Float + } } /// Get an index into the (not necessarily contiguous) index space of @@ -109,20 +110,15 @@ impl PReg { /// all PRegs and index it efficiently. #[inline(always)] pub fn index(self) -> usize { - ((self.class as u8 as usize) << Self::MAX_BITS) | (self.hw_enc as usize) + self.bits as usize } /// Construct a PReg from the value returned from `.index()`. #[inline(always)] pub fn from_index(index: usize) -> Self { - let class = (index >> Self::MAX_BITS) & 1; - let class = match class { - 0 => RegClass::Int, - 1 => RegClass::Float, - _ => unreachable!(), - }; - let index = index & Self::MAX; - PReg::new(index, class) + PReg { + bits: (index & (Self::NUM_INDEX - 1)) as u8, + } } /// Return the "invalid PReg", which can be used to initialize From 51493ab03ad313b5579b8ecec6152b527d9127be Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Sun, 12 Dec 2021 00:33:30 +0000 Subject: [PATCH 7/7] Apply review feedback --- Cargo.toml | 1 + src/ion/data_structures.rs | 13 ++- src/ion/liveranges.rs | 25 +++-- src/ion/merge.rs | 10 +- src/ion/moves.rs | 22 ++-- src/ion/process.rs | 51 ++++----- src/ion/requirement.rs | 63 +++++++---- src/lib.rs | 1 - src/util.rs | 224 ------------------------------------- 9 files changed, 106 insertions(+), 304 deletions(-) delete mode 100644 src/util.rs diff --git a/Cargo.toml b/Cargo.toml index 6f7f309..5410cf1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,7 @@ repository = "https://github.com/bytecodealliance/regalloc2" log = { version = "0.4.8", default-features = false } smallvec = "1.6.1" fxhash = "0.2.1" +slice-group-by = "0.3.0" # The below are only needed for fuzzing. # Keep this in sync with libfuzzer_sys's crate version: diff --git a/src/ion/data_structures.rs b/src/ion/data_structures.rs index 23b6594..09f7834 100644 --- a/src/ion/data_structures.rs +++ b/src/ion/data_structures.rs @@ -269,6 +269,15 @@ pub struct PRegData { pub is_stack: bool, } +#[derive(Clone, Debug)] +pub struct MultiFixedRegFixup { + pub pos: ProgPoint, + pub from_slot: u8, + pub to_slot: u8, + pub to_preg: PRegIndex, + pub vreg: VRegIndex, +} + #[derive(Clone, Debug)] pub struct Env<'a, F: Function> { pub func: &'a F, @@ -330,9 +339,7 @@ pub struct Env<'a, F: Function> { // the register available. When we produce the final edit-list, we // will insert a copy from wherever the VReg's primary allocation // was to the approprate PReg. - // - // (progpoint, from-slot, copy-to-preg, vreg, to-slot) - pub multi_fixed_reg_fixups: Vec<(ProgPoint, u8, PRegIndex, VRegIndex, u8)>, + pub multi_fixed_reg_fixups: Vec, pub inserted_moves: Vec, diff --git a/src/ion/liveranges.rs b/src/ion/liveranges.rs index 3d18326..404b438 100644 --- a/src/ion/liveranges.rs +++ b/src/ion/liveranges.rs @@ -18,12 +18,13 @@ use super::{ SpillSetIndex, Use, VRegData, VRegIndex, SLOT_NONE, }; use crate::indexset::IndexSet; -use crate::util::SliceGroupBy; +use crate::ion::data_structures::MultiFixedRegFixup; use crate::{ Allocation, Block, Function, Inst, InstPosition, Operand, OperandConstraint, OperandKind, OperandPos, PReg, ProgPoint, RegAllocError, VReg, }; use fxhash::FxHashSet; +use slice_group_by::GroupByMut; use smallvec::{smallvec, SmallVec}; use std::collections::{HashSet, VecDeque}; @@ -1184,7 +1185,7 @@ impl<'a, F: Function> Env<'a, F> { // Find groups of uses that occur in at the same program point. for uses in self.ranges[range.index()] .uses - .group_by_mut_(|a, b| a.pos == b.pos) + .linear_group_by_key_mut(|u| u.pos) { if uses.len() < 2 { continue; @@ -1254,21 +1255,23 @@ impl<'a, F: Function> Env<'a, F> { // FixedStack is incompatible if there are any // Reg/FixedReg constraints. FixedReg is // incompatible if there already is a different - // FixedReg constraint. + // FixedReg constraint. If either condition is true, + // we edit the constraint below; otherwise, we can + // skip this edit. if !(requires_reg && self.pregs[preg.index()].is_stack) && *first_preg.get_or_insert(preg) == preg { continue; } - log::trace!(" -> duplicate; switching to constraint Reg"); - self.multi_fixed_reg_fixups.push(( - u.pos, - source_slot, - preg_idx, - vreg_idx, - u.slot, - )); + log::trace!(" -> duplicate; switching to constraint Any"); + self.multi_fixed_reg_fixups.push(MultiFixedRegFixup { + pos: u.pos, + from_slot: source_slot, + to_slot: u.slot, + to_preg: preg_idx, + vreg: vreg_idx, + }); u.operand = Operand::new( u.operand.vreg(), OperandConstraint::Any, diff --git a/src/ion/merge.rs b/src/ion/merge.rs index b32a1b0..a8e1fe5 100644 --- a/src/ion/merge.rs +++ b/src/ion/merge.rs @@ -13,8 +13,8 @@ //! Bundle merging. use super::{ - Env, LiveBundleIndex, LiveRangeIndex, LiveRangeKey, Requirement, SpillSet, SpillSetIndex, - SpillSlotIndex, VRegIndex, + Env, LiveBundleIndex, LiveRangeIndex, LiveRangeKey, SpillSet, SpillSetIndex, SpillSlotIndex, + VRegIndex, }; use crate::{Function, Inst, OperandConstraint, PReg}; use smallvec::smallvec; @@ -99,11 +99,7 @@ impl<'a, F: Function> Env<'a, F> { || self.bundles[to.index()].cached_stack() || self.bundles[to.index()].cached_fixed() { - let req = self - .compute_requirement(from) - .0 - .merge(self.compute_requirement(to).0); - if req == Requirement::Conflict { + if self.merge_bundle_requirements(from, to).is_err() { log::trace!(" -> conflicting requirements; aborting merge"); return false; } diff --git a/src/ion/moves.rs b/src/ion/moves.rs index ea27827..9f913c4 100644 --- a/src/ion/moves.rs +++ b/src/ion/moves.rs @@ -695,29 +695,27 @@ impl<'a, F: Function> Env<'a, F> { } // Handle multi-fixed-reg constraints by copying. - for (progpoint, from_slot, to_preg, to_vreg, to_slot) in - std::mem::replace(&mut self.multi_fixed_reg_fixups, vec![]) - { - let from_alloc = self.get_alloc(progpoint.inst(), from_slot as usize); - let to_alloc = Allocation::reg(self.pregs[to_preg.index()].reg); + for fixup in std::mem::replace(&mut self.multi_fixed_reg_fixups, vec![]) { + let from_alloc = self.get_alloc(fixup.pos.inst(), fixup.from_slot as usize); + let to_alloc = Allocation::reg(self.pregs[fixup.to_preg.index()].reg); log::trace!( "multi-fixed-move constraint at {:?} from {} to {} for v{}", - progpoint, + fixup.pos, from_alloc, to_alloc, - to_vreg.index(), + fixup.vreg.index(), ); self.insert_move( - progpoint, + fixup.pos, InsertMovePrio::MultiFixedReg, from_alloc, to_alloc, - Some(self.vreg_regs[to_vreg.index()]), + Some(self.vreg_regs[fixup.vreg.index()]), ); self.set_alloc( - progpoint.inst(), - to_slot as usize, - Allocation::reg(self.pregs[to_preg.index()].reg), + fixup.pos.inst(), + fixup.to_slot as usize, + Allocation::reg(self.pregs[fixup.to_preg.index()].reg), ); } diff --git a/src/ion/process.rs b/src/ion/process.rs index 0afe894..c45bc5b 100644 --- a/src/ion/process.rs +++ b/src/ion/process.rs @@ -18,9 +18,12 @@ use super::{ Requirement, SpillWeight, UseList, }; use crate::{ - ion::data_structures::{ - CodeRange, BUNDLE_MAX_NORMAL_SPILL_WEIGHT, MINIMAL_BUNDLE_SPILL_WEIGHT, - MINIMAL_FIXED_BUNDLE_SPILL_WEIGHT, + ion::{ + data_structures::{ + CodeRange, BUNDLE_MAX_NORMAL_SPILL_WEIGHT, MINIMAL_BUNDLE_SPILL_WEIGHT, + MINIMAL_FIXED_BUNDLE_SPILL_WEIGHT, + }, + requirement::RequirementConflictAt, }, Allocation, Function, Inst, InstPosition, OperandConstraint, OperandKind, PReg, ProgPoint, RegAllocError, @@ -729,7 +732,6 @@ impl<'a, F: Function> Env<'a, F> { reg_hint: PReg, ) -> Result<(), RegAllocError> { let class = self.spillsets[self.bundles[bundle.index()].spillset.index()].class; - let (req, split_point) = self.compute_requirement(bundle); // Grab a hint from either the queue or our spillset, if any. let mut hint_reg = if reg_hint != PReg::invalid() { reg_hint @@ -741,27 +743,30 @@ impl<'a, F: Function> Env<'a, F> { } log::trace!("process_bundle: bundle {:?} hint {:?}", bundle, hint_reg,); - if let Requirement::Conflict = req { - // We have to split right away. We'll find a point to - // split that would allow at least the first half of the - // split to be conflict-free. - assert!( - !self.minimal_bundle(bundle), - "Minimal bundle with conflict!" - ); - self.split_and_requeue_bundle( - bundle, - /* split_at_point = */ split_point, - reg_hint, - ); - return Ok(()); - } + let req = match self.compute_requirement(bundle) { + Ok(req) => req, + Err(RequirementConflictAt(split_point)) => { + // We have to split right away. We'll find a point to + // split that would allow at least the first half of the + // split to be conflict-free. + assert!( + !self.minimal_bundle(bundle), + "Minimal bundle with conflict!" + ); + self.split_and_requeue_bundle( + bundle, + /* split_at_point = */ split_point, + reg_hint, + ); + return Ok(()); + } + }; // If no requirement at all (because no uses), and *if* a // spill bundle is already present, then move the LRs over to // the spill bundle right away. match req { - Requirement::Unknown | Requirement::Any => { + Requirement::Any => { if let Some(spill) = self.get_or_create_spill_bundle(bundle, /* create_if_absent = */ false) { @@ -794,14 +799,10 @@ impl<'a, F: Function> Env<'a, F> { return Ok(()); } - Requirement::Any | Requirement::Unknown => { + Requirement::Any => { self.spilled_bundles.push(bundle); return Ok(()); } - - Requirement::Conflict => { - unreachable!() - } }; // Scan all pregs, or the one fixed preg, and attempt to allocate. diff --git a/src/ion/requirement.rs b/src/ion/requirement.rs index 6e7920b..c14cc38 100644 --- a/src/ion/requirement.rs +++ b/src/ion/requirement.rs @@ -15,32 +15,36 @@ use super::{Env, LiveBundleIndex}; use crate::{Function, Operand, OperandConstraint, PReg, ProgPoint}; +pub struct RequirementConflict; + +pub struct RequirementConflictAt(pub ProgPoint); + #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Requirement { - Unknown, FixedReg(PReg), FixedStack(PReg), Register, Stack, Any, - Conflict, } impl Requirement { #[inline(always)] - pub fn merge(self, other: Requirement) -> Requirement { + pub fn merge(self, other: Requirement) -> Result { match (self, other) { - (Requirement::Unknown, other) | (other, Requirement::Unknown) => other, - (Requirement::Conflict, _) | (_, Requirement::Conflict) => Requirement::Conflict, - (other, Requirement::Any) | (Requirement::Any, other) => other, - (Requirement::Register, Requirement::Register) => self, - (Requirement::Stack, Requirement::Stack) => self, + (other, Requirement::Any) | (Requirement::Any, other) => Ok(other), + (Requirement::Register, Requirement::Register) => Ok(self), + (Requirement::Stack, Requirement::Stack) => Ok(self), (Requirement::Register, Requirement::FixedReg(preg)) - | (Requirement::FixedReg(preg), Requirement::Register) => Requirement::FixedReg(preg), + | (Requirement::FixedReg(preg), Requirement::Register) => { + Ok(Requirement::FixedReg(preg)) + } (Requirement::Stack, Requirement::FixedStack(preg)) - | (Requirement::FixedStack(preg), Requirement::Stack) => Requirement::FixedStack(preg), - (Requirement::FixedReg(a), Requirement::FixedReg(b)) if a == b => self, - (Requirement::FixedStack(a), Requirement::FixedStack(b)) if a == b => self, - _ => Requirement::Conflict, + | (Requirement::FixedStack(preg), Requirement::Stack) => { + Ok(Requirement::FixedStack(preg)) + } + (Requirement::FixedReg(a), Requirement::FixedReg(b)) if a == b => Ok(self), + (Requirement::FixedStack(a), Requirement::FixedStack(b)) if a == b => Ok(self), + _ => Err(RequirementConflict), } } } @@ -58,12 +62,15 @@ impl<'a, F: Function> Env<'a, F> { } OperandConstraint::Reg | OperandConstraint::Reuse(_) => Requirement::Register, OperandConstraint::Stack => Requirement::Stack, - _ => Requirement::Any, + OperandConstraint::Any => Requirement::Any, } } - pub fn compute_requirement(&self, bundle: LiveBundleIndex) -> (Requirement, ProgPoint) { - let mut req = Requirement::Unknown; + pub fn compute_requirement( + &self, + bundle: LiveBundleIndex, + ) -> Result { + let mut req = Requirement::Any; log::trace!("compute_requirement: {:?}", bundle); let ranges = &self.bundles[bundle.index()].ranges; for entry in ranges { @@ -71,14 +78,28 @@ impl<'a, F: Function> Env<'a, F> { for u in &self.ranges[entry.index.index()].uses { log::trace!(" -> use {:?}", u); let r = self.requirement_from_operand(u.operand); - req = req.merge(r); + req = req.merge(r).map_err(|_| { + log::trace!(" -> conflict"); + RequirementConflictAt(u.pos) + })?; log::trace!(" -> req {:?}", req); - if req == Requirement::Conflict { - return (req, u.pos); - } } } log::trace!(" -> final: {:?}", req); - (req, ranges.first().unwrap().range.from) + Ok(req) + } + + pub fn merge_bundle_requirements( + &self, + a: LiveBundleIndex, + b: LiveBundleIndex, + ) -> Result { + let req_a = self + .compute_requirement(a) + .map_err(|_| RequirementConflict)?; + let req_b = self + .compute_requirement(b) + .map_err(|_| RequirementConflict)?; + req_a.merge(req_b) } } diff --git a/src/lib.rs b/src/lib.rs index f3c695f..36b8042 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,7 +19,6 @@ pub(crate) mod ion; pub(crate) mod moves; pub(crate) mod postorder; pub(crate) mod ssa; -pub(crate) mod util; #[macro_use] mod index; diff --git a/src/util.rs b/src/util.rs deleted file mode 100644 index 55c4a21..0000000 --- a/src/util.rs +++ /dev/null @@ -1,224 +0,0 @@ -// This file contains a port of the unstable GroupBy slice iterator from the -// Rust standard library. The methods have a trailing underscore to avoid -// naming conflicts with the standard library. - -use std::fmt; -use std::iter::FusedIterator; -use std::mem; - -pub trait SliceGroupBy { - /// Returns an iterator over the slice producing non-overlapping runs - /// of elements using the predicate to separate them. - /// - /// The predicate is called on two elements following themselves, - /// it means the predicate is called on `slice[0]` and `slice[1]` - /// then on `slice[1]` and `slice[2]` and so on. - fn group_by_(&self, pred: F) -> GroupBy<'_, T, F> - where - F: FnMut(&T, &T) -> bool; - - /// Returns an iterator over the slice producing non-overlapping mutable - /// runs of elements using the predicate to separate them. - /// - /// The predicate is called on two elements following themselves, - /// it means the predicate is called on `slice[0]` and `slice[1]` - /// then on `slice[1]` and `slice[2]` and so on. - fn group_by_mut_(&mut self, pred: F) -> GroupByMut<'_, T, F> - where - F: FnMut(&T, &T) -> bool; -} - -impl SliceGroupBy for [T] { - fn group_by_(&self, pred: F) -> GroupBy<'_, T, F> - where - F: FnMut(&T, &T) -> bool, - { - GroupBy::new(self, pred) - } - - fn group_by_mut_(&mut self, pred: F) -> GroupByMut<'_, T, F> - where - F: FnMut(&T, &T) -> bool, - { - GroupByMut::new(self, pred) - } -} - -/// An iterator over slice in (non-overlapping) chunks separated by a predicate. -pub struct GroupBy<'a, T: 'a, P> { - slice: &'a [T], - predicate: P, -} - -impl<'a, T: 'a, P> GroupBy<'a, T, P> { - pub(super) fn new(slice: &'a [T], predicate: P) -> Self { - GroupBy { slice, predicate } - } -} - -impl<'a, T: 'a, P> Iterator for GroupBy<'a, T, P> -where - P: FnMut(&T, &T) -> bool, -{ - type Item = &'a [T]; - - #[inline] - fn next(&mut self) -> Option { - if self.slice.is_empty() { - None - } else { - let mut len = 1; - let mut iter = self.slice.windows(2); - while let Some([l, r]) = iter.next() { - if (self.predicate)(l, r) { - len += 1 - } else { - break; - } - } - let (head, tail) = self.slice.split_at(len); - self.slice = tail; - Some(head) - } - } - - #[inline] - fn size_hint(&self) -> (usize, Option) { - if self.slice.is_empty() { - (0, Some(0)) - } else { - (1, Some(self.slice.len())) - } - } - - #[inline] - fn last(mut self) -> Option { - self.next_back() - } -} - -impl<'a, T: 'a, P> DoubleEndedIterator for GroupBy<'a, T, P> -where - P: FnMut(&T, &T) -> bool, -{ - #[inline] - fn next_back(&mut self) -> Option { - if self.slice.is_empty() { - None - } else { - let mut len = 1; - let mut iter = self.slice.windows(2); - while let Some([l, r]) = iter.next_back() { - if (self.predicate)(l, r) { - len += 1 - } else { - break; - } - } - let (head, tail) = self.slice.split_at(self.slice.len() - len); - self.slice = head; - Some(tail) - } - } -} - -impl<'a, T: 'a, P> FusedIterator for GroupBy<'a, T, P> where P: FnMut(&T, &T) -> bool {} - -impl<'a, T: 'a + fmt::Debug, P> fmt::Debug for GroupBy<'a, T, P> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("GroupBy") - .field("slice", &self.slice) - .finish() - } -} - -/// An iterator over slice in (non-overlapping) mutable chunks separated -/// by a predicate. -pub struct GroupByMut<'a, T: 'a, P> { - slice: &'a mut [T], - predicate: P, -} - -impl<'a, T: 'a, P> GroupByMut<'a, T, P> { - pub(super) fn new(slice: &'a mut [T], predicate: P) -> Self { - GroupByMut { slice, predicate } - } -} - -impl<'a, T: 'a, P> Iterator for GroupByMut<'a, T, P> -where - P: FnMut(&T, &T) -> bool, -{ - type Item = &'a mut [T]; - - #[inline] - fn next(&mut self) -> Option { - if self.slice.is_empty() { - None - } else { - let mut len = 1; - let mut iter = self.slice.windows(2); - while let Some([l, r]) = iter.next() { - if (self.predicate)(l, r) { - len += 1 - } else { - break; - } - } - let slice = mem::take(&mut self.slice); - let (head, tail) = slice.split_at_mut(len); - self.slice = tail; - Some(head) - } - } - - #[inline] - fn size_hint(&self) -> (usize, Option) { - if self.slice.is_empty() { - (0, Some(0)) - } else { - (1, Some(self.slice.len())) - } - } - - #[inline] - fn last(mut self) -> Option { - self.next_back() - } -} - -impl<'a, T: 'a, P> DoubleEndedIterator for GroupByMut<'a, T, P> -where - P: FnMut(&T, &T) -> bool, -{ - #[inline] - fn next_back(&mut self) -> Option { - if self.slice.is_empty() { - None - } else { - let mut len = 1; - let mut iter = self.slice.windows(2); - while let Some([l, r]) = iter.next_back() { - if (self.predicate)(l, r) { - len += 1 - } else { - break; - } - } - let slice = mem::take(&mut self.slice); - let (head, tail) = slice.split_at_mut(slice.len() - len); - self.slice = head; - Some(tail) - } - } -} - -impl<'a, T: 'a, P> FusedIterator for GroupByMut<'a, T, P> where P: FnMut(&T, &T) -> bool {} - -impl<'a, T: 'a + fmt::Debug, P> fmt::Debug for GroupByMut<'a, T, P> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("GroupByMut") - .field("slice", &self.slice) - .finish() - } -}