From 9733cb2227a24f5956dfc46377abb948f9d34e4e Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Mon, 27 Jun 2022 12:27:19 -0700 Subject: [PATCH] Clobbers: use a more efficient bitmask representation in API. (#58) * Clobbers: use a more efficient bitmask representation in API. Currently, the `Function` trait requires a `&[PReg]` for the clobber-list for a given instruction. In most cases where clobbers are used, the list may be long: e.g., ABIs specify a fixed set of registers that are clobbered and there may be ~half of all registers in this list. What's more, the list can't be shared for e.g. all calls of a given ABI, because actual return-values (defs) can't be clobbers. So we need to allocate space for long, sometimes-slightly-different lists; this is inefficient for the embedder and for us. It's much more efficient to use a bitmask to represent a set of physical registers. With current data structure bitpacking limitations, we can support at most 128 physical registers; this means we can use a `u128` bitmask. This also allows e.g. an embedder to start with a constant for a given ABI, and mask out bits for actual return-value registers on call instructions. This PR makes that change, for minor but positive performance impact. * Review comments. --- src/checker.rs | 2 +- src/fuzzing/func.rs | 10 +++-- src/ion/dump.rs | 2 +- src/ion/liveranges.rs | 4 +- src/ion/moves.rs | 2 +- src/lib.rs | 87 +++++++++++++++++++++++++++++++++++++++++-- 6 files changed, 94 insertions(+), 13 deletions(-) diff --git a/src/checker.rs b/src/checker.rs index 952f612..1d905ac 100644 --- a/src/checker.rs +++ b/src/checker.rs @@ -935,7 +935,7 @@ impl<'a, F: Function> Checker<'a, F> { else if !self.f.is_branch(inst) { let operands: Vec<_> = self.f.inst_operands(inst).iter().cloned().collect(); let allocs: Vec<_> = out.inst_allocs(inst).iter().cloned().collect(); - let clobbers: Vec<_> = self.f.inst_clobbers(inst).iter().cloned().collect(); + let clobbers: Vec<_> = self.f.inst_clobbers(inst).into_iter().collect(); let checkinst = CheckerInst::Op { inst, operands, diff --git a/src/fuzzing/func.rs b/src/fuzzing/func.rs index 7956d0c..47899d5 100644 --- a/src/fuzzing/func.rs +++ b/src/fuzzing/func.rs @@ -5,7 +5,7 @@ use crate::{ domtree, postorder, Allocation, Block, Function, Inst, InstRange, MachineEnv, Operand, - OperandConstraint, OperandKind, OperandPos, PReg, RegClass, VReg, + OperandConstraint, OperandKind, OperandPos, PReg, PRegSet, RegClass, VReg, }; use arbitrary::Result as ArbitraryResult; @@ -132,8 +132,12 @@ impl Function for Func { &self.insts[insn.index()].operands[..] } - fn inst_clobbers(&self, insn: Inst) -> &[PReg] { - &self.insts[insn.index()].clobbers[..] + fn inst_clobbers(&self, insn: Inst) -> PRegSet { + let mut set = PRegSet::default(); + for &preg in &self.insts[insn.index()].clobbers { + set = set.with(preg); + } + set } fn num_vregs(&self) -> usize { diff --git a/src/ion/dump.rs b/src/ion/dump.rs index d57d59c..ba4f74f 100644 --- a/src/ion/dump.rs +++ b/src/ion/dump.rs @@ -95,7 +95,7 @@ impl<'a, F: Function> Env<'a, F> { let clobbers = self .func .inst_clobbers(inst) - .iter() + .into_iter() .map(|preg| format!("{}", preg)) .collect::>(); let allocs = (0..ops.len()) diff --git a/src/ion/liveranges.rs b/src/ion/liveranges.rs index df612ee..02dfa33 100644 --- a/src/ion/liveranges.rs +++ b/src/ion/liveranges.rs @@ -491,9 +491,7 @@ impl<'a, F: Function> Env<'a, F> { // operands and clobbers. for inst in insns.rev().iter() { // Mark clobbers with CodeRanges on PRegs. - for i in 0..self.func.inst_clobbers(inst).len() { - // don't borrow `self` - let clobber = self.func.inst_clobbers(inst)[i]; + for clobber in self.func.inst_clobbers(inst) { // Clobber range is at After point only: an // instruction can still take an input in a reg // that it later clobbers. (In other words, the diff --git a/src/ion/moves.rs b/src/ion/moves.rs index 3da42fe..3e01417 100644 --- a/src/ion/moves.rs +++ b/src/ion/moves.rs @@ -950,7 +950,7 @@ impl<'a, F: Function> Env<'a, F> { } } for reg in this.func.inst_clobbers(inst) { - redundant_moves.clear_alloc(Allocation::reg(*reg)); + redundant_moves.clear_alloc(Allocation::reg(reg)); } } } diff --git a/src/lib.rs b/src/lib.rs index 95ca1e5..be7552c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -123,13 +123,13 @@ impl PReg { /// all physical registers. Allows one to maintain an array of data for /// all PRegs and index it efficiently. #[inline(always)] - pub fn index(self) -> usize { + pub const fn index(self) -> usize { self.bits as usize } /// Construct a PReg from the value returned from `.index()`. #[inline(always)] - pub fn from_index(index: usize) -> Self { + pub const fn from_index(index: usize) -> Self { PReg { bits: (index & (Self::NUM_INDEX - 1)) as u8, } @@ -165,6 +165,78 @@ impl std::fmt::Display for PReg { } } +/// A physical register set. Used to represent clobbers +/// efficiently. +/// +/// The set is `Copy` and is guaranteed to have constant, and small, +/// size, as it is based on a bitset internally. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Default)] +#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))] +pub struct PRegSet { + bits: u128, +} + +impl PRegSet { + /// Create an empty set. + pub const fn empty() -> Self { + Self { bits: 0 } + } + + /// Add a physical register (PReg) to the set, returning the new value. + pub const fn with(self, reg: PReg) -> Self { + let bit = reg.index(); + debug_assert!(bit < 128); + Self { + bits: self.bits | (1u128 << bit), + } + } + + /// Add a physical register (PReg) to the set. + pub fn add(&mut self, reg: PReg) { + let bit = reg.index(); + debug_assert!(bit < 128); + self.bits |= 1u128 << bit; + } + + /// Remove a physical register (PReg) from the set. + pub fn remove(&mut self, reg: PReg) { + let bit = reg.index(); + debug_assert!(bit < 128); + self.bits &= !(1u128 << bit); + } + + /// Add all of the registers in one set to this one, mutating in + /// place. + pub fn union_from(&mut self, other: PRegSet) { + self.bits |= other.bits; + } +} + +impl IntoIterator for PRegSet { + type Item = PReg; + type IntoIter = PRegSetIter; + fn into_iter(self) -> PRegSetIter { + PRegSetIter { bits: self.bits } + } +} + +pub struct PRegSetIter { + bits: u128, +} + +impl Iterator for PRegSetIter { + type Item = PReg; + fn next(&mut self) -> Option { + if self.bits == 0 { + None + } else { + let index = self.bits.trailing_zeros(); + self.bits &= !(1u128 << index); + Some(PReg::from_index(index as usize)) + } + } +} + /// A virtual register. Contains a virtual register number and a /// class. /// @@ -198,7 +270,7 @@ impl VReg { } #[inline(always)] - pub fn vreg(self) -> usize { + pub const fn vreg(self) -> usize { let vreg = (self.bits >> 1) as usize; vreg } @@ -972,7 +1044,14 @@ pub trait Function { /// fixed physical registers written by an instruction but not /// used as a vreg output, or fixed physical registers used as /// temps within an instruction out of necessity. - fn inst_clobbers(&self, insn: Inst) -> &[PReg]; + /// + /// Note that it is legal for a register to be both a clobber and + /// an actual def (via pinned vreg or via operand constrained to + /// the reg). This is for convenience: e.g., a call instruction + /// might have a constant clobber set determined by the ABI, but + /// some of those clobbered registers are sometimes return + /// value(s). + fn inst_clobbers(&self, insn: Inst) -> PRegSet; /// Get the number of `VReg` in use in this function. fn num_vregs(&self) -> usize;