From 1e6c13d83eeaf11c1ab885050bdea8023a1c541c Mon Sep 17 00:00:00 2001 From: Trevor Elliott Date: Tue, 17 Jan 2023 16:31:15 -0800 Subject: [PATCH] cranelift: Rework block instructions to use BlockCall (#5464) Add a new type BlockCall that represents the pair of a block name with arguments to be passed to it. (The mnemonic here is that it looks a bit like a function call.) Rework the implementation of jump, brz, and brnz to use BlockCall instead of storing the block arguments as varargs in the instruction's ValueList. To ensure that we're processing block arguments from BlockCall values in instructions, three new functions have been introduced on DataFlowGraph that both sets of arguments: inst_values - returns an iterator that traverses values in the instruction and block arguments map_inst_values - applies a function to each value in the instruction and block arguments overwrite_inst_values - overwrite all values in an instruction and block arguments with values from the iterator Co-authored-by: Jamey Sharp --- cranelift/codegen/meta/src/cdsl/operands.rs | 4 + cranelift/codegen/meta/src/gen_inst.rs | 105 +++++++--------- cranelift/codegen/meta/src/shared/entities.rs | 17 ++- cranelift/codegen/meta/src/shared/formats.rs | 7 +- .../codegen/meta/src/shared/instructions.rs | 13 +- cranelift/codegen/src/dce.rs | 5 +- cranelift/codegen/src/dominator_tree.rs | 4 +- cranelift/codegen/src/egraph.rs | 17 +-- cranelift/codegen/src/egraph/elaborate.rs | 57 ++++----- cranelift/codegen/src/flowgraph.rs | 4 +- cranelift/codegen/src/inst_predicates.rs | 6 +- cranelift/codegen/src/ir/dfg.rs | 82 ++++++++++--- cranelift/codegen/src/ir/function.rs | 6 +- cranelift/codegen/src/ir/instructions.rs | 111 ++++++++++++++--- cranelift/codegen/src/ir/mod.rs | 2 +- cranelift/codegen/src/isa/aarch64/lower.isle | 18 +-- cranelift/codegen/src/isa/riscv64/inst.isle | 18 +-- cranelift/codegen/src/isa/s390x/lower.isle | 6 +- cranelift/codegen/src/isa/x64/lower.isle | 22 ++-- cranelift/codegen/src/licm.rs | 5 +- cranelift/codegen/src/machinst/lower.rs | 17 ++- cranelift/codegen/src/opts.rs | 6 +- cranelift/codegen/src/remove_constant_phis.rs | 61 ++++------ cranelift/codegen/src/simple_preopt.rs | 115 +++++------------- cranelift/codegen/src/verifier/mod.rs | 40 +++--- cranelift/codegen/src/write.rs | 21 ++-- cranelift/frontend/src/frontend.rs | 14 ++- cranelift/frontend/src/ssa.rs | 26 ++-- cranelift/interpreter/src/step.rs | 28 +++-- cranelift/preopt/src/constant_folding.rs | 37 +++--- cranelift/reader/src/parser.rs | 15 +-- cranelift/src/bugpoint.rs | 8 +- 32 files changed, 475 insertions(+), 422 deletions(-) diff --git a/cranelift/codegen/meta/src/cdsl/operands.rs b/cranelift/codegen/meta/src/cdsl/operands.rs index 8420c62158..15c10fe4e7 100644 --- a/cranelift/codegen/meta/src/cdsl/operands.rs +++ b/cranelift/codegen/meta/src/cdsl/operands.rs @@ -152,6 +152,10 @@ impl OperandKind { | OperandKindFields::VariableArgs => unreachable!(), } } + + pub(crate) fn is_block(&self) -> bool { + self.rust_type == "ir::BlockCall" + } } impl Into for &TypeVar { diff --git a/cranelift/codegen/meta/src/gen_inst.rs b/cranelift/codegen/meta/src/gen_inst.rs index 073869dbb7..85bee66abe 100644 --- a/cranelift/codegen/meta/src/gen_inst.rs +++ b/cranelift/codegen/meta/src/gen_inst.rs @@ -158,8 +158,6 @@ fn gen_arguments_method(formats: &[&InstructionFormat], fmt: &mut Formatter, is_ /// - `pub fn opcode(&self) -> Opcode` /// - `pub fn arguments(&self, &pool) -> &[Value]` /// - `pub fn arguments_mut(&mut self, &pool) -> &mut [Value]` -/// - `pub fn value_list(&self) -> Option` -/// - `pub fn value_list_mut(&mut self) -> Option<&mut ir::ValueList>` /// - `pub fn eq(&self, &other: Self, &pool) -> bool` /// - `pub fn hash(&self, state: &mut H, &pool)` fn gen_instruction_data_impl(formats: &[&InstructionFormat], fmt: &mut Formatter) { @@ -213,51 +211,6 @@ fn gen_instruction_data_impl(formats: &[&InstructionFormat], fmt: &mut Formatter gen_arguments_method(formats, fmt, true); fmt.empty_line(); - fmt.doc_comment(r#" - The ValueList for the instruction. - "#); - fmt.line("pub fn value_list(&self) -> Option {"); - fmt.indent(|fmt| { - let mut m = Match::new("*self"); - - for format in formats { - if format.has_value_list { - m.arm(format!("Self::{}", format.name), - vec!["args", ".."], - "Some(args)".to_string()); - } - } - - m.arm_no_fields("_", "None"); - - fmt.add_match(m); - }); - fmt.line("}"); - fmt.empty_line(); - - fmt.doc_comment(r#" - A mutable reference to the ValueList for this instruction, if it - has one. - "#); - fmt.line("pub fn value_list_mut(&mut self) -> Option<&mut ir::ValueList> {"); - fmt.indent(|fmt| { - let mut m = Match::new("*self"); - - for format in formats { - if format.has_value_list { - m.arm(format!("Self::{}", format.name), - vec!["ref mut args", ".."], - "Some(args)".to_string()); - } - } - - m.arm_no_fields("_", "None"); - - fmt.add_match(m); - }); - fmt.line("}"); - fmt.empty_line(); - fmt.doc_comment(r#" Compare two `InstructionData` for equality. @@ -898,12 +851,7 @@ fn gen_format_constructor(format: &InstructionFormat, fmt: &mut Formatter) { /// instruction reference itself for instructions that don't have results. fn gen_inst_builder(inst: &Instruction, format: &InstructionFormat, fmt: &mut Formatter) { // Construct method arguments. - let mut args = vec![if format.has_value_list { - "mut self" - } else { - "self" - } - .to_string()]; + let mut args = vec![String::new()]; let mut args_doc = Vec::new(); let mut rets_doc = Vec::new(); @@ -922,17 +870,39 @@ fn gen_inst_builder(inst: &Instruction, format: &InstructionFormat, fmt: &mut Fo let mut tmpl_types = Vec::new(); let mut into_args = Vec::new(); + let mut block_args = Vec::new(); for op in &inst.operands_in { - let t = if op.is_immediate() { - let t = format!("T{}", tmpl_types.len() + 1); - tmpl_types.push(format!("{}: Into<{}>", t, op.kind.rust_type)); - into_args.push(op.name); - t + if op.kind.is_block() { + args.push(format!("{}_label: {}", op.name, "ir::Block")); + args_doc.push(format!( + "- {}_label: {}", + op.name, "Destination basic block" + )); + + args.push(format!("{}_args: {}", op.name, "&[Value]")); + args_doc.push(format!("- {}_args: {}", op.name, "Block arguments")); + + block_args.push(op); } else { - op.kind.rust_type.to_string() - }; - args.push(format!("{}: {}", op.name, t)); - args_doc.push(format!("- {}: {}", op.name, op.doc())); + let t = if op.is_immediate() { + let t = format!("T{}", tmpl_types.len() + 1); + tmpl_types.push(format!("{}: Into<{}>", t, op.kind.rust_type)); + into_args.push(op.name); + t + } else { + op.kind.rust_type.to_string() + }; + args.push(format!("{}: {}", op.name, t)); + args_doc.push(format!("- {}: {}", op.name, op.doc())); + } + } + + // We need to mutate `self` if this instruction accepts a value list, or will construct + // BlockCall values. + if format.has_value_list || !block_args.is_empty() { + args[0].push_str("mut self"); + } else { + args[0].push_str("self"); } for op in &inst.operands_out { @@ -981,10 +951,19 @@ fn gen_inst_builder(inst: &Instruction, format: &InstructionFormat, fmt: &mut Fo fmtln!(fmt, "fn {} {{", proto); fmt.indent(|fmt| { // Convert all of the `Into<>` arguments. - for arg in &into_args { + for arg in into_args { fmtln!(fmt, "let {} = {}.into();", arg, arg); } + // Convert block references + for op in block_args { + fmtln!( + fmt, + "let {0} = self.data_flow_graph_mut().block_call({0}_label, {0}_args);", + op.name + ); + } + // Arguments for instruction constructor. let first_arg = format!("Opcode::{}", inst.camel_name); let mut args = vec![first_arg.as_str()]; diff --git a/cranelift/codegen/meta/src/shared/entities.rs b/cranelift/codegen/meta/src/shared/entities.rs index 6a3ae9b0fd..fdad737c7d 100644 --- a/cranelift/codegen/meta/src/shared/entities.rs +++ b/cranelift/codegen/meta/src/shared/entities.rs @@ -11,9 +11,13 @@ fn new(format_field_name: &'static str, rust_type: &'static str, doc: &'static s } pub(crate) struct EntityRefs { + /// A reference to a basic block in the same function, with its arguments provided. + /// This is primarily used in control flow instructions. + pub(crate) block_call: OperandKind, + /// A reference to a basic block in the same function. - /// This is primarliy used in control flow instructions. - pub(crate) block: OperandKind, + /// This is primarily used in control flow instructions. + pub(crate) label: OperandKind, /// A reference to a stack slot declared in the function preamble. pub(crate) stack_slot: OperandKind, @@ -45,11 +49,18 @@ pub(crate) struct EntityRefs { impl EntityRefs { pub fn new() -> Self { Self { - block: new( + block_call: new( + "destination", + "ir::BlockCall", + "a basic block in the same function, with its arguments provided.", + ), + + label: new( "destination", "ir::Block", "a basic block in the same function.", ), + stack_slot: new("stack_slot", "ir::StackSlot", "A stack slot"), dynamic_stack_slot: new( diff --git a/cranelift/codegen/meta/src/shared/formats.rs b/cranelift/codegen/meta/src/shared/formats.rs index ad3bdd7e31..c017836091 100644 --- a/cranelift/codegen/meta/src/shared/formats.rs +++ b/cranelift/codegen/meta/src/shared/formats.rs @@ -111,17 +111,16 @@ impl Formats { .value() .build(), - jump: Builder::new("Jump").imm(&entities.block).varargs().build(), + jump: Builder::new("Jump").imm(&entities.block_call).build(), branch: Builder::new("Branch") .value() - .imm(&entities.block) - .varargs() + .imm(&entities.block_call) .build(), branch_table: Builder::new("BranchTable") .value() - .imm(&entities.block) + .imm(&entities.label) .imm(&entities.jump_table) .build(), diff --git a/cranelift/codegen/meta/src/shared/instructions.rs b/cranelift/codegen/meta/src/shared/instructions.rs index 8cb16e33ba..6bc318b12e 100755 --- a/cranelift/codegen/meta/src/shared/instructions.rs +++ b/cranelift/codegen/meta/src/shared/instructions.rs @@ -17,8 +17,9 @@ fn define_control_flow( imm: &Immediates, entities: &EntityRefs, ) { - let block = &Operand::new("block", &entities.block).with_doc("Destination basic block"); - let args = &Operand::new("args", &entities.varargs).with_doc("block arguments"); + let block_call = &Operand::new("block_call", &entities.block_call) + .with_doc("Destination basic block, with its arguments provided"); + let label = &Operand::new("label", &entities.label).with_doc("Destination basic block"); ig.push( Inst::new( @@ -32,7 +33,7 @@ fn define_control_flow( "#, &formats.jump, ) - .operands_in(vec![block, args]) + .operands_in(vec![block_call]) .is_terminator(true) .is_branch(true), ); @@ -56,7 +57,7 @@ fn define_control_flow( "#, &formats.branch, ) - .operands_in(vec![c, block, args]) + .operands_in(vec![c, block_call]) .is_branch(true), ); @@ -70,7 +71,7 @@ fn define_control_flow( "#, &formats.branch, ) - .operands_in(vec![c, block, args]) + .operands_in(vec![c, block_call]) .is_branch(true), ); } @@ -105,7 +106,7 @@ fn define_control_flow( "#, &formats.branch_table, ) - .operands_in(vec![x, block, JT]) + .operands_in(vec![x, label, JT]) .is_terminator(true) .is_branch(true), ); diff --git a/cranelift/codegen/src/dce.rs b/cranelift/codegen/src/dce.rs index f2850a8a16..2b52f92bcf 100644 --- a/cranelift/codegen/src/dce.rs +++ b/cranelift/codegen/src/dce.rs @@ -23,10 +23,11 @@ pub fn do_dce(func: &mut Function, domtree: &DominatorTree) { if has_side_effect(pos.func, inst) || any_inst_results_used(inst, &live, &pos.func.dfg) { - for arg in pos.func.dfg.inst_args(inst) { - let v = pos.func.dfg.resolve_aliases(*arg); + for arg in pos.func.dfg.inst_values(inst) { + let v = pos.func.dfg.resolve_aliases(arg); live[v.index()] = true; } + continue; } } diff --git a/cranelift/codegen/src/dominator_tree.rs b/cranelift/codegen/src/dominator_tree.rs index 5077354f7a..175dad5f80 100644 --- a/cranelift/codegen/src/dominator_tree.rs +++ b/cranelift/codegen/src/dominator_tree.rs @@ -353,7 +353,9 @@ impl DominatorTree { fn push_successors(&mut self, func: &Function, block: Block) { for inst in func.layout.block_likely_branches(block) { match func.dfg.analyze_branch(inst) { - BranchInfo::SingleDest(succ, _) => self.push_if_unseen(succ), + BranchInfo::SingleDest(succ) => { + self.push_if_unseen(succ.block(&func.dfg.value_lists)) + } BranchInfo::Table(jt, dest) => { for succ in func.jump_tables[jt].iter() { self.push_if_unseen(*succ); diff --git a/cranelift/codegen/src/egraph.rs b/cranelift/codegen/src/egraph.rs index f49e61dece..6f8bdbb73d 100644 --- a/cranelift/codegen/src/egraph.rs +++ b/cranelift/codegen/src/egraph.rs @@ -408,12 +408,12 @@ impl<'a> EgraphPass<'a> { // Rewrite args of *all* instructions using the // value-to-opt-value map. cursor.func.dfg.resolve_aliases_in_arguments(inst); - for arg in cursor.func.dfg.inst_args_mut(inst) { - let new_value = value_to_opt_value[*arg]; + cursor.func.dfg.map_inst_values(inst, |_, arg| { + let new_value = value_to_opt_value[arg]; trace!("rewriting arg {} of inst {} to {}", arg, inst, new_value); debug_assert_ne!(new_value, Value::reserved_value()); - *arg = new_value; - } + new_value + }); // Build a context for optimization, with borrows of // state. We can't invoke a method on `self` because @@ -497,8 +497,10 @@ impl<'a> EgraphPass<'a> { // layout. for block in self.func.layout.blocks() { for inst in self.func.layout.block_insts(block) { - for &arg in self.func.dfg.inst_args(inst) { - match self.func.dfg.value_def(arg) { + self.func + .dfg + .inst_values(inst) + .for_each(|arg| match self.func.dfg.value_def(arg) { ValueDef::Result(i, _) => { debug_assert!(self.func.layout.inst_block(i).is_some()); } @@ -506,8 +508,7 @@ impl<'a> EgraphPass<'a> { panic!("egraph union node {} still reachable at {}!", arg, inst); } _ => {} - } - } + }) } } } diff --git a/cranelift/codegen/src/egraph/elaborate.rs b/cranelift/codegen/src/egraph/elaborate.rs index 735834d064..4d308b5f2e 100644 --- a/cranelift/codegen/src/egraph/elaborate.rs +++ b/cranelift/codegen/src/egraph/elaborate.rs @@ -6,8 +6,7 @@ use super::domtree::DomTreeWithChildren; use super::Stats; use crate::dominator_tree::DominatorTree; use crate::fx::FxHashSet; -use crate::ir::ValueDef; -use crate::ir::{Block, Function, Inst, Value}; +use crate::ir::{Block, Function, Inst, Value, ValueDef}; use crate::loop_analysis::{Loop, LoopAnalysis, LoopLevel}; use crate::scoped_hash_map::ScopedHashMap; use crate::trace; @@ -15,7 +14,6 @@ use crate::unionfind::UnionFind; use alloc::vec::Vec; use cranelift_entity::{packed_option::ReservedValue, SecondaryMap}; use smallvec::{smallvec, SmallVec}; -use std::ops::Add; pub(crate) struct Elaborator<'a> { func: &'a mut Function, @@ -228,16 +226,10 @@ impl<'a> Elaborator<'a> { // N.B.: at this point we know that the opcode is // pure, so `pure_op_cost`'s precondition is // satisfied. - let cost = pure_op_cost(inst_data.opcode()).at_level(loop_level.level()) - + self - .func - .dfg - .inst_args(inst) - .iter() - .map(|value| best[*value].0) - // Can't use `.sum()` for `Cost` types; do - // an explicit reduce instead. - .fold(Cost::zero(), Cost::add); + let cost = self.func.dfg.inst_values(inst).fold( + pure_op_cost(inst_data.opcode()).at_level(loop_level.level()), + |cost, value| cost + best[value].0, + ); best[value] = (cost, value); } }; @@ -366,8 +358,7 @@ impl<'a> Elaborator<'a> { // layout. First, enqueue all args to be // elaborated. Push state to receive the results // and later elab this inst. - let args = self.func.dfg.inst_args(inst); - let num_args = args.len(); + let num_args = self.func.dfg.inst_values(inst).count(); self.elab_stack.push(ElabStackEntry::PendingInst { inst, result_idx, @@ -375,9 +366,10 @@ impl<'a> Elaborator<'a> { remat, before, }); + // Push args in reverse order so we process the // first arg first. - for &arg in args.iter().rev() { + for arg in self.func.dfg.inst_values(inst).rev() { debug_assert_ne!(arg, Value::reserved_value()); self.elab_stack .push(ElabStackEntry::Start { value: arg, before }); @@ -560,10 +552,9 @@ impl<'a> Elaborator<'a> { self.func.layout.insert_inst(inst, before); // Update the inst's arguments. - let args_dest = self.func.dfg.inst_args_mut(inst); - for (dest, val) in args_dest.iter_mut().zip(arg_values.iter()) { - *dest = val.value; - } + self.func + .dfg + .overwrite_inst_values(inst, arg_values.into_iter().map(|ev| ev.value)); // Now that we've consumed the arg values, pop // them off the stack. @@ -580,7 +571,7 @@ impl<'a> Elaborator<'a> { } } - fn elaborate_block(&mut self, idom: Option, block: Block) { + fn elaborate_block(&mut self, elab_values: &mut Vec, idom: Option, block: Block) { trace!("elaborate_block: block {}", block); self.start_block(idom, block); @@ -608,18 +599,19 @@ impl<'a> Elaborator<'a> { let before = first_branch.unwrap_or(inst); trace!(" -> inserting before {}", before); - // For each arg of the inst, elaborate its value. - for i in 0..self.func.dfg.inst_args(inst).len() { - // Don't borrow across the below. - let arg = self.func.dfg.inst_args(inst)[i]; - trace!(" -> arg {}", arg); + elab_values.extend(self.func.dfg.inst_values(inst)); + for arg in elab_values.iter_mut() { + trace!(" -> arg {}", *arg); // Elaborate the arg, placing any newly-inserted insts // before `before`. Get the updated value, which may // be different than the original. - let arg = self.elaborate_eclass_use(arg, before); - trace!(" -> rewrote arg to {:?}", arg); - self.func.dfg.inst_args_mut(inst)[i] = arg.value; + let new_arg = self.elaborate_eclass_use(*arg, before); + trace!(" -> rewrote arg to {:?}", new_arg); + *arg = new_arg.value; } + self.func + .dfg + .overwrite_inst_values(inst, elab_values.drain(..)); // We need to put the results of this instruction in the // map now. @@ -645,13 +637,18 @@ impl<'a> Elaborator<'a> { block: root, idom: None, }); + + // A temporary workspace for elaborate_block, allocated here to maximize the use of the + // allocation. + let mut elab_values = Vec::new(); + while let Some(top) = self.block_stack.pop() { match top { BlockStackEntry::Elaborate { block, idom } => { self.block_stack.push(BlockStackEntry::Pop); self.value_to_elaborated_value.increment_depth(); - self.elaborate_block(idom, block); + self.elaborate_block(&mut elab_values, idom, block); // Push children. We are doing a preorder // traversal so we do this after processing this diff --git a/cranelift/codegen/src/flowgraph.rs b/cranelift/codegen/src/flowgraph.rs index 9c6ccbaea5..4fd06e3c6a 100644 --- a/cranelift/codegen/src/flowgraph.rs +++ b/cranelift/codegen/src/flowgraph.rs @@ -122,8 +122,8 @@ impl ControlFlowGraph { fn compute_block(&mut self, func: &Function, block: Block) { for inst in func.layout.block_likely_branches(block) { match func.dfg.analyze_branch(inst) { - BranchInfo::SingleDest(dest, _) => { - self.add_edge(block, inst, dest); + BranchInfo::SingleDest(dest) => { + self.add_edge(block, inst, dest.block(&func.dfg.value_lists)); } BranchInfo::Table(jt, dest) => { if let Some(dest) = dest { diff --git a/cranelift/codegen/src/inst_predicates.rs b/cranelift/codegen/src/inst_predicates.rs index 368366360f..f5e4905769 100644 --- a/cranelift/codegen/src/inst_predicates.rs +++ b/cranelift/codegen/src/inst_predicates.rs @@ -164,10 +164,10 @@ pub(crate) fn visit_block_succs( } fn visit_branch_targets(f: &Function, inst: Inst, visit: &mut F) { - match f.dfg.insts[inst].analyze_branch(&f.dfg.value_lists) { + match f.dfg.insts[inst].analyze_branch() { BranchInfo::NotABranch => {} - BranchInfo::SingleDest(dest, _) => { - visit(inst, dest, false); + BranchInfo::SingleDest(dest) => { + visit(inst, dest.block(&f.dfg.value_lists), false); } BranchInfo::Table(table, maybe_dest) => { if let Some(dest) = maybe_dest { diff --git a/cranelift/codegen/src/ir/dfg.rs b/cranelift/codegen/src/ir/dfg.rs index e47a609aba..74058938bb 100644 --- a/cranelift/codegen/src/ir/dfg.rs +++ b/cranelift/codegen/src/ir/dfg.rs @@ -7,8 +7,8 @@ use crate::ir::dynamic_type::{DynamicTypeData, DynamicTypes}; use crate::ir::instructions::{BranchInfo, CallInfo, InstructionData}; use crate::ir::{types, ConstantData, ConstantPool, Immediate}; use crate::ir::{ - Block, DynamicType, FuncRef, Inst, SigRef, Signature, Type, Value, ValueLabelAssignments, - ValueList, ValueListPool, + Block, BlockCall, DynamicType, FuncRef, Inst, SigRef, Signature, Type, Value, + ValueLabelAssignments, ValueList, ValueListPool, }; use crate::ir::{ExtFuncData, RelSourceLoc}; use crate::packed_option::ReservedValue; @@ -167,6 +167,11 @@ impl DataFlowGraph { self.blocks.is_valid(block) } + /// Make a BlockCall, bundling together the block and its arguments. + pub fn block_call(&mut self, block: Block, args: &[Value]) -> BlockCall { + BlockCall::new(block, args, &mut self.value_lists) + } + /// Get the total number of values. pub fn num_values(&self) -> usize { self.values.len() @@ -328,12 +333,7 @@ impl DataFlowGraph { /// For each argument of inst which is defined by an alias, replace the /// alias with the aliased value. pub fn resolve_aliases_in_arguments(&mut self, inst: Inst) { - for arg in self.insts[inst].arguments_mut(&mut self.value_lists) { - let resolved = resolve_aliases(&self.values, *arg); - if resolved != *arg { - *arg = resolved; - } - } + self.map_inst_values(inst, |dfg, arg| resolve_aliases(&dfg.values, arg)); } /// Turn a value into an alias of another. @@ -665,6 +665,60 @@ impl DataFlowGraph { } } + /// Construct a read-only visitor context for the values of this instruction. + pub fn inst_values<'dfg>( + &'dfg self, + inst: Inst, + ) -> impl DoubleEndedIterator + 'dfg { + self.inst_args(inst) + .iter() + .chain( + self.insts[inst] + .branch_destination() + .into_iter() + .flat_map(|branch| branch.args_slice(&self.value_lists).iter()), + ) + .copied() + } + + /// Map a function over the values of the instruction. + pub fn map_inst_values(&mut self, inst: Inst, mut body: F) + where + F: FnMut(&mut DataFlowGraph, Value) -> Value, + { + for i in 0..self.inst_args(inst).len() { + let arg = self.inst_args(inst)[i]; + self.inst_args_mut(inst)[i] = body(self, arg); + } + + for mut block in self.insts[inst].branch_destination().into_iter() { + // We aren't changing the size of the args list, so we won't need to write the branch + // back to the instruction. + for i in 0..block.args_slice(&self.value_lists).len() { + let arg = block.args_slice(&self.value_lists)[i]; + block.args_slice_mut(&mut self.value_lists)[i] = body(self, arg); + } + } + } + + /// Overwrite the instruction's value references with values from the iterator. + /// NOTE: the iterator provided is expected to yield at least as many values as the instruction + /// currently has. + pub fn overwrite_inst_values(&mut self, inst: Inst, mut values: I) + where + I: Iterator, + { + for arg in self.inst_args_mut(inst) { + *arg = values.next().unwrap(); + } + + for mut block in self.insts[inst].branch_destination().into_iter() { + for arg in block.args_slice_mut(&mut self.value_lists) { + *arg = values.next().unwrap(); + } + } + } + /// Get all value arguments on `inst` as a slice. pub fn inst_args(&self, inst: Inst) -> &[Value] { self.insts[inst].arguments(&self.value_lists) @@ -866,16 +920,6 @@ impl DataFlowGraph { }) } - /// Append a new value argument to an instruction. - /// - /// Panics if the instruction doesn't support arguments. - pub fn append_inst_arg(&mut self, inst: Inst, new_arg: Value) { - self.insts[inst] - .value_list_mut() - .expect("the instruction doesn't have value arguments") - .push(new_arg, &mut self.value_lists); - } - /// Clone an instruction, attaching new result `Value`s and /// returning them. pub fn clone_inst(&mut self, inst: Inst) -> Inst { @@ -933,7 +977,7 @@ impl DataFlowGraph { /// Check if `inst` is a branch. pub fn analyze_branch(&self, inst: Inst) -> BranchInfo { - self.insts[inst].analyze_branch(&self.value_lists) + self.insts[inst].analyze_branch() } /// Compute the type of an instruction result from opcode constraints and call signatures. diff --git a/cranelift/codegen/src/ir/function.rs b/cranelift/codegen/src/ir/function.rs index f1b1917104..d315b58cb9 100644 --- a/cranelift/codegen/src/ir/function.rs +++ b/cranelift/codegen/src/ir/function.rs @@ -284,7 +284,7 @@ impl FunctionStencil { pub fn change_branch_destination(&mut self, inst: Inst, new_dest: Block) { match self.dfg.insts[inst].branch_destination_mut() { None => (), - Some(inst_dest) => *inst_dest = new_dest, + Some(inst_dest) => inst_dest.set_block(new_dest, &mut self.dfg.value_lists), } } @@ -295,8 +295,8 @@ impl FunctionStencil { /// rewrite the destinations of multi-destination branches like `br_table`. pub fn rewrite_branch_destination(&mut self, inst: Inst, old_dest: Block, new_dest: Block) { match self.dfg.analyze_branch(inst) { - BranchInfo::SingleDest(dest, ..) => { - if dest == old_dest { + BranchInfo::SingleDest(dest) => { + if dest.block(&self.dfg.value_lists) == old_dest { self.change_branch_destination(inst, new_dest); } } diff --git a/cranelift/codegen/src/ir/instructions.rs b/cranelift/codegen/src/ir/instructions.rs index 24aadd553a..a16ef5848d 100644 --- a/cranelift/codegen/src/ir/instructions.rs +++ b/cranelift/codegen/src/ir/instructions.rs @@ -31,6 +31,95 @@ pub type ValueList = entity::EntityList; /// Memory pool for holding value lists. See `ValueList`. pub type ValueListPool = entity::ListPool; +/// A pair of a Block and its arguments, stored in a single EntityList internally. +/// +/// NOTE: We don't expose either value_to_block or block_to_value outside of this module because +/// this operation is not generally safe. However, as the two share the same underlying layout, +/// they can be stored in the same value pool. +/// +/// BlockCall makes use of this shared layout by storing all of its contents (a block and its +/// argument) in a single EntityList. This is a bit better than introducing a new entity type for +/// the pair of a block name and the arguments entity list, as we don't pay any indirection penalty +/// to get to the argument values -- they're stored in-line with the block in the same list. +/// +/// The BlockCall::new function guarantees this layout by requiring a block argument that's written +/// in as the first element of the EntityList. Any subsequent entries are always assumed to be real +/// Values. +#[derive(Debug, Clone, Copy, PartialEq, Hash)] +#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))] +pub struct BlockCall { + /// The underlying storage for the BlockCall. The first element of the values EntityList is + /// guaranteed to always be a Block encoded as a Value via BlockCall::block_to_value. + /// Consequently, the values entity list is never empty. + values: entity::EntityList, +} + +impl BlockCall { + // NOTE: the only uses of this function should be internal to BlockCall. See the block comment + // on BlockCall for more context. + fn value_to_block(val: Value) -> Block { + Block::from_u32(val.as_u32()) + } + + // NOTE: the only uses of this function should be internal to BlockCall. See the block comment + // on BlockCall for more context. + fn block_to_value(block: Block) -> Value { + Value::from_u32(block.as_u32()) + } + + /// Construct a BlockCall with the given block and arguments. + pub fn new(block: Block, args: &[Value], pool: &mut ValueListPool) -> Self { + let mut values = ValueList::default(); + values.push(Self::block_to_value(block), pool); + values.extend(args.iter().copied(), pool); + Self { values } + } + + /// Return the block for this BlockCall. + pub fn block(&self, pool: &ValueListPool) -> Block { + let val = self.values.first(pool).unwrap(); + Self::value_to_block(val) + } + + /// Replace the block for this BlockCall. + pub fn set_block(&mut self, block: Block, pool: &mut ValueListPool) { + *self.values.get_mut(0, pool).unwrap() = Self::block_to_value(block); + } + + /// Append an argument to the block args. + pub fn append_argument(&mut self, arg: Value, pool: &mut ValueListPool) { + self.values.push(arg, pool); + } + + /// Return a slice for the arguments of this block. + pub fn args_slice<'a>(&self, pool: &'a ValueListPool) -> &'a [Value] { + &self.values.as_slice(pool)[1..] + } + + /// Return a slice for the arguments of this block. + pub fn args_slice_mut<'a>(&'a mut self, pool: &'a mut ValueListPool) -> &'a mut [Value] { + &mut self.values.as_mut_slice(pool)[1..] + } + + /// Remove the argument at ix from the argument list. + pub fn remove(&mut self, ix: usize, pool: &mut ValueListPool) { + self.values.remove(1 + ix, pool) + } + + /// Clear out the arguments list. + pub fn clear(&mut self, pool: &mut ValueListPool) { + self.values.truncate(1, pool) + } + + /// Appends multiple elements to the arguments. + pub fn extend(&mut self, elements: I, pool: &mut ValueListPool) + where + I: IntoIterator, + { + self.values.extend(elements, pool) + } +} + // Include code generated by `cranelift-codegen/meta/src/gen_inst.rs`. This file contains: // // - The `pub enum InstructionFormat` enum with all the instruction formats. @@ -178,18 +267,10 @@ impl InstructionData { /// /// Any instruction that can transfer control to another block reveals its possible destinations /// here. - pub fn analyze_branch<'a>(&'a self, pool: &'a ValueListPool) -> BranchInfo<'a> { + pub fn analyze_branch(&self) -> BranchInfo { match *self { - Self::Jump { - destination, - ref args, - .. - } => BranchInfo::SingleDest(destination, args.as_slice(pool)), - Self::Branch { - destination, - ref args, - .. - } => BranchInfo::SingleDest(destination, &args.as_slice(pool)[1..]), + Self::Jump { destination, .. } => BranchInfo::SingleDest(destination), + Self::Branch { destination, .. } => BranchInfo::SingleDest(destination), Self::BranchTable { table, destination, .. } => BranchInfo::Table(table, Some(destination)), @@ -204,7 +285,7 @@ impl InstructionData { /// branch or jump. /// /// Multi-destination branches like `br_table` return `None`. - pub fn branch_destination(&self) -> Option { + pub fn branch_destination(&self) -> Option { match *self { Self::Jump { destination, .. } | Self::Branch { destination, .. } => Some(destination), Self::BranchTable { .. } => None, @@ -219,7 +300,7 @@ impl InstructionData { /// single destination branch or jump. /// /// Multi-destination branches like `br_table` return `None`. - pub fn branch_destination_mut(&mut self) -> Option<&mut Block> { + pub fn branch_destination_mut(&mut self) -> Option<&mut BlockCall> { match *self { Self::Jump { ref mut destination, @@ -366,14 +447,14 @@ impl InstructionData { } /// Information about branch and jump instructions. -pub enum BranchInfo<'a> { +pub enum BranchInfo { /// This is not a branch or jump instruction. /// This instruction will not transfer control to another block in the function, but it may still /// affect control flow by returning or trapping. NotABranch, /// This is a branch or jump to a single destination block, possibly taking value arguments. - SingleDest(Block, &'a [Value]), + SingleDest(BlockCall), /// This is a jump table branch which can have many destination blocks and maybe one default block. Table(JumpTable, Option), diff --git a/cranelift/codegen/src/ir/mod.rs b/cranelift/codegen/src/ir/mod.rs index 0457308b0f..56a98a0db2 100644 --- a/cranelift/codegen/src/ir/mod.rs +++ b/cranelift/codegen/src/ir/mod.rs @@ -46,7 +46,7 @@ pub use crate::ir::extname::{ExternalName, UserExternalName, UserFuncName}; pub use crate::ir::function::{DisplayFunctionAnnotations, Function}; pub use crate::ir::globalvalue::GlobalValueData; pub use crate::ir::instructions::{ - InstructionData, Opcode, ValueList, ValueListPool, VariableArgs, + BlockCall, InstructionData, Opcode, ValueList, ValueListPool, VariableArgs, }; pub use crate::ir::jumptable::JumpTableData; pub use crate::ir::known_symbol::KnownSymbol; diff --git a/cranelift/codegen/src/isa/aarch64/lower.isle b/cranelift/codegen/src/isa/aarch64/lower.isle index 388758c012..2f2a2ecfa3 100644 --- a/cranelift/codegen/src/isa/aarch64/lower.isle +++ b/cranelift/codegen/src/isa/aarch64/lower.isle @@ -2464,7 +2464,7 @@ ;;; Rules for `brz`/`brnz` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; `brz` following `icmp` -(rule (lower_branch (brz (maybe_uextend (icmp cc x @ (value_type ty) y)) _ _) targets) +(rule (lower_branch (brz (maybe_uextend (icmp cc x @ (value_type ty) y)) _) targets) (let ((comparison FlagsAndCC (lower_icmp_into_flags cc x y ty)) ;; Negate the condition for `brz`. (cond Cond (invert_cond (cond_code (flags_and_cc_cc comparison)))) @@ -2476,7 +2476,7 @@ not_taken (cond_br_cond cond)))))) ;; `brnz` following `icmp` -(rule (lower_branch (brnz (maybe_uextend (icmp cc x @ (value_type ty) y)) _ _) targets) +(rule (lower_branch (brnz (maybe_uextend (icmp cc x @ (value_type ty) y)) _) targets) (let ((comparison FlagsAndCC (lower_icmp_into_flags cc x y ty)) (cond Cond (cond_code (flags_and_cc_cc comparison))) (taken BranchTarget (branch_target targets 0)) @@ -2487,7 +2487,7 @@ not_taken (cond_br_cond cond)))))) ;; `brz` following `fcmp` -(rule (lower_branch (brz (maybe_uextend (fcmp cc x @ (value_type (ty_scalar_float ty)) y)) _ _) targets) +(rule (lower_branch (brz (maybe_uextend (fcmp cc x @ (value_type (ty_scalar_float ty)) y)) _) targets) (let ((cond Cond (fp_cond_code cc)) (cond Cond (invert_cond cond)) ;; negate for `brz` (taken BranchTarget (branch_target targets 0)) @@ -2497,7 +2497,7 @@ (cond_br taken not_taken (cond_br_cond cond)))))) ;; `brnz` following `fcmp` -(rule (lower_branch (brnz (maybe_uextend (fcmp cc x @ (value_type (ty_scalar_float ty)) y)) _ _) targets) +(rule (lower_branch (brnz (maybe_uextend (fcmp cc x @ (value_type (ty_scalar_float ty)) y)) _) targets) (let ((cond Cond (fp_cond_code cc)) (taken BranchTarget (branch_target targets 0)) (not_taken BranchTarget (branch_target targets 1))) @@ -2506,7 +2506,7 @@ (cond_br taken not_taken (cond_br_cond cond)))))) ;; standard `brz` -(rule -1 (lower_branch (brz c @ (value_type $I128) _ _) targets) +(rule -1 (lower_branch (brz c @ (value_type $I128) _) targets) (let ((flags ProducesFlags (flags_to_producesflags c)) (c ValueRegs (put_in_regs c)) (c_lo Reg (value_regs_get c 0)) @@ -2517,7 +2517,7 @@ (emit_side_effect (with_flags_side_effect flags (cond_br taken not_taken (cond_br_zero rt)))))) -(rule -2 (lower_branch (brz c @ (value_type ty) _ _) targets) +(rule -2 (lower_branch (brz c @ (value_type ty) _) targets) (if (ty_int_ref_scalar_64 ty)) (let ((flags ProducesFlags (flags_to_producesflags c)) (rt Reg (put_in_reg_zext64 c)) @@ -2527,7 +2527,7 @@ (with_flags_side_effect flags (cond_br taken not_taken (cond_br_zero rt)))))) ;; standard `brnz` -(rule -1 (lower_branch (brnz c @ (value_type $I128) _ _) targets) +(rule -1 (lower_branch (brnz c @ (value_type $I128) _) targets) (let ((flags ProducesFlags (flags_to_producesflags c)) (c ValueRegs (put_in_regs c)) (c_lo Reg (value_regs_get c 0)) @@ -2538,7 +2538,7 @@ (emit_side_effect (with_flags_side_effect flags (cond_br taken not_taken (cond_br_not_zero rt)))))) -(rule -2 (lower_branch (brnz c @ (value_type ty) _ _) targets) +(rule -2 (lower_branch (brnz c @ (value_type ty) _) targets) (if (ty_int_ref_scalar_64 ty)) (let ((flags ProducesFlags (flags_to_producesflags c)) (rt Reg (put_in_reg_zext64 c)) @@ -2550,7 +2550,7 @@ ;;; Rules for `jump` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(rule (lower_branch (jump _ _) targets) +(rule (lower_branch (jump _) targets) (emit_side_effect (aarch64_jump (branch_target targets 0)))) ;;; Rules for `br_table` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/cranelift/codegen/src/isa/riscv64/inst.isle b/cranelift/codegen/src/isa/riscv64/inst.isle index 1791b579ba..31fdad31a9 100644 --- a/cranelift/codegen/src/isa/riscv64/inst.isle +++ b/cranelift/codegen/src/isa/riscv64/inst.isle @@ -1857,7 +1857,7 @@ (extern constructor vec_label_get vec_label_get) (decl partial lower_branch (Inst VecMachLabel) Unit) -(rule (lower_branch (jump _ _) targets ) +(rule (lower_branch (jump _) targets ) (emit_side_effect (SideEffectNoResult.Inst (gen_jump (vec_label_get targets 0))))) ;;; cc a b targets Type @@ -1900,44 +1900,44 @@ ;;;;; (rule - (lower_branch (brz v @ (value_type ty) _ _) targets) + (lower_branch (brz v @ (value_type ty) _) targets) (lower_brz_or_nz (IntCC.Equal) (normalize_cmp_value ty v) targets ty)) ;; Special case for SI128 to reify the comparison value and branch on it. (rule 2 - (lower_branch (brz v @ (value_type $I128) _ _) targets) + (lower_branch (brz v @ (value_type $I128) _) targets) (let ((zero ValueRegs (value_regs (zero_reg) (zero_reg))) (cmp Reg (gen_icmp (IntCC.Equal) v zero $I128))) (lower_brz_or_nz (IntCC.NotEqual) cmp targets $I64))) (rule 1 - (lower_branch (brz (icmp cc a @ (value_type ty) b) _ _) targets) + (lower_branch (brz (icmp cc a @ (value_type ty) b) _) targets) (lower_br_icmp (intcc_inverse cc) a b targets ty)) (rule 1 - (lower_branch (brz (fcmp cc a @ (value_type ty) b) _ _) targets) + (lower_branch (brz (fcmp cc a @ (value_type ty) b) _) targets) (let ((then BranchTarget (label_to_br_target (vec_label_get targets 0))) (else BranchTarget (label_to_br_target (vec_label_get targets 1)))) (emit_side_effect (cond_br (emit_fcmp (floatcc_inverse cc) ty a b) then else)))) ;;;; (rule - (lower_branch (brnz v @ (value_type ty) _ _) targets) + (lower_branch (brnz v @ (value_type ty) _) targets) (lower_brz_or_nz (IntCC.NotEqual) (normalize_cmp_value ty v) targets ty)) ;; Special case for SI128 to reify the comparison value and branch on it. (rule 2 - (lower_branch (brnz v @ (value_type $I128) _ _) targets) + (lower_branch (brnz v @ (value_type $I128) _) targets) (let ((zero ValueRegs (value_regs (zero_reg) (zero_reg))) (cmp Reg (gen_icmp (IntCC.NotEqual) v zero $I128))) (lower_brz_or_nz (IntCC.NotEqual) cmp targets $I64))) (rule 1 - (lower_branch (brnz (icmp cc a @ (value_type ty) b) _ _) targets) + (lower_branch (brnz (icmp cc a @ (value_type ty) b) _) targets) (lower_br_icmp cc a b targets ty)) (rule 1 - (lower_branch (brnz (fcmp cc a @ (value_type ty) b) _ _) targets) + (lower_branch (brnz (fcmp cc a @ (value_type ty) b) _) targets) (let ((then BranchTarget (label_to_br_target (vec_label_get targets 0))) (else BranchTarget (label_to_br_target (vec_label_get targets 1)))) (emit_side_effect (cond_br (emit_fcmp cc ty a b) then else)))) diff --git a/cranelift/codegen/src/isa/s390x/lower.isle b/cranelift/codegen/src/isa/s390x/lower.isle index e97e08b65f..96bffdf350 100644 --- a/cranelift/codegen/src/isa/s390x/lower.isle +++ b/cranelift/codegen/src/isa/s390x/lower.isle @@ -3714,7 +3714,7 @@ ;; Unconditional branch. The target is found as first (and only) element in ;; the list of the current block's branch targets passed as `targets`. -(rule (lower_branch (jump _ _) targets) +(rule (lower_branch (jump _) targets) (emit_side_effect (jump_impl (vec_element targets 0)))) @@ -3755,7 +3755,7 @@ ;; Two-way conditional branch on zero. `targets` contains: ;; - element 0: target if the condition is true (i.e. value is zero) ;; - element 1: target if the condition is false (i.e. value is nonzero) -(rule (lower_branch (brz val_cond _ _) targets) +(rule (lower_branch (brz val_cond _) targets) (emit_side_effect (cond_br_bool (invert_bool (value_nonzero val_cond)) (vec_element targets 0) (vec_element targets 1)))) @@ -3766,7 +3766,7 @@ ;; Two-way conditional branch on nonzero. `targets` contains: ;; - element 0: target if the condition is true (i.e. value is nonzero) ;; - element 1: target if the condition is false (i.e. value is zero) -(rule (lower_branch (brnz val_cond _ _) targets) +(rule (lower_branch (brnz val_cond _) targets) (emit_side_effect (cond_br_bool (value_nonzero val_cond) (vec_element targets 0) (vec_element targets 1)))) diff --git a/cranelift/codegen/src/isa/x64/lower.isle b/cranelift/codegen/src/isa/x64/lower.isle index 69f8df5120..191e703f8c 100644 --- a/cranelift/codegen/src/isa/x64/lower.isle +++ b/cranelift/codegen/src/isa/x64/lower.isle @@ -2882,46 +2882,46 @@ ;; Rules for `jump` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(rule (lower_branch (jump _ _) (single_target target)) +(rule (lower_branch (jump _) (single_target target)) (emit_side_effect (jmp_known target))) ;; Rules for `brz` and `brnz` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -(rule 2 (lower_branch (brz (maybe_uextend (icmp cc a b)) _ _) (two_targets taken not_taken)) +(rule 2 (lower_branch (brz (maybe_uextend (icmp cc a b)) _) (two_targets taken not_taken)) (let ((cmp IcmpCondResult (invert_icmp_cond_result (emit_cmp cc a b)))) (emit_side_effect (jmp_cond_icmp cmp taken not_taken)))) -(rule 2 (lower_branch (brz (maybe_uextend (fcmp cc a b)) _ _) (two_targets taken not_taken)) +(rule 2 (lower_branch (brz (maybe_uextend (fcmp cc a b)) _) (two_targets taken not_taken)) (let ((cmp FcmpCondResult (emit_fcmp (floatcc_inverse cc) a b))) (emit_side_effect (jmp_cond_fcmp cmp taken not_taken)))) -(rule 1 (lower_branch (brz val @ (value_type $I128) _ _) (two_targets taken not_taken)) +(rule 1 (lower_branch (brz val @ (value_type $I128) _) (two_targets taken not_taken)) (emit_side_effect (jmp_cond_icmp (cmp_zero_i128 (CC.NZ) val) taken not_taken))) -(rule 0 (lower_branch (brz val @ (value_type (ty_int_bool_or_ref)) _ _) (two_targets taken not_taken)) +(rule 0 (lower_branch (brz val @ (value_type (ty_int_bool_or_ref)) _) (two_targets taken not_taken)) (emit_side_effect (with_flags_side_effect (cmp_zero_int_bool_ref val) (jmp_cond (CC.Z) taken not_taken)))) -(rule 2 (lower_branch (brnz (icmp cc a b) _ _) (two_targets taken not_taken)) +(rule 2 (lower_branch (brnz (icmp cc a b) _) (two_targets taken not_taken)) (emit_side_effect (jmp_cond_icmp (emit_cmp cc a b) taken not_taken))) -(rule 2 (lower_branch (brnz (fcmp cc a b) _ _) (two_targets taken not_taken)) +(rule 2 (lower_branch (brnz (fcmp cc a b) _) (two_targets taken not_taken)) (let ((cmp FcmpCondResult (emit_fcmp cc a b))) (emit_side_effect (jmp_cond_fcmp cmp taken not_taken)))) -(rule 2 (lower_branch (brnz (uextend (icmp cc a b)) _ _) (two_targets taken not_taken)) +(rule 2 (lower_branch (brnz (uextend (icmp cc a b)) _) (two_targets taken not_taken)) (emit_side_effect (jmp_cond_icmp (emit_cmp cc a b) taken not_taken))) -(rule 2 (lower_branch (brnz (uextend (fcmp cc a b)) _ _) (two_targets taken not_taken)) +(rule 2 (lower_branch (brnz (uextend (fcmp cc a b)) _) (two_targets taken not_taken)) (let ((cmp FcmpCondResult (emit_fcmp cc a b))) (emit_side_effect (jmp_cond_fcmp cmp taken not_taken)))) -(rule 1 (lower_branch (brnz val @ (value_type $I128) _ _) (two_targets taken not_taken)) +(rule 1 (lower_branch (brnz val @ (value_type $I128) _) (two_targets taken not_taken)) (emit_side_effect (jmp_cond_icmp (cmp_zero_i128 (CC.Z) val) taken not_taken))) -(rule 0 (lower_branch (brnz val @ (value_type (ty_int_bool_or_ref)) _ _) (two_targets taken not_taken)) +(rule 0 (lower_branch (brnz val @ (value_type (ty_int_bool_or_ref)) _) (two_targets taken not_taken)) (emit_side_effect (with_flags_side_effect (cmp_zero_int_bool_ref val) (jmp_cond (CC.NZ) taken not_taken)))) diff --git a/cranelift/codegen/src/licm.rs b/cranelift/codegen/src/licm.rs index 04551918e5..9f543392cd 100644 --- a/cranelift/codegen/src/licm.rs +++ b/cranelift/codegen/src/licm.rs @@ -161,9 +161,8 @@ fn is_loop_invariant(inst: Inst, dfg: &DataFlowGraph, loop_values: &FxHashSet Lower<'func, I> { let uses = |value| { trace!(" -> pushing args for {} onto stack", value); if let ValueDef::Result(src_inst, _) = f.dfg.value_def(value) { - Some(f.dfg.inst_args(src_inst).iter().copied()) + Some(f.dfg.inst_values(src_inst)) } else { None } @@ -494,9 +494,9 @@ impl<'func, I: VCodeInst> Lower<'func, I> { // could come in as Once on our two different results. let force_multiple = f.dfg.inst_results(inst).len() > 1; - // Iterate over all args of all instructions, noting an + // Iterate over all values used by all instructions, noting an // additional use on each operand. - for &arg in f.dfg.inst_args(inst) { + for arg in f.dfg.inst_values(inst) { let arg = f.dfg.resolve_aliases(arg); let old = value_ir_uses[arg]; if force_multiple { @@ -911,7 +911,7 @@ impl<'func, I: VCodeInst> Lower<'func, I> { // conditional branch; a conditional branch can be followed only by an // unconditional branch or fallthrough. Otherwise, if only one branch, // it may be an unconditional branch, a fallthrough, a return, or a - // trap. These conditions are verified by `is_ebb_basic()` during the + // trap. These conditions are verified by `is_block_basic()` during the // verifier pass. assert!(branches.len() <= 2); if branches.len() == 2 { @@ -942,7 +942,11 @@ impl<'func, I: VCodeInst> Lower<'func, I> { // Avoid immutable borrow by explicitly indexing. let (inst, succ) = self.vcode.block_order().succ_indices(block)[succ_idx]; // Get branch args and convert to Regs. - let branch_args = self.f.dfg.inst_variable_args(inst); + let branch_args = self.f.dfg.insts[inst] + .branch_destination() + .into_iter() + .flat_map(|block| block.args_slice(&self.f.dfg.value_lists)); + let mut branch_arg_vregs: SmallVec<[Reg; 16]> = smallvec![]; for &arg in branch_args { let arg = self.f.dfg.resolve_aliases(arg); @@ -1142,7 +1146,8 @@ impl<'func, I: VCodeInst> Lower<'func, I> { self.f.rel_srclocs()[ir_inst] } - /// Get the number of inputs to the given IR instruction. + /// Get the number of inputs to the given IR instruction. This is a count only of the Value + /// arguments to the instruction: block arguments will not be included in this count. pub fn num_inputs(&self, ir_inst: Inst) -> usize { self.f.dfg.inst_args(ir_inst).len() } diff --git a/cranelift/codegen/src/opts.rs b/cranelift/codegen/src/opts.rs index 41f5fb884c..fa6fa600f9 100644 --- a/cranelift/codegen/src/opts.rs +++ b/cranelift/codegen/src/opts.rs @@ -7,9 +7,9 @@ use crate::ir::dfg::ValueDef; pub use crate::ir::immediates::{Ieee32, Ieee64, Imm64, Offset32, Uimm32, Uimm64, Uimm8}; pub use crate::ir::types::*; pub use crate::ir::{ - dynamic_to_fixed, AtomicRmwOp, Block, Constant, DataFlowGraph, DynamicStackSlot, FuncRef, - GlobalValue, Immediate, InstructionData, JumpTable, MemFlags, Opcode, StackSlot, Table, - TrapCode, Type, Value, + dynamic_to_fixed, AtomicRmwOp, Block, BlockCall, Constant, DataFlowGraph, DynamicStackSlot, + FuncRef, GlobalValue, Immediate, InstructionData, JumpTable, MemFlags, Opcode, StackSlot, + Table, TrapCode, Type, Value, }; use crate::isle_common_prelude_methods; use crate::machinst::isle::*; diff --git a/cranelift/codegen/src/remove_constant_phis.rs b/cranelift/codegen/src/remove_constant_phis.rs index 5f31947386..4f03c7d8ca 100644 --- a/cranelift/codegen/src/remove_constant_phis.rs +++ b/cranelift/codegen/src/remove_constant_phis.rs @@ -1,13 +1,12 @@ //! A Constant-Phi-Node removal pass. use crate::dominator_tree::DominatorTree; -use crate::entity::EntityList; use crate::fx::FxHashMap; use crate::fx::FxHashSet; use crate::ir; use crate::ir::instructions::BranchInfo; use crate::ir::Function; -use crate::ir::{Block, Inst, Value}; +use crate::ir::{Block, BlockCall, Inst, Value}; use crate::timing; use arrayvec::ArrayVec; use bumpalo::Bump; @@ -127,8 +126,8 @@ impl<'a> OutEdge<'a> { /// Returns `None` if this is an edge without any block arguments, which /// means we can ignore it for this analysis's purposes. #[inline] - fn new(bump: &'a Bump, dfg: &ir::DataFlowGraph, inst: Inst, block: Block) -> Option { - let inst_var_args = dfg.inst_variable_args(inst); + fn new(bump: &'a Bump, dfg: &ir::DataFlowGraph, inst: Inst, block: BlockCall) -> Option { + let inst_var_args = block.args_slice(&dfg.value_lists); // Skip edges without params. if inst_var_args.is_empty() { @@ -137,7 +136,7 @@ impl<'a> OutEdge<'a> { Some(OutEdge { inst, - block, + block: block.block(&dfg.value_lists), args: bump.alloc_slice_fill_iter( inst_var_args .iter() @@ -236,8 +235,7 @@ pub fn do_remove_constant_phis(func: &mut Function, domtree: &mut DominatorTree) // Note that multi-dest transfers (i.e., branch tables) don't // carry parameters in our IR, so we only have to care about // `SingleDest` here. - if let BranchInfo::SingleDest(dest, _) = idetails.analyze_branch(&func.dfg.value_lists) - { + if let BranchInfo::SingleDest(dest) = idetails.analyze_branch() { if let Some(edge) = OutEdge::new(&bump, &func.dfg, inst, dest) { summary.dests.push(edge); } @@ -372,47 +370,34 @@ pub fn do_remove_constant_phis(func: &mut Function, domtree: &mut DominatorTree) // Secondly, visit all branch insns. If the destination has had its // formals changed, change the actuals accordingly. Don't scan all insns, // rather just visit those as listed in the summaries we prepared earlier. + let mut old_actuals = alloc::vec::Vec::new(); for summary in summaries.values() { for edge in &summary.dests { if !need_editing.contains(&edge.block) { continue; } - let old_actuals = func.dfg.insts[edge.inst].value_list().unwrap(); - let num_old_actuals = old_actuals.len(&func.dfg.value_lists); - let num_fixed_actuals = func.dfg.insts[edge.inst] - .opcode() - .constraints() - .num_fixed_value_arguments(); - let dst_summary = &summaries[edge.block]; + let dfg = &mut func.dfg; + let block = dfg.insts[edge.inst].branch_destination_mut().unwrap(); + old_actuals.extend(block.args_slice(&dfg.value_lists)); // Check that the numbers of arguments make sense. - assert!(num_fixed_actuals <= num_old_actuals); - assert_eq!( - num_fixed_actuals + dst_summary.formals.len(), - num_old_actuals - ); + let formals = &summaries[edge.block].formals; + assert_eq!(formals.len(), old_actuals.len()); - // Create a new value list. - let mut new_actuals = EntityList::::new(); - // Copy the fixed args to the new list - for i in 0..num_fixed_actuals { - let val = old_actuals.get(i, &func.dfg.value_lists).unwrap(); - new_actuals.push(val, &mut func.dfg.value_lists); - } + // Filter out redundant block arguments. + let mut formals = formals.iter(); + old_actuals.retain(|_| { + let formal_i = formals.next().unwrap(); + !state.get(*formal_i).is_one() + }); - // Copy the variable args (the actual block params) to the new - // list, filtering out redundant ones. - for (i, formal_i) in dst_summary.formals.iter().enumerate() { - let actual_i = old_actuals - .get(num_fixed_actuals + i, &func.dfg.value_lists) - .unwrap(); - let is_redundant = state.get(*formal_i).is_one(); - if !is_redundant { - new_actuals.push(actual_i, &mut func.dfg.value_lists); - } - } - *func.dfg.insts[edge.inst].value_list_mut().unwrap() = new_actuals; + // Replace the block with a new one that only includes the non-redundant arguments. + // This leaks the value list from the old block, + // https://github.com/bytecodealliance/wasmtime/issues/5451 for more information. + let destination = block.block(&dfg.value_lists); + *block = BlockCall::new(destination, &old_actuals, &mut dfg.value_lists); + old_actuals.clear(); } } diff --git a/cranelift/codegen/src/simple_preopt.rs b/cranelift/codegen/src/simple_preopt.rs index bfe8ccbdc8..41b5e7d706 100644 --- a/cranelift/codegen/src/simple_preopt.rs +++ b/cranelift/codegen/src/simple_preopt.rs @@ -11,7 +11,7 @@ use crate::flowgraph::ControlFlowGraph; use crate::ir::{ condcodes::{CondCode, IntCC}, instructions::Opcode, - types::{I128, I32, I64}, + types::{I128, I32, I64, INVALID}, Block, DataFlowGraph, Function, Inst, InstBuilder, InstructionData, Type, Value, }; use crate::isa::TargetIsa; @@ -466,23 +466,17 @@ fn do_divrem_transformation(divrem_info: &DivRemByConstInfo, pos: &mut FuncCurso } } -enum BranchOrderKind { - BrzToBrnz(Value), - BrnzToBrz(Value), -} - /// Reorder branches to encourage fallthroughs. /// /// When a block ends with a conditional branch followed by an unconditional /// branch, this will reorder them if one of them is branching to the next Block /// layout-wise. The unconditional jump can then become a fallthrough. fn branch_order(pos: &mut FuncCursor, cfg: &mut ControlFlowGraph, block: Block, inst: Inst) { - let (term_inst, term_inst_args, term_dest, cond_inst, cond_inst_args, cond_dest, kind) = + let (term_inst, term_dest, cond_inst, cond_dest, kind, cond_arg) = match pos.func.dfg.insts[inst] { InstructionData::Jump { opcode: Opcode::Jump, destination, - ref args, } => { let next_block = if let Some(next_block) = pos.func.layout.next_block(block) { next_block @@ -490,7 +484,7 @@ fn branch_order(pos: &mut FuncCursor, cfg: &mut ControlFlowGraph, block: Block, return; }; - if destination == next_block { + if destination.block(&pos.func.dfg.value_lists) == next_block { return; } @@ -500,42 +494,23 @@ fn branch_order(pos: &mut FuncCursor, cfg: &mut ControlFlowGraph, block: Block, return; }; - let prev_inst_data = &pos.func.dfg.insts[prev_inst]; - - if let Some(prev_dest) = prev_inst_data.branch_destination() { - if prev_dest != next_block { - return; - } - } else { - return; - } - - match prev_inst_data { - InstructionData::Branch { + match &pos.func.dfg.insts[prev_inst] { + &InstructionData::Branch { opcode, - args: ref prev_args, + arg, destination: cond_dest, } => { - let cond_arg = { - let args = pos.func.dfg.inst_args(prev_inst); - args[0] - }; + if cond_dest.block(&pos.func.dfg.value_lists) != next_block { + return; + } let kind = match opcode { - Opcode::Brz => BranchOrderKind::BrzToBrnz(cond_arg), - Opcode::Brnz => BranchOrderKind::BrnzToBrz(cond_arg), + Opcode::Brz => Opcode::Brnz, + Opcode::Brnz => Opcode::Brz, _ => panic!("unexpected opcode"), }; - ( - inst, - args.clone(), - destination, - prev_inst, - prev_args.clone(), - *cond_dest, - kind, - ) + (inst, destination, prev_inst, cond_dest, kind, arg) } _ => return, } @@ -544,31 +519,14 @@ fn branch_order(pos: &mut FuncCursor, cfg: &mut ControlFlowGraph, block: Block, _ => return, }; - let cond_args = cond_inst_args.as_slice(&pos.func.dfg.value_lists).to_vec(); - let term_args = term_inst_args.as_slice(&pos.func.dfg.value_lists).to_vec(); - - match kind { - BranchOrderKind::BrnzToBrz(cond_arg) => { - pos.func - .dfg - .replace(term_inst) - .jump(cond_dest, &cond_args[1..]); - pos.func - .dfg - .replace(cond_inst) - .brz(cond_arg, term_dest, &term_args); - } - BranchOrderKind::BrzToBrnz(cond_arg) => { - pos.func - .dfg - .replace(term_inst) - .jump(cond_dest, &cond_args[1..]); - pos.func - .dfg - .replace(cond_inst) - .brnz(cond_arg, term_dest, &term_args); - } - } + pos.func + .dfg + .replace(term_inst) + .Jump(Opcode::Jump, INVALID, cond_dest); + pos.func + .dfg + .replace(cond_inst) + .Branch(kind, INVALID, term_dest, cond_arg); cfg.recompute_block(pos.func, block); } @@ -578,7 +536,7 @@ mod simplify { use crate::ir::{ dfg::ValueDef, immediates, - instructions::{Opcode, ValueList}, + instructions::Opcode, types::{I16, I32, I8}, }; use std::marker::PhantomData; @@ -829,30 +787,18 @@ mod simplify { } } - struct BranchOptInfo { - br_inst: Inst, - cmp_arg: Value, - args: ValueList, - new_opcode: Opcode, - } - /// Fold comparisons into branch operations when possible. /// /// This matches against operations which compare against zero, then use the /// result in a `brz` or `brnz` branch. It folds those two operations into a /// single `brz` or `brnz`. fn branch_opt(pos: &mut FuncCursor, inst: Inst) { - let mut info = if let InstructionData::Branch { + let (cmp_arg, new_opcode) = if let InstructionData::Branch { opcode: br_opcode, - args: ref br_args, + arg: first_arg, .. } = pos.func.dfg.insts[inst] { - let first_arg = { - let args = pos.func.dfg.inst_args(inst); - args[0] - }; - let icmp_inst = if let ValueDef::Result(icmp_inst, _) = pos.func.dfg.value_def(first_arg) { icmp_inst @@ -886,12 +832,7 @@ mod simplify { _ => return, }; - BranchOptInfo { - br_inst: inst, - cmp_arg, - args: br_args.clone(), - new_opcode, - } + (cmp_arg, new_opcode) } else { return; } @@ -899,11 +840,11 @@ mod simplify { return; }; - info.args.as_mut_slice(&mut pos.func.dfg.value_lists)[0] = info.cmp_arg; - if let InstructionData::Branch { ref mut opcode, .. } = pos.func.dfg.insts[info.br_inst] { - *opcode = info.new_opcode; + if let InstructionData::Branch { opcode, arg, .. } = &mut pos.func.dfg.insts[inst] { + *opcode = new_opcode; + *arg = cmp_arg; } else { - panic!(); + unreachable!(); } } } diff --git a/cranelift/codegen/src/verifier/mod.rs b/cranelift/codegen/src/verifier/mod.rs index 30d14bb62d..ec790faaa5 100644 --- a/cranelift/codegen/src/verifier/mod.rs +++ b/cranelift/codegen/src/verifier/mod.rs @@ -562,7 +562,7 @@ impl<'a> Verifier<'a> { ) -> VerifierStepResult<()> { use crate::ir::instructions::InstructionData::*; - for &arg in self.func.dfg.inst_args(inst) { + for arg in self.func.dfg.inst_values(inst) { self.verify_inst_arg(inst, arg, errors)?; // All used values must be attached to something. @@ -584,18 +584,8 @@ impl<'a> Verifier<'a> { MultiAry { ref args, .. } => { self.verify_value_list(inst, args, errors)?; } - Jump { - destination, - ref args, - .. - } - | Branch { - destination, - ref args, - .. - } => { - self.verify_block(inst, destination, errors)?; - self.verify_value_list(inst, args, errors)?; + Jump { destination, .. } | Branch { destination, .. } => { + self.verify_block(inst, destination.block(&self.func.dfg.value_lists), errors)?; } BranchTable { table, destination, .. @@ -1295,20 +1285,23 @@ impl<'a> Verifier<'a> { Ok(()) } + /// Typecheck both instructions that contain variable arguments like calls, and those that + /// include references to basic blocks with their arguments. fn typecheck_variable_args( &self, inst: Inst, errors: &mut VerifierErrors, ) -> VerifierStepResult<()> { match self.func.dfg.analyze_branch(inst) { - BranchInfo::SingleDest(block, _) => { + BranchInfo::SingleDest(block) => { let iter = self .func .dfg - .block_params(block) + .block_params(block.block(&self.func.dfg.value_lists)) .iter() .map(|&v| self.func.dfg.value_type(v)); - self.typecheck_variable_args_iterator(inst, iter, errors)?; + let args = block.args_slice(&self.func.dfg.value_lists); + self.typecheck_variable_args_iterator(inst, iter, args, errors)?; } BranchInfo::Table(table, block) => { if let Some(block) = block { @@ -1342,20 +1335,20 @@ impl<'a> Verifier<'a> { } match self.func.dfg.insts[inst].analyze_call(&self.func.dfg.value_lists) { - CallInfo::Direct(func_ref, _) => { + CallInfo::Direct(func_ref, args) => { let sig_ref = self.func.dfg.ext_funcs[func_ref].signature; let arg_types = self.func.dfg.signatures[sig_ref] .params .iter() .map(|a| a.value_type); - self.typecheck_variable_args_iterator(inst, arg_types, errors)?; + self.typecheck_variable_args_iterator(inst, arg_types, args, errors)?; } - CallInfo::Indirect(sig_ref, _) => { + CallInfo::Indirect(sig_ref, args) => { let arg_types = self.func.dfg.signatures[sig_ref] .params .iter() .map(|a| a.value_type); - self.typecheck_variable_args_iterator(inst, arg_types, errors)?; + self.typecheck_variable_args_iterator(inst, arg_types, args, errors)?; } CallInfo::NotACall => {} } @@ -1366,9 +1359,9 @@ impl<'a> Verifier<'a> { &self, inst: Inst, iter: I, + variable_args: &[Value], errors: &mut VerifierErrors, ) -> VerifierStepResult<()> { - let variable_args = self.func.dfg.inst_variable_args(inst); let mut i = 0; for expected_type in iter { @@ -1743,7 +1736,6 @@ impl<'a> Verifier<'a> { #[cfg(test)] mod tests { use super::{Verifier, VerifierError, VerifierErrors}; - use crate::entity::EntityList; use crate::ir::instructions::{InstructionData, Opcode}; use crate::ir::{types, AbiParam, Function}; use crate::settings; @@ -1785,11 +1777,11 @@ mod tests { imm: 0.into(), }); func.layout.append_inst(nullary_with_bad_opcode, block0); + let destination = func.dfg.block_call(block0, &[]); func.stencil.layout.append_inst( func.stencil.dfg.make_inst(InstructionData::Jump { opcode: Opcode::Jump, - destination: block0, - args: EntityList::default(), + destination, }), block0, ); diff --git a/cranelift/codegen/src/write.rs b/cranelift/codegen/src/write.rs index 3b1e8a6f1c..49e6e76d29 100644 --- a/cranelift/codegen/src/write.rs +++ b/cranelift/codegen/src/write.rs @@ -414,22 +414,15 @@ pub fn write_operands(w: &mut dyn Write, dfg: &DataFlowGraph, inst: Inst) -> fmt IntCompareImm { cond, arg, imm, .. } => write!(w, " {} {}, {}", cond, arg, imm), IntAddTrap { args, code, .. } => write!(w, " {}, {}, {}", args[0], args[1], code), FloatCompare { cond, args, .. } => write!(w, " {} {}, {}", cond, args[0], args[1]), - Jump { - destination, - ref args, - .. - } => { - write!(w, " {}", destination)?; - write_block_args(w, args.as_slice(pool)) + Jump { destination, .. } => { + write!(w, " {}", destination.block(pool))?; + write_block_args(w, destination.args_slice(pool)) } Branch { - destination, - ref args, - .. + arg, destination, .. } => { - let args = args.as_slice(pool); - write!(w, " {}, {}", args[0], destination)?; - write_block_args(w, &args[1..]) + write!(w, " {}, {}", arg, destination.block(pool))?; + write_block_args(w, destination.args_slice(pool)) } BranchTable { arg, @@ -485,7 +478,7 @@ pub fn write_operands(w: &mut dyn Write, dfg: &DataFlowGraph, inst: Inst) -> fmt }?; let mut sep = " ; "; - for &arg in dfg.inst_args(inst) { + for arg in dfg.inst_values(inst) { if let ValueDef::Result(src, _) = dfg.value_def(arg) { let imm = match dfg.insts[src] { UnaryImm { imm, .. } => imm.to_string(), diff --git a/cranelift/frontend/src/frontend.rs b/cranelift/frontend/src/frontend.rs index 0a2a191d72..f25e6c37b1 100644 --- a/cranelift/frontend/src/frontend.rs +++ b/cranelift/frontend/src/frontend.rs @@ -111,7 +111,10 @@ impl<'short, 'long> InstBuilderBase<'short> for FuncInstBuilder<'short, 'long> { Some(dest_block) => { // If the user has supplied jump arguments we must adapt the arguments of // the destination block - self.builder.declare_successor(dest_block, inst); + self.builder.declare_successor( + dest_block.block(&self.builder.func.dfg.value_lists), + inst, + ); } None => { // branch_destination() doesn't detect jump_tables @@ -676,11 +679,14 @@ impl<'a> FunctionBuilder<'a> { /// **Note:** You are responsible for maintaining the coherence with the arguments of /// other jump instructions. pub fn change_jump_destination(&mut self, inst: Inst, new_dest: Block) { - let old_dest = self.func.dfg.insts[inst] + let dfg = &mut self.func.dfg; + let old_dest = dfg.insts[inst] .branch_destination_mut() .expect("you want to change the jump destination of a non-jump instruction"); - self.func_ctx.ssa.remove_block_predecessor(*old_dest, inst); - *old_dest = new_dest; + self.func_ctx + .ssa + .remove_block_predecessor(old_dest.block(&dfg.value_lists), inst); + old_dest.set_block(new_dest, &mut dfg.value_lists); self.func_ctx.ssa.declare_block_predecessor(new_dest, inst); } diff --git a/cranelift/frontend/src/ssa.rs b/cranelift/frontend/src/ssa.rs index ded88909e2..058c95d2b5 100644 --- a/cranelift/frontend/src/ssa.rs +++ b/cranelift/frontend/src/ssa.rs @@ -584,8 +584,12 @@ impl SSABuilder { } // For a single destination appending a jump argument to the instruction // is sufficient. - BranchInfo::SingleDest(_, _) => { - func.dfg.append_inst_arg(branch, val); + BranchInfo::SingleDest(_) => { + let dfg = &mut func.dfg; + dfg.insts[branch] + .branch_destination_mut() + .unwrap() + .append_argument(val, &mut dfg.value_lists); None } BranchInfo::Table(mut jt, _default_block) => { @@ -832,23 +836,23 @@ mod tests { assert_eq!(x_ssa, x_use3); assert_eq!(y_ssa, y_use3); match func.dfg.analyze_branch(brnz_block0_block2) { - BranchInfo::SingleDest(dest, jump_args) => { - assert_eq!(dest, block2); - assert_eq!(jump_args.len(), 0); + BranchInfo::SingleDest(dest) => { + assert_eq!(dest.block(&func.dfg.value_lists), block2); + assert_eq!(dest.args_slice(&func.dfg.value_lists).len(), 0); } _ => assert!(false), }; match func.dfg.analyze_branch(jump_block0_block1) { - BranchInfo::SingleDest(dest, jump_args) => { - assert_eq!(dest, block1); - assert_eq!(jump_args.len(), 0); + BranchInfo::SingleDest(dest) => { + assert_eq!(dest.block(&func.dfg.value_lists), block1); + assert_eq!(dest.args_slice(&func.dfg.value_lists).len(), 0); } _ => assert!(false), }; match func.dfg.analyze_branch(jump_block1_block2) { - BranchInfo::SingleDest(dest, jump_args) => { - assert_eq!(dest, block2); - assert_eq!(jump_args.len(), 0); + BranchInfo::SingleDest(dest) => { + assert_eq!(dest.block(&func.dfg.value_lists), block2); + assert_eq!(dest.args_slice(&func.dfg.value_lists).len(), 0); } _ => assert!(false), }; diff --git a/cranelift/interpreter/src/step.rs b/cranelift/interpreter/src/step.rs index 365efc9634..89a63bae7e 100644 --- a/cranelift/interpreter/src/step.rs +++ b/cranelift/interpreter/src/step.rs @@ -235,21 +235,25 @@ where }; // Retrieve an instruction's branch destination; expects the instruction to be a branch. - let branch = || -> Block { inst.branch_destination().unwrap() }; + + let continue_at = || { + let block = inst.branch_destination().unwrap(); + let branch_args = state + .collect_values(block.args_slice(&state.get_current_function().dfg.value_lists)) + .map_err(|v| StepError::UnknownValue(v))?; + Ok(ControlFlow::ContinueAt( + block.block(&state.get_current_function().dfg.value_lists), + branch_args, + )) + }; // Based on `condition`, indicate where to continue the control flow. let branch_when = |condition: bool| -> Result, StepError> { - let branch_args = match inst { - InstructionData::Jump { .. } => args_range(0..), - InstructionData::Branch { .. } => args_range(1..), - _ => panic!("Unrecognized branch inst: {:?}", inst), - }?; - - Ok(if condition { - ControlFlow::ContinueAt(branch(), branch_args) + if condition { + continue_at() } else { - ControlFlow::Continue - }) + Ok(ControlFlow::Continue) + } }; // Retrieve an instruction's trap code; expects the instruction to be a trap. @@ -275,7 +279,7 @@ where // Interpret a Cranelift instruction. Ok(match inst.opcode() { - Opcode::Jump => ControlFlow::ContinueAt(branch(), args()?), + Opcode::Jump => continue_at()?, Opcode::Brz => branch_when( !arg(0)? .convert(ValueConversionKind::ToBoolean)? diff --git a/cranelift/preopt/src/constant_folding.rs b/cranelift/preopt/src/constant_folding.rs index 8333f6c8e2..ca11d44b8d 100644 --- a/cranelift/preopt/src/constant_folding.rs +++ b/cranelift/preopt/src/constant_folding.rs @@ -3,7 +3,7 @@ use cranelift_codegen::{ cursor::{Cursor, FuncCursor}, - ir::{self, dfg::ValueDef, InstBuilder}, + ir::{self, dfg::ValueDef, instructions::Opcode, types, InstBuilder}, }; // use rustc_apfloat::{ // ieee::{Double, Single}, @@ -52,8 +52,12 @@ pub fn fold_constants(func: &mut ir::Function) { Unary { opcode, arg } => { fold_unary(&mut pos.func.dfg, inst, opcode, arg); } - Branch { opcode, .. } => { - fold_branch(&mut pos, inst, opcode); + Branch { + opcode, + arg, + destination, + } => { + fold_branch(&mut pos, inst, opcode, arg, destination); } _ => {} } @@ -215,18 +219,16 @@ fn fold_unary(dfg: &mut ir::DataFlowGraph, inst: ir::Inst, opcode: ir::Opcode, a } } -fn fold_branch(pos: &mut FuncCursor, inst: ir::Inst, opcode: ir::Opcode) { - let (cond, block, args) = { - let values = pos.func.dfg.inst_args(inst); - let inst_data = &pos.func.dfg.insts[inst]; - ( - match resolve_value_to_imm(&pos.func.dfg, values[0]) { - Some(imm) => imm, - None => return, - }, - inst_data.branch_destination().unwrap(), - values[1..].to_vec(), - ) +fn fold_branch( + pos: &mut FuncCursor, + inst: ir::Inst, + opcode: ir::Opcode, + cond: ir::Value, + block: ir::BlockCall, +) { + let cond = match resolve_value_to_imm(&pos.func.dfg, cond) { + Some(imm) => imm, + None => return, }; let truthiness = cond.evaluate_truthiness(); @@ -237,7 +239,10 @@ fn fold_branch(pos: &mut FuncCursor, inst: ir::Inst, opcode: ir::Opcode) { }; if (branch_if_zero && !truthiness) || (!branch_if_zero && truthiness) { - pos.func.dfg.replace(inst).jump(block, &args); + pos.func + .dfg + .replace(inst) + .Jump(Opcode::Jump, types::INVALID, block); // remove the rest of the block to avoid verifier errors while let Some(next_inst) = pos.func.layout.next_inst(inst) { pos.func.layout.remove_inst(next_inst); diff --git a/cranelift/reader/src/parser.rs b/cranelift/reader/src/parser.rs index 57629a02ff..1a6b05d2c8 100644 --- a/cranelift/reader/src/parser.rs +++ b/cranelift/reader/src/parser.rs @@ -1899,8 +1899,8 @@ impl<'a> Parser<'a> { // all references refer to a definition. for block in &ctx.function.layout { for inst in ctx.function.layout.block_insts(block) { - for value in ctx.function.dfg.inst_args(inst) { - if !ctx.map.contains_value(*value) { + for value in ctx.function.dfg.inst_values(inst) { + if !ctx.map.contains_value(value) { return err!( ctx.map.location(AnyEntity::Inst(inst)).unwrap(), "undefined operand value {}", @@ -2582,21 +2582,22 @@ impl<'a> Parser<'a> { // Parse the destination block number. let block_num = self.match_block("expected jump destination block")?; let args = self.parse_opt_value_list()?; + let destination = ctx.function.dfg.block_call(block_num, &args); InstructionData::Jump { opcode, - destination: block_num, - args: args.into_value_list(&[], &mut ctx.function.dfg.value_lists), + destination, } } InstructionFormat::Branch => { - let ctrl_arg = self.match_value("expected SSA value control operand")?; + let arg = self.match_value("expected SSA value control operand")?; self.match_token(Token::Comma, "expected ',' between operands")?; let block_num = self.match_block("expected branch destination block")?; let args = self.parse_opt_value_list()?; + let destination = ctx.function.dfg.block_call(block_num, &args); InstructionData::Branch { opcode, - destination: block_num, - args: args.into_value_list(&[ctrl_arg], &mut ctx.function.dfg.value_lists), + arg, + destination, } } InstructionFormat::BranchTable => { diff --git a/cranelift/src/bugpoint.rs b/cranelift/src/bugpoint.rs index 01f18ae589..606cfac348 100644 --- a/cranelift/src/bugpoint.rs +++ b/cranelift/src/bugpoint.rs @@ -422,11 +422,9 @@ impl Mutator for ReplaceBlockParamWithConst { // Remove parameters in branching instructions that point to this block for pred in cfg.pred_iter(self.block) { let dfg = &mut func.dfg; - let inst = &mut dfg.insts[pred.inst]; - let num_fixed_args = inst.opcode().constraints().num_fixed_value_arguments(); - inst.value_list_mut() - .unwrap() - .remove(num_fixed_args + param_index, &mut dfg.value_lists); + for branch in dfg.insts[pred.inst].branch_destination_mut().into_iter() { + branch.remove(param_index, &mut dfg.value_lists); + } } if Some(self.block) == func.layout.entry_block() {