From 8619ae79d159dc698cc71ff7aea62f92bdcbe97c Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Mon, 21 Jul 2025 20:16:25 -0700 Subject: [PATCH 1/3] Revert "Fix bit packing of bundle params (#192)" This reverts commit d251c8e2357dedc56ea775b6d4443532929b321e. --- src/ion/data_structures.rs | 12 +++++++++++- src/ion/merge.rs | 12 +++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/ion/data_structures.rs b/src/ion/data_structures.rs index 208be91d..06bc3810 100644 --- a/src/ion/data_structures.rs +++ b/src/ion/data_structures.rs @@ -241,6 +241,11 @@ impl LiveBundle { self.spill_weight_and_props & (1 << 29) != 0 } + #[inline(always)] + pub fn cached_stack(&self) -> bool { + self.spill_weight_and_props & (1 << 28) != 0 + } + #[inline(always)] pub fn set_cached_fixed(&mut self) { self.spill_weight_and_props |= 1 << 30; @@ -251,9 +256,14 @@ impl LiveBundle { self.spill_weight_and_props |= 1 << 29; } + #[inline(always)] + pub fn set_cached_stack(&mut self) { + self.spill_weight_and_props |= 1 << 28; + } + #[inline(always)] pub fn cached_spill_weight(&self) -> u32 { - self.spill_weight_and_props & BUNDLE_MAX_SPILL_WEIGHT + self.spill_weight_and_props & ((1 << 28) - 1) } } diff --git a/src/ion/merge.rs b/src/ion/merge.rs index f503acf1..0b8dad12 100644 --- a/src/ion/merge.rs +++ b/src/ion/merge.rs @@ -112,7 +112,11 @@ impl<'a, F: Function> Env<'a, F> { } // Check for a requirements conflict. - if self.bundles[from].cached_fixed() || self.bundles[to].cached_fixed() { + if self.bundles[from].cached_stack() + || self.bundles[from].cached_fixed() + || self.bundles[to].cached_stack() + || self.bundles[to].cached_fixed() + { if self.merge_bundle_requirements(from, to).is_err() { trace!(" -> conflicting requirements; aborting merge"); return false; @@ -154,6 +158,9 @@ 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(); } @@ -236,6 +243,9 @@ 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(); } From 6a3a61593ad260a4e537080002887627797b027a Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Mon, 21 Jul 2025 20:17:41 -0700 Subject: [PATCH 2/3] Revert "Remove the OperandConstraint::Stack variant (#185)" This reverts commit abdc3d6477e5021d0e9ab37a951758f15db952f7. --- doc/ION.md | 13 ++++++++----- src/checker.rs | 11 +++++++++++ src/ion/data_structures.rs | 6 ++++-- src/ion/liveranges.rs | 4 ++++ src/ion/merge.rs | 9 ++++++++- src/ion/process.rs | 15 ++++++++++++++- src/ion/requirement.rs | 11 +++++++++-- src/lib.rs | 5 +++++ 8 files changed, 63 insertions(+), 11 deletions(-) diff --git a/doc/ION.md b/doc/ION.md index aea2be23..55a3e6dc 100644 --- a/doc/ION.md +++ b/doc/ION.md @@ -442,11 +442,14 @@ other): ```plain - Any(rc) - / \ - FixedReg(reg) FixedStack(reg) - \ / - Conflict + ___Unknown_____ + | | | + | | | + | ____Any(rc) | + |/ | | + Stack(rc) FixedReg(reg) + \ / + Conflict ``` Once we have the Requirement for a bundle, we can decide what to do. diff --git a/src/checker.rs b/src/checker.rs index 91943a5e..3d67a995 100644 --- a/src/checker.rs +++ b/src/checker.rs @@ -640,6 +640,17 @@ impl CheckerState { } 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 checker.machine_env.fixed_stack_slots.contains(&preg) { + return Ok(()); + } + } + return Err(CheckerError::AllocationIsNotStack { inst, op, alloc }); + } + } OperandConstraint::FixedReg(preg) => { if alloc != Allocation::reg(preg) { return Err(CheckerError::AllocationIsNotFixedReg { inst, op, alloc }); diff --git a/src/ion/data_structures.rs b/src/ion/data_structures.rs index 06bc3810..c7422f29 100644 --- a/src/ion/data_structures.rs +++ b/src/ion/data_structures.rs @@ -205,7 +205,7 @@ pub struct LiveBundle { pub spill_weight_and_props: u32, } -pub const BUNDLE_MAX_SPILL_WEIGHT: u32 = (1 << 29) - 1; +pub const BUNDLE_MAX_SPILL_WEIGHT: u32 = (1 << 28) - 1; pub const MINIMAL_FIXED_BUNDLE_SPILL_WEIGHT: u32 = BUNDLE_MAX_SPILL_WEIGHT; pub const MINIMAL_BUNDLE_SPILL_WEIGHT: u32 = BUNDLE_MAX_SPILL_WEIGHT - 1; pub const BUNDLE_MAX_NORMAL_SPILL_WEIGHT: u32 = BUNDLE_MAX_SPILL_WEIGHT - 2; @@ -218,12 +218,14 @@ impl LiveBundle { minimal: bool, fixed: bool, fixed_def: bool, + stack: bool, ) { debug_assert!(spill_weight <= BUNDLE_MAX_SPILL_WEIGHT); self.spill_weight_and_props = spill_weight | (if minimal { 1 << 31 } else { 0 }) | (if fixed { 1 << 30 } else { 0 }) - | (if fixed_def { 1 << 29 } else { 0 }); + | (if fixed_def { 1 << 29 } else { 0 }) + | (if stack { 1 << 28 } else { 0 }); } #[inline(always)] diff --git a/src/ion/liveranges.rs b/src/ion/liveranges.rs index 55818280..874df5a9 100644 --- a/src/ion/liveranges.rs +++ b/src/ion/liveranges.rs @@ -824,6 +824,10 @@ impl<'a, F: Function> Env<'a, F> { 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" + ), } } diff --git a/src/ion/merge.rs b/src/ion/merge.rs index 0b8dad12..3ca3a9c3 100644 --- a/src/ion/merge.rs +++ b/src/ion/merge.rs @@ -282,6 +282,7 @@ impl<'a, F: Function> Env<'a, F> { let mut fixed = false; let mut fixed_def = false; + let mut stack = false; for entry in &self.bundles[bundle].ranges { for u in &self.ranges[entry.index].uses { if let OperandConstraint::FixedReg(_) = u.operand.constraint() { @@ -290,7 +291,10 @@ impl<'a, F: Function> Env<'a, F> { fixed_def = true; } } - if fixed && fixed_def { + if let OperandConstraint::Stack = u.operand.constraint() { + stack = true; + } + if fixed && stack && fixed_def { break; } } @@ -301,6 +305,9 @@ impl<'a, F: Function> Env<'a, F> { if fixed_def { self.bundles[bundle].set_cached_fixed_def(); } + if stack { + self.bundles[bundle].set_cached_stack(); + } // Create a spillslot for this bundle. let reg = self.vreg(vreg); diff --git a/src/ion/process.rs b/src/ion/process.rs index 13f04e8b..80954cbd 100644 --- a/src/ion/process.rs +++ b/src/ion/process.rs @@ -267,6 +267,7 @@ impl<'a, F: Function> Env<'a, F> { let minimal; let mut fixed = false; let mut fixed_def = false; + let mut stack = false; let bundledata = &self.ctx.bundles[bundle]; let num_ranges = bundledata.ranges.len(); let first_range = bundledata.ranges[0].index; @@ -288,7 +289,12 @@ impl<'a, F: Function> Env<'a, F> { trace!(" -> is fixed def"); fixed_def = true; } - + } + if let OperandConstraint::Stack = u.operand.constraint() { + trace!(" -> stack operand at {:?}: {:?}", u.pos, u.operand); + stack = true; + } + if stack && fixed { break; } } @@ -351,6 +357,7 @@ impl<'a, F: Function> Env<'a, F> { minimal, fixed, fixed_def, + stack, ); } @@ -1036,6 +1043,12 @@ impl<'a, F: Function> Env<'a, F> { let fixed_preg = match req { Requirement::FixedReg(preg) | Requirement::FixedStack(preg) => Some(preg), Requirement::Register => None, + Requirement::Stack => { + // If we must be on the stack, mark our spillset + // as required immediately. + self.spillsets[self.bundles[bundle].spillset].required = true; + return Ok(()); + } Requirement::Any => { self.ctx.spilled_bundles.push(bundle); diff --git a/src/ion/requirement.rs b/src/ion/requirement.rs index 5e9af2a3..fc049e20 100644 --- a/src/ion/requirement.rs +++ b/src/ion/requirement.rs @@ -61,6 +61,7 @@ pub enum Requirement { FixedReg(PReg), FixedStack(PReg), Register, + Stack, Any, } impl Requirement { @@ -69,10 +70,15 @@ impl 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)) + } (Requirement::FixedReg(a), Requirement::FixedReg(b)) if a == b => Ok(self), (Requirement::FixedStack(a), Requirement::FixedStack(b)) if a == b => Ok(self), _ => Err(RequirementConflict), @@ -82,7 +88,7 @@ impl Requirement { #[inline(always)] pub fn is_stack(self) -> bool { match self { - Requirement::FixedStack(..) => true, + Requirement::Stack | Requirement::FixedStack(..) => true, Requirement::Register | Requirement::FixedReg(..) => false, Requirement::Any => false, } @@ -92,7 +98,7 @@ impl Requirement { pub fn is_reg(self) -> bool { match self { Requirement::Register | Requirement::FixedReg(..) => true, - Requirement::FixedStack(..) => false, + Requirement::Stack | Requirement::FixedStack(..) => false, Requirement::Any => false, } } @@ -110,6 +116,7 @@ impl<'a, F: Function> Env<'a, F> { } } OperandConstraint::Reg | OperandConstraint::Reuse(_) => Requirement::Register, + OperandConstraint::Stack => Requirement::Stack, OperandConstraint::Any => Requirement::Any, } } diff --git a/src/lib.rs b/src/lib.rs index a672583f..8433ae3d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -528,6 +528,8 @@ pub enum OperandConstraint { Any, /// Operand must be in a register. Register is read-only for Uses. Reg, + /// Operand must be on the stack. + Stack, /// Operand must be in a fixed register. FixedReg(PReg), /// On defs only: reuse a use's register. @@ -539,6 +541,7 @@ impl core::fmt::Display for OperandConstraint { match self { 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), } @@ -634,6 +637,7 @@ impl Operand { let constraint_field = match constraint { OperandConstraint::Any => 0, OperandConstraint::Reg => 1, + OperandConstraint::Stack => 2, OperandConstraint::FixedReg(preg) => { debug_assert_eq!(preg.class(), vreg.class()); 0b1000000 | preg.hw_enc() as u32 @@ -906,6 +910,7 @@ impl Operand { match constraint_field { 0 => OperandConstraint::Any, 1 => OperandConstraint::Reg, + 2 => OperandConstraint::Stack, _ => unreachable!(), } } From e5c97c07250feee44cf28ead1c17495afacdf1f9 Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Mon, 21 Jul 2025 20:19:28 -0700 Subject: [PATCH 3/3] fixups --- src/fastalloc/mod.rs | 8 ++++++++ src/ion/data_structures.rs | 2 +- src/ion/process.rs | 3 ++- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/fastalloc/mod.rs b/src/fastalloc/mod.rs index 98c28c68..e0bc9343 100644 --- a/src/fastalloc/mod.rs +++ b/src/fastalloc/mod.rs @@ -453,6 +453,10 @@ impl<'a, F: Function> Env<'a, F> { OperandConstraint::Reuse(_) => { unreachable!() } + + OperandConstraint::Stack => { + panic!("Stack constraints not supported in fastalloc"); + } } } @@ -576,6 +580,10 @@ impl<'a, F: Function> Env<'a, F> { // This is handled elsewhere. unreachable!(); } + + OperandConstraint::Stack => { + panic!("Stack operand constraints not supported in fastalloc"); + } }; self.allocs[(inst.index(), op_idx)] = new_alloc; Ok(new_alloc) diff --git a/src/ion/data_structures.rs b/src/ion/data_structures.rs index c7422f29..780ca4cc 100644 --- a/src/ion/data_structures.rs +++ b/src/ion/data_structures.rs @@ -265,7 +265,7 @@ impl LiveBundle { #[inline(always)] pub fn cached_spill_weight(&self) -> u32 { - self.spill_weight_and_props & ((1 << 28) - 1) + self.spill_weight_and_props & BUNDLE_MAX_SPILL_WEIGHT } } diff --git a/src/ion/process.rs b/src/ion/process.rs index 80954cbd..2d174815 100644 --- a/src/ion/process.rs +++ b/src/ion/process.rs @@ -1046,7 +1046,8 @@ impl<'a, F: Function> Env<'a, F> { Requirement::Stack => { // If we must be on the stack, mark our spillset // as required immediately. - self.spillsets[self.bundles[bundle].spillset].required = true; + let spillset = self.bundles[bundle].spillset; + self.spillsets[spillset].required = true; return Ok(()); }