Properly handle fixed stack slots during multi-fixed-reg fixup

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)
+    // (progpoint, from-slot, copy-to-preg, vreg, to-slot)
-    pub multi_fixed_reg_fixups: Vec<(ProgPoint, PRegIndex, PRegIndex, VRegIndex, usize)>,
+    pub multi_fixed_reg_fixups: Vec<(ProgPoint, u8, PRegIndex, VRegIndex, u8)>,
 
     pub inserted_moves: Vec<InsertedMove>,
 

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() {
+                // Find groups of uses that occur in at the same program point.
-                        let vreg_idx = VRegIndex::new(op.vreg().vreg());
+                for uses in self.ranges[range.index()]
-                        let preg_idx = PRegIndex::new(preg.index());
+                    .uses
-                        log::trace!(
+                    .group_by_mut_(|a, b| a.pos == b.pos)
-                            "at pos {:?}, vreg {:?} has fixed constraint to preg {:?}",
+                {
-                            pos,
+                    if uses.len() < 2 {
-                            vreg_idx,
+                        continue;
-                            preg_idx
+                    }
-                        );
+
-                        if let Some(idx) = seen_fixed_for_vreg.iter().position(|r| *r == op.vreg())
+                    // Search for conflicting constraints in the uses.
-                        {
+                    let mut requires_reg = false;
-                            let orig_preg = first_preg[idx];
+                    let mut num_fixed_reg = 0;
-                            if orig_preg != preg_idx {
+                    let mut num_fixed_stack = 0;
-                                log::trace!(" -> duplicate; switching to constraint Reg");
+                    let mut first_reg_slot = None;
-                                fixups.push((pos, orig_preg, preg_idx, vreg_idx, slot));
+                    let mut first_stack_slot = None;
-                                *op = Operand::new(
+                    for u in uses.iter() {
-                                    op.vreg(),
+                        match u.operand.constraint() {
-                                    OperandConstraint::Reg,
+                            OperandConstraint::Any => {
-                                    op.kind(),
+                                first_reg_slot.get_or_insert(u.slot);
-                                    op.pos(),
+                                first_stack_slot.get_or_insert(u.slot);
-                                );
-                                log::trace!(
-                                    " -> extra clobber {} at inst{}",
-                                    preg,
-                                    pos.inst().index()
-                                );
-                                extra_clobbers.push((preg, pos.inst()));
                             }
-                        } else {
+                            OperandConstraint::Reg | OperandConstraint::Reuse(_) => {
-                            seen_fixed_for_vreg.push(op.vreg());
+                                first_reg_slot.get_or_insert(u.slot);
-                            first_preg.push(preg_idx);
+                                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 {
+                    // Fast path if there are no conflicts.
-                    let pos = u.pos;
+                    if num_fixed_reg + num_fixed_stack <= 1
-                    let slot = u.slot as usize;
+                        && !(requires_reg && num_fixed_stack != 0)
-                    fixup_multi_fixed_vregs(
+                    {
-                        pos,
+                        continue;
-                        slot,
+                    }
-                        &mut u.operand,
+
-                        &mut self.multi_fixed_reg_fixups,
+                    // 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();
             }
         }
     }

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(),
+                from_alloc,
-                to_preg.index(),
+                to_alloc,
                 to_vreg.index(),
             );
             self.insert_move(
                 progpoint,
                 InsertMovePrio::MultiFixedReg,
-                Allocation::reg(self.pregs[from_preg.index()].reg),
+                from_alloc,
-                Allocation::reg(self.pregs[to_preg.index()].reg),
+                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),
             );
         }

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;

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<T> {
+    /// 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_<F>(&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_<F>(&mut self, pred: F) -> GroupByMut<'_, T, F>
+    where
+        F: FnMut(&T, &T) -> bool;
+}
+
+impl<T> SliceGroupBy<T> for [T] {
+    fn group_by_<F>(&self, pred: F) -> GroupBy<'_, T, F>
+    where
+        F: FnMut(&T, &T) -> bool,
+    {
+        GroupBy::new(self, pred)
+    }
+
+    fn group_by_mut_<F>(&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<Self::Item> {
+        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<usize>) {
+        if self.slice.is_empty() {
+            (0, Some(0))
+        } else {
+            (1, Some(self.slice.len()))
+        }
+    }
+
+    #[inline]
+    fn last(mut self) -> Option<Self::Item> {
+        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<Self::Item> {
+        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<Self::Item> {
+        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<usize>) {
+        if self.slice.is_empty() {
+            (0, Some(0))
+        } else {
+            (1, Some(self.slice.len()))
+        }
+    }
+
+    #[inline]
+    fn last(mut self) -> Option<Self::Item> {
+        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<Self::Item> {
+        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()
+    }
+}