Remove safepoint support (#183)

As we have switched to modeling safepoints in clif directly, it's not
longer necessary to track them in RA2. This PR is the first cut at
removing safepoints from RA2.

Co-authored-by: Nick Fitzgerald <[email protected]>

---------

Co-authored-by: Nick Fitzgerald <[email protected]>

Author: elliottt

doc/DESIGN.md

@@ -667,12 +667,6 @@ If the requirement indicates that no register is needed (`Unknown` or
 bundle already exists for this bundle's spillset, then we move all the
 liveranges over to the spill bundle, as described above.
 
-If the requirement indicates that the stack is needed explicitly
-(e.g., for a safepoint), we set our spillset as "required" (this will
-cause it to allocate a spillslot) and return; because the bundle has
-no other allocation set, it will look to the spillset's spillslot by
-default.
-
 If the requirement indicates a conflict, we immediately split and
 requeue the split pieces. This split is performed at the point at
 which the conflict is first introduced, i.e. just before the first use

src/checker.rs

@@ -518,30 +518,6 @@ impl CheckerState {
                     self.check_val(inst, *op, *alloc, val, allocs, checker)?;
                 }
             }
-            &CheckerInst::Safepoint { inst, ref allocs } => {
-                for &alloc in allocs {
-                    let val = self.get_value(&alloc).unwrap_or(&default_val);
-                    trace!(
-                        "checker: checkinst {:?}: safepoint slot {}, checker value {:?}",
-                        checkinst,
-                        alloc,
-                        val
-                    );
-
-                    let reffy = val
-                        .vregs()
-                        .expect("checker value should not be Universe set")
-                        .iter()
-                        .any(|vreg| checker.reftyped_vregs.contains(vreg));
-                    if !reffy {
-                        return Err(CheckerError::NonRefValuesInStackmap {
-                            inst,
-                            alloc,
-                            vregs: val.vregs().unwrap().clone(),
-                        });
-                    }
-                }
-            }
             &CheckerInst::Move { into, from } => {
                 // Ensure that the allocator never returns stack-to-stack moves.
                 let is_stack = |alloc: Allocation| {
@@ -565,7 +541,7 @@ impl CheckerState {
     }
 
     /// Update according to instruction.
-    fn update<'a, F: Function>(&mut self, checkinst: &CheckerInst, checker: &Checker<'a, F>) {
+    fn update(&mut self, checkinst: &CheckerInst) {
         self.become_defined();
 
         match checkinst {
@@ -645,23 +621,6 @@ impl CheckerState {
                     self.remove_value(&Allocation::reg(*clobber));
                 }
             }
-            &CheckerInst::Safepoint { ref allocs, .. } => {
-                for (alloc, value) in self.get_mappings_mut() {
-                    if alloc.is_reg() {
-                        continue;
-                    }
-                    if !allocs.contains(&alloc) {
-                        // Remove all reftyped vregs as labels.
-                        let new_vregs = value
-                            .vregs()
-                            .unwrap()
-                            .difference(&checker.reftyped_vregs)
-                            .cloned()
-                            .collect();
-                        *value = CheckerValue::VRegs(new_vregs);
-                    }
-                }
-            }
         }
     }
 
@@ -749,10 +708,6 @@ pub(crate) enum CheckerInst {
         allocs: Vec<Allocation>,
         clobbers: Vec<PReg>,
     },
-
-    /// A safepoint, with the given Allocations specified as containing
-    /// reftyped values. All other reftyped values become invalid.
-    Safepoint { inst: Inst, allocs: Vec<Allocation> },
 }
 
 #[derive(Debug)]
@@ -761,7 +716,6 @@ pub struct Checker<'a, F: Function> {
     bb_in: FxHashMap<Block, CheckerState>,
     bb_insts: FxHashMap<Block, Vec<CheckerInst>>,
     edge_insts: FxHashMap<(Block, Block), Vec<CheckerInst>>,
-    reftyped_vregs: FxHashSet<VReg>,
     machine_env: &'a MachineEnv,
     stack_pregs: PRegSet,
 }
