From 92cf6620e42219a8a5952808b32d7f12634a684a Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Sun, 26 Jun 2022 10:56:26 -0700 Subject: [PATCH 1/2] 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. --- 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 | 84 ++++++++++++++++++++++++++++++++++++++++--- 6 files changed, 91 insertions(+), 13 deletions(-) diff --git a/src/checker.rs b/src/checker.rs index 952f6123..1d905ac7 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 7956d0cd..ef8e9020 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, PRegMask, 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) -> PRegMask { + let mut mask = PRegMask::default(); + for &preg in &self.insts[insn.index()].clobbers { + mask = mask.with(preg); + } + mask } fn num_vregs(&self) -> usize { diff --git a/src/ion/dump.rs b/src/ion/dump.rs index d57d59cd..ba4f74f5 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 350002d6..992f297a 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 3da42fe1..3e01417b 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 95ca1e5e..70991ee7 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,75 @@ impl std::fmt::Display for PReg { } } +/// A physical register mask: a set of physical registers. Used to +/// represent clobbers efficiently. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Default)] +#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))] +pub struct PRegMask { + bits: u128, +} + +impl PRegMask { + /// Create an empty mask. + pub const fn empty() -> Self { + Self { bits: 0 } + } + + /// Add a physical register (PReg) to the mask, 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 mask. + 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 mask. + 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 clobber mask to this one, + /// mutating in place. + pub fn add_all(&mut self, other: PRegMask) { + self.bits |= other.bits; + } +} + +impl IntoIterator for PRegMask { + type Item = PReg; + type IntoIter = PRegMaskIter; + fn into_iter(self) -> PRegMaskIter { + PRegMaskIter { bits: self.bits } + } +} + +pub struct PRegMaskIter { + bits: u128, +} + +impl Iterator for PRegMaskIter { + 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 +267,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 +1041,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 mask determined by the ABI, but + /// some of those clobbered registers are sometimes return + /// value(s). + fn inst_clobbers(&self, insn: Inst) -> PRegMask; /// Get the number of `VReg` in use in this function. fn num_vregs(&self) -> usize; From 86475ef05f96286554a7f39bf7e34fbca6785c60 Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Mon, 27 Jun 2022 11:56:20 -0700 Subject: [PATCH 2/2] Review comments. --- src/fuzzing/func.rs | 10 +++++----- src/lib.rs | 41 ++++++++++++++++++++++------------------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/src/fuzzing/func.rs b/src/fuzzing/func.rs index ef8e9020..47899d5e 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, PRegMask, RegClass, VReg, + OperandConstraint, OperandKind, OperandPos, PReg, PRegSet, RegClass, VReg, }; use arbitrary::Result as ArbitraryResult; @@ -132,12 +132,12 @@ impl Function for Func { &self.insts[insn.index()].operands[..] } - fn inst_clobbers(&self, insn: Inst) -> PRegMask { - let mut mask = PRegMask::default(); + fn inst_clobbers(&self, insn: Inst) -> PRegSet { + let mut set = PRegSet::default(); for &preg in &self.insts[insn.index()].clobbers { - mask = mask.with(preg); + set = set.with(preg); } - mask + set } fn num_vregs(&self) -> usize { diff --git a/src/lib.rs b/src/lib.rs index 70991ee7..be7552c1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -165,21 +165,24 @@ impl std::fmt::Display for PReg { } } -/// A physical register mask: a set of physical registers. Used to -/// represent clobbers efficiently. +/// 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 PRegMask { +pub struct PRegSet { bits: u128, } -impl PRegMask { - /// Create an empty mask. +impl PRegSet { + /// Create an empty set. pub const fn empty() -> Self { Self { bits: 0 } } - /// Add a physical register (PReg) to the mask, returning the new value. + /// 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); @@ -188,40 +191,40 @@ impl PRegMask { } } - /// Add a physical register (PReg) to the mask. + /// 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 mask. + /// 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 clobber mask to this one, - /// mutating in place. - pub fn add_all(&mut self, other: PRegMask) { + /// 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 PRegMask { +impl IntoIterator for PRegSet { type Item = PReg; - type IntoIter = PRegMaskIter; - fn into_iter(self) -> PRegMaskIter { - PRegMaskIter { bits: self.bits } + type IntoIter = PRegSetIter; + fn into_iter(self) -> PRegSetIter { + PRegSetIter { bits: self.bits } } } -pub struct PRegMaskIter { +pub struct PRegSetIter { bits: u128, } -impl Iterator for PRegMaskIter { +impl Iterator for PRegSetIter { type Item = PReg; fn next(&mut self) -> Option { if self.bits == 0 { @@ -1045,10 +1048,10 @@ pub trait Function { /// 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 mask determined by the ABI, but + /// 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) -> PRegMask; + fn inst_clobbers(&self, insn: Inst) -> PRegSet; /// Get the number of `VReg` in use in this function. fn num_vregs(&self) -> usize;