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

@@ -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,

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] {
+    fn inst_clobbers(&self, insn: Inst) -> PRegSet {
-        &self.insts[insn.index()].clobbers[..]
+        let mut set = PRegSet::default();
+        for &preg in &self.insts[insn.index()].clobbers {
+            set = set.with(preg);
+        }
+        set
     }
 
     fn num_vregs(&self) -> usize {

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::<Vec<_>>();
                 let allocs = (0..ops.len())

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() {
+                for clobber in self.func.inst_clobbers(inst) {
-                    // don't borrow `self`
-                    let clobber = self.func.inst_clobbers(inst)[i];
                     // 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

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));
                 }
             }
         }

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<PReg> {
+        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;