From 3e948a5fcd4e9d590c6d390204d2be7469f34ff0 Mon Sep 17 00:00:00 2001 From: Demetrios Chiuratto Agourakis Date: Sat, 26 Sep 2026 01:16:28 +0000 Subject: [PATCH 1/2] ion: reject a fixed def that conflicts with a clobber A clobber is a fixed late def of a throwaway vreg. When a real fixed def occupies that same register, the bundle is minimal and the reservation cannot be evicted, so allocation is impossible. Return TooManyLiveRegs instead of panicking. Fixes #222. Co-Authored-By: Claude --- src/ion/mod.rs | 121 +++++++++++++++++++++++++++++++++++++++++++++ src/ion/process.rs | 10 ++++ 2 files changed, 131 insertions(+) diff --git a/src/ion/mod.rs b/src/ion/mod.rs index 99181191..9731c7f3 100644 --- a/src/ion/mod.rs +++ b/src/ion/mod.rs @@ -132,3 +132,124 @@ pub fn run( Ok(()) } + +#[cfg(test)] +mod tests { + use crate::{ + Algorithm, Block, Function, Inst, InstRange, MachineEnv, Operand, OperandConstraint, + OperandKind, OperandPos, PReg, PRegSet, RegAllocError, RegClass, RegallocOptions, VReg, + Vec, + }; + use alloc::vec; + + struct ConflictFunc { + operands: Vec>, + clobbers: Vec, + } + + impl Function for ConflictFunc { + fn num_insts(&self) -> usize { + self.operands.len() + } + fn num_blocks(&self) -> usize { + 1 + } + fn entry_block(&self) -> Block { + Block::new(0) + } + fn block_insns(&self, _: Block) -> InstRange { + InstRange::new(Inst::new(0), Inst::new(self.operands.len())) + } + fn block_succs(&self, _: Block) -> &[Block] { + &[] + } + fn block_preds(&self, _: Block) -> &[Block] { + &[] + } + fn block_params(&self, _: Block) -> &[VReg] { + &[] + } + fn is_ret(&self, insn: Inst) -> bool { + insn.index() + 1 == self.operands.len() + } + fn is_branch(&self, _: Inst) -> bool { + false + } + fn branch_blockparams(&self, _: Block, _: Inst, _: usize) -> &[VReg] { + &[] + } + fn inst_operands(&self, insn: Inst) -> &[Operand] { + &self.operands[insn.index()] + } + fn inst_clobbers(&self, insn: Inst) -> PRegSet { + self.clobbers[insn.index()] + } + fn num_vregs(&self) -> usize { + 2 + } + fn spillslot_size(&self, _: RegClass) -> usize { + 1 + } + } + + fn int_env(nregs: usize) -> MachineEnv { + let mut regs = PRegSet::empty(); + for hw in 0..nregs { + regs.add(PReg::new(hw, RegClass::Int)); + } + MachineEnv { + preferred_regs_by_class: [regs, PRegSet::empty(), PRegSet::empty()], + non_preferred_regs_by_class: [PRegSet::empty(); 3], + scratch_by_class: [None, None, None], + fixed_stack_slots: vec![], + } + } + + fn int_op(vreg: usize, constraint: OperandConstraint, kind: OperandKind) -> Operand { + Operand::new( + VReg::new(vreg, RegClass::Int), + constraint, + kind, + match kind { + OperandKind::Use => OperandPos::Early, + OperandKind::Def => OperandPos::Late, + }, + ) + } + + #[test] + fn fixed_def_conflicting_with_clobber_is_an_error() { + let p = |hw| PReg::new(hw, RegClass::Int); + let mut clobber = PRegSet::empty(); + for hw in 0..3 { + clobber.add(p(hw)); + } + let func = ConflictFunc { + operands: vec![ + vec![int_op(0, OperandConstraint::Any, OperandKind::Def)], + vec![ + int_op(1, OperandConstraint::FixedReg(p(0)), OperandKind::Def), + int_op(0, OperandConstraint::FixedReg(p(1)), OperandKind::Use), + ], + vec![int_op( + 1, + OperandConstraint::FixedReg(p(0)), + OperandKind::Use, + )], + ], + clobbers: vec![PRegSet::empty(), clobber, PRegSet::empty()], + }; + let env = int_env(3); + let opts = RegallocOptions { + verbose_log: false, + validate_ssa: true, + algorithm: Algorithm::Ion, + }; + let err = crate::run(&func, &env, &opts).unwrap_err(); + assert!( + matches!(err, RegAllocError::TooManyLiveRegs), + "expected TooManyLiveRegs, got {:?}", + err + ); + } +} diff --git a/src/ion/process.rs b/src/ion/process.rs index a6e5db90..57b81e69 100644 --- a/src/ion/process.rs +++ b/src/ion/process.rs @@ -1214,6 +1214,16 @@ impl<'a, F: Function> Env<'a, F> { || lowest_cost_evict_conflict_cost.is_none() || lowest_cost_evict_conflict_cost.unwrap() >= our_spill_weight) { + // A minimal bundle pinned to one physical register cannot + // move, and a fixed reservation on that register (a clobber + // is modeled as one) cannot be evicted. The overlap is + // illegal: a clobber must not collide with a fixed def or + // late use. Reject it instead of panicking. + if matches!(req, Requirement::FixedReg(_)) + && lowest_cost_evict_conflict_cost.is_none() + { + return Err(RegAllocError::TooManyLiveRegs); + } 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; From 1e777241ebddf9caa0897df7b4d492ac3162f3e6 Mon Sep 17 00:00:00 2001 From: Demetrios Agourakis Date: Sun, 4 Oct 2026 23:32:11 -0300 Subject: [PATCH 2/2] fuzzing: cover fixed-def and clobber conflicts --- src/fuzzing/func.rs | 31 ++++++++++++ src/fuzzing/ion.rs | 31 ++++++++++-- src/ion/mod.rs | 121 -------------------------------------------- 3 files changed, 57 insertions(+), 126 deletions(-) diff --git a/src/fuzzing/func.rs b/src/fuzzing/func.rs index b2a4817f..c936d10f 100644 --- a/src/fuzzing/func.rs +++ b/src/fuzzing/func.rs @@ -367,6 +367,7 @@ pub struct Options { pub fixed_regs: bool, pub fixed_nonallocatable: bool, pub clobbers: bool, + pub fixed_def_clobbers: bool, pub reftypes: bool, pub callsite_ish_constraints: bool, pub num_blocks: RangeInclusive, @@ -383,6 +384,7 @@ impl Options { fixed_regs: false, fixed_nonallocatable: false, clobbers: false, + fixed_def_clobbers: false, reftypes: false, callsite_ish_constraints: false, num_blocks: 1..=100, @@ -582,6 +584,19 @@ impl Func { ))); } + if opts.fixed_def_clobbers && bool::arbitrary(u)? { + // Exercise an impossible fixed output, not just allocatable functions. + if let (OperandKind::Def, OperandPos::Late, OperandConstraint::FixedReg(preg)) = ( + operands[0].kind(), + operands[0].pos(), + operands[0].constraint(), + ) { + if preg.hw_enc() < 32 { + clobbers.push(preg); + } + } + } + builder.add_inst( Block::new(block), InstData { @@ -638,6 +653,22 @@ impl Func { Ok(builder.finalize()) } + + pub fn has_fixed_def_clobber(&self) -> bool { + self.insts.iter().any(|inst| { + inst.clobbers + .iter() + .any(|&preg| inst.operands.iter().any(has_fixed_def_with(preg))) + }) + } + + pub fn remove_fixed_def_clobbers(&mut self) { + for inst in &mut self.insts { + let operands = &inst.operands; + inst.clobbers + .retain(|&preg| !operands.iter().any(has_fixed_def_with(preg))); + } + } } impl core::fmt::Debug for Func { diff --git a/src/fuzzing/ion.rs b/src/fuzzing/ion.rs index e64b5a31..81167dc0 100644 --- a/src/fuzzing/ion.rs +++ b/src/fuzzing/ion.rs @@ -1,6 +1,6 @@ //! Fuzz the `ion` register allocator. -use crate::{checker, fuzzing::func, ion}; +use crate::{checker, fuzzing::func, ion, RegAllocError}; use arbitrary::{Arbitrary, Result, Unstructured}; use core::cell::RefCell; use std::thread_local; @@ -11,6 +11,7 @@ const OPTIONS: func::Options = func::Options { fixed_regs: true, fixed_nonallocatable: true, clobbers: true, + fixed_def_clobbers: true, reftypes: true, callsite_ish_constraints: true, ..func::Options::DEFAULT @@ -53,18 +54,38 @@ pub fn check(t: TestCase) { log::trace!("func:\n{func:?}"); let env = func::machine_env(); + let allocatable_func = if func.has_fixed_def_clobber() { + let mut allocatable_func = func.clone(); + allocatable_func.remove_fixed_def_clobbers(); + Some(allocatable_func) + } else { + None + }; + let valid_func = allocatable_func.as_ref().unwrap_or(func); thread_local! { // We test that ctx is cleared properly between runs. static CTX: RefCell = RefCell::default(); } CTX.with(|ctx| { - ion::run(func, &env, &mut *ctx.borrow_mut(), *annotate, *check_ssa) + let mut ctx = ctx.borrow_mut(); + ion::run(valid_func, &env, &mut ctx, *annotate, *check_ssa) .expect("regalloc did not succeed"); - let mut checker = checker::Checker::new(func, &env); - checker.prepare(&ctx.borrow().output); - checker.run().expect("checker failed"); + { + let mut checker = checker::Checker::new(valid_func, &env); + checker.prepare(&ctx.output); + checker.run().expect("checker failed"); + } + + if allocatable_func.is_some() { + let result = ion::run(func, &env, &mut ctx, *annotate, *check_ssa); + assert!( + matches!(result, Err(RegAllocError::TooManyLiveRegs)), + "expected TooManyLiveRegs for a fixed-def/clobber conflict, got {:?}", + result + ); + } }); } diff --git a/src/ion/mod.rs b/src/ion/mod.rs index 9731c7f3..99181191 100644 --- a/src/ion/mod.rs +++ b/src/ion/mod.rs @@ -132,124 +132,3 @@ pub fn run( Ok(()) } - -#[cfg(test)] -mod tests { - use crate::{ - Algorithm, Block, Function, Inst, InstRange, MachineEnv, Operand, OperandConstraint, - OperandKind, OperandPos, PReg, PRegSet, RegAllocError, RegClass, RegallocOptions, VReg, - Vec, - }; - use alloc::vec; - - struct ConflictFunc { - operands: Vec>, - clobbers: Vec, - } - - impl Function for ConflictFunc { - fn num_insts(&self) -> usize { - self.operands.len() - } - fn num_blocks(&self) -> usize { - 1 - } - fn entry_block(&self) -> Block { - Block::new(0) - } - fn block_insns(&self, _: Block) -> InstRange { - InstRange::new(Inst::new(0), Inst::new(self.operands.len())) - } - fn block_succs(&self, _: Block) -> &[Block] { - &[] - } - fn block_preds(&self, _: Block) -> &[Block] { - &[] - } - fn block_params(&self, _: Block) -> &[VReg] { - &[] - } - fn is_ret(&self, insn: Inst) -> bool { - insn.index() + 1 == self.operands.len() - } - fn is_branch(&self, _: Inst) -> bool { - false - } - fn branch_blockparams(&self, _: Block, _: Inst, _: usize) -> &[VReg] { - &[] - } - fn inst_operands(&self, insn: Inst) -> &[Operand] { - &self.operands[insn.index()] - } - fn inst_clobbers(&self, insn: Inst) -> PRegSet { - self.clobbers[insn.index()] - } - fn num_vregs(&self) -> usize { - 2 - } - fn spillslot_size(&self, _: RegClass) -> usize { - 1 - } - } - - fn int_env(nregs: usize) -> MachineEnv { - let mut regs = PRegSet::empty(); - for hw in 0..nregs { - regs.add(PReg::new(hw, RegClass::Int)); - } - MachineEnv { - preferred_regs_by_class: [regs, PRegSet::empty(), PRegSet::empty()], - non_preferred_regs_by_class: [PRegSet::empty(); 3], - scratch_by_class: [None, None, None], - fixed_stack_slots: vec![], - } - } - - fn int_op(vreg: usize, constraint: OperandConstraint, kind: OperandKind) -> Operand { - Operand::new( - VReg::new(vreg, RegClass::Int), - constraint, - kind, - match kind { - OperandKind::Use => OperandPos::Early, - OperandKind::Def => OperandPos::Late, - }, - ) - } - - #[test] - fn fixed_def_conflicting_with_clobber_is_an_error() { - let p = |hw| PReg::new(hw, RegClass::Int); - let mut clobber = PRegSet::empty(); - for hw in 0..3 { - clobber.add(p(hw)); - } - let func = ConflictFunc { - operands: vec![ - vec![int_op(0, OperandConstraint::Any, OperandKind::Def)], - vec![ - int_op(1, OperandConstraint::FixedReg(p(0)), OperandKind::Def), - int_op(0, OperandConstraint::FixedReg(p(1)), OperandKind::Use), - ], - vec![int_op( - 1, - OperandConstraint::FixedReg(p(0)), - OperandKind::Use, - )], - ], - clobbers: vec![PRegSet::empty(), clobber, PRegSet::empty()], - }; - let env = int_env(3); - let opts = RegallocOptions { - verbose_log: false, - validate_ssa: true, - algorithm: Algorithm::Ion, - }; - let err = crate::run(&func, &env, &opts).unwrap_err(); - assert!( - matches!(err, RegAllocError::TooManyLiveRegs), - "expected TooManyLiveRegs, got {:?}", - err - ); - } -}