From 0a2d1bd5861ca5974760f655fa4669170ba25d90 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 17 Sep 2025 14:25:50 -0700 Subject: [PATCH 01/11] Add `OperandConstraint::Limit` This allows users of `regalloc2` to specify a register range, `0..n-1`, in which to allocate an operand. The upper limit, `n`, must be a power of two to fit in the bits available in the `Operand` encoding. --- src/lib.rs | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 4d10e452..c7dcb5f3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -547,6 +547,13 @@ pub enum OperandConstraint { FixedReg(PReg), /// On defs only: reuse a use's register. Reuse(usize), + /// Operand must be in a specific range of registers. + /// + /// The contained `usize` indicates the (exclusive) upper limit of a + /// register range, `n`. An operand with this constraint may allocate a + /// register between `0 ..= n-1`. Due to encoding constraints, `n` must be a + /// power of two and below 2^16. + Limit(usize), } impl core::fmt::Display for OperandConstraint { @@ -555,8 +562,9 @@ impl core::fmt::Display for OperandConstraint { Self::Any => write!(f, "any"), Self::Reg => write!(f, "reg"), Self::Stack => write!(f, "stack"), - Self::FixedReg(preg) => write!(f, "fixed({})", preg), - Self::Reuse(idx) => write!(f, "reuse({})", idx), + Self::FixedReg(preg) => write!(f, "fixed({preg})"), + Self::Reuse(idx) => write!(f, "reuse({idx})"), + Self::Limit(max) => write!(f, "limit(0..={})", max - 1), } } } @@ -631,6 +639,7 @@ pub struct Operand { /// The constraints are encoded as follows: /// - 1xxxxxx => FixedReg(preg) /// - 01xxxxx => Reuse(index) + /// - 001xxxx => Limit(max) /// - 0000000 => Any /// - 0000001 => Reg /// - 0000010 => Stack @@ -659,6 +668,12 @@ impl Operand { debug_assert!(which <= 0b11111); 0b0100000 | which as u32 } + OperandConstraint::Limit(max) => { + assert!(max.is_power_of_two()); + let log2 = max.ilog2(); + debug_assert!(log2 <= 0b1111); + 0b0010000 | log2 as u32 + } }; let class_field = vreg.class() as u8 as u32; let pos_field = pos as u8 as u32; @@ -919,6 +934,8 @@ impl Operand { OperandConstraint::FixedReg(PReg::new(constraint_field & 0b0111111, self.class())) } else if constraint_field & 0b0100000 != 0 { OperandConstraint::Reuse(constraint_field & 0b0011111) + } else if constraint_field & 0b0010000 != 0 { + OperandConstraint::Limit(1 << (constraint_field & 0b0001111)) } else { match constraint_field { 0 => OperandConstraint::Any, From 54d19268c68a071caf2a6b1ef19a47d668dd5db8 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 17 Sep 2025 14:28:35 -0700 Subject: [PATCH 02/11] Check that are operands are allocated within their `Limit` Failure to allocate within the limit results in a `CheckerError::AllocationOutsideLimit`. --- src/checker.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/checker.rs b/src/checker.rs index 3d67a995..58719e66 100644 --- a/src/checker.rs +++ b/src/checker.rs @@ -104,6 +104,7 @@ use alloc::vec::Vec; use alloc::{format, vec}; use core::default::Default; use core::hash::Hash; +use core::ops::Range; use core::result::Result; use smallvec::{smallvec, SmallVec}; @@ -166,6 +167,12 @@ pub enum CheckerError { into: Allocation, from: Allocation, }, + AllocationOutsideLimit { + inst: Inst, + op: Operand, + alloc: Allocation, + range: Range, + }, } /// Abstract state for an allocation. @@ -669,6 +676,18 @@ impl CheckerState { }); } } + OperandConstraint::Limit(max) => { + if let Some(preg) = alloc.as_reg() { + if preg.hw_enc() >= max { + return Err(CheckerError::AllocationOutsideLimit { + inst, + op, + alloc, + range: (0..max), + }); + } + } + } } Ok(()) } From 60b4cb376d362532a4d3799dfc05c70eaf3c63c4 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 17 Sep 2025 14:31:19 -0700 Subject: [PATCH 03/11] Ensure all fixed registers fit within their `Limit` Operands can be constrained to fixed registers, potentially multiple _different_ fixed registers; `Env::fixup_multi_fixed_vregs` fixes this. Here we also check that, for all the uses of a fixed register, the fixed register chosen does not fall outside of any limit constraints. To do this, we find the _lowest_ (minimum) limit in all uses of the operand and ensure the fixed register is below that. --- src/ion/liveranges.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/ion/liveranges.rs b/src/ion/liveranges.rs index 874df5a9..1162740f 100644 --- a/src/ion/liveranges.rs +++ b/src/ion/liveranges.rs @@ -25,6 +25,7 @@ use crate::{ OperandPos, PReg, ProgPoint, RegAllocError, VReg, VecExt, }; use core::convert::TryFrom; +use core::usize; use smallvec::{smallvec, SmallVec}; /// A spill weight computed for a certain Use. @@ -804,6 +805,8 @@ impl<'a, F: Function> Env<'a, F> { let mut num_fixed_stack = 0; let mut first_reg_slot = None; let mut first_stack_slot = None; + let mut min_limit = usize::MAX; + let mut max_fixed_reg = usize::MIN; for u in uses.iter() { match u.operand.constraint() { OperandConstraint::Any => { @@ -814,7 +817,13 @@ impl<'a, F: Function> Env<'a, F> { first_reg_slot.get_or_insert(u.slot); requires_reg = true; } + OperandConstraint::Limit(max) => { + first_reg_slot.get_or_insert(u.slot); + min_limit = min_limit.min(max); + requires_reg = true; + } OperandConstraint::FixedReg(preg) => { + max_fixed_reg = max_fixed_reg.max(preg.hw_enc()); if self.ctx.pregs[preg.index()].is_stack { num_fixed_stack += 1; first_stack_slot.get_or_insert(u.slot); @@ -834,6 +843,7 @@ impl<'a, F: Function> Env<'a, F> { // Fast path if there are no conflicts. if num_fixed_reg + num_fixed_stack <= 1 && !(requires_reg && num_fixed_stack != 0) + && max_fixed_reg < min_limit { continue; } @@ -867,6 +877,7 @@ impl<'a, F: Function> Env<'a, F> { // skip this edit. if !(requires_reg && self.ctx.pregs[preg.index()].is_stack) && *first_preg.get_or_insert(preg) == preg + && preg.hw_enc() < min_limit { continue; } From e13bb3d8a45fec3288afcb69781ff96168528097 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 17 Sep 2025 14:41:20 -0700 Subject: [PATCH 04/11] Transform an limit into `Requirement::Limit` In order to properly merge and split bundles, we need to propagate any limits to the registers available. Here we transform an `OperandConstraint::Limit` into a new `Requirement::Limit` and add merge rules: - when two limits meet we pick the smaller limit - when a limit meets a register we pick the limit - when a limit meets a fixed register, we pick the fixed register, but only if it fits under the limit --- src/ion/requirement.rs | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/src/ion/requirement.rs b/src/ion/requirement.rs index fc049e20..85890130 100644 --- a/src/ion/requirement.rs +++ b/src/ion/requirement.rs @@ -58,29 +58,36 @@ impl RequirementConflictAt { #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Requirement { - FixedReg(PReg), - FixedStack(PReg), + Any, Register, + FixedReg(PReg), + Limit(usize), Stack, - Any, + FixedStack(PReg), } impl Requirement { #[inline(always)] pub fn merge(self, other: Requirement) -> Result { + use Requirement::*; + match (self, other) { - (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) => { - Ok(Requirement::FixedReg(preg)) - } - (Requirement::Stack, Requirement::FixedStack(preg)) - | (Requirement::FixedStack(preg), Requirement::Stack) => { - Ok(Requirement::FixedStack(preg)) + // `Any` matches anything. + (other, Any) | (Any, other) => Ok(other), + // Same kinds match. + (Register, Register) => Ok(self), + (Stack, Stack) => Ok(self), + (Limit(a), Limit(b)) => Ok(Limit(a.min(b))), + (FixedReg(a), FixedReg(b)) if a == b => Ok(self), + (FixedStack(a), FixedStack(b)) if a == b => Ok(self), + // Limit a 'Register|FixedReg`. + (Limit(a), Register) | (Register, Limit(a)) => Ok(Limit(a)), + (Limit(a), FixedReg(b)) | (FixedReg(b), Limit(a)) if usize::from(a) > b.hw_enc() => { + Ok(FixedReg(b)) } - (Requirement::FixedReg(a), Requirement::FixedReg(b)) if a == b => Ok(self), - (Requirement::FixedStack(a), Requirement::FixedStack(b)) if a == b => Ok(self), + // Constrain `Register|Stack` to `Fixed{Reg|Stack}`. + (Register, FixedReg(preg)) | (FixedReg(preg), Register) => Ok(FixedReg(preg)), + (Stack, FixedStack(preg)) | (FixedStack(preg), Stack) => Ok(FixedStack(preg)), + // Fail otherwise. _ => Err(RequirementConflict), } } @@ -89,7 +96,7 @@ impl Requirement { pub fn is_stack(self) -> bool { match self { Requirement::Stack | Requirement::FixedStack(..) => true, - Requirement::Register | Requirement::FixedReg(..) => false, + Requirement::Register | Requirement::FixedReg(..) | Requirement::Limit(..) => false, Requirement::Any => false, } } @@ -97,7 +104,7 @@ impl Requirement { #[inline(always)] pub fn is_reg(self) -> bool { match self { - Requirement::Register | Requirement::FixedReg(..) => true, + Requirement::Register | Requirement::FixedReg(..) | Requirement::Limit(..) => true, Requirement::Stack | Requirement::FixedStack(..) => false, Requirement::Any => false, } @@ -116,6 +123,7 @@ impl<'a, F: Function> Env<'a, F> { } } OperandConstraint::Reg | OperandConstraint::Reuse(_) => Requirement::Register, + OperandConstraint::Limit(max) => Requirement::Limit(max), OperandConstraint::Stack => Requirement::Stack, OperandConstraint::Any => Requirement::Any, } From 4b0c5ff7b712e46461b4cf254d58a35f3a8d8cae Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 17 Sep 2025 15:58:37 -0700 Subject: [PATCH 05/11] Merge cached limits on `LiveBundle`s In line with the current performance considerations, we cache the limit for each bundle and merge them when bundles are merged. --- src/ion/data_structures.rs | 2 + src/ion/merge.rs | 77 +++++++++++++++++++++----------------- 2 files changed, 45 insertions(+), 34 deletions(-) diff --git a/src/ion/data_structures.rs b/src/ion/data_structures.rs index a81ee8fb..8f1a1f4b 100644 --- a/src/ion/data_structures.rs +++ b/src/ion/data_structures.rs @@ -203,6 +203,7 @@ pub struct LiveBundle { pub allocation: Allocation, pub prio: u32, // recomputed after every bulk update pub spill_weight_and_props: u32, + pub limit: Option, } pub const BUNDLE_MAX_SPILL_WEIGHT: u32 = (1 << 28) - 1; @@ -413,6 +414,7 @@ impl LiveBundles { spillset: SpillSetIndex::invalid(), prio: 0, spill_weight_and_props: 0, + limit: None, }) } } diff --git a/src/ion/merge.rs b/src/ion/merge.rs index 0385abd9..7d212733 100644 --- a/src/ion/merge.rs +++ b/src/ion/merge.rs @@ -12,17 +12,32 @@ //! Bundle merging. -use super::{Env, LiveBundleIndex, SpillSet, SpillSlotIndex, VRegIndex}; -use crate::{ - ion::{ - data_structures::{BlockparamOut, CodeRange}, - LiveRangeList, - }, - Function, Inst, OperandConstraint, OperandKind, PReg, ProgPoint, +use crate::ion::data_structures::{ + BlockparamOut, CodeRange, Env, LiveBundleIndex, LiveRangeList, SpillSet, SpillSlotIndex, + VRegIndex, }; +use crate::{Function, Inst, OperandConstraint, OperandKind, PReg, ProgPoint}; use alloc::format; impl<'a, F: Function> Env<'a, F> { + fn merge_bundle_properties(&mut self, from: LiveBundleIndex, to: LiveBundleIndex) { + if self.bundles[from].cached_fixed() { + self.bundles[to].set_cached_fixed(); + } + if self.bundles[from].cached_fixed_def() { + self.bundles[to].set_cached_fixed_def(); + } + if self.bundles[from].cached_stack() { + self.bundles[to].set_cached_stack(); + } + if let Some(theirs) = self.bundles[from].limit { + match self.bundles[to].limit { + Some(ours) => self.bundles[to].limit = Some(ours.min(theirs)), + None => self.bundles[to].limit = Some(theirs), + } + } + } + pub fn merge_bundles(&mut self, from: LiveBundleIndex, to: LiveBundleIndex) -> bool { if from == to { // Merge bundle into self -- trivial merge. @@ -114,8 +129,10 @@ impl<'a, F: Function> Env<'a, F> { // Check for a requirements conflict. if self.bundles[from].cached_stack() || self.bundles[from].cached_fixed() + || self.bundles[from].limit.is_some() || self.bundles[to].cached_stack() || self.bundles[to].cached_fixed() + || self.bundles[to].limit.is_some() { if self.merge_bundle_requirements(from, to).is_err() { trace!(" -> conflicting requirements; aborting merge"); @@ -157,17 +174,7 @@ impl<'a, F: Function> Env<'a, F> { } } self.bundles[to].ranges = list; - - if self.bundles[from].cached_stack() { - self.bundles[to].set_cached_stack(); - } - if self.bundles[from].cached_fixed() { - self.bundles[to].set_cached_fixed(); - } - if self.bundles[from].cached_fixed_def() { - self.bundles[to].set_cached_fixed_def(); - } - + self.merge_bundle_properties(from, to); return true; } @@ -243,15 +250,7 @@ impl<'a, F: Function> Env<'a, F> { *to_range = to_range.join(from_range); } - if self.bundles[from].cached_stack() { - self.bundles[to].set_cached_stack(); - } - if self.bundles[from].cached_fixed() { - self.bundles[to].set_cached_fixed(); - } - if self.bundles[from].cached_fixed_def() { - self.bundles[to].set_cached_fixed_def(); - } + self.merge_bundle_properties(from, to); true } @@ -283,16 +282,25 @@ impl<'a, F: Function> Env<'a, F> { let mut fixed = false; let mut fixed_def = false; let mut stack = false; + let mut limit: Option = None; for entry in &self.bundles[bundle].ranges { for u in &self.ranges[entry.index].uses { - if let OperandConstraint::FixedReg(_) = u.operand.constraint() { - fixed = true; - if u.operand.kind() == OperandKind::Def { - fixed_def = true; + use OperandConstraint::*; + match u.operand.constraint() { + FixedReg(_) => { + fixed = true; + if u.operand.kind() == OperandKind::Def { + fixed_def = true; + } + } + Stack => stack = true, + Limit(current) => match limit { + Some(prev) => limit = Some(prev.min(current)), + None => limit = Some(current), + }, + Any | Reg | Reuse(_) => { + continue; } - } - if let OperandConstraint::Stack = u.operand.constraint() { - stack = true; } if fixed && stack && fixed_def { break; @@ -308,6 +316,7 @@ impl<'a, F: Function> Env<'a, F> { if stack { self.bundles[bundle].set_cached_stack(); } + self.bundles[bundle].limit = limit; // Create a spillslot for this bundle. let reg = self.vreg(vreg); From 97d0894d0d1a78ddb27acbfb3aab27f0bdd7bbdf Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 17 Sep 2025 16:13:24 -0700 Subject: [PATCH 06/11] Detect when limits prevent allocation In `Env::process_bundle`, we check if allocation _must_ fail and return a `RegAllocError::TooManyLiveRegs`. This change extends the current logic to understand how limit constraints affect this calculation. --- src/ion/process.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/ion/process.rs b/src/ion/process.rs index 6dba89cb..125f18c8 100644 --- a/src/ion/process.rs +++ b/src/ion/process.rs @@ -1036,7 +1036,7 @@ impl<'a, F: Function> Env<'a, F> { let fixed_preg = match req { Requirement::FixedReg(preg) | Requirement::FixedStack(preg) => Some(preg), - Requirement::Register => None, + Requirement::Register | Requirement::Limit(..) => None, Requirement::Stack => { // If we must be on the stack, mark our spillset // as required immediately. @@ -1200,7 +1200,7 @@ impl<'a, F: Function> Env<'a, F> { || lowest_cost_evict_conflict_cost.is_none() || lowest_cost_evict_conflict_cost.unwrap() >= our_spill_weight) { - if let Requirement::Register = req { + if matches!(req, Requirement::Register | Requirement::Limit(_)) { // Check if this is a too-many-live-registers situation. let range = self.ctx.bundles[bundle].ranges[0].range; trace!("checking for too many live regs"); @@ -1240,6 +1240,15 @@ impl<'a, F: Function> Env<'a, F> { fixed_assigned += 1; } } + + // We also need to discard any registers that do not fit + // under the limit--we cannot allocate to them. + if let Requirement::Limit(limit) = req { + if preg.hw_enc() >= limit as usize { + continue; + } + } + total_regs += 1; } trace!( From effa5d16a5b52e7e8032a57dbf8bf2b6ca0a3183 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 17 Sep 2025 16:16:01 -0700 Subject: [PATCH 07/11] Disable limits in fastalloc for now More discussion is necessary before integrating `Limit` constraints with the `available_pregs_for_regs` introduced in #233. --- src/fastalloc/mod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/fastalloc/mod.rs b/src/fastalloc/mod.rs index 81654e63..22a46625 100644 --- a/src/fastalloc/mod.rs +++ b/src/fastalloc/mod.rs @@ -509,6 +509,9 @@ impl<'a, F: Function> Env<'a, F> { } OperandConstraint::Stack => self.edits.is_stack(alloc), + OperandConstraint::Limit(_) => { + todo!("limit constraints are not yet supported in fastalloc") + } } } @@ -667,6 +670,9 @@ impl<'a, F: Function> Env<'a, F> { } OperandConstraint::Stack => Allocation::stack(self.get_spillslot(op.vreg())), + OperandConstraint::Limit(_) => { + todo!("limit constraints are not yet supported in fastalloc") + } }; self.allocs[(inst.index(), op_idx)] = new_alloc; Ok(new_alloc) From 17d2b2296eb9866ff7a1ea8b05e50eb950162961 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 17 Sep 2025 16:50:51 -0700 Subject: [PATCH 08/11] Constrain iteration of registers with a `Limit` During allocation, we iterate over the available registers and pick one within all the constraints (i.e., fixed, hints, preferred/non-preferred lists). This adds a limit to all of those constraints: unless the register is fixed, we only pick a register with a HW encoding below the limit, if one is given. --- src/ion/moves.rs | 5 ++++- src/ion/process.rs | 12 +++++++++--- src/ion/reg_traversal.rs | 15 ++++++++++----- src/ion/spill.rs | 4 +++- 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/ion/moves.rs b/src/ion/moves.rs index 95195238..ddabd9d8 100644 --- a/src/ion/moves.rs +++ b/src/ion/moves.rs @@ -892,7 +892,10 @@ impl<'a, F: Function> Env<'a, F> { } let resolved = parallel_moves.resolve(); - let mut scratch_iter = RegTraversalIter::new(self.env, regclass, None, None, 0); + let mut scratch_iter = RegTraversalIter::new( + self.env, regclass, None, None, 0, + None, // We assume there is no limit on the set of registers available for moves. + ); let mut dedicated_scratch = self.env.scratch_by_class[regclass as usize]; let key = LiveRangeKey::from_range(&CodeRange { from: pos_prio.pos, diff --git a/src/ion/process.rs b/src/ion/process.rs index 125f18c8..7b81318f 100644 --- a/src/ion/process.rs +++ b/src/ion/process.rs @@ -1072,9 +1072,15 @@ impl<'a, F: Function> Env<'a, F> { + bundle.index(); self.ctx.output.stats.process_bundle_reg_probe_start_any += 1; - for preg in - RegTraversalIter::new(self.env, class, fixed_preg, hint.as_valid(), scan_offset) - { + let limit = self.bundles[bundle].limit; + for preg in RegTraversalIter::new( + self.env, + class, + fixed_preg, + hint.as_valid(), + scan_offset, + limit, + ) { self.ctx.output.stats.process_bundle_reg_probes_any += 1; let preg_idx = PRegIndex::new(preg.index()); trace!("trying preg {:?}", preg_idx); diff --git a/src/ion/reg_traversal.rs b/src/ion/reg_traversal.rs index 7ad4611d..3659fe97 100644 --- a/src/ion/reg_traversal.rs +++ b/src/ion/reg_traversal.rs @@ -72,6 +72,7 @@ pub struct RegTraversalIter<'a> { hint: Option, preferred: Cursor<'a>, non_preferred: Cursor<'a>, + limit: Option, } impl<'a> RegTraversalIter<'a> { @@ -81,6 +82,7 @@ impl<'a> RegTraversalIter<'a> { fixed: Option, hint: Option, offset: usize, + limit: Option, ) -> Self { debug_assert!(fixed != Some(PReg::invalid())); debug_assert!(hint != Some(PReg::invalid())); @@ -96,6 +98,7 @@ impl<'a> RegTraversalIter<'a> { hint, preferred, non_preferred, + limit, } } } @@ -110,21 +113,23 @@ impl<'a> core::iter::Iterator for RegTraversalIter<'a> { if self.use_hint { self.use_hint = false; - return self.hint; + if self.hint.unwrap().hw_enc() < self.limit.unwrap_or(usize::MAX) { + return self.hint; + } } while !self.preferred.done() { let reg = self.preferred.advance(); - if Some(reg) == self.hint { - continue; // Try again; we already tried the hint. + if Some(reg) == self.hint || reg.hw_enc() >= self.limit.unwrap_or(usize::MAX) { + continue; // Try again; we already tried the hint or we are outside of the register range limit. } return Some(reg); } while !self.non_preferred.done() { let reg = self.non_preferred.advance(); - if Some(reg) == self.hint { - continue; // Try again; we already tried the hint. + if Some(reg) == self.hint || reg.hw_enc() >= self.limit.unwrap_or(usize::MAX) { + continue; // Try again; we already tried the hint or we are outside of the register range limit. } return Some(reg); } diff --git a/src/ion/spill.rs b/src/ion/spill.rs index 3e1f8d2f..69269b07 100644 --- a/src/ion/spill.rs +++ b/src/ion/spill.rs @@ -42,7 +42,8 @@ impl<'a, F: Function> Env<'a, F> { let mut success = false; self.ctx.output.stats.spill_bundle_reg_probes += 1; - for preg in RegTraversalIter::new(self.env, class, None, hint, bundle.index()) { + let limit = self.bundles[bundle].limit; + for preg in RegTraversalIter::new(self.env, class, None, hint, bundle.index(), limit) { trace!("trying bundle {:?} to preg {:?}", bundle, preg); let preg_idx = PRegIndex::new(preg.index()); if let AllocRegResult::Allocated(_) = @@ -53,6 +54,7 @@ impl<'a, F: Function> Env<'a, F> { break; } } + if !success { trace!( "spilling bundle {:?}: marking spillset {:?} as required", From 4511ac6a7c4c28b823f1afeb050bdc813af04318 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 17 Sep 2025 17:59:56 -0700 Subject: [PATCH 09/11] Recalculate cached limits When splitting bundles, we need the ability to recalculate the cached limit. --- src/ion/merge.rs | 22 ++++++++++++++++++++++ src/ion/process.rs | 1 + 2 files changed, 23 insertions(+) diff --git a/src/ion/merge.rs b/src/ion/merge.rs index 7d212733..539ccd73 100644 --- a/src/ion/merge.rs +++ b/src/ion/merge.rs @@ -392,6 +392,28 @@ impl<'a, F: Function> Env<'a, F> { total } + pub fn compute_bundle_limit(&self, bundle: LiveBundleIndex) -> Option { + let mut limit: Option = None; + for entry in &self.bundles[bundle].ranges { + for u in &self.ranges[entry.index].uses { + use OperandConstraint::*; + match u.operand.constraint() { + Limit(current) => match limit { + Some(prev) => limit = Some(prev.min(current)), + None => limit = Some(current), + }, + FixedReg(_) | Stack => { + break; + } + Any | Reg | Reuse(_) => { + continue; + } + } + } + } + limit + } + pub fn queue_bundles(&mut self) { for bundle in 0..self.bundles.len() { trace!("enqueueing bundle{}", bundle); diff --git a/src/ion/process.rs b/src/ion/process.rs index 7b81318f..1fab2b06 100644 --- a/src/ion/process.rs +++ b/src/ion/process.rs @@ -274,6 +274,7 @@ impl<'a, F: Function> Env<'a, F> { let first_range_data = &self.ctx.ranges[first_range]; self.ctx.bundles[bundle].prio = self.compute_bundle_prio(bundle); + self.ctx.bundles[bundle].limit = self.compute_bundle_limit(bundle); if first_range_data.vreg.is_invalid() { trace!(" -> no vreg; minimal and fixed"); From 66901223547cde77dcb01d5b1373c07bac84bf6a Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Fri, 19 Sep 2025 12:13:03 -0700 Subject: [PATCH 10/11] Only cache limits as `u8` To avoid possible cache issues, Chris [asked] to store a smaller size for the cached limit. This change drops the `Option` to a `Option` adding conversions between `usize` and `u8` as needed. The `usize` is retained for consistency in the public API (`OperandConstraint::Limit`) and for ease of use in comparing with HW encodings and slice indexes (all `usize`). [asked]: https://github.com/bytecodealliance/regalloc2/pull/239#discussion_r2361118861 --- src/ion/data_structures.rs | 2 +- src/ion/merge.rs | 31 ++++++++++++++++++++----------- src/ion/process.rs | 2 +- src/ion/spill.rs | 2 +- 4 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/ion/data_structures.rs b/src/ion/data_structures.rs index 8f1a1f4b..99fd7b1f 100644 --- a/src/ion/data_structures.rs +++ b/src/ion/data_structures.rs @@ -203,7 +203,7 @@ pub struct LiveBundle { pub allocation: Allocation, pub prio: u32, // recomputed after every bulk update pub spill_weight_and_props: u32, - pub limit: Option, + pub limit: Option, } pub const BUNDLE_MAX_SPILL_WEIGHT: u32 = (1 << 28) - 1; diff --git a/src/ion/merge.rs b/src/ion/merge.rs index 539ccd73..f6e87d68 100644 --- a/src/ion/merge.rs +++ b/src/ion/merge.rs @@ -18,6 +18,7 @@ use crate::ion::data_structures::{ }; use crate::{Function, Inst, OperandConstraint, OperandKind, PReg, ProgPoint}; use alloc::format; +use core::convert::TryFrom; impl<'a, F: Function> Env<'a, F> { fn merge_bundle_properties(&mut self, from: LiveBundleIndex, to: LiveBundleIndex) { @@ -282,7 +283,7 @@ impl<'a, F: Function> Env<'a, F> { let mut fixed = false; let mut fixed_def = false; let mut stack = false; - let mut limit: Option = None; + let mut limit: Option = None; for entry in &self.bundles[bundle].ranges { for u in &self.ranges[entry.index].uses { use OperandConstraint::*; @@ -294,10 +295,14 @@ impl<'a, F: Function> Env<'a, F> { } } Stack => stack = true, - Limit(current) => match limit { - Some(prev) => limit = Some(prev.min(current)), - None => limit = Some(current), - }, + Limit(current) => { + let current = u8::try_from(current) + .expect("the current limit is too large to fit in a u8"); + match limit { + Some(prev) => limit = Some(prev.min(current)), + None => limit = Some(current), + } + } Any | Reg | Reuse(_) => { continue; } @@ -392,16 +397,20 @@ impl<'a, F: Function> Env<'a, F> { total } - pub fn compute_bundle_limit(&self, bundle: LiveBundleIndex) -> Option { - let mut limit: Option = None; + pub fn compute_bundle_limit(&self, bundle: LiveBundleIndex) -> Option { + let mut limit: Option = None; for entry in &self.bundles[bundle].ranges { for u in &self.ranges[entry.index].uses { use OperandConstraint::*; match u.operand.constraint() { - Limit(current) => match limit { - Some(prev) => limit = Some(prev.min(current)), - None => limit = Some(current), - }, + Limit(current) => { + let current = u8::try_from(current) + .expect("the current limit is too large to fit in a u8"); + match limit { + Some(prev) => limit = Some(prev.min(current)), + None => limit = Some(current), + } + } FixedReg(_) | Stack => { break; } diff --git a/src/ion/process.rs b/src/ion/process.rs index 1fab2b06..8c5a3d0a 100644 --- a/src/ion/process.rs +++ b/src/ion/process.rs @@ -1073,7 +1073,7 @@ impl<'a, F: Function> Env<'a, F> { + bundle.index(); self.ctx.output.stats.process_bundle_reg_probe_start_any += 1; - let limit = self.bundles[bundle].limit; + let limit = self.bundles[bundle].limit.map(|l| l as usize); for preg in RegTraversalIter::new( self.env, class, diff --git a/src/ion/spill.rs b/src/ion/spill.rs index 69269b07..8eb44cb9 100644 --- a/src/ion/spill.rs +++ b/src/ion/spill.rs @@ -42,7 +42,7 @@ impl<'a, F: Function> Env<'a, F> { let mut success = false; self.ctx.output.stats.spill_bundle_reg_probes += 1; - let limit = self.bundles[bundle].limit; + let limit = self.bundles[bundle].limit.map(|l| l as usize); for preg in RegTraversalIter::new(self.env, class, None, hint, bundle.index(), limit) { trace!("trying bundle {:?} to preg {:?}", bundle, preg); let preg_idx = PRegIndex::new(preg.index()); From 2454d72887018fe6a5abcfa6faef89108b785246 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Fri, 19 Sep 2025 12:25:28 -0700 Subject: [PATCH 11/11] Prevent over-large `OperandConstraint::Limit`s For consistency, internally and externally, we do not really want to handle limits that are larger that the maximum encodable register. --- src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index c7dcb5f3..1f2d3276 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -670,6 +670,10 @@ impl Operand { } OperandConstraint::Limit(max) => { assert!(max.is_power_of_two()); + assert!( + max <= PReg::MAX + 1, + "limit is larger than the allowed register encoding" + ); let log2 = max.ilog2(); debug_assert!(log2 <= 0b1111); 0b0010000 | log2 as u32