diff --git a/cranelift/codegen/src/dominator_tree.rs b/cranelift/codegen/src/dominator_tree.rs index 0df5951075..d70db21971 100644 --- a/cranelift/codegen/src/dominator_tree.rs +++ b/cranelift/codegen/src/dominator_tree.rs @@ -2,8 +2,7 @@ use crate::entity::SecondaryMap; use crate::flowgraph::{BlockPredecessor, ControlFlowGraph}; -use crate::ir::instructions::BranchInfo; -use crate::ir::{Block, ExpandedProgramPoint, Function, Inst, Layout, ProgramOrder, Value}; +use crate::ir::{self, Block, ExpandedProgramPoint, Function, Inst, Layout, ProgramOrder, Value}; use crate::packed_option::PackedOption; use crate::timing; use alloc::vec::Vec; @@ -352,21 +351,28 @@ impl DominatorTree { /// post-order except for the insertion of the new block header at the split point. fn push_successors(&mut self, func: &Function, block: Block) { if let Some(inst) = func.layout.last_inst(block) { - match func.dfg.analyze_branch(inst) { - BranchInfo::SingleDest(succ) => { - self.push_if_unseen(succ.block(&func.dfg.value_lists)) - } - BranchInfo::Conditional(block_then, block_else) => { + match func.dfg.insts[inst] { + ir::InstructionData::Jump { + destination: succ, .. + } => self.push_if_unseen(succ.block(&func.dfg.value_lists)), + ir::InstructionData::Brif { + blocks: [block_then, block_else], + .. + } => { self.push_if_unseen(block_then.block(&func.dfg.value_lists)); self.push_if_unseen(block_else.block(&func.dfg.value_lists)); } - BranchInfo::Table(jt, dest) => { + ir::InstructionData::BranchTable { + table: jt, + destination: dest, + .. + } => { for succ in func.jump_tables[jt].iter() { self.push_if_unseen(*succ); } self.push_if_unseen(dest); } - BranchInfo::NotABranch => {} + _ => {} } } } diff --git a/cranelift/codegen/src/flowgraph.rs b/cranelift/codegen/src/flowgraph.rs index ead9cba2f3..9b04a5abc6 100644 --- a/cranelift/codegen/src/flowgraph.rs +++ b/cranelift/codegen/src/flowgraph.rs @@ -22,8 +22,7 @@ use crate::bforest; use crate::entity::SecondaryMap; -use crate::ir::instructions::BranchInfo; -use crate::ir::{Block, Function, Inst}; +use crate::ir::{self, Block, Function, Inst}; use crate::timing; use core::mem; @@ -118,22 +117,31 @@ impl ControlFlowGraph { fn compute_block(&mut self, func: &Function, block: Block) { if let Some(inst) = func.layout.last_inst(block) { - match func.dfg.analyze_branch(inst) { - BranchInfo::SingleDest(dest) => { + match func.dfg.insts[inst] { + ir::InstructionData::Jump { + destination: dest, .. + } => { self.add_edge(block, inst, dest.block(&func.dfg.value_lists)); } - BranchInfo::Conditional(block_then, block_else) => { + ir::InstructionData::Brif { + blocks: [block_then, block_else], + .. + } => { self.add_edge(block, inst, block_then.block(&func.dfg.value_lists)); self.add_edge(block, inst, block_else.block(&func.dfg.value_lists)); } - BranchInfo::Table(jt, dest) => { + ir::InstructionData::BranchTable { + table: jt, + destination: dest, + .. + } => { self.add_edge(block, inst, dest); for dest in func.jump_tables[jt].iter() { self.add_edge(block, inst, *dest); } } - BranchInfo::NotABranch => {} + _ => {} } } } diff --git a/cranelift/codegen/src/inst_predicates.rs b/cranelift/codegen/src/inst_predicates.rs index eb8a53202b..9e9280675f 100644 --- a/cranelift/codegen/src/inst_predicates.rs +++ b/cranelift/codegen/src/inst_predicates.rs @@ -1,7 +1,6 @@ //! Instruction predicates/properties, shared by various analyses. use crate::ir::immediates::Offset32; -use crate::ir::instructions::BranchInfo; -use crate::ir::{Block, DataFlowGraph, Function, Inst, InstructionData, Opcode, Type, Value}; +use crate::ir::{self, Block, DataFlowGraph, Function, Inst, InstructionData, Opcode, Type, Value}; use cranelift_entity::EntityRef; /// Preserve instructions with used result values. @@ -176,16 +175,26 @@ pub(crate) fn visit_block_succs( mut visit: F, ) { if let Some(inst) = f.layout.last_inst(block) { - match f.dfg.insts[inst].analyze_branch() { - BranchInfo::NotABranch => {} - BranchInfo::SingleDest(dest) => { + match f.dfg.insts[inst] { + ir::InstructionData::Jump { + destination: dest, .. + } => { visit(inst, dest.block(&f.dfg.value_lists), false); } - BranchInfo::Conditional(block_then, block_else) => { + + ir::InstructionData::Brif { + blocks: [block_then, block_else], + .. + } => { visit(inst, block_then.block(&f.dfg.value_lists), false); visit(inst, block_else.block(&f.dfg.value_lists), false); } - BranchInfo::Table(table, dest) => { + + ir::InstructionData::BranchTable { + table, + destination: dest, + .. + } => { // The default block is reached via a direct conditional branch, // so it is not part of the table. visit(inst, dest, false); @@ -194,6 +203,8 @@ pub(crate) fn visit_block_succs( visit(inst, dest, true); } } + + _ => {} } } } diff --git a/cranelift/codegen/src/ir/dfg.rs b/cranelift/codegen/src/ir/dfg.rs index a1f4c34f43..d180695219 100644 --- a/cranelift/codegen/src/ir/dfg.rs +++ b/cranelift/codegen/src/ir/dfg.rs @@ -4,7 +4,7 @@ use crate::entity::{self, PrimaryMap, SecondaryMap}; use crate::ir; use crate::ir::builder::ReplaceBuilder; use crate::ir::dynamic_type::{DynamicTypeData, DynamicTypes}; -use crate::ir::instructions::{BranchInfo, CallInfo, InstructionData}; +use crate::ir::instructions::{CallInfo, InstructionData}; use crate::ir::{types, ConstantData, ConstantPool, Immediate}; use crate::ir::{ Block, BlockCall, DynamicType, FuncRef, Inst, SigRef, Signature, Type, Value, @@ -1035,11 +1035,6 @@ impl DataFlowGraph { impl ExactSizeIterator for InstResultTypes<'_> {} } - /// Check if `inst` is a branch. - pub fn analyze_branch(&self, inst: Inst) -> BranchInfo { - self.insts[inst].analyze_branch() - } - /// Compute the type of an instruction result from opcode constraints and call signatures. /// /// This computes the same sequence of result types that `make_inst_results()` above would diff --git a/cranelift/codegen/src/ir/entities.rs b/cranelift/codegen/src/ir/entities.rs index 8573b4cd7d..51c7633207 100644 --- a/cranelift/codegen/src/ir/entities.rs +++ b/cranelift/codegen/src/ir/entities.rs @@ -92,11 +92,6 @@ impl Value { /// the `Value` of such instructions, use [`inst_results`](super::DataFlowGraph::inst_results) or /// its analogue in `cranelift_frontend::FuncBuilder`. /// -/// If you look around the API, you can find many inventive uses for `Inst`, -/// such as [annotating specific instructions with a comment][inst_comment] -/// or [performing reflection at compile time](super::DataFlowGraph::analyze_branch) -/// on the type of instruction. -/// /// [inst_comment]: https://github.com/bjorn3/rustc_codegen_cranelift/blob/0f8814fd6da3d436a90549d4bb19b94034f2b19c/src/pretty_clif.rs /// /// While the order is stable, it is arbitrary and does not necessarily resemble the layout order. diff --git a/cranelift/codegen/src/ir/function.rs b/cranelift/codegen/src/ir/function.rs index 5414831076..b0a14d426a 100644 --- a/cranelift/codegen/src/ir/function.rs +++ b/cranelift/codegen/src/ir/function.rs @@ -4,15 +4,12 @@ //! instructions. use crate::entity::{PrimaryMap, SecondaryMap}; -use crate::ir; -use crate::ir::JumpTables; use crate::ir::{ - instructions::BranchInfo, Block, DynamicStackSlot, DynamicStackSlotData, DynamicType, - ExtFuncData, FuncRef, GlobalValue, GlobalValueData, Inst, InstructionData, JumpTable, - JumpTableData, Opcode, SigRef, StackSlot, StackSlotData, Table, TableData, Type, + self, Block, DataFlowGraph, DynamicStackSlot, DynamicStackSlotData, DynamicStackSlots, + DynamicType, ExtFuncData, FuncRef, GlobalValue, GlobalValueData, Inst, InstructionData, + JumpTable, JumpTableData, JumpTables, Layout, Opcode, SigRef, Signature, SourceLocs, StackSlot, + StackSlotData, StackSlots, Table, TableData, Type, }; -use crate::ir::{DataFlowGraph, Layout, Signature}; -use crate::ir::{DynamicStackSlots, SourceLocs, StackSlots}; use crate::isa::CallConv; use crate::value_label::ValueLabelsRanges; use crate::write::write_function; @@ -280,8 +277,10 @@ impl FunctionStencil { /// Rewrite the branch destination to `new_dest` if the destination matches `old_dest`. /// Does nothing if called with a non-jump or non-branch instruction. pub fn rewrite_branch_destination(&mut self, inst: Inst, old_dest: Block, new_dest: Block) { - match self.dfg.analyze_branch(inst) { - BranchInfo::SingleDest(dest) => { + match self.dfg.insts[inst] { + InstructionData::Jump { + destination: dest, .. + } => { if dest.block(&self.dfg.value_lists) == old_dest { for block in self.dfg.insts[inst].branch_destination_mut() { block.set_block(new_dest, &mut self.dfg.value_lists) @@ -289,7 +288,10 @@ impl FunctionStencil { } } - BranchInfo::Conditional(block_then, block_else) => { + InstructionData::Brif { + blocks: [block_then, block_else], + .. + } => { if block_then.block(&self.dfg.value_lists) == old_dest { if let InstructionData::Brif { blocks: [block_then, _], @@ -315,7 +317,11 @@ impl FunctionStencil { } } - BranchInfo::Table(table, default_dest) => { + InstructionData::BranchTable { + table, + destination: default_dest, + .. + } => { self.jump_tables[table].iter_mut().for_each(|entry| { if *entry == old_dest { *entry = new_dest; @@ -335,7 +341,7 @@ impl FunctionStencil { } } - BranchInfo::NotABranch => {} + _ => {} } } diff --git a/cranelift/codegen/src/ir/instructions.rs b/cranelift/codegen/src/ir/instructions.rs index dec1e87282..2771915a54 100644 --- a/cranelift/codegen/src/ir/instructions.rs +++ b/cranelift/codegen/src/ir/instructions.rs @@ -20,7 +20,7 @@ use crate::ir::{ self, condcodes::{FloatCC, IntCC}, trapcode::TrapCode, - types, Block, FuncRef, JumpTable, MemFlags, SigRef, StackSlot, Type, Value, + types, Block, FuncRef, MemFlags, SigRef, StackSlot, Type, Value, }; /// Some instructions use an external list of argument values because there is not enough space in @@ -292,27 +292,6 @@ impl Default for VariableArgs { /// Avoid large matches on instruction formats by using the methods defined here to examine /// instructions. impl InstructionData { - /// Return information about the destination of a branch or jump instruction. - /// - /// Any instruction that can transfer control to another block reveals its possible destinations - /// here. - pub fn analyze_branch(&self) -> BranchInfo { - match *self { - Self::Jump { destination, .. } => BranchInfo::SingleDest(destination), - Self::Brif { - blocks: [block_then, block_else], - .. - } => BranchInfo::Conditional(block_then, block_else), - Self::BranchTable { - table, destination, .. - } => BranchInfo::Table(table, destination), - _ => { - debug_assert!(!self.opcode().is_branch()); - BranchInfo::NotABranch - } - } - } - /// Get the destinations of this instruction, if it's a branch. /// /// `br_table` returns the empty slice. @@ -476,23 +455,6 @@ impl InstructionData { } } -/// Information about branch and jump instructions. -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(BlockCall), - - /// This is a conditional branch - Conditional(BlockCall, BlockCall), - - /// This is a jump table branch which can have many destination blocks and one default block. - Table(JumpTable, Block), -} - /// Information about call instructions. pub enum CallInfo<'a> { /// This is not a call instruction. diff --git a/cranelift/codegen/src/machinst/lower.rs b/cranelift/codegen/src/machinst/lower.rs index ea29cb3468..250bfa0783 100644 --- a/cranelift/codegen/src/machinst/lower.rs +++ b/cranelift/codegen/src/machinst/lower.rs @@ -9,9 +9,9 @@ use crate::entity::SecondaryMap; use crate::fx::{FxHashMap, FxHashSet}; use crate::inst_predicates::{has_lowering_side_effect, is_constant_64bit}; use crate::ir::{ - instructions, ArgumentPurpose, Block, Constant, ConstantData, DataFlowGraph, ExternalName, - Function, GlobalValue, GlobalValueData, Immediate, Inst, InstructionData, MemFlags, Opcode, - RelSourceLoc, Type, Value, ValueDef, ValueLabelAssignments, ValueLabelStart, + ArgumentPurpose, Block, Constant, ConstantData, DataFlowGraph, ExternalName, Function, + GlobalValue, GlobalValueData, Immediate, Inst, InstructionData, MemFlags, Opcode, RelSourceLoc, + Type, Value, ValueDef, ValueLabelAssignments, ValueLabelStart, }; use crate::machinst::{ writable_value_regs, BlockIndex, BlockLoweringOrder, Callee, LoweredBlock, MachLabel, Reg, @@ -943,17 +943,18 @@ impl<'func, I: VCodeInst> Lower<'func, I> { let (inst, succ) = self.vcode.block_order().succ_indices(block)[succ_idx]; // Get branch args and convert to Regs. - let branch_args = match self.f.dfg.analyze_branch(inst) { - instructions::BranchInfo::NotABranch => unreachable!(), - instructions::BranchInfo::SingleDest(block) => { - block.args_slice(&self.f.dfg.value_lists) - } - instructions::BranchInfo::Conditional(then_block, else_block) => { + let branch_args = match self.f.dfg.insts[inst] { + InstructionData::Jump { + destination: block, .. + } => block.args_slice(&self.f.dfg.value_lists), + InstructionData::Brif { + blocks: [then_block, else_block], + .. + } => { // NOTE: `succ_idx == 0` implying that we're traversing the `then_block` is // enforced by the traversal order defined in `visit_block_succs`. Eventually - // we should traverse the `branch_destination` slice instead of the result of - // analyze_branch there, which would simplify computing the branch args - // significantly. + // we should traverse the `branch_destination` slice there, which would + // simplify computing the branch args significantly. if succ_idx == 0 { then_block.args_slice(&self.f.dfg.value_lists) } else { @@ -961,7 +962,8 @@ impl<'func, I: VCodeInst> Lower<'func, I> { else_block.args_slice(&self.f.dfg.value_lists) } } - instructions::BranchInfo::Table(_, _) => &[], + InstructionData::BranchTable { .. } => &[], + _ => unreachable!(), }; let mut branch_arg_vregs: SmallVec<[Reg; 16]> = smallvec![]; for &arg in branch_args { diff --git a/cranelift/codegen/src/verifier/mod.rs b/cranelift/codegen/src/verifier/mod.rs index b71ccf41e5..523e32d7f3 100644 --- a/cranelift/codegen/src/verifier/mod.rs +++ b/cranelift/codegen/src/verifier/mod.rs @@ -62,7 +62,7 @@ use crate::dominator_tree::DominatorTree; use crate::entity::SparseSet; use crate::flowgraph::{BlockPredecessor, ControlFlowGraph}; use crate::ir::entities::AnyEntity; -use crate::ir::instructions::{BranchInfo, CallInfo, InstructionFormat, ResolvedConstraint}; +use crate::ir::instructions::{CallInfo, InstructionFormat, ResolvedConstraint}; use crate::ir::{self, ArgumentExtension}; use crate::ir::{ types, ArgumentPurpose, Block, Constant, DynamicStackSlot, FuncRef, Function, GlobalValue, @@ -1293,8 +1293,10 @@ impl<'a> Verifier<'a> { inst: Inst, errors: &mut VerifierErrors, ) -> VerifierStepResult<()> { - match self.func.dfg.analyze_branch(inst) { - BranchInfo::SingleDest(block) => { + match self.func.dfg.insts[inst] { + ir::InstructionData::Jump { + destination: block, .. + } => { let iter = self .func .dfg @@ -1304,7 +1306,10 @@ impl<'a> Verifier<'a> { let args = block.args_slice(&self.func.dfg.value_lists); self.typecheck_variable_args_iterator(inst, iter, args, errors)?; } - BranchInfo::Conditional(block_then, block_else) => { + ir::InstructionData::Brif { + blocks: [block_then, block_else], + .. + } => { let iter = self .func .dfg @@ -1323,7 +1328,11 @@ impl<'a> Verifier<'a> { let args_else = block_else.args_slice(&self.func.dfg.value_lists); self.typecheck_variable_args_iterator(inst, iter, args_else, errors)?; } - BranchInfo::Table(table, block) => { + ir::InstructionData::BranchTable { + table, + destination: block, + .. + } => { let arg_count = self.func.dfg.num_block_params(block); if arg_count != 0 { return errors.nonfatal(( @@ -1349,7 +1358,7 @@ impl<'a> Verifier<'a> { } } } - BranchInfo::NotABranch => {} + _ => {} } match self.func.dfg.insts[inst].analyze_call(&self.func.dfg.value_lists) { diff --git a/cranelift/frontend/src/frontend.rs b/cranelift/frontend/src/frontend.rs index dff1601cca..a0b5e2e295 100644 --- a/cranelift/frontend/src/frontend.rs +++ b/cranelift/frontend/src/frontend.rs @@ -106,17 +106,20 @@ impl<'short, 'long> InstBuilderBase<'short> for FuncInstBuilder<'short, 'long> { self.builder.func.set_srcloc(inst, self.builder.srcloc); } - match self.builder.func.dfg.analyze_branch(inst) { - ir::instructions::BranchInfo::NotABranch => (), - - ir::instructions::BranchInfo::SingleDest(dest) => { + match self.builder.func.dfg.insts[inst] { + ir::InstructionData::Jump { + destination: dest, .. + } => { // If the user has supplied jump arguments we must adapt the arguments of // the destination block let block = dest.block(&self.builder.func.dfg.value_lists); self.builder.declare_successor(block, inst); } - ir::instructions::BranchInfo::Conditional(branch_then, branch_else) => { + ir::InstructionData::Brif { + blocks: [branch_then, branch_else], + .. + } => { let block_then = branch_then.block(&self.builder.func.dfg.value_lists); let block_else = branch_else.block(&self.builder.func.dfg.value_lists); @@ -126,7 +129,9 @@ impl<'short, 'long> InstBuilderBase<'short> for FuncInstBuilder<'short, 'long> { } } - ir::instructions::BranchInfo::Table(table, destination) => { + ir::InstructionData::BranchTable { + table, destination, .. + } => { // Unlike all other jumps/branches, jump tables are // capable of having the same successor appear // multiple times, so we must deduplicate. @@ -149,6 +154,8 @@ impl<'short, 'long> InstBuilderBase<'short> for FuncInstBuilder<'short, 'long> { } self.builder.declare_successor(destination, inst); } + + _ => {} } if data.opcode().is_terminator() { diff --git a/cranelift/frontend/src/ssa.rs b/cranelift/frontend/src/ssa.rs index ea53765e54..0623ddc743 100644 --- a/cranelift/frontend/src/ssa.rs +++ b/cranelift/frontend/src/ssa.rs @@ -14,7 +14,6 @@ use core::mem; use cranelift_codegen::cursor::{Cursor, FuncCursor}; use cranelift_codegen::entity::{EntityList, EntitySet, ListPool, SecondaryMap}; use cranelift_codegen::ir::immediates::{Ieee32, Ieee64}; -use cranelift_codegen::ir::instructions::BranchInfo; use cranelift_codegen::ir::types::{F32, F64, I128, I64}; use cranelift_codegen::ir::{ Block, Function, Inst, InstBuilder, InstructionData, JumpTableData, Type, Value, @@ -578,20 +577,17 @@ impl SSABuilder { dest_block: Block, val: Value, ) -> Option<(Block, Inst)> { - match func.dfg.analyze_branch(branch) { - BranchInfo::NotABranch => { - panic!("you have declared a non-branch instruction as a predecessor to a block"); - } + match func.dfg.insts[branch] { // For a single destination appending a jump argument to the instruction // is sufficient. - BranchInfo::SingleDest(_) => { + InstructionData::Jump { .. } => { let dfg = &mut func.dfg; for dest in dfg.insts[branch].branch_destination_mut() { dest.append_argument(val, &mut dfg.value_lists); } None } - BranchInfo::Conditional(_, _) => { + InstructionData::Brif { .. } => { let dfg = &mut func.dfg; for block in dfg.insts[branch].branch_destination_mut() { if block.block(&dfg.value_lists) == dest_block { @@ -600,7 +596,7 @@ impl SSABuilder { } None } - BranchInfo::Table(mut jt, _default_block) => { + InstructionData::BranchTable { table: mut jt, .. } => { // In the case of a jump table, the situation is tricky because br_table doesn't // support arguments. We have to split the critical edge. let middle_block = func.dfg.make_block(); @@ -622,7 +618,7 @@ impl SSABuilder { jt = func.create_jump_table(copied); } - // Redo the match from `analyze_branch` but this time capture mutable references + // Redo the match from above, but this time capture mutable references match &mut func.dfg.insts[branch] { InstructionData::BranchTable { destination, table, .. @@ -639,6 +635,9 @@ impl SSABuilder { let middle_jump_inst = cur.ins().jump(dest_block, &[val]); Some((middle_block, middle_jump_inst)) } + _ => { + panic!("you have declared a non-branch instruction as a predecessor to a block"); + } } } @@ -689,7 +688,7 @@ mod tests { use crate::Variable; use cranelift_codegen::cursor::{Cursor, FuncCursor}; use cranelift_codegen::entity::EntityRef; - use cranelift_codegen::ir::instructions::BranchInfo; + use cranelift_codegen::ir; use cranelift_codegen::ir::types::*; use cranelift_codegen::ir::{Function, Inst, InstBuilder, JumpTableData, Opcode}; use cranelift_codegen::settings; @@ -838,8 +837,11 @@ mod tests { assert_eq!(x_ssa, x_use3); assert_eq!(y_ssa, y_use3); - match func.dfg.analyze_branch(brif_block0_block2_block1) { - BranchInfo::Conditional(block_then, block_else) => { + match func.dfg.insts[brif_block0_block2_block1] { + ir::InstructionData::Brif { + blocks: [block_then, block_else], + .. + } => { assert_eq!(block_then.block(&func.dfg.value_lists), block2); assert_eq!(block_then.args_slice(&func.dfg.value_lists).len(), 0); assert_eq!(block_else.block(&func.dfg.value_lists), block1); @@ -847,8 +849,11 @@ mod tests { } _ => assert!(false), }; - match func.dfg.analyze_branch(brif_block0_block2_block1) { - BranchInfo::Conditional(block_then, block_else) => { + match func.dfg.insts[brif_block0_block2_block1] { + ir::InstructionData::Brif { + blocks: [block_then, block_else], + .. + } => { assert_eq!(block_then.block(&func.dfg.value_lists), block2); assert_eq!(block_then.args_slice(&func.dfg.value_lists).len(), 0); assert_eq!(block_else.block(&func.dfg.value_lists), block1); @@ -856,8 +861,10 @@ mod tests { } _ => assert!(false), }; - match func.dfg.analyze_branch(jump_block1_block2) { - BranchInfo::SingleDest(dest) => { + match func.dfg.insts[jump_block1_block2] { + ir::InstructionData::Jump { + destination: dest, .. + } => { assert_eq!(dest.block(&func.dfg.value_lists), block2); assert_eq!(dest.args_slice(&func.dfg.value_lists).len(), 0); }