Fix spillslot allocation to actually reuse spillslots. (#56)

* Fix spillslot allocation to actually reuse spillslots.

The old logic, which did some linked-list rearranging to try to probe
more-likely-to-be-free slots first and which was inherited straight from
the original IonMonkey allocator, was slightly broken (error in
translation and not in IonMonkey, to be clear): it did not get the
list-splicing right, so quite often dropped a slot on the floor and
failed to consider it for further reuse.

On some experimentation, it seems to work just as well to keep a
SmallVec of spillslot indices per size class instead, and save the last
probe-point in order to spread load throughout the allocated slots while
limiting the number of probes (to bound quadratic behavior).

This change moves the maximum slot count from 285 to 92 in `python.wasm`
from bytecodealliance/wasmtime#4214, and the maximum frame size from
2384 bytes to 752 bytes.

src/ion/data_structures.rs

@@ -439,13 +439,25 @@ pub struct SpillSlotData {
     pub ranges: LiveRangeSet,
     pub class: RegClass,
     pub alloc: Allocation,
-    pub next_spillslot: SpillSlotIndex,
 }
 
 #[derive(Clone, Debug)]
 pub struct SpillSlotList {
-    pub first_spillslot: SpillSlotIndex,
-    pub last_spillslot: SpillSlotIndex,
+    pub slots: SmallVec<[SpillSlotIndex; 32]>,
+    pub probe_start: usize,
+}
+
+impl SpillSlotList {
+    /// Get the next spillslot index in probing order, wrapping around
+    /// at the end of the slots list.
+    pub(crate) fn next_index(&self, index: usize) -> usize {
+        debug_assert!(index < self.slots.len());
+        if index == self.slots.len() - 1 {
+            0
+        } else {
+            index + 1
+        }
+    }
 }
 
 #[derive(Clone, Debug)]

src/ion/spill.rs

@@ -17,6 +17,7 @@ use super::{
     SpillSetIndex, SpillSlotData, SpillSlotIndex, SpillSlotList,
 };
 use crate::{Allocation, Function, SpillSlot};
+use smallvec::smallvec;
 
 impl<'a, F: Function> Env<'a, F> {
     pub fn try_allocating_regs_for_spilled_bundles(&mut self) {
@@ -110,6 +111,8 @@ impl<'a, F: Function> Env<'a, F> {
     }
 
     pub fn allocate_spillslots(&mut self) {
+        const MAX_ATTEMPTS: usize = 10;
+
         for spillset in 0..self.spillsets.len() {
             trace!("allocate spillslot: {}", spillset);
             let spillset = SpillSetIndex::new(spillset);
@@ -122,70 +125,46 @@ impl<'a, F: Function> Env<'a, F> {
                 self.slots_by_size.resize(
                     size + 1,
                     SpillSlotList {
-                        first_spillslot: SpillSlotIndex::invalid(),
-                        last_spillslot: SpillSlotIndex::invalid(),
+                        slots: smallvec![],
+                        probe_start: 0,
                     },
                 );
             }
             // Try a few existing spillslots.
-            let mut spillslot_iter = self.slots_by_size[size].first_spillslot;
-            let mut first_slot = SpillSlotIndex::invalid();
-            let mut prev = SpillSlotIndex::invalid();
+            let mut i = self.slots_by_size[size].probe_start;
             let mut success = false;
-            for _attempt in 0..10 {
-                if spillslot_iter.is_invalid() {
-                    break;
-                }
-                if spillslot_iter == first_slot {
-                    // We've started looking at slots we placed at the end; end search.
-                    break;
-                }
-                if first_slot.is_invalid() {
-                    first_slot = spillslot_iter;
-                }
-
-                if self.spillslot_can_fit_spillset(spillslot_iter, spillset) {
-                    self.allocate_spillset_to_spillslot(spillset, spillslot_iter);
+            // Never probe the same element more than once: limit the
+            // attempt count to the number of slots in existence.
+            for _attempt in 0..std::cmp::min(self.slots_by_size[size].slots.len(), MAX_ATTEMPTS) {
+                // Note: this indexing of `slots` is always valid
+                // because either the `slots` list is empty and the
+                // iteration limit above consequently means we don't
+                // run this loop at all, or else `probe_start` is
+                // in-bounds (because it is made so below when we add
+                // a slot, and it always takes on the last index `i`
+                // after this loop).
+                let spillslot = self.slots_by_size[size].slots[i];
+
+                if self.spillslot_can_fit_spillset(spillslot, spillset) {
+                    self.allocate_spillset_to_spillslot(spillset, spillslot);
                     success = true;
+                    self.slots_by_size[size].probe_start = i;
                     break;
                 }
-                // Remove the slot and place it at the end of the respective list.
-                let next = self.spillslots[spillslot_iter.index()].next_spillslot;
-                if prev.is_valid() {
-                    self.spillslots[prev.index()].next_spillslot = next;
-                } else {
-                    self.slots_by_size[size].first_spillslot = next;
-                }
-                if !next.is_valid() {
-                    self.slots_by_size[size].last_spillslot = prev;
-                }
 
-                let last = self.slots_by_size[size].last_spillslot;
-                if last.is_valid() {
-                    self.spillslots[last.index()].next_spillslot = spillslot_iter;
-                } else {
-                    self.slots_by_size[size].first_spillslot = spillslot_iter;
-                }
-                self.slots_by_size[size].last_spillslot = spillslot_iter;
-
-                prev = spillslot_iter;
-                spillslot_iter = next;
+                i = self.slots_by_size[size].next_index(i);
             }
 
             if !success {
                 // Allocate a new spillslot.
                 let spillslot = SpillSlotIndex::new(self.spillslots.len());
-                let next = self.slots_by_size[size].first_spillslot;
                 self.spillslots.push(SpillSlotData {
                     ranges: LiveRangeSet::new(),
-                    next_spillslot: next,
                     alloc: Allocation::none(),
                     class: self.spillsets[spillset.index()].class,
                 });
-                self.slots_by_size[size].first_spillslot = spillslot;
-                if !next.is_valid() {
-                    self.slots_by_size[size].last_spillslot = spillslot;
-                }
+                self.slots_by_size[size].slots.push(spillslot);
+                self.slots_by_size[size].probe_start = self.slots_by_size[size].slots.len() - 1;
 
                 self.allocate_spillset_to_spillslot(spillset, spillslot);
             }