Skip to content

Commit

Permalink
Add support for reftypes/stackmaps and Stack constraints, and misc AP…
Browse files Browse the repository at this point in the history
…I changes.

The main enhancement in this commit is support for reference types and
stackmaps. This requires tracking whether each VReg is a "reference" or
"pointer". At certain instructions designated as "safepoints", the
regalloc will (i) ensure that all references are in spillslots rather
than in registers, and (ii) provide a list of exactly which spillslots
have live references at that program point. This can be used by, e.g., a
GC to trace and possibly modify pointers. The stackmap of spillslots is
precise: it includes all live references, and *only* live references.

This commit also brings in some API tweaks as part of the in-progress
Cranelift glue. In particular, it makes Allocations and Operands
mutually disjoint by using the same bitfield for the type-tag in both
and choosing non-overlapping tags. This will allow instructions to carry
an Operand for each register slot and then overwrite these in place with
Allocations. The `OperandOrAllocation` type does the necessary magic to
make this look like an enum, but staying in 32 bits.
  • Loading branch information
cfallin committed Apr 18, 2021
1 parent 33ac6cb commit a08b012
Show file tree
Hide file tree
Showing 6 changed files with 686 additions and 91 deletions.
1 change: 1 addition & 0 deletions fuzz/fuzz_targets/ion_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ impl Arbitrary for TestCase {
reducible: false,
block_params: true,
always_local_uses: false,
reftypes: true,
})?,
})
}
Expand Down
44 changes: 42 additions & 2 deletions src/bitvec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,18 @@ impl BitVec {
}
}

pub fn assign(&mut self, other: &Self) {
if other.bits.len() > 0 {
self.ensure_idx(other.bits.len() - 1);
}
for i in 0..other.bits.len() {
self.bits[i] = other.bits[i];
}
for i in other.bits.len()..self.bits.len() {
self.bits[i] = 0;
}
}

#[inline(always)]
pub fn get(&mut self, idx: usize) -> bool {
let word = idx / BITS_PER_WORD;
Expand All @@ -59,16 +71,21 @@ impl BitVec {
}
}

pub fn or(&mut self, other: &Self) {
pub fn or(&mut self, other: &Self) -> bool {
if other.bits.is_empty() {
return;
return false;
}
let last_idx = other.bits.len() - 1;
self.ensure_idx(last_idx);

let mut changed = false;
for (self_word, other_word) in self.bits.iter_mut().zip(other.bits.iter()) {
if *other_word & !*self_word != 0 {
changed = true;
}
*self_word |= *other_word;
}
changed
}

pub fn and(&mut self, other: &Self) {
Expand All @@ -91,6 +108,29 @@ impl BitVec {
}
}

impl std::cmp::PartialEq for BitVec {
fn eq(&self, other: &Self) -> bool {
let limit = std::cmp::min(self.bits.len(), other.bits.len());
for i in 0..limit {
if self.bits[i] != other.bits[i] {
return false;
}
}
for i in limit..self.bits.len() {
if self.bits[i] != 0 {
return false;
}
}
for i in limit..other.bits.len() {
if other.bits[i] != 0 {
return false;
}
}
true
}
}
impl std::cmp::Eq for BitVec {}

pub struct SetBitsIter<'a> {
words: &'a [u64],
word_idx: usize,
Expand Down
142 changes: 129 additions & 13 deletions src/checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@

use crate::{
Allocation, AllocationKind, Block, Edit, Function, Inst, InstPosition, Operand, OperandKind,
OperandPolicy, OperandPos, Output, ProgPoint, VReg,
OperandPolicy, OperandPos, Output, PReg, ProgPoint, SpillSlot, VReg,
};

use std::collections::{HashMap, VecDeque};
use std::collections::{HashMap, HashSet, VecDeque};
use std::default::Default;
use std::hash::Hash;
use std::result::Result;
Expand Down Expand Up @@ -127,6 +127,20 @@ pub enum CheckerError {
alloc: Allocation,
expected_alloc: Allocation,
},
AllocationIsNotStack {
inst: Inst,
op: Operand,
alloc: Allocation,
},
ConflictedValueInStackmap {
inst: Inst,
slot: SpillSlot,
},
NonRefValueInStackmap {
inst: Inst,
slot: SpillSlot,
vreg: VReg,
},
}

