Fix review comments

This commit is contained in:
bjorn3
2019-08-31 12:24:31 +02:00
committed by Dan Gohman
parent e2b2b520eb
commit e8d4ef7c3d
2 changed files with 13 additions and 9 deletions

View File

@@ -89,6 +89,8 @@ fn unwrap_inst(
fmtln!(fmt, "func.dfg.resolve_aliases(args[{}]),", n); fmtln!(fmt, "func.dfg.resolve_aliases(args[{}]),", n);
} else if op.is_varargs() { } else if op.is_varargs() {
let n = inst.imm_opnums.iter().chain(inst.value_opnums.iter()).max().map(|n| n + 1).unwrap_or(0); let n = inst.imm_opnums.iter().chain(inst.value_opnums.iter()).max().map(|n| n + 1).unwrap_or(0);
// We need to create a `Vec` here, as using a slice would result in a borrowck
// error later on.
fmtln!(fmt, "\ fmtln!(fmt, "\
args.iter().skip({}).map(|&arg| func.dfg.resolve_aliases(arg)).collect::<Vec<_>>(),\ args.iter().skip({}).map(|&arg| func.dfg.resolve_aliases(arg)).collect::<Vec<_>>(),\
", n); ", n);
@@ -115,6 +117,9 @@ fn unwrap_inst(
let name = var_pool let name = var_pool
.get(apply.args[i].maybe_var().expect("vararg without name")) .get(apply.args[i].maybe_var().expect("vararg without name"))
.name; .name;
// Above name is set to an `Vec` representing the varargs. However it is expected to be
// `&[Value]` below, so we borrow it.
fmtln!(fmt, "let {} = &{};", name, name); fmtln!(fmt, "let {} = &{};", name, name);
} }
} }
@@ -419,6 +424,8 @@ fn gen_transform<'a>(
} }
if transform.def_pool.get(transform.src).apply.inst.is_branch { if transform.def_pool.get(transform.src).apply.inst.is_branch {
// A branch might have been legalized into multiple branches, so we need to recompute
// the cfg.
fmt.line("cfg.recompute_ebb(pos.func, pos.current_ebb().unwrap());"); fmt.line("cfg.recompute_ebb(pos.func, pos.current_ebb().unwrap());");
} }

View File

@@ -75,12 +75,11 @@ fn legalize_inst(
match pos.func.dfg.value_def(arg) { match pos.func.dfg.value_def(arg) {
ir::ValueDef::Result(inst, _num) => { ir::ValueDef::Result(inst, _num) => {
if let ir::InstructionData::Binary { if let ir::InstructionData::Binary {
opcode, args: _, .. opcode: ir::Opcode::Iconcat,
..
} = pos.func.dfg[inst] } = pos.func.dfg[inst]
{ {
if opcode != ir::Opcode::Iconcat { // `arg` was created by an `iconcat` instruction.
return LegalizeInstResult::SplitLegalizePending;
}
} else { } else {
// `arg` was not created by an `iconcat` instruction. Don't try to resolve it, // `arg` was not created by an `iconcat` instruction. Don't try to resolve it,
// as otherwise `split::isplit` will re-insert the original `isplit`, causing // as otherwise `split::isplit` will re-insert the original `isplit`, causing
@@ -153,7 +152,9 @@ pub fn legalize_function(func: &mut ir::Function, cfg: &mut ControlFlowGraph, is
// Process EBBs in layout order. Some legalization actions may split the current EBB or append // Process EBBs in layout order. Some legalization actions may split the current EBB or append
// new ones to the end. We need to make sure we visit those new EBBs too. // new ones to the end. We need to make sure we visit those new EBBs too.
while let Some(_ebb) = pos.next_ebb() { while let Some(ebb) = pos.next_ebb() {
split::split_ebb_params(pos.func, cfg, ebb);
// Keep track of the cursor position before the instruction being processed, so we can // Keep track of the cursor position before the instruction being processed, so we can
// double back when replacing instructions. // double back when replacing instructions.
let mut prev_pos = pos.position(); let mut prev_pos = pos.position();
@@ -176,10 +177,6 @@ pub fn legalize_function(func: &mut ir::Function, cfg: &mut ControlFlowGraph, is
} }
} }
while let Some(ebb) = pos.next_ebb() {
split::split_ebb_params(pos.func, cfg, ebb);
}
// Try legalizing `isplit` and `vsplit` instructions, which could not previously be legalized. // Try legalizing `isplit` and `vsplit` instructions, which could not previously be legalized.
for inst in pending_splits { for inst in pending_splits {
pos.goto_inst(inst); pos.goto_inst(inst);