From af527aca88bf8b28857ad2c537f946ff1f276b26 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Thu, 16 Sep 2021 14:49:23 +0100 Subject: [PATCH 1/4] Fix PReg indexing with >32 pregs --- src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 31822514..86afea73 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -108,13 +108,13 @@ impl PReg { /// all PRegs and index it efficiently. #[inline(always)] pub fn index(self) -> usize { - ((self.class as u8 as usize) << 5) | (self.hw_enc as usize) + ((self.class as u8 as usize) << Self::MAX_BITS) | (self.hw_enc as usize) } /// Construct a PReg from the value returned from `.index()`. #[inline(always)] pub fn from_index(index: usize) -> Self { - let class = (index >> 5) & 1; + let class = (index >> Self::MAX_BITS) & 1; let class = match class { 0 => RegClass::Int, 1 => RegClass::Float, From 358c831b318ec9f52a60cb74153e314985fbdef0 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Thu, 16 Sep 2021 09:14:07 +0100 Subject: [PATCH 2/4] Remove regs from MachineEnv It isn't exactly clear what purpose it serves. --- src/fuzzing/func.rs | 1 - src/ion/liveranges.rs | 7 +++++-- src/lib.rs | 6 ------ 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/fuzzing/func.rs b/src/fuzzing/func.rs index 320fe95f..38e7400f 100644 --- a/src/fuzzing/func.rs +++ b/src/fuzzing/func.rs @@ -589,7 +589,6 @@ pub fn machine_env() -> MachineEnv { [regs.iter().cloned().skip(24).collect(), vec![]]; let scratch_by_class: [PReg; 2] = [PReg::new(31, RegClass::Int), PReg::new(0, RegClass::Float)]; MachineEnv { - regs, preferred_regs_by_class, non_preferred_regs_by_class, scratch_by_class, diff --git a/src/ion/liveranges.rs b/src/ion/liveranges.rs index 501d9f5c..4c9e9497 100644 --- a/src/ion/liveranges.rs +++ b/src/ion/liveranges.rs @@ -105,8 +105,11 @@ impl<'a, F: Function> Env<'a, F> { allocations: LiveRangeSet::new(), }, ); - for &preg in &self.env.regs { - self.pregs[preg.index()].reg = preg; + for i in 0..=PReg::MAX { + let preg_int = PReg::new(i, RegClass::Int); + self.pregs[preg_int.index()].reg = preg_int; + let preg_float = PReg::new(i, RegClass::Float); + self.pregs[preg_float.index()].reg = preg_float; } // Create VRegs from the vreg count. for idx in 0..self.func.num_vregs() { diff --git a/src/lib.rs b/src/lib.rs index 86afea73..2c56d5ae 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1139,12 +1139,6 @@ pub enum Edit { /// as well. #[derive(Clone, Debug)] pub struct MachineEnv { - /// Physical registers. Every register that might be mentioned in - /// any constraint must be listed here, even if it is not - /// allocatable (present in one of - /// `{preferred,non_preferred}_regs_by_class`). - pub regs: Vec, - /// Preferred physical registers for each class. These are the /// registers that will be allocated first, if free. pub preferred_regs_by_class: [Vec; 2], From a527a6d25adc6fbf2f3379f8edd5a6aaa3a84b63 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Mon, 1 Nov 2021 00:08:18 +0000 Subject: [PATCH 3/4] Remove unused clobbers vector --- src/ion/data_structures.rs | 1 - src/ion/liveranges.rs | 5 ----- src/ion/mod.rs | 1 - 3 files changed, 7 deletions(-) diff --git a/src/ion/data_structures.rs b/src/ion/data_structures.rs index dbf1d777..2c8e616a 100644 --- a/src/ion/data_structures.rs +++ b/src/ion/data_structures.rs @@ -293,7 +293,6 @@ pub struct Env<'a, F: Function> { pub vreg_regs: Vec, pub pregs: Vec, pub allocation_queue: PrioQueue, - pub clobbers: Vec, // Sorted list of insts with clobbers. pub safepoints: Vec, // Sorted list of safepoint insts. pub safepoints_per_vreg: HashMap>, diff --git a/src/ion/liveranges.rs b/src/ion/liveranges.rs index 4c9e9497..2aa078d1 100644 --- a/src/ion/liveranges.rs +++ b/src/ion/liveranges.rs @@ -448,10 +448,6 @@ impl<'a, F: Function> Env<'a, F> { // For each instruction, in reverse order, process // operands and clobbers. for inst in insns.rev().iter() { - if self.func.inst_clobbers(inst).len() > 0 { - self.clobbers.push(inst); - } - // Mark clobbers with CodeRanges on PRegs. for i in 0..self.func.inst_clobbers(inst).len() { // don't borrow `self` @@ -1234,7 +1230,6 @@ impl<'a, F: Function> Env<'a, F> { } } - self.clobbers.sort_unstable(); self.blockparam_ins.sort_unstable(); self.blockparam_outs.sort_unstable(); self.prog_move_srcs.sort_unstable_by_key(|(pos, _)| *pos); diff --git a/src/ion/mod.rs b/src/ion/mod.rs index e2d73b58..8d902c8c 100644 --- a/src/ion/mod.rs +++ b/src/ion/mod.rs @@ -62,7 +62,6 @@ impl<'a, F: Function> Env<'a, F> { vreg_regs: Vec::with_capacity(n), pregs: vec![], allocation_queue: PrioQueue::new(), - clobbers: vec![], safepoints: vec![], safepoints_per_vreg: HashMap::new(), spilled_bundles: vec![], From a516e6d6f3b5b4a462297151c4172f32abcdc804 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Thu, 16 Sep 2021 14:02:26 +0100 Subject: [PATCH 4/4] Return safepoint_slots as Allocations instead of SpillSlots This enables us to support reftype vregs in register locations in the future. --- src/checker.rs | 37 +++++++++++++++++-------------------- src/ion/data_structures.rs | 4 ++-- src/ion/stackmap.rs | 6 ++---- src/lib.rs | 7 +++++-- 4 files changed, 26 insertions(+), 28 deletions(-) diff --git a/src/checker.rs b/src/checker.rs index 146dbeeb..55f48601 100644 --- a/src/checker.rs +++ b/src/checker.rs @@ -78,7 +78,7 @@ use crate::{ Allocation, AllocationKind, Block, Edit, Function, Inst, InstPosition, Operand, - OperandConstraint, OperandKind, OperandPos, Output, PReg, ProgPoint, SpillSlot, VReg, + OperandConstraint, OperandKind, OperandPos, Output, PReg, ProgPoint, RegClass, VReg, }; use std::collections::{HashMap, HashSet, VecDeque}; @@ -143,11 +143,11 @@ pub enum CheckerError { }, ConflictedValueInStackmap { inst: Inst, - slot: SpillSlot, + alloc: Allocation, }, NonRefValueInStackmap { inst: Inst, - slot: SpillSlot, + alloc: Allocation, vreg: VReg, }, } @@ -332,9 +332,8 @@ impl CheckerState { self.check_val(inst, *op, *alloc, val, allocs)?; } } - &CheckerInst::Safepoint { inst, ref slots } => { - for &slot in slots { - let alloc = Allocation::stack(slot); + &CheckerInst::Safepoint { inst, ref allocs } => { + for &alloc in allocs { let val = self .allocations .get(&alloc) @@ -343,17 +342,17 @@ impl CheckerState { log::trace!( "checker: checkinst {:?}: safepoint slot {}, checker value {:?}", checkinst, - slot, + alloc, val ); match val { CheckerValue::Unknown => {} CheckerValue::Conflicted => { - return Err(CheckerError::ConflictedValueInStackmap { inst, slot }); + return Err(CheckerError::ConflictedValueInStackmap { inst, alloc }); } CheckerValue::Reg(vreg, false) => { - return Err(CheckerError::NonRefValueInStackmap { inst, slot, vreg }); + return Err(CheckerError::NonRefValueInStackmap { inst, alloc, vreg }); } CheckerValue::Reg(_, true) => {} } @@ -405,12 +404,10 @@ impl CheckerState { self.allocations .insert(alloc, CheckerValue::Reg(vreg, reftyped)); } - &CheckerInst::Safepoint { ref slots, .. } => { + &CheckerInst::Safepoint { ref allocs, .. } => { for (alloc, value) in &mut self.allocations { if let CheckerValue::Reg(_, true) = *value { - if alloc.is_reg() { - *value = CheckerValue::Conflicted; - } else if alloc.is_stack() && !slots.contains(&alloc.as_stack().unwrap()) { + if !allocs.contains(&alloc) { *value = CheckerValue::Conflicted; } } @@ -483,9 +480,9 @@ pub(crate) enum CheckerInst { /// of a value is logically transferred to a new vreg. DefAlloc { alloc: Allocation, vreg: VReg }, - /// A safepoint, with the given SpillSlots specified as containing + /// A safepoint, with the given Allocations specified as containing /// reftyped values. All other reftyped values become invalid. - Safepoint { inst: Inst, slots: Vec }, + Safepoint { inst: Inst, allocs: Vec }, } #[derive(Debug)] @@ -529,7 +526,7 @@ impl<'a, F: Function> Checker<'a, F> { pub fn prepare(&mut self, out: &Output) { log::trace!("checker: out = {:?}", out); // Preprocess safepoint stack-maps into per-inst vecs. - let mut safepoint_slots: HashMap> = HashMap::new(); + let mut safepoint_slots: HashMap> = HashMap::new(); for &(progpoint, slot) in &out.safepoint_slots { safepoint_slots .entry(progpoint.inst()) @@ -551,9 +548,9 @@ impl<'a, F: Function> Checker<'a, F> { // If this is a safepoint, then check the spillslots at this point. if self.f.requires_refs_on_stack(inst) { - let slots = safepoint_slots.remove(&inst).unwrap_or_else(|| vec![]); + let allocs = safepoint_slots.remove(&inst).unwrap_or_else(|| vec![]); - let checkinst = CheckerInst::Safepoint { inst, slots }; + let checkinst = CheckerInst::Safepoint { inst, allocs }; self.bb_insts.get_mut(&block).unwrap().push(checkinst); } @@ -727,9 +724,9 @@ impl<'a, F: Function> Checker<'a, F> { &CheckerInst::DefAlloc { alloc, vreg } => { log::trace!(" defalloc: {}:{}", vreg, alloc); } - &CheckerInst::Safepoint { ref slots, .. } => { + &CheckerInst::Safepoint { ref allocs, .. } => { let mut slotargs = vec![]; - for &slot in slots { + for &slot in allocs { slotargs.push(format!("{}", slot)); } log::trace!(" safepoint: {}", slotargs.join(", ")); diff --git a/src/ion/data_structures.rs b/src/ion/data_structures.rs index 2c8e616a..635ac263 100644 --- a/src/ion/data_structures.rs +++ b/src/ion/data_structures.rs @@ -19,7 +19,7 @@ use crate::index::ContainerComparator; use crate::indexset::IndexSet; use crate::{ define_index, Allocation, Block, Edit, Function, Inst, MachineEnv, Operand, PReg, ProgPoint, - RegClass, SpillSlot, VReg, + RegClass, VReg, }; use smallvec::SmallVec; use std::cmp::Ordering; @@ -336,7 +336,7 @@ pub struct Env<'a, F: Function> { pub allocs: Vec, pub inst_alloc_offsets: Vec, pub num_spillslots: u32, - pub safepoint_slots: Vec<(ProgPoint, SpillSlot)>, + pub safepoint_slots: Vec<(ProgPoint, Allocation)>, pub allocated_bundle_count: usize, diff --git a/src/ion/stackmap.rs b/src/ion/stackmap.rs index c48475cc..f9c534ad 100644 --- a/src/ion/stackmap.rs +++ b/src/ion/stackmap.rs @@ -58,10 +58,8 @@ impl<'a, F: Function> Env<'a, F> { } log::trace!(" -> covers safepoint {:?}", safepoints[safepoint_idx]); - let slot = alloc - .as_stack() - .expect("Reference-typed value not in spillslot at safepoint"); - self.safepoint_slots.push((safepoints[safepoint_idx], slot)); + self.safepoint_slots + .push((safepoints[safepoint_idx], alloc)); safepoint_idx += 1; } } diff --git a/src/lib.rs b/src/lib.rs index 2c56d5ae..d91bf970 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1180,8 +1180,11 @@ pub struct Output { /// Allocation offset in `allocs` for each instruction. pub inst_alloc_offsets: Vec, - /// Safepoint records: at a given program point, a reference-typed value lives in the given SpillSlot. - pub safepoint_slots: Vec<(ProgPoint, SpillSlot)>, + /// Safepoint records: at a given program point, a reference-typed value + /// lives in the given Allocation. Currently these are guaranteed to be + /// stack slots, but in the future an option may be added to allow + /// reftype value to be kept in registers at safepoints. + pub safepoint_slots: Vec<(ProgPoint, Allocation)>, /// Debug info: a labeled value (as applied to vregs by /// `Function::debug_value_labels()` on the input side) is located