/// Abstract state for an allocation.
Expand Down Expand Up @@ -162,8 +176,10 @@ impl CheckerValue {
(_, &CheckerValue::Unknown) => *self,
(&CheckerValue::Conflicted, _) => *self,
(_, &CheckerValue::Conflicted) => *other,
(&CheckerValue::Reg(r1, ref1), &CheckerValue::Reg(r2, ref2)) if r1 == r2 => {
CheckerValue::Reg(r1, ref1 || ref2)
(&CheckerValue::Reg(r1, ref1), &CheckerValue::Reg(r2, ref2))
if r1 == r2 && ref1 == ref2 =>
{
CheckerValue::Reg(r1, ref1)
}
_ => {
log::debug!("{:?} and {:?} meet to Conflicted", self, other);
Expand Down Expand Up @@ -192,7 +208,8 @@ impl std::fmt::Display for CheckerValue {
match self {
CheckerValue::Unknown => write!(f, "?"),
CheckerValue::Conflicted => write!(f, "!"),
CheckerValue::Reg(r, _) => write!(f, "{}", r),
CheckerValue::Reg(r, false) => write!(f, "{}", r),
CheckerValue::Reg(r, true) => write!(f, "{}/ref", r),
}
}
}
Expand Down Expand Up @@ -305,13 +322,38 @@ impl CheckerState {
self.check_val(inst, *op, *alloc, val, allocs)?;
}
}
&CheckerInst::Safepoint { inst, ref slots } => {
for &slot in slots {
let alloc = Allocation::stack(slot);
let val = self
.allocations
.get(&alloc)
.cloned()
.unwrap_or(Default::default());
debug!(
"checker: checkinst {:?}: safepoint slot {}, checker value {:?}",
checkinst, slot, val
);

match val {
CheckerValue::Unknown => {}
CheckerValue::Conflicted => {
return Err(CheckerError::ConflictedValueInStackmap { inst, slot });
}
CheckerValue::Reg(vreg, false) => {
return Err(CheckerError::NonRefValueInStackmap { inst, slot, vreg });
}
CheckerValue::Reg(_, true) => {}
}
}
}
_ => {}
}
Ok(())
}

/// Update according to instruction.
fn update(&mut self, checkinst: &CheckerInst) {
fn update<'a, F: Function>(&mut self, checkinst: &CheckerInst, checker: &Checker<'a, F>) {
match checkinst {
&CheckerInst::Move { into, from } => {
let val = self
Expand All @@ -328,14 +370,19 @@ impl CheckerState {
&CheckerInst::Op {
ref operands,
ref allocs,
ref clobbers,
..
} => {
for (op, alloc) in operands.iter().zip(allocs.iter()) {
if op.kind() != OperandKind::Def {
continue;
}
let reftyped = checker.reftyped_vregs.contains(&op.vreg());
self.allocations
.insert(*alloc, CheckerValue::Reg(op.vreg(), false));
.insert(*alloc, CheckerValue::Reg(op.vreg(), reftyped));
}
for clobber in clobbers {
self.allocations.remove(&Allocation::reg(*clobber));
}
}
&CheckerInst::BlockParams {
Expand All @@ -344,8 +391,20 @@ impl CheckerState {
..
} => {
for (vreg, alloc) in vregs.iter().zip(allocs.iter()) {
let reftyped = checker.reftyped_vregs.contains(vreg);
self.allocations
.insert(*alloc, CheckerValue::Reg(*vreg, false));
.insert(*alloc, CheckerValue::Reg(*vreg, reftyped));
}
}
&CheckerInst::Safepoint { ref slots, .. } => {
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()) {
*value = CheckerValue::Conflicted;
}
}
}
}
}
Expand All @@ -365,6 +424,11 @@ impl CheckerState {
return Err(CheckerError::AllocationIsNotReg { inst, op, alloc });
}
}
OperandPolicy::Stack => {
if alloc.kind() != AllocationKind::Stack {
return Err(CheckerError::AllocationIsNotStack { inst, op, alloc });
}
}
OperandPolicy::FixedReg(preg) => {
if alloc != Allocation::reg(preg) {
return Err(CheckerError::AllocationIsNotFixedReg { inst, op, alloc });
Expand Down Expand Up @@ -402,6 +466,7 @@ pub(crate) enum CheckerInst {
inst: Inst,
operands: Vec<Operand>,
allocs: Vec<Allocation>,
clobbers: Vec<PReg>,
},

/// The top of a block with blockparams. We define the given vregs
Expand All @@ -411,13 +476,18 @@ pub(crate) enum CheckerInst {
vregs: Vec<VReg>,
allocs: Vec<Allocation>,
},

/// A safepoint, with the given SpillSlots specified as containing
/// reftyped values. All other reftyped values become invalid.
Safepoint { inst: Inst, slots: Vec<SpillSlot> },
}

#[derive(Debug)]
pub struct Checker<'a, F: Function> {
f: &'a F,
bb_in: HashMap<Block, CheckerState>,
bb_insts: HashMap<Block, Vec<CheckerInst>>,
reftyped_vregs: HashSet<VReg>,
}

