Handle conflict-related liverange splits arising from stack constraints without falling back to spill bundle. (#49)

Currently, we unconditionally trim the ends of liveranges around a split
when we do a split, including splits due to conflicts in a
liverange/bundle's requirements (e.g., a liverange with both a register
and a stack use). These trimmed ends, if they exist, go to the spill
bundle, and the spill bundle may receive a register during second-chance
allocation or otherwise will receive a stack slot.

This was previously measured to reduce contention significantly, because
it reduces the sizes of liveranges that participate in the first-chance
competition for allocations. When a split has to occur, we might as well
relegate the "connecting pieces" to a process that comes later, with a
hint to try to get the right register if possible but no hard connection
to either end.

However, in the case of a split arising from a reg-to-stack /
stack-to-reg conflict, as happens when references are used or def'd as
registers and then cross safepoints, this extra step in the connectivity
(normal LR with register use, then spill bundle, then normal LR with
stack use) can lead to extra moves. Additionally, when one of the LRs
has a stack constraint, contention is far less important; so it doesn't
hurt to skip the trimming step. In fact, it's likely much better to put
the "connecting piece" together with the stack side of the conflict.

Ideally we would handle this with the same move-cost logic we use for
conflicts detected during backtracking, but the requirements-related
splitting happens separately and that logic would need to be generalized
further. For now, this is sufficient to eliminate redundant moves as
seen in e.g. bytecodealliance/wasmtime#3785.

src/ion/process.rs

