diff --git a/src/ion/data_structures.rs b/src/ion/data_structures.rs index cf92d5a..5e8fac4 100644 --- a/src/ion/data_structures.rs +++ b/src/ion/data_structures.rs @@ -53,6 +53,14 @@ impl CodeRange { pub fn len(&self) -> usize { self.to.inst().index() - self.from.inst().index() } + /// Returns the range covering just one program point. + #[inline(always)] + pub fn singleton(pos: ProgPoint) -> CodeRange { + CodeRange { + from: pos, + to: pos.next(), + } + } } impl std::cmp::PartialOrd for CodeRange { @@ -185,7 +193,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; @@ -197,13 +205,15 @@ impl LiveBundle { spill_weight: u32, 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 stack { 1 << 29 } else { 0 }); + | (if fixed_def { 1 << 29 } else { 0 }) + | (if stack { 1 << 28 } else { 0 }); } #[inline(always)] @@ -217,23 +227,33 @@ impl LiveBundle { } #[inline(always)] - pub fn cached_stack(&self) -> bool { + pub fn cached_fixed_def(&self) -> bool { 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; } #[inline(always)] - pub fn set_cached_stack(&mut self) { + pub fn set_cached_fixed_def(&mut self) { 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 & ((1 << 29) - 1) + self.spill_weight_and_props & ((1 << 28) - 1) } } diff --git a/src/ion/liveranges.rs b/src/ion/liveranges.rs index b717fcd..bdee6f1 100644 --- a/src/ion/liveranges.rs +++ b/src/ion/liveranges.rs @@ -935,11 +935,8 @@ impl<'a, F: Function> Env<'a, F> { // constraint, and (ii) move the def to Early position // to reserve the register for the whole instruction. let mut operand_rewrites: FxHashMap = FxHashMap::default(); - let mut late_def_fixed: SmallVec<[(PReg, Operand, usize); 2]> = smallvec![]; - for (i, &operand) in self.func.inst_operands(inst).iter().enumerate() { - if operand.as_fixed_nonallocatable().is_some() { - continue; - } + let mut late_def_fixed: SmallVec<[PReg; 8]> = smallvec![]; + for &operand in self.func.inst_operands(inst) { if let OperandConstraint::FixedReg(preg) = operand.constraint() { match operand.pos() { OperandPos::Late => { @@ -954,7 +951,7 @@ impl<'a, F: Function> Env<'a, F> { "Invalid operand: fixed constraint on Use/Mod at Late point" ); - late_def_fixed.push((preg, operand, i)); + late_def_fixed.push(preg); } _ => {} } @@ -966,19 +963,19 @@ impl<'a, F: Function> Env<'a, F> { } if let OperandConstraint::FixedReg(preg) = operand.constraint() { match operand.pos() { - OperandPos::Early => { + OperandPos::Early if live.get(operand.vreg().vreg()) => { assert!(operand.kind() == OperandKind::Use, "Invalid operand: fixed constraint on Def/Mod at Early position"); // If we have a constraint at the // Early point for a fixed preg, and // this preg is also constrained with - // a *separate* def at Late, and *if* - // the vreg is live downward, we have - // to use the multi-fixed-reg - // mechanism for a fixup and rewrite - // here without the constraint. See - // #53. + // a *separate* def at Late or is + // clobbered, and *if* the vreg is + // live downward, we have to use the + // multi-fixed-reg mechanism for a + // fixup and rewrite here without the + // constraint. See #53. // // We adjust the def liverange and Use // to an "early" position to reserve @@ -990,10 +987,18 @@ impl<'a, F: Function> Env<'a, F> { // conflicting constraints for the // same vreg in a separate pass (see // `fixup_multi_fixed_vregs` below). - if let Some((_, def_op, def_slot)) = late_def_fixed - .iter() - .find(|(def_preg, _, _)| *def_preg == preg) + if late_def_fixed.contains(&preg) + || self.func.inst_clobbers(inst).contains(preg) { + log::trace!( + concat!( + "-> operand {:?} is fixed to preg {:?}, ", + "is downward live, and there is also a ", + "def or clobber at this preg" + ), + operand, + preg + ); let pos = ProgPoint::before(inst); self.multi_fixed_reg_fixups.push(MultiFixedRegFixup { pos, @@ -1004,6 +1009,14 @@ impl<'a, F: Function> Env<'a, F> { level: FixedRegFixupLevel::Initial, }); + // We need to insert a reservation + // at the before-point to reserve + // the reg for the use too. + let range = CodeRange::singleton(pos); + self.add_liverange_to_preg(range, preg); + + // Remove the fixed-preg + // constraint from the Use. operand_rewrites.insert( i, Operand::new( @@ -1013,15 +1026,6 @@ impl<'a, F: Function> Env<'a, F> { operand.pos(), ), ); - operand_rewrites.insert( - *def_slot, - Operand::new( - def_op.vreg(), - def_op.constraint(), - def_op.kind(), - OperandPos::Early, - ), - ); } } _ => {} @@ -1310,7 +1314,7 @@ impl<'a, F: Function> Env<'a, F> { // have to split the multiple uses at the same progpoint into // different bundles, which breaks invariants related to // disjoint ranges and bundles). - let mut extra_clobbers: SmallVec<[(PReg, Inst); 8]> = smallvec![]; + let mut extra_clobbers: SmallVec<[(PReg, ProgPoint); 8]> = smallvec![]; for vreg in 0..self.vregs.len() { for range_idx in 0..self.vregs[vreg].ranges.len() { let entry = self.vregs[vreg].ranges[range_idx]; @@ -1419,15 +1423,15 @@ impl<'a, F: Function> Env<'a, F> { u.operand.pos(), ); trace!(" -> extra clobber {} at inst{}", preg, u.pos.inst().index()); - extra_clobbers.push((preg, u.pos.inst())); + extra_clobbers.push((preg, u.pos)); } } } - for &(clobber, inst) in &extra_clobbers { + for &(clobber, pos) in &extra_clobbers { let range = CodeRange { - from: ProgPoint::before(inst), - to: ProgPoint::before(inst.next()), + from: pos, + to: pos.next(), }; self.add_liverange_to_preg(range, clobber); } diff --git a/src/ion/merge.rs b/src/ion/merge.rs index c3a0216..ec685d3 100644 --- a/src/ion/merge.rs +++ b/src/ion/merge.rs @@ -16,7 +16,9 @@ use super::{ Env, LiveBundleIndex, LiveRangeIndex, LiveRangeKey, SpillSet, SpillSetIndex, SpillSlotIndex, VRegIndex, }; -use crate::{ion::data_structures::BlockparamOut, Function, Inst, OperandConstraint, PReg}; +use crate::{ + ion::data_structures::BlockparamOut, Function, Inst, OperandConstraint, OperandKind, PReg, +}; use smallvec::smallvec; impl<'a, F: Function> Env<'a, F> { @@ -93,6 +95,17 @@ impl<'a, F: Function> Env<'a, F> { } } + // Avoid merging if either side has a fixed-reg def: this can + // result in an impossible-to-solve allocation problem if + // there is a fixed-reg use in the same reg on the same + // instruction. + if self.bundles[from.index()].cached_fixed_def() + || self.bundles[to.index()].cached_fixed_def() + { + trace!(" -> one bundle has a fixed def; aborting merge"); + return false; + } + // Check for a requirements conflict. if self.bundles[from.index()].cached_stack() || self.bundles[from.index()].cached_fixed() @@ -258,16 +271,20 @@ 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.index()].ranges { for u in &self.ranges[entry.index.index()].uses { if let OperandConstraint::FixedReg(_) = u.operand.constraint() { fixed = true; + if u.operand.kind() == OperandKind::Def { + fixed_def = true; + } } if let OperandConstraint::Stack = u.operand.constraint() { stack = true; } - if fixed && stack { + if fixed && stack && fixed_def { break; } } @@ -275,6 +292,9 @@ impl<'a, F: Function> Env<'a, F> { if fixed { self.bundles[bundle.index()].set_cached_fixed(); } + if fixed_def { + self.bundles[bundle.index()].set_cached_fixed_def(); + } if stack { self.bundles[bundle.index()].set_cached_stack(); } diff --git a/src/ion/process.rs b/src/ion/process.rs index 2d7c15b..d412d3b 100644 --- a/src/ion/process.rs +++ b/src/ion/process.rs @@ -262,6 +262,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.bundles[bundle.index()]; let first_range = bundledata.ranges[0].index; @@ -277,11 +278,15 @@ impl<'a, F: Function> Env<'a, F> { for u in &first_range_data.uses { trace!(" -> use: {:?}", u); if let OperandConstraint::FixedReg(_) = u.operand.constraint() { - trace!(" -> fixed use at {:?}: {:?}", u.pos, u.operand); + trace!(" -> fixed operand at {:?}: {:?}", u.pos, u.operand); fixed = true; + if u.operand.kind() == OperandKind::Def { + trace!(" -> is fixed def"); + fixed_def = true; + } } if let OperandConstraint::Stack = u.operand.constraint() { - trace!(" -> stack use at {:?}: {:?}", u.pos, u.operand); + trace!(" -> stack operand at {:?}: {:?}", u.pos, u.operand); stack = true; } if stack && fixed { @@ -340,6 +345,7 @@ impl<'a, F: Function> Env<'a, F> { spill_weight, minimal, fixed, + fixed_def, stack, ); } @@ -794,6 +800,7 @@ impl<'a, F: Function> Env<'a, F> { for entry_idx in 0..self.bundles[bundle.index()].ranges.len() { // Iterate manually; don't borrow `self`. let entry = self.bundles[bundle.index()].ranges[entry_idx]; + let lr_to = entry.range.to; removed_lrs.insert(entry.index); let vreg = self.ranges[entry.index.index()].vreg; @@ -813,14 +820,17 @@ impl<'a, F: Function> Env<'a, F> { continue; } + // The minimal bundle runs through the whole inst + // (up to the Before of the next inst), *unless* + // the original LR was only over the Before (up to + // the After) of this inst. + let to = std::cmp::min(ProgPoint::before(u.pos.inst().next()), lr_to); + // If the last bundle was at the same inst, add a new // LR to the same bundle; otherwise, create a LR and a // new bundle. if Some(u.pos.inst()) == last_inst { - let cr = CodeRange { - from: u.pos, - to: ProgPoint::before(u.pos.inst().next()), - }; + let cr = CodeRange { from: u.pos, to }; let lr = self.create_liverange(cr); new_lrs.push((vreg, lr)); self.ranges[lr.index()].uses.push(u); @@ -853,10 +863,7 @@ impl<'a, F: Function> Env<'a, F> { // Otherwise, create a new LR. let pos = ProgPoint::before(u.pos.inst()); - let cr = CodeRange { - from: pos, - to: ProgPoint::before(u.pos.inst().next()), - }; + let cr = CodeRange { from: pos, to }; let lr = self.create_liverange(cr); new_lrs.push((vreg, lr)); self.ranges[lr.index()].uses.push(u); diff --git a/src/ion/requirement.rs b/src/ion/requirement.rs index f1c273a..4fa7260 100644 --- a/src/ion/requirement.rs +++ b/src/ion/requirement.rs @@ -130,7 +130,7 @@ impl<'a, F: Function> Env<'a, F> { trace!("compute_requirement: {:?}", bundle); let ranges = &self.bundles[bundle.index()].ranges; for entry in ranges { - trace!(" -> LR {:?}", entry.index); + trace!(" -> LR {:?}: {:?}", entry.index, entry.range); for u in &self.ranges[entry.index.index()].uses { trace!(" -> use {:?}", u); let r = self.requirement_from_operand(u.operand); diff --git a/src/ion/stackmap.rs b/src/ion/stackmap.rs index 517de15..108835a 100644 --- a/src/ion/stackmap.rs +++ b/src/ion/stackmap.rs @@ -55,6 +55,7 @@ impl<'a, F: Function> Env<'a, F> { safepoint_idx += 1; continue; } + trace!(" -> covers safepoint {:?}", safepoints[safepoint_idx]); self.safepoint_slots