diff --git a/src/checker.rs b/src/checker.rs index 4adecb3..fa638cc 100644 --- a/src/checker.rs +++ b/src/checker.rs @@ -394,6 +394,11 @@ impl CheckerState { .insert(*alloc, CheckerValue::Reg(*vreg, reftyped)); } } + &CheckerInst::DefAlloc { alloc, vreg } => { + let reftyped = checker.reftyped_vregs.contains(&vreg); + self.allocations + .insert(alloc, CheckerValue::Reg(vreg, reftyped)); + } &CheckerInst::Safepoint { ref slots, .. } => { for (alloc, value) in &mut self.allocations { if let CheckerValue::Reg(_, true) = *value { @@ -475,6 +480,11 @@ pub(crate) enum CheckerInst { allocs: Vec, }, + /// Define an allocation's contents. Like BlockParams but for one + /// allocation. Used sometimes when moves are elided but ownership + /// of a value is logically transferred to a new vreg. + DefAlloc { alloc: Allocation, vreg: VReg }, + /// A safepoint, with the given SpillSlots specified as containing /// reftyped values. All other reftyped values become invalid. Safepoint { inst: Inst, slots: Vec }, @@ -583,17 +593,23 @@ impl<'a, F: Function> Checker<'a, F> { } debug!("checker: adding edit {:?} at pos {:?}", edit, pos); match edit { - &Edit::Move { from, to, .. } => { + &Edit::Move { from, to, to_vreg } => { self.bb_insts .get_mut(&block) .unwrap() .push(CheckerInst::Move { into: to, from }); + if let Some(vreg) = to_vreg { + self.bb_insts + .get_mut(&block) + .unwrap() + .push(CheckerInst::DefAlloc { alloc: to, vreg }); + } } - &Edit::DefAlloc { .. } => { - unimplemented!(concat!( - "DefAlloc is used only when dealing with pinned vregs, ", - "which are only used by regalloc.rs shim; use checker at that level!" - )); + &Edit::DefAlloc { alloc, vreg } => { + self.bb_insts + .get_mut(&block) + .unwrap() + .push(CheckerInst::DefAlloc { alloc, vreg }); } &Edit::BlockParams { ref vregs, @@ -732,6 +748,9 @@ impl<'a, F: Function> Checker<'a, F> { } debug!(" blockparams: {}", args.join(", ")); } + &CheckerInst::DefAlloc { alloc, vreg } => { + debug!(" defalloc: {}:{}", vreg, alloc); + } &CheckerInst::Safepoint { ref slots, .. } => { let mut slotargs = vec![]; for &slot in slots { diff --git a/src/ion/mod.rs b/src/ion/mod.rs index 5be5406..34c026a 100644 --- a/src/ion/mod.rs +++ b/src/ion/mod.rs @@ -3849,8 +3849,6 @@ impl<'a, F: Function> Env<'a, F> { None }; - let mut clean_spillslot: Option = None; - // For each range in each vreg, insert moves or // half-moves. We also scan over `blockparam_ins` and // `blockparam_outs`, which are sorted by (block, vreg), @@ -3925,19 +3923,9 @@ impl<'a, F: Function> Env<'a, F> { self.ranges[entry.index.index()].has_flag(LiveRangeFlag::StartsAtDef); debug_assert!(prev_alloc != Allocation::none()); - // If this is a stack-to-reg move, track that the reg is a clean copy of a spillslot. - if prev_alloc.is_stack() && alloc.is_reg() { - clean_spillslot = Some(prev_alloc.as_stack().unwrap()); - } - // If this is a reg-to-stack move, elide it if the spillslot is still clean. - let skip_spill = prev_alloc.is_reg() - && alloc.is_stack() - && clean_spillslot == alloc.as_stack(); - if prev_range.to == range.from && !self.is_start_of_block(range.from) && !first_is_def - && !skip_spill { log::debug!( "prev LR {} abuts LR {} in same block; moving {} -> {} for v{}", @@ -3958,33 +3946,6 @@ impl<'a, F: Function> Env<'a, F> { } } - // If this range either spans any block boundary, or - // has any mods/defs, then the spillslot (if any) that - // its value came from is no longer 'clean'. - if clean_spillslot.is_some() { - if self.cfginfo.insn_block[range.from.inst().index()] - != self.cfginfo.insn_block[range.to.prev().inst().index()] - || range.from - == self.cfginfo.block_entry - [self.cfginfo.insn_block[range.from.inst().index()].index()] - { - clean_spillslot = None; - } else if self.ranges[entry.index.index()].has_flag(LiveRangeFlag::StartsAtDef) - { - clean_spillslot = None; - } else { - for u in &self.ranges[entry.index.index()].uses { - match u.operand.kind() { - OperandKind::Def | OperandKind::Mod => { - clean_spillslot = None; - break; - } - _ => {} - } - } - } - } - // The block-to-block edge-move logic is not // applicable to pinned vregs, which are always in one // PReg (so never need moves within their own vreg @@ -4540,6 +4501,132 @@ impl<'a, F: Function> Env<'a, F> { let mut i = 0; self.inserted_moves .sort_unstable_by_key(|m| (m.pos.to_index(), m.prio)); + + // Simple redundant-spill/reload removal: track which + // stackslot each preg is a clean copy of, if any, and remove + // spills and reloads that are unnecessary. + let mut preg_state: [(Block, SpillSlot); PReg::MAX_INDEX] = + [(Block::new(0), SpillSlot::invalid()); PReg::MAX_INDEX]; + + fn process_move_maybe_elide( + preg_state: &mut [(Block, SpillSlot); PReg::MAX_INDEX], + block: Block, + from: Allocation, + to: Allocation, + ) -> bool { + if from.is_reg() && to.is_reg() { + let from = from.as_reg().unwrap().hw_enc(); + let to = to.as_reg().unwrap().hw_enc(); + if preg_state[to] == preg_state[from] + && preg_state[from].1.is_valid() + && preg_state[from].0 == block + { + true + } else { + preg_state[to] = preg_state[from]; + false + } + } else if from.is_stack() && to.is_stack() { + let from = from.as_stack().unwrap(); + let to = to.as_stack().unwrap(); + update_copies_of_slot(preg_state, to, from); + false + } else if from.is_reg() && to.is_stack() { + // Reg-to-stack spill: determine if value in slot is + // still up-to-date, and skip the store if so. + let from = from.as_reg().unwrap().hw_enc(); + let to = to.as_stack().unwrap(); + if preg_state[from] == (block, to) { + // Redundant spill: elide. + true + } else { + // Don't elide; but this register now has a clean + // copy of the slot (because its value is now + // being written to the slot). However, no other + // register that previously had a clean copy of + // this slot does anymore. + update_copies_of_slot(preg_state, to, SpillSlot::invalid()); + preg_state[from] = (block, to); + false + } + } else { + // Stack-to-reg reload: determine if reg is already an + // up-to-date copy of value in slot, and skip reload + // if so. Otherwise, don't skip reload, and update + // metadata to indicate reg is now up-to-date. + let from = from.as_stack().unwrap(); + let to = to.as_reg().unwrap().hw_enc(); + if preg_state[to] == (block, from) { + true + } else { + preg_state[to] = (block, from); + false + } + } + } + fn update_copies_of_slot( + preg_state: &mut [(Block, SpillSlot); PReg::MAX_INDEX], + slot: SpillSlot, + updated: SpillSlot, + ) { + for entry in preg_state.iter_mut() { + if entry.1 == slot { + entry.1 = updated; + } + } + } + fn clear_preg_state<'a, F: Function>( + this: &Env<'a, F>, + preg_state: &mut [(Block, SpillSlot); PReg::MAX_INDEX], + from: ProgPoint, + to: ProgPoint, + ) { + let start_inst = if from.pos() == InstPosition::Before { + from.inst() + } else { + from.inst().next() + }; + let end_inst = if to.pos() == InstPosition::Before { + to.inst() + } else { + to.inst().next() + }; + for inst in start_inst.index()..end_inst.index() { + let inst = Inst::new(inst); + + if this.func.is_safepoint(inst) { + for entry in preg_state.iter_mut() { + entry.1 = SpillSlot::invalid(); + } + continue; + } + + for (i, op) in this.func.inst_operands(inst).iter().enumerate() { + match op.kind() { + OperandKind::Def | OperandKind::Use => { + let alloc = this.get_alloc(inst, i); + if let Some(preg) = alloc.as_reg() { + preg_state[preg.hw_enc()].1 = SpillSlot::invalid(); + } else if let Some(stack) = alloc.as_stack() { + update_copies_of_slot(preg_state, stack, SpillSlot::invalid()); + } + } + _ => {} + } + } + for reg in this.func.inst_clobbers(inst) { + preg_state[reg.hw_enc()].1 = SpillSlot::invalid(); + } + } + if to.pos() == InstPosition::Before && this.func.is_safepoint(to.inst()) { + for entry in preg_state.iter_mut() { + entry.1 = SpillSlot::invalid(); + } + } + } + + let mut last_pos = ProgPoint::before(Inst::new(0)); + while i < self.inserted_moves.len() { let start = i; let pos = self.inserted_moves[i].pos; @@ -4552,6 +4639,11 @@ impl<'a, F: Function> Env<'a, F> { } let moves = &self.inserted_moves[start..i]; + let block = self.cfginfo.insn_block[pos.inst().index()]; + + clear_preg_state(self, &mut preg_state, last_pos, pos); + last_pos = pos; + // Gather all the moves with Int class and Float class // separately. These cannot interact, so it is safe to // have two separate ParallelMove instances. They need to @@ -4616,6 +4708,13 @@ impl<'a, F: Function> Env<'a, F> { for (src, dst, to_vreg) in resolved { log::debug!(" resolved: {} -> {} ({:?})", src, dst, to_vreg); + if process_move_maybe_elide(&mut preg_state, block, src, dst) { + log::debug!(" -> eliding (redundant)!"); + if let Some(vreg) = to_vreg { + self.add_edit(pos, prio, Edit::DefAlloc { alloc: dst, vreg }); + } + continue; + } self.add_edit( pos, prio, diff --git a/src/lib.rs b/src/lib.rs index c96004f..7941ebe 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -43,7 +43,7 @@ pub struct PReg(u8, RegClass); impl PReg { pub const MAX_BITS: usize = 5; pub const MAX: usize = (1 << Self::MAX_BITS) - 1; - pub const MAX_INDEX: usize = 2 * Self::MAX; // including RegClass bit + pub const MAX_INDEX: usize = 1 << (Self::MAX_BITS + 1); // including RegClass bit /// Create a new PReg. The `hw_enc` range is 6 bits. #[inline(always)] @@ -188,6 +188,19 @@ impl SpillSlot { pub fn plus(self, offset: usize) -> Self { SpillSlot::new(self.index() + offset, self.class()) } + + #[inline(always)] + pub fn invalid() -> Self { + SpillSlot(0xffff_ffff) + } + #[inline(always)] + pub fn is_invalid(self) -> bool { + self == Self::invalid() + } + #[inline(always)] + pub fn is_valid(self) -> bool { + self != Self::invalid() + } } impl std::fmt::Display for SpillSlot {