impl<'a, F: Function> Checker<'a, F> {
Expand All @@ -428,20 +498,39 @@ impl<'a, F: Function> Checker<'a, F> {
pub fn new(f: &'a F) -> Checker<'a, F> {
let mut bb_in = HashMap::new();
let mut bb_insts = HashMap::new();
let mut reftyped_vregs = HashSet::new();

for block in 0..f.blocks() {
let block = Block::new(block);
bb_in.insert(block, Default::default());
bb_insts.insert(block, vec![]);
}

Checker { f, bb_in, bb_insts }
for &vreg in f.reftype_vregs() {
reftyped_vregs.insert(vreg);
}

Checker {
f,
bb_in,
bb_insts,
reftyped_vregs,
}
}

/// Build the list of checker instructions based on the given func
/// and allocation results.
pub fn prepare(&mut self, out: &Output) {
debug!("checker: out = {:?}", out);
// Preprocess safepoint stack-maps into per-inst vecs.
let mut safepoint_slots: HashMap<Inst, Vec<SpillSlot>> = HashMap::new();
for &(progpoint, slot) in &out.safepoint_slots {
safepoint_slots
.entry(progpoint.inst)
.or_insert_with(|| vec![])
.push(slot);
}

// For each original instruction, create an `Op`.
let mut last_inst = None;
let mut insert_idx = 0;
Expand All @@ -454,13 +543,23 @@ impl<'a, F: Function> Checker<'a, F> {
// Any inserted edits before instruction.
self.handle_edits(block, out, &mut insert_idx, ProgPoint::before(inst));

// If this is a safepoint, then check the spillslots at this point.
if self.f.is_safepoint(inst) {
let slots = safepoint_slots.remove(&inst).unwrap_or_else(|| vec![]);

let checkinst = CheckerInst::Safepoint { inst, slots };
self.bb_insts.get_mut(&block).unwrap().push(checkinst);
}

// Instruction itself.
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 checkinst = CheckerInst::Op {
inst,
operands,
allocs,
clobbers,
};
debug!("checker: adding inst {:?}", checkinst);
self.bb_insts.get_mut(&block).unwrap().push(checkinst);
Expand Down Expand Up @@ -511,7 +610,7 @@ impl<'a, F: Function> Checker<'a, F> {
let mut state = self.bb_in.get(&block).cloned().unwrap();
debug!("analyze: block {} has state {:?}", block.index(), state);
for inst in self.bb_insts.get(&block).unwrap() {
state.update(inst);
state.update(inst, self);
debug!("analyze: inst {:?} -> state {:?}", inst, state);
}

Expand Down Expand Up @@ -546,7 +645,7 @@ impl<'a, F: Function> Checker<'a, F> {
debug!("Checker error: {:?}", e);
errors.push(e);
}
state.update(inst);
state.update(inst, self);
if let Err(e) = state.check(InstPosition::After, inst) {
debug!("Checker error: {:?}", e);
errors.push(e);
Expand Down Expand Up @@ -575,6 +674,9 @@ impl<'a, F: Function> Checker<'a, F> {
}
debug!(" {{ {} }}", s.join(", "))
}
for vreg in self.f.reftype_vregs() {
debug!(" REF: {}", vreg);
}
for bb in 0..self.f.blocks() {
let bb = Block::new(bb);
debug!("block{}:", bb.index());
Expand All @@ -587,8 +689,15 @@ impl<'a, F: Function> Checker<'a, F> {
inst,
ref operands,
ref allocs,
ref clobbers,
} => {
debug!(" inst{}: {:?} ({:?})", inst.index(), operands, allocs);
debug!(
" inst{}: {:?} ({:?}) clobbers:{:?}",
inst.index(),
operands,
allocs,
clobbers
);
}
&CheckerInst::Move { from, into } => {
debug!(" {} -> {}", from, into);
Expand All @@ -604,8 +713,15 @@ impl<'a, F: Function> Checker<'a, F> {
}
debug!(" blockparams: {}", args.join(", "));
}
&CheckerInst::Safepoint { ref slots, .. } => {
let mut slotargs = vec![];
for &slot in slots {
slotargs.push(format!("{}", slot));
}
debug!(" safepoint: {}", slotargs.join(", "));
}
}
state.update(inst);
state.update(inst, &self);
print_state(&state);
}
}
Expand Down
Loading

0 comments on commit a08b012

Please sign in to comment.