T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove analyze_branch and BranchInfo (#5730)

Browse Source 
We don't have overlap in behavior for branch instructions anymore, so we can remove analyze_branch and instead match on the InstructionData directly.

Co-authored-by: Jamey Sharp <jamey@minilop.net>
This commit is contained in:
Trevor Elliott
2023-02-06 17:06:57 -08:00
committed by GitHub
parent 75ae976adc
commit c8a6adf825
11 changed files with 134 additions and 126 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
cranelift/codegen/src/dominator_tree.rs
View File

@@ -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 => {}
_ => {}
}
}
}

22
cranelift/codegen/src/flowgraph.rs
View File

@@ -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 => {}
_ => {}
}
}
}

25
cranelift/codegen/src/inst_predicates.rs
View File

@@ -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<F: FnMut(Inst, Block, bool)>(
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<F: FnMut(Inst, Block, bool)>(
visit(inst, dest, true);
}
}
_ => {}
}
}
}

7
cranelift/codegen/src/ir/dfg.rs
View File

@@ -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

5
cranelift/codegen/src/ir/entities.rs
View File

@@ -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.

30
cranelift/codegen/src/ir/function.rs
View File

@@ -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 => {}
_ => {}
}
}

40
cranelift/codegen/src/ir/instructions.rs
View File

@@ -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.

28
cranelift/codegen/src/machinst/lower.rs
View File

@@ -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 {

21
cranelift/codegen/src/verifier/mod.rs
View File

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

19
cranelift/frontend/src/frontend.rs
View File

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

39
cranelift/frontend/src/ssa.rs
View File

@@ -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);
}