@@ -775,7 +729,6 @@ impl<'a, F: Function> Checker<'a, F> {
         let mut bb_in = FxHashMap::default();
         let mut bb_insts = FxHashMap::default();
         let mut edge_insts = FxHashMap::default();
-        let mut reftyped_vregs = FxHashSet::default();
 
         for block in 0..f.num_blocks() {
             let block = Block::new(block);
@@ -786,10 +739,6 @@ impl<'a, F: Function> Checker<'a, F> {
             }
         }
 
-        for &vreg in f.reftype_vregs() {
-            reftyped_vregs.insert(vreg);
-        }
-
         bb_in.insert(f.entry_block(), CheckerState::default());
 
         let mut stack_pregs = PRegSet::empty();
@@ -802,7 +751,6 @@ impl<'a, F: Function> Checker<'a, F> {
             bb_in,
             bb_insts,
             edge_insts,
-            reftyped_vregs,
             machine_env,
             stack_pregs,
         }
@@ -812,15 +760,6 @@ impl<'a, F: Function> Checker<'a, F> {
     /// and allocation results.
     pub fn prepare(&mut self, out: &Output) {
         trace!("checker: out = {:?}", out);
-        // Preprocess safepoint stack-maps into per-inst vecs.
-        let mut safepoint_slots: FxHashMap<Inst, Vec<Allocation>> = FxHashMap::default();
-        for &(progpoint, slot) in &out.safepoint_slots {
-            safepoint_slots
-                .entry(progpoint.inst())
-                .or_insert_with(|| vec![])
-                .push(slot);
-        }
-
         let mut last_inst = None;
         for block in 0..self.f.num_blocks() {
             let block = Block::new(block);
@@ -829,7 +768,7 @@ impl<'a, F: Function> Checker<'a, F> {
                     InstOrEdit::Inst(inst) => {
                         debug_assert!(last_inst.is_none() || inst > last_inst.unwrap());
                         last_inst = Some(inst);
-                        self.handle_inst(block, inst, &mut safepoint_slots, out);
+                        self.handle_inst(block, inst, out);
                     }
                     InstOrEdit::Edit(edit) => self.handle_edit(block, edit),
                 }
@@ -838,21 +777,7 @@ impl<'a, F: Function> Checker<'a, F> {
     }
 
     /// For each original instruction, create an `Op`.
-    fn handle_inst(
-        &mut self,
-        block: Block,
-        inst: Inst,
-        safepoint_slots: &mut FxHashMap<Inst, Vec<Allocation>>,
-        out: &Output,
-    ) {
-        // If this is a safepoint, then check the spillslots at this point.
-        if self.f.requires_refs_on_stack(inst) {
-            let allocs = safepoint_slots.remove(&inst).unwrap_or_else(|| vec![]);
-
-            let checkinst = CheckerInst::Safepoint { inst, allocs };
-            self.bb_insts.get_mut(&block).unwrap().push(checkinst);
-        }
-
+    fn handle_inst(&mut self, block: Block, inst: Inst, out: &Output) {
         // Skip normal checks if this is a branch: the blockparams do
         // not exist in post-regalloc code, and the edge-moves have to
         // be inserted before the branch rather than after.
@@ -931,14 +856,14 @@ impl<'a, F: Function> Checker<'a, F> {
             let mut state = self.bb_in.get(&block).cloned().unwrap();
             trace!("analyze: block {} has state {:?}", block.index(), state);
             for inst in self.bb_insts.get(&block).unwrap() {
-                state.update(inst, self);
+                state.update(inst);
                 trace!("analyze: inst {:?} -> state {:?}", inst, state);
             }
 
             for &succ in self.f.block_succs(block) {
                 let mut new_state = state.clone();
                 for edge_inst in self.edge_insts.get(&(block, succ)).unwrap() {
-                    new_state.update(edge_inst, self);
+                    new_state.update(edge_inst);
                     trace!(
                         "analyze: succ {:?}: inst {:?} -> state {:?}",
                         succ,
@@ -987,7 +912,7 @@ impl<'a, F: Function> Checker<'a, F> {
                     trace!("Checker error: {:?}", e);
                     errors.push(e);
                 }
-                state.update(inst, self);
+                state.update(inst);
                 if let Err(e) = state.check(InstPosition::After, inst, self) {
                     trace!("Checker error: {:?}", e);
                     errors.push(e);
@@ -1021,9 +946,6 @@ impl<'a, F: Function> Checker<'a, F> {
                 trace!("    {{ {} }}", s.join(", "))
             }
         }
-        for vreg in self.f.reftype_vregs() {
-            trace!("  REF: {}", vreg);
-        }
         for bb in 0..self.f.num_blocks() {
             let bb = Block::new(bb);
             trace!("block{}:", bb.index());
@@ -1049,18 +971,11 @@ impl<'a, F: Function> Checker<'a, F> {
                     &CheckerInst::Move { from, into } => {
                         trace!("    {} -> {}", from, into);
                     }
-                    &CheckerInst::Safepoint { ref allocs, .. } => {
-                        let mut slotargs = vec![];
-                        for &slot in allocs {
-                            slotargs.push(format!("{}", slot));
-                        }
-                        trace!("    safepoint: {}", slotargs.join(", "));
-                    }
                     &CheckerInst::ParallelMove { .. } => {
                         panic!("unexpected parallel_move in body (non-edge)")
                     }
                 }
-                state.update(inst, &self);
+                state.update(inst);
                 print_state(&state);
             }
 
@@ -1078,7 +993,7 @@ impl<'a, F: Function> Checker<'a, F> {
                         }
                         _ => panic!("unexpected edge_inst: not a parallel move"),
                     }
-                    state.update(edge_inst, &self);
+                    state.update(edge_inst);
                     print_state(&state);
                 }
             }

src/fuzzing/func.rs

@@ -26,7 +26,6 @@ pub struct InstData {
     op: InstOpcode,
     operands: Vec<Operand>,
     clobbers: Vec<PReg>,
-    is_safepoint: bool,
 }
 
 impl InstData {
@@ -35,15 +34,13 @@ impl InstData {
             op: InstOpcode::Branch,
             operands: vec![],
             clobbers: vec![],
-            is_safepoint: false,
         }
     }
     pub fn ret() -> InstData {
         InstData {
             op: InstOpcode::Ret,
             operands: vec![],
             clobbers: vec![],
-            is_safepoint: false,
         }
     }
 }
@@ -103,14 +100,6 @@ impl Function for Func {
         &self.block_params_out[block.index()][succ][..]
     }
 
-    fn requires_refs_on_stack(&self, insn: Inst) -> bool {
-        self.insts[insn.index()].is_safepoint
-    }
-
-    fn reftype_vregs(&self) -> &[VReg] {
-        &self.reftype_vregs[..]
-    }
-
     fn debug_value_labels(&self) -> &[(VReg, Inst, Inst, u32)] {
         &self.debug_value_labels[..]
     }
@@ -515,19 +504,12 @@ impl Func {
                     )));
                 }
 
-                let is_safepoint = opts.reftypes
-                    && operands
-                        .iter()
-                        .all(|op| !builder.f.reftype_vregs.contains(&op.vreg()))
-                    && bool::arbitrary(u)?;
-
                 builder.add_inst(
                     Block::new(block),
                     InstData {
                         op: InstOpcode::Op,
                         operands,
                         clobbers,
-                        is_safepoint,
                     },
                 );
                 avail.push(vreg);
@@ -583,9 +565,6 @@ impl Func {
 impl core::fmt::Debug for Func {
     fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
         write!(f, "{{\n")?;
-        for vreg in self.reftype_vregs() {
-            write!(f, "  REF: {}\n", vreg)?;
-        }
         for (i, blockrange) in self.blocks.iter().enumerate() {
             let succs = self.block_succs[i]
                 .iter()
@@ -620,9 +599,6 @@ impl core::fmt::Debug for Func {
                 i, params_in, succs, preds
             )?;
             for inst in blockrange.iter() {
-                if self.requires_refs_on_stack(inst) {
-                    write!(f, "    -- SAFEPOINT --\n")?;
-                }
                 write!(
                     f,
                     "    inst{}: {:?} ops:{:?} clobber:{:?}\n",

src/ion/data_structures.rs

@@ -17,15 +17,14 @@ use crate::cfg::CFGInfo;
 use crate::index::ContainerComparator;
 use crate::indexset::IndexSet;
 use crate::{
-    define_index, Allocation, Block, Edit, Function, FxHashSet, Inst, MachineEnv, Operand, PReg,
+    define_index, Allocation, Block, Edit, Function, FxHashSet, MachineEnv, Operand, PReg,
     ProgPoint, RegClass, VReg,
 };
 use alloc::collections::BTreeMap;
 use alloc::string::String;
 use alloc::vec::Vec;
 use core::cmp::Ordering;
 use core::fmt::Debug;
-use hashbrown::{HashMap, HashSet};
 use smallvec::{smallvec, SmallVec};
 
 /// A range from `from` (inclusive) to `to` (exclusive).
@@ -195,8 +194,6 @@ impl Use {
     }
 }
 
-pub const SLOT_NONE: u8 = u8::MAX;
-
 #[derive(Clone, Debug)]
 pub struct LiveBundle {
     pub ranges: LiveRangeList,
@@ -315,7 +312,6 @@ pub(crate) const MAX_SPLITS_PER_SPILLSET: u8 = 2;
 pub struct VRegData {
     pub ranges: LiveRangeList,
     pub blockparam: Block,
-    pub is_ref: bool,
     // We don't initially know the RegClass until we observe a use of the VReg.
     pub class: Option<RegClass>,
 }
@@ -460,8 +456,6 @@ pub struct Env<'a, F: Function> {
     pub vregs: VRegs,
     pub pregs: Vec<PRegData>,
     pub allocation_queue: PrioQueue,
-    pub safepoints: Vec<Inst>, // Sorted list of safepoint insts.
-    pub safepoints_per_vreg: HashMap<usize, HashSet<Inst>>,
 
     pub spilled_bundles: Vec<LiveBundleIndex>,
     pub spillslots: Vec<SpillSlotData>,
@@ -484,7 +478,6 @@ pub struct Env<'a, F: Function> {
     pub allocs: Vec<Allocation>,
     pub inst_alloc_offsets: Vec<u32>,
     pub num_spillslots: u32,
-    pub safepoint_slots: Vec<(ProgPoint, Allocation)>,
     pub debug_locations: Vec<(u32, ProgPoint, ProgPoint, Allocation)>,
 
     pub allocated_bundle_count: usize,