Skip to content

Rework the API for outgoing blockparams #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 1, 2021
Merged

Rework the API for outgoing blockparams #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0cb3a80
Rework the API for outgoing blockparams
Amanieu Nov 30, 2021
6621a57
Fix liveranges for branch parameters
Amanieu Dec 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cfg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ impl CFGInfo {
}
if require_no_branch_args {
let last = f.block_insns(block).last();
if f.branch_blockparam_arg_offset(block, last) > 0 {
if !f.inst_operands(last).is_empty() {
return Err(RegAllocError::DisallowedBranchArg(last));
}
}
Expand Down
68 changes: 44 additions & 24 deletions src/fuzzing/func.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use arbitrary::{Arbitrary, Unstructured};

#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum InstOpcode {
Phi,
Op,
Ret,
Branch,
Expand All @@ -40,14 +39,10 @@ impl InstData {
is_safepoint: false,
}
}
pub fn branch(uses: &[usize]) -> InstData {
let mut operands = vec![];
for &u in uses {
operands.push(Operand::reg_use(VReg::new(u, RegClass::Int)));
}
pub fn branch() -> InstData {
InstData {
op: InstOpcode::Branch,
operands,
operands: vec![],
clobbers: vec![],
is_safepoint: false,
}
Expand All @@ -68,7 +63,8 @@ pub struct Func {
blocks: Vec<InstRange>,
block_preds: Vec<Vec<Block>>,
block_succs: Vec<Vec<Block>>,
block_params: Vec<Vec<VReg>>,
block_params_in: Vec<Vec<VReg>>,
block_params_out: Vec<Vec<Vec<VReg>>>,
num_vregs: usize,
reftype_vregs: Vec<VReg>,
}
Expand Down Expand Up @@ -100,7 +96,7 @@ impl Function for Func {
}

fn block_params(&self, block: Block) -> &[VReg] {
&self.block_params[block.index()][..]
&self.block_params_in[block.index()][..]
}

fn is_ret(&self, insn: Inst) -> bool {
Expand All @@ -111,10 +107,8 @@ impl Function for Func {
self.insts[insn.index()].op == InstOpcode::Branch
}

fn branch_blockparam_arg_offset(&self, _: Block, _: Inst) -> usize {
// Branch blockparam args always start at zero for this
// Function implementation.
0
fn branch_blockparams(&self, block: Block, _: Inst, succ: usize) -> &[VReg] {
&self.block_params_out[block.index()][succ][..]
}

fn requires_refs_on_stack(&self, insn: Inst) -> bool {
Expand Down Expand Up @@ -164,7 +158,8 @@ impl FuncBuilder {
f: Func {
block_preds: vec![],
block_succs: vec![],
block_params: vec![],
block_params_in: vec![],
block_params_out: vec![],
insts: vec![],
blocks: vec![],
num_vregs: 0,
Expand All @@ -181,7 +176,8 @@ impl FuncBuilder {
.push(InstRange::forward(Inst::new(0), Inst::new(0)));
self.f.block_preds.push(vec![]);
self.f.block_succs.push(vec![]);
self.f.block_params.push(vec![]);
self.f.block_params_in.push(vec![]);
self.f.block_params_out.push(vec![]);
self.insts_per_block.push(vec![]);
b
}
Expand All @@ -195,8 +191,12 @@ impl FuncBuilder {
self.f.block_preds[to.index()].push(from);
}

pub fn set_block_params(&mut self, block: Block, params: &[VReg]) {
self.f.block_params[block.index()] = params.iter().cloned().collect();
pub fn set_block_params_in(&mut self, block: Block, params: &[VReg]) {
self.f.block_params_in[block.index()] = params.iter().cloned().collect();
}

pub fn set_block_params_out(&mut self, block: Block, params: Vec<Vec<VReg>>) {
self.f.block_params_out[block.index()] = params;
}

fn compute_doms(&mut self) {
Expand Down Expand Up @@ -388,7 +388,7 @@ impl Func {
}
}
vregs_by_block_to_be_defined.last_mut().unwrap().reverse();
builder.set_block_params(Block::new(block), &block_params[block][..]);
builder.set_block_params_in(Block::new(block), &block_params[block][..]);
}

for block in 0..num_blocks {
Expand Down Expand Up @@ -510,9 +510,10 @@ impl Func {
// Define the branch with blockparam args that must end
// the block.
if builder.f.block_succs[block].len() > 0 {
let mut args = vec![];
let mut params = vec![];
for &succ in &builder.f.block_succs[block] {
for _ in 0..builder.f.block_params[succ.index()].len() {
let mut args = vec![];
for _ in 0..builder.f.block_params_in[succ.index()].len() {
let dom_block = choose_dominating_block(
&builder.idom[..],
Block::new(block),
Expand All @@ -524,10 +525,12 @@ impl Func {
} else {
u.choose(&avail[..])?
};
args.push(vreg.vreg());
args.push(*vreg);
}
params.push(args);
}
builder.add_inst(Block::new(block), InstData::branch(&args[..]));
builder.set_block_params_out(Block::new(block), params);
builder.add_inst(Block::new(block), InstData::branch());
} else {
builder.add_inst(Block::new(block), InstData::ret());
}
Expand All @@ -552,15 +555,29 @@ impl std::fmt::Debug for Func {
.iter()
.map(|b| b.index())
.collect::<Vec<_>>();
let params = self.block_params[i]
let params_in = self.block_params_in[i]
.iter()
.map(|v| format!("v{}", v.vreg()))
.collect::<Vec<_>>()
.join(", ");
let params_out = self.block_params_out[i]
.iter()
.enumerate()
.map(|(succ_idx, vec)| {
let succ = self.block_succs[i][succ_idx];
let params = vec
.iter()
.map(|v| format!("v{}", v.vreg()))
.collect::<Vec<_>>()
.join(", ");
format!("block{}({})", succ.index(), params)
})
.collect::<Vec<_>>()
.join(", ");
write!(
f,
" block{}({}): # succs:{:?} preds:{:?}\n",
i, params, succs, preds
i, params_in, succs, preds
)?;
for inst in blockrange.iter() {
if self.requires_refs_on_stack(inst) {
Expand All @@ -574,6 +591,9 @@ impl std::fmt::Debug for Func {
self.insts[inst.index()].operands,
self.insts[inst.index()].clobbers
)?;
if let InstOpcode::Branch = self.insts[inst.index()].op {
write!(f, " params: {}\n", params_out)?;
}
}
}
write!(f, "}}\n")?;
Expand Down
58 changes: 32 additions & 26 deletions src/ion/liveranges.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ impl<'a, F: Function> Env<'a, F> {
while !workqueue.is_empty() {
let block = workqueue.pop_front().unwrap();
workqueue_set.remove(&block);
let insns = self.func.block_insns(block);

log::trace!("computing liveins for block{}", block.index());

Expand All @@ -323,7 +324,16 @@ impl<'a, F: Function> Env<'a, F> {
let mut live = self.liveouts[block.index()].clone();
log::trace!(" -> initial liveout set: {:?}", live);

for inst in self.func.block_insns(block).rev().iter() {
// Include outgoing blockparams in the initial live set.
if self.func.is_branch(insns.last()) {
for i in 0..self.func.block_succs(block).len() {
for param in self.func.branch_blockparams(block, insns.last(), i) {
live.set(param.vreg(), true);
}
}
}

for inst in insns.rev().iter() {
if let Some((src, dst)) = self.func.is_move(inst) {
live.set(dst.vreg().vreg(), false);
live.set(src.vreg().vreg(), true);
Expand Down Expand Up @@ -399,12 +409,33 @@ impl<'a, F: Function> Env<'a, F> {

for i in (0..self.func.num_blocks()).rev() {
let block = Block::new(i);
let insns = self.func.block_insns(block);

self.stats.livein_blocks += 1;

// Init our local live-in set.
let mut live = self.liveouts[block.index()].clone();

// If the last instruction is a branch (rather than
// return), create blockparam_out entries.
if self.func.is_branch(insns.last()) {
for (i, &succ) in self.func.block_succs(block).iter().enumerate() {
let blockparams_in = self.func.block_params(succ);
let blockparams_out = self.func.branch_blockparams(block, insns.last(), i);
for (&blockparam_in, &blockparam_out) in
blockparams_in.iter().zip(blockparams_out)
{
let blockparam_out = VRegIndex::new(blockparam_out.vreg());
let blockparam_in = VRegIndex::new(blockparam_in.vreg());
self.blockparam_outs
.push((blockparam_out, block, succ, blockparam_in));

// Include outgoing blockparams in the initial live set.
live.set(blockparam_out.index(), true);
}
}
}

// Initially, registers are assumed live for the whole block.
for vreg in live.iter() {
let range = CodeRange {
Expand All @@ -426,24 +457,6 @@ impl<'a, F: Function> Env<'a, F> {
self.vregs[param.vreg()].blockparam = block;
}

let insns = self.func.block_insns(block);

// If the last instruction is a branch (rather than
// return), create blockparam_out entries.
if self.func.is_branch(insns.last()) {
let operands = self.func.inst_operands(insns.last());
let mut i = self.func.branch_blockparam_arg_offset(block, insns.last());
for &succ in self.func.block_succs(block) {
for &blockparam in self.func.block_params(succ) {
let from_vreg = VRegIndex::new(operands[i].vreg().vreg());
let blockparam_vreg = VRegIndex::new(blockparam.vreg());
self.blockparam_outs
.push((from_vreg, block, succ, blockparam_vreg));
i += 1;
}
}
}

// For each instruction, in reverse order, process
// operands and clobbers.
for inst in insns.rev().iter() {
Expand Down Expand Up @@ -892,13 +905,6 @@ impl<'a, F: Function> Env<'a, F> {
(OperandKind::Def, OperandPos::Early) => ProgPoint::before(inst),
(OperandKind::Def, OperandPos::Late) => ProgPoint::after(inst),
(OperandKind::Use, OperandPos::Late) => ProgPoint::after(inst),
// If this is a branch, extend `pos` to
// the end of the block. (Branch uses are
// blockparams and need to be live at the
// end of the block.)
(OperandKind::Use, _) if self.func.is_branch(inst) => {
self.cfginfo.block_exit[block.index()]
}
// If there are any reused inputs in this
// instruction, and this is *not* the
// reused input, force `pos` to
Expand Down
17 changes: 5 additions & 12 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -860,21 +860,14 @@ pub trait Function {
fn is_ret(&self, insn: Inst) -> bool;

/// Determine whether an instruction is the end-of-block
/// branch. If so, its operands at the indices given by
/// `branch_blockparam_arg_offset()` below *must* be the block
/// parameters for each of its block's `block_succs` successor
/// blocks, in order.
/// branch.
fn is_branch(&self, insn: Inst) -> bool;

/// If `insn` is a branch at the end of `block`, returns the
/// operand index at which outgoing blockparam arguments are
/// found. Starting at this index, blockparam arguments for each
/// successor block's blockparams, in order, must be found.
///
/// It is an error if `self.inst_operands(insn).len() -
/// self.branch_blockparam_arg_offset(insn)` is not exactly equal
/// to the sum of blockparam counts for all successor blocks.
fn branch_blockparam_arg_offset(&self, block: Block, insn: Inst) -> usize;
/// outgoing blockparam arguments for the given successor. The
/// number of arguments must match the number incoming blockparams
/// for each respective successor block.
fn branch_blockparams(&self, block: Block, insn: Inst, succ_idx: usize) -> &[VReg];

/// Determine whether an instruction requires all reference-typed
/// values to be placed onto the stack. For these instructions,
Expand Down
13 changes: 6 additions & 7 deletions src/ssa.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,12 @@ pub fn validate_ssa<F: Function>(f: &F, cfginfo: &CFGInfo) -> Result<(), RegAllo
return Err(RegAllocError::BB(block));
}
if f.is_branch(insn) {
let expected = f
.block_succs(block)
.iter()
.map(|&succ| f.block_params(succ).len())
.sum();
if f.inst_operands(insn).len() != expected {
return Err(RegAllocError::Branch(insn));
for (i, &succ) in f.block_succs(block).iter().enumerate() {
let blockparams_in = f.block_params(succ);
let blockparams_out = f.branch_blockparams(block, insn, i);
if blockparams_in.len() != blockparams_out.len() {
return Err(RegAllocError::Branch(insn));
}
}
}
} else {
Expand Down