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() {