From a6e312882190fe0cff1071f4f62cbac23f0d7a1d Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Fri, 7 May 2021 19:48:34 -0700 Subject: [PATCH] Support `mod` (modify) operands, for better efficiency with regalloc.rs/Cranelift shim. --- src/checker.rs | 6 ++-- src/ion/mod.rs | 76 ++++++++++++++++++++++++++++---------------------- src/lib.rs | 55 +++++++++++++++++++++++------------- src/ssa.rs | 6 ++++ 4 files changed, 87 insertions(+), 56 deletions(-) diff --git a/src/checker.rs b/src/checker.rs index 05f32f5..244a1e7 100644 --- a/src/checker.rs +++ b/src/checker.rs @@ -297,10 +297,8 @@ impl CheckerState { // the requirements of the OperandPolicy. for (op, alloc) in operands.iter().zip(allocs.iter()) { let is_here = match (op.pos(), pos) { - (OperandPos::Before, InstPosition::Before) - | (OperandPos::Both, InstPosition::Before) => true, - (OperandPos::After, InstPosition::After) - | (OperandPos::Both, InstPosition::After) => true, + (OperandPos::Before, InstPosition::Before) => true, + (OperandPos::After, InstPosition::After) => true, _ => false, }; if !is_here { diff --git a/src/ion/mod.rs b/src/ion/mod.rs index cc714a0..86ca609 100644 --- a/src/ion/mod.rs +++ b/src/ion/mod.rs @@ -326,19 +326,16 @@ struct PRegData { /* * Environment setup: * - * We have seven fundamental objects: LiveRange, LiveBundle, SpillSet, Use, Def, VReg, PReg. + * We have seven fundamental objects: LiveRange, LiveBundle, SpillSet, Use, VReg, PReg. * * The relationship is as follows: * * LiveRange --(vreg)--> shared(VReg) * LiveRange --(bundle)--> shared(LiveBundle) - * LiveRange --(def)--> owns(Def) * LiveRange --(use) --> list(Use) * * Use --(vreg)--> shared(VReg) * - * Def --(vreg) --> owns(VReg) - * * LiveBundle --(range)--> list(LiveRange) * LiveBundle --(spillset)--> shared(SpillSet) * LiveBundle --(parent)--> parent(LiveBundle) @@ -565,6 +562,7 @@ enum Requirement { Any(RegClass), } impl Requirement { + #[inline(always)] fn class(self) -> RegClass { match self { Requirement::Fixed(preg) => preg.class(), @@ -573,7 +571,7 @@ impl Requirement { } } } - + #[inline(always)] fn merge(self, other: Requirement) -> Option { if self.class() != other.class() { return None; @@ -590,6 +588,7 @@ impl Requirement { _ => None, } } + #[inline(always)] fn from_operand(op: Operand) -> Requirement { match op.policy() { OperandPolicy::FixedReg(preg) => Requirement::Fixed(preg), @@ -1034,7 +1033,7 @@ impl<'a, F: Function> Env<'a, F> { ); lrdata.uses_spill_weight_and_flags -= spill_weight_from_policy(usedata.operand.policy()); - if usedata.operand.kind() == OperandKind::Def { + if usedata.operand.kind() != OperandKind::Use { lrdata.uses_spill_weight_and_flags -= 2000; } } @@ -1083,7 +1082,7 @@ impl<'a, F: Function> Env<'a, F> { spill_weight_from_policy(policy) ); self.ranges[into.index()].uses_spill_weight_and_flags += spill_weight_from_policy(policy); - if self.uses[u.index()].operand.kind() == OperandKind::Def { + if self.uses[u.index()].operand.kind() != OperandKind::Use { self.ranges[into.index()].uses_spill_weight_and_flags += 2000; } log::debug!(" -> now {}", self.ranges[into.index()].uses_spill_weight()); @@ -1149,11 +1148,11 @@ impl<'a, F: Function> Env<'a, F> { live.set(dst.vreg(), false); live.set(src.vreg(), true); } - for pos in &[OperandPos::After, OperandPos::Both, OperandPos::Before] { + for pos in &[OperandPos::After, OperandPos::Before] { for op in self.func.inst_operands(inst) { if op.pos() == *pos { match op.kind() { - OperandKind::Use => { + OperandKind::Use | OperandKind::Mod => { live.set(op.vreg().vreg(), true); } OperandKind::Def => { @@ -1355,11 +1354,12 @@ impl<'a, F: Function> Env<'a, F> { // don't borrow `self` let operand = self.func.inst_operands(inst)[i]; match operand.kind() { - OperandKind::Def => { + OperandKind::Def | OperandKind::Mod => { // Create the Def object. - let pos = match operand.pos() { - OperandPos::Before | OperandPos::Both => ProgPoint::before(inst), - OperandPos::After => ProgPoint::after(inst), + let pos = match (operand.kind(), operand.pos()) { + (OperandKind::Mod, _) => ProgPoint::before(inst), + (_, OperandPos::Before) => ProgPoint::before(inst), + (_, OperandPos::After) => ProgPoint::after(inst), }; let u = UseIndex(self.uses.len() as u32); self.uses @@ -1370,11 +1370,6 @@ impl<'a, F: Function> Env<'a, F> { // Fill in vreg's actual data. self.vreg_regs[operand.vreg().vreg()] = operand.vreg(); - // Trim the range for this vreg to start - // at `pos` if it previously ended at the - // start of this block (i.e. was not - // merged into some larger LiveRange due - // to out-of-order blocks). let mut lr = vreg_ranges[operand.vreg().vreg()]; log::debug!(" -> has existing LR {:?}", lr); // If there was no liverange (dead def), create a trivial one. @@ -1389,22 +1384,34 @@ impl<'a, F: Function> Env<'a, F> { ); log::debug!(" -> invalid; created {:?}", lr); } - if self.ranges[lr.index()].range.from - == self.cfginfo.block_entry[block.index()] - { - log::debug!(" -> started at block start; trimming to {:?}", pos); - self.ranges[lr.index()].range.from = pos; - } self.insert_use_into_liverange_and_update_stats(lr, u); - // Remove from live-set. - live.set(operand.vreg().vreg(), false); - vreg_ranges[operand.vreg().vreg()] = LiveRangeIndex::invalid(); + + if operand.kind() == OperandKind::Def { + // Trim the range for this vreg to start + // at `pos` if it previously ended at the + // start of this block (i.e. was not + // merged into some larger LiveRange due + // to out-of-order blocks). + if self.ranges[lr.index()].range.from + == self.cfginfo.block_entry[block.index()] + { + log::debug!( + " -> started at block start; trimming to {:?}", + pos + ); + self.ranges[lr.index()].range.from = pos; + } + + // Remove from live-set. + live.set(operand.vreg().vreg(), false); + vreg_ranges[operand.vreg().vreg()] = LiveRangeIndex::invalid(); + } } OperandKind::Use => { // Establish where the use occurs. let mut pos = match operand.pos() { OperandPos::Before => ProgPoint::before(inst), - OperandPos::Both | OperandPos::After => ProgPoint::after(inst), + OperandPos::After => ProgPoint::after(inst), }; // If there are any reused inputs in this // instruction, and this is *not* the @@ -2612,8 +2619,9 @@ impl<'a, F: Function> Env<'a, F> { // the def so we don't need to insert a move. use_data.pos } else { - // For an use, split before the instruction -- - // this allows us to insert a move if necessary. + // For an use or mod, split before the instruction + // -- this allows us to insert a move if + // necessary. ProgPoint::before(use_data.pos.inst()) }; let after_use_inst = ProgPoint::before(use_data.pos.inst().next()); @@ -4118,9 +4126,11 @@ impl<'a, F: Function> Env<'a, F> { ); } - // Ensure edits are in sorted ProgPoint order. - self.edits - .sort_unstable_by_key(|&(pos, prio, _)| (pos, prio)); + // Ensure edits are in sorted ProgPoint order. N.B.: this must + // be a stable sort! We have to keep the order produced by the + // parallel-move resolver for all moves within a single sort + // key. + self.edits.sort_by_key(|&(pos, prio, _)| (pos, prio)); self.stats.edits_count = self.edits.len(); // Add debug annotations. diff --git a/src/lib.rs b/src/lib.rs index 8b8b69d..3dfc41f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -209,7 +209,7 @@ pub struct Operand { /// so that clients, if they wish, can track just one `u32` per /// register slot and edit it in-place after allocation. /// - /// policy:3 kind:1 pos:2 class:1 preg:5 vreg:20 + /// policy:3 kind:2 pos:1 class:1 preg:5 vreg:20 bits: u32, } @@ -237,7 +237,7 @@ impl Operand { | (preg_field << 20) | (class_field << 25) | (pos_field << 26) - | (kind_field << 28) + | (kind_field << 27) | (policy_field << 29), } } @@ -253,7 +253,12 @@ impl Operand { } #[inline(always)] pub fn reg_use_at_end(vreg: VReg) -> Self { - Operand::new(vreg, OperandPolicy::Reg, OperandKind::Use, OperandPos::Both) + Operand::new( + vreg, + OperandPolicy::Reg, + OperandKind::Use, + OperandPos::After, + ) } #[inline(always)] pub fn reg_def(vreg: VReg) -> Self { @@ -266,11 +271,21 @@ impl Operand { } #[inline(always)] pub fn reg_def_at_start(vreg: VReg) -> Self { - Operand::new(vreg, OperandPolicy::Reg, OperandKind::Def, OperandPos::Both) + Operand::new( + vreg, + OperandPolicy::Reg, + OperandKind::Def, + OperandPos::Before, + ) } #[inline(always)] pub fn reg_temp(vreg: VReg) -> Self { - Operand::new(vreg, OperandPolicy::Reg, OperandKind::Def, OperandPos::Both) + Operand::new( + vreg, + OperandPolicy::Reg, + OperandKind::Def, + OperandPos::Before, + ) } #[inline(always)] pub fn reg_reuse_def(vreg: VReg, idx: usize) -> Self { @@ -278,7 +293,7 @@ impl Operand { vreg, OperandPolicy::Reuse(idx), OperandKind::Def, - OperandPos::Both, + OperandPos::After, ) } #[inline(always)] @@ -318,21 +333,21 @@ impl Operand { #[inline(always)] pub fn kind(self) -> OperandKind { - let kind_field = (self.bits >> 28) & 1; + let kind_field = (self.bits >> 27) & 3; match kind_field { 0 => OperandKind::Def, - 1 => OperandKind::Use, + 1 => OperandKind::Mod, + 2 => OperandKind::Use, _ => unreachable!(), } } #[inline(always)] pub fn pos(self) -> OperandPos { - let pos_field = (self.bits >> 26) & 3; + let pos_field = (self.bits >> 26) & 1; match pos_field { 0 => OperandPos::Before, 1 => OperandPos::After, - 2 => OperandPos::Both, _ => unreachable!(), } } @@ -415,14 +430,14 @@ impl std::fmt::Display for OperandPolicy { #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum OperandKind { Def = 0, - Use = 1, + Mod = 1, + Use = 2, } #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum OperandPos { Before = 0, After = 1, - Both = 2, } /// An Allocation represents the end result of regalloc for an @@ -585,7 +600,8 @@ impl OperandOrAllocation { let bits = alloc.bits() | match kind { OperandKind::Def => 0, - OperandKind::Use => 1 << 28, + OperandKind::Mod => 1 << 27, + OperandKind::Use => 2 << 27, }; Self { bits } } @@ -604,11 +620,11 @@ impl OperandOrAllocation { } pub fn as_allocation(&self) -> Option { if self.is_allocation() { - // Remove the def/use bit -- the canonical `Allocation` - // does not have this, and we want allocs to continue to - // be comparable whether they are used for reads or - // writes. - Some(Allocation::from_bits(self.bits & !(1 << 28))) + // Remove the kind (def/use/mod) bits -- the canonical + // `Allocation` does not have this, and we want allocs to + // continue to be comparable whether they are used for + // reads or writes. + Some(Allocation::from_bits(self.bits & !(3 << 27))) } else { None } @@ -618,7 +634,8 @@ impl OperandOrAllocation { let kind_field = (self.bits >> 28) & 1; match kind_field { 0 => OperandKind::Def, - 1 => OperandKind::Use, + 1 => OperandKind::Mod, + 2 => OperandKind::Use, _ => unreachable!(), } } diff --git a/src/ssa.rs b/src/ssa.rs index 2d6e625..de69841 100644 --- a/src/ssa.rs +++ b/src/ssa.rs @@ -47,6 +47,12 @@ pub fn validate_ssa(f: &F, cfginfo: &CFGInfo) -> Result<(), RegAllo } defined[operand.vreg().vreg()] = true; } + OperandKind::Mod => { + // Mod (modify) operands are not used in SSA, + // but can be used by non-SSA code (e.g. with + // the regalloc.rs compatibility shim). + return Err(RegAllocError::SSA(operand.vreg(), iix)); + } } } }