@@ -18,13 +18,10 @@ use super::{
     Requirement, SpillWeight, UseList,
 };
 use crate::{
-    ion::{
+    ion::data_structures::{
-        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,
 };
@@ -395,6 +392,9 @@ impl<'a, F: Function> Env<'a, F> {
         bundle: LiveBundleIndex,
         mut split_at: ProgPoint,
         reg_hint: PReg,
+        // Do we trim the parts around the split and put them in the
+        // spill bundle?
+        trim_ends_into_spill_bundle: bool,
     ) {
         self.stats.splits += 1;
         trace!(
@@ -567,6 +567,7 @@ impl<'a, F: Function> Env<'a, F> {
         }
         self.bundles[new_bundle.index()].ranges = new_lr_list;
 
+        if trim_ends_into_spill_bundle {
             // Finally, handle moving LRs to the spill bundle when
             // appropriate: If the first range in `new_bundle` or last
             // range in `bundle` has "empty space" beyond the first or
@@ -711,6 +712,7 @@ impl<'a, F: Function> Env<'a, F> {
                 }
                 break;
             }
+        }
 
         if self.bundles[bundle.index()].ranges.len() > 0 {
             self.recompute_bundle_properties(bundle);
@@ -745,7 +747,7 @@ impl<'a, F: Function> Env<'a, F> {
 
         let req = match self.compute_requirement(bundle) {
             Ok(req) => req,
-            Err(RequirementConflictAt(split_point)) => {
+            Err(conflict) => {
                 // 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.
@@ -755,8 +757,10 @@ impl<'a, F: Function> Env<'a, F> {
                 );
                 self.split_and_requeue_bundle(
                     bundle,
-                    /* split_at_point = */ split_point,
+                    /* split_at_point = */ conflict.suggested_split_point(),
                     reg_hint,
+                    /* trim_ends_into_spill_bundle = */
+                    conflict.should_trim_edges_around_split(),
                 );
                 return Ok(());
             }
@@ -1047,7 +1051,12 @@ impl<'a, F: Function> Env<'a, F> {
                     }
                 }
 
-                self.split_and_requeue_bundle(bundle, split_at_point, requeue_with_reg);
+                self.split_and_requeue_bundle(
+                    bundle,
+                    split_at_point,
+                    requeue_with_reg,
+                    /* should_trim = */ true,
+                );
                 return Ok(());
             } else {
                 // Evict all bundles in `conflicting bundles` and try again.

src/ion/requirement.rs

@@ -13,11 +13,48 @@
 //! Requirements computation.
 
 use super::{Env, LiveBundleIndex};
-use crate::{Function, Operand, OperandConstraint, PReg, ProgPoint};
+use crate::{Function, Inst, Operand, OperandConstraint, PReg, ProgPoint};
 
 pub struct RequirementConflict;
 
-pub struct RequirementConflictAt(pub ProgPoint);
+#[derive(Clone, Copy, Debug)]
+pub enum RequirementConflictAt {
+    /// A transition from a stack-constrained to a reg-constrained
+    /// segment. The suggested split point is late, to keep the
+    /// intervening region with the stackslot (which is cheaper).
+    StackToReg(ProgPoint),
+    /// A transition from a reg-constraint to a stack-constrained
+    /// segment. Mirror of above: the suggested split point is early
+    /// (just after the last register use).
+    RegToStack(ProgPoint),
+    /// Any other transition. The suggested split point is late (just
+    /// before the conflicting use), but the split will also trim the
+    /// ends and create a split bundle, so the intervening region will
+    /// not appear with either side. This is probably for the best
+    /// when e.g. the two sides of the split are both constrained to
+    /// different physical registers: the part in the middle should be
+    /// constrained to neither.
+    Other(ProgPoint),
+}
+
+impl RequirementConflictAt {
+    #[inline(always)]
+    pub fn should_trim_edges_around_split(self) -> bool {
+        match self {
+            RequirementConflictAt::RegToStack(..) | RequirementConflictAt::StackToReg(..) => false,
+            RequirementConflictAt::Other(..) => true,
+        }
+    }
+
+    #[inline(always)]
+    pub fn suggested_split_point(self) -> ProgPoint {
+        match self {
+            RequirementConflictAt::RegToStack(pt)
+            | RequirementConflictAt::StackToReg(pt)
+            | RequirementConflictAt::Other(pt) => pt,
+        }
+    }
+}
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum Requirement {
@@ -47,6 +84,24 @@ impl Requirement {
             _ => Err(RequirementConflict),
         }
     }
+
+    #[inline(always)]
+    pub fn is_stack(self) -> bool {
+        match self {
+            Requirement::Stack | Requirement::FixedStack(..) => true,
+            Requirement::Register | Requirement::FixedReg(..) => false,
+            Requirement::Any => false,
+        }
+    }
+
+    #[inline(always)]
+    pub fn is_reg(self) -> bool {
+        match self {
+            Requirement::Register | Requirement::FixedReg(..) => true,
+            Requirement::Stack | Requirement::FixedStack(..) => false,
+            Requirement::Any => false,
+        }
+    }
 }
 
 impl<'a, F: Function> Env<'a, F> {
@@ -71,6 +126,7 @@ impl<'a, F: Function> Env<'a, F> {
         bundle: LiveBundleIndex,
     ) -> Result<Requirement, RequirementConflictAt> {
         let mut req = Requirement::Any;
+        let mut last_pos = ProgPoint::before(Inst::new(0));
         trace!("compute_requirement: {:?}", bundle);
         let ranges = &self.bundles[bundle.index()].ranges;
         for entry in ranges {
@@ -80,8 +136,21 @@ impl<'a, F: Function> Env<'a, F> {
                 let r = self.requirement_from_operand(u.operand);
                 req = req.merge(r).map_err(|_| {
                     trace!("     -> conflict");
-                    RequirementConflictAt(u.pos)
+                    if req.is_stack() && r.is_reg() {
+                        // Suggested split point just before the reg (i.e., late split).
+                        RequirementConflictAt::StackToReg(u.pos)
+                    } else if req.is_reg() && r.is_stack() {
+                        // Suggested split point just after the stack
+                        // (i.e., early split). Note that splitting
+                        // with a use *right* at the beginning is
+                        // interpreted by `split_and_requeue_bundle`
+                        // as splitting off the first use.
+                        RequirementConflictAt::RegToStack(last_pos)
+                    } else {
+                        RequirementConflictAt::Other(u.pos)
+                    }
                 })?;
+                last_pos = u.pos;
                 trace!("     -> req {:?}", req);
             }
         }