Skip to content

Commit c8a6adf

Browse files
elliotttjameysharp
elliottt
and
jameysharp
authored
Remove analyze_branch and BranchInfo (#5730)
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 <[email protected]>
1 parent 75ae976 commit c8a6adf

File tree

11 files changed

+134
-126
lines changed

11 files changed

+134
-126
lines changed

‎cranelift/codegen/src/dominator_tree.rs‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22
33
use crate::entity::SecondaryMap;
44
use crate::flowgraph::{BlockPredecessor, ControlFlowGraph};
5-
use crate::ir::instructions::BranchInfo;
6-
use crate::ir::{Block, ExpandedProgramPoint, Function, Inst, Layout, ProgramOrder, Value};
5+
use crate::ir::{self, Block, ExpandedProgramPoint, Function, Inst, Layout, ProgramOrder, Value};
76
use crate::packed_option::PackedOption;
87
use crate::timing;
98
use alloc::vec::Vec;
@@ -352,21 +351,28 @@ impl DominatorTree {
352351
/// post-order except for the insertion of the new block header at the split point.
353352
fn push_successors(&mut self, func: &Function, block: Block) {
354353
if let Some(inst) = func.layout.last_inst(block) {
355-
match func.dfg.analyze_branch(inst) {
356-
BranchInfo::SingleDest(succ) => {
357-
self.push_if_unseen(succ.block(&func.dfg.value_lists))
358-
}
359-
BranchInfo::Conditional(block_then, block_else) => {
354+
match func.dfg.insts[inst] {
355+
ir::InstructionData::Jump {
356+
destination: succ, ..
357+
} => self.push_if_unseen(succ.block(&func.dfg.value_lists)),
358+
ir::InstructionData::Brif {
359+
blocks: [block_then, block_else],
360+
..
361+
} => {
360362
self.push_if_unseen(block_then.block(&func.dfg.value_lists));
361363
self.push_if_unseen(block_else.block(&func.dfg.value_lists));
362364
}
363-
BranchInfo::Table(jt, dest) => {
365+
ir::InstructionData::BranchTable {
366+
table: jt,
367+
destination: dest,
368+
..
369+
} => {
364370
for succ in func.jump_tables[jt].iter() {
365371
self.push_if_unseen(*succ);
366372
}
367373
self.push_if_unseen(dest);
368374
}
369-
BranchInfo::NotABranch => {}
375+
_ => {}
370376
}
371377
}
372378
}

‎cranelift/codegen/src/flowgraph.rs‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,7 @@
2222
2323
use crate::bforest;
2424
use crate::entity::SecondaryMap;
25-
use crate::ir::instructions::BranchInfo;
26-
use crate::ir::{Block, Function, Inst};
25+
use crate::ir::{self, Block, Function, Inst};
2726
use crate::timing;
2827
use core::mem;
2928

@@ -118,22 +117,31 @@ impl ControlFlowGraph {
118117

119118
fn compute_block(&mut self, func: &Function, block: Block) {
120119
if let Some(inst) = func.layout.last_inst(block) {
121-
match func.dfg.analyze_branch(inst) {
122-
BranchInfo::SingleDest(dest) => {
120+
match func.dfg.insts[inst] {
121+
ir::InstructionData::Jump {
122+
destination: dest, ..
123+
} => {
123124
self.add_edge(block, inst, dest.block(&func.dfg.value_lists));
124125
}
125-
BranchInfo::Conditional(block_then, block_else) => {
126+
ir::InstructionData::Brif {
127+
blocks: [block_then, block_else],
128+
..
129+
} => {
126130
self.add_edge(block, inst, block_then.block(&func.dfg.value_lists));
127131
self.add_edge(block, inst, block_else.block(&func.dfg.value_lists));
128132
}
129-
BranchInfo::Table(jt, dest) => {
133+
ir::InstructionData::BranchTable {
134+
table: jt,
135+
destination: dest,
136+
..
137+
} => {
130138
self.add_edge(block, inst, dest);
131139

132140
for dest in func.jump_tables[jt].iter() {
133141
self.add_edge(block, inst, *dest);
134142
}
135143
}
136-
BranchInfo::NotABranch => {}
144+
_ => {}
137145
}
138146
}
139147
}

‎cranelift/codegen/src/inst_predicates.rs‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//! Instruction predicates/properties, shared by various analyses.
22
use crate::ir::immediates::Offset32;
3-
use crate::ir::instructions::BranchInfo;
4-
use crate::ir::{Block, DataFlowGraph, Function, Inst, InstructionData, Opcode, Type, Value};
3+
use crate::ir::{self, Block, DataFlowGraph, Function, Inst, InstructionData, Opcode, Type, Value};
54
use cranelift_entity::EntityRef;
65

76
/// Preserve instructions with used result values.
@@ -176,16 +175,26 @@ pub(crate) fn visit_block_succs<F: FnMut(Inst, Block, bool)>(
176175
mut visit: F,
177176
) {
178177
if let Some(inst) = f.layout.last_inst(block) {
179-
match f.dfg.insts[inst].analyze_branch() {
180-
BranchInfo::NotABranch => {}
181-
BranchInfo::SingleDest(dest) => {
178+
match f.dfg.insts[inst] {
179+
ir::InstructionData::Jump {
180+
destination: dest, ..
181+
} => {
182182
visit(inst, dest.block(&f.dfg.value_lists), false);
183183
}
184-
BranchInfo::Conditional(block_then, block_else) => {
184+
185+
ir::InstructionData::Brif {
186+
blocks: [block_then, block_else],
187+
..
188+
} => {
185189
visit(inst, block_then.block(&f.dfg.value_lists), false);
186190
visit(inst, block_else.block(&f.dfg.value_lists), false);
187191
}
188-
BranchInfo::Table(table, dest) => {
192+
193+
ir::InstructionData::BranchTable {
194+
table,
195+
destination: dest,
196+
..
197+
} => {
189198
// The default block is reached via a direct conditional branch,
190199
// so it is not part of the table.
191200
visit(inst, dest, false);
@@ -194,6 +203,8 @@ pub(crate) fn visit_block_succs<F: FnMut(Inst, Block, bool)>(
194203
visit(inst, dest, true);
195204
}
196205
}
206+
207+
_ => {}
197208
}
198209
}
199210
}

‎cranelift/codegen/src/ir/dfg.rs‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::entity::{self, PrimaryMap, SecondaryMap};
44
use crate::ir;
55
use crate::ir::builder::ReplaceBuilder;
66
use crate::ir::dynamic_type::{DynamicTypeData, DynamicTypes};
7-
use crate::ir::instructions::{BranchInfo, CallInfo, InstructionData};
7+
use crate::ir::instructions::{CallInfo, InstructionData};
88
use crate::ir::{types, ConstantData, ConstantPool, Immediate};
99
use crate::ir::{
1010
Block, BlockCall, DynamicType, FuncRef, Inst, SigRef, Signature, Type, Value,
@@ -1035,11 +1035,6 @@ impl DataFlowGraph {
10351035
impl ExactSizeIterator for InstResultTypes<'_> {}
10361036
}
10371037

1038-
/// Check if `inst` is a branch.
1039-
pub fn analyze_branch(&self, inst: Inst) -> BranchInfo {
1040-
self.insts[inst].analyze_branch()
1041-
}
1042-
10431038
/// Compute the type of an instruction result from opcode constraints and call signatures.
10441039
///
10451040
/// This computes the same sequence of result types that `make_inst_results()` above would

‎cranelift/codegen/src/ir/entities.rs‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,6 @@ impl Value {
9292
/// the `Value` of such instructions, use [`inst_results`](super::DataFlowGraph::inst_results) or
9393
/// its analogue in `cranelift_frontend::FuncBuilder`.
9494
///
95-
/// If you look around the API, you can find many inventive uses for `Inst`,
96-
/// such as [annotating specific instructions with a comment][inst_comment]
97-
/// or [performing reflection at compile time](super::DataFlowGraph::analyze_branch)
98-
/// on the type of instruction.
99-
///
10095
/// [inst_comment]: https://github.com/bjorn3/rustc_codegen_cranelift/blob/0f8814fd6da3d436a90549d4bb19b94034f2b19c/src/pretty_clif.rs
10196
///
10297
/// While the order is stable, it is arbitrary and does not necessarily resemble the layout order.

‎cranelift/codegen/src/ir/function.rs‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,12 @@
44
//! instructions.
55
66
use crate::entity::{PrimaryMap, SecondaryMap};
7-
use crate::ir;
8-
use crate::ir::JumpTables;
97
use crate::ir::{
10-
instructions::BranchInfo, Block, DynamicStackSlot, DynamicStackSlotData, DynamicType,
11-
ExtFuncData, FuncRef, GlobalValue, GlobalValueData, Inst, InstructionData, JumpTable,
12-
JumpTableData, Opcode, SigRef, StackSlot, StackSlotData, Table, TableData, Type,
8+
self, Block, DataFlowGraph, DynamicStackSlot, DynamicStackSlotData, DynamicStackSlots,
9+
DynamicType, ExtFuncData, FuncRef, GlobalValue, GlobalValueData, Inst, InstructionData,
10+
JumpTable, JumpTableData, JumpTables, Layout, Opcode, SigRef, Signature, SourceLocs, StackSlot,
11+
StackSlotData, StackSlots, Table, TableData, Type,
1312
};
14-
use crate::ir::{DataFlowGraph, Layout, Signature};
15-
use crate::ir::{DynamicStackSlots, SourceLocs, StackSlots};
1613
use crate::isa::CallConv;
1714
use crate::value_label::ValueLabelsRanges;
1815
use crate::write::write_function;
@@ -280,16 +277,21 @@ impl FunctionStencil {
280277
/// Rewrite the branch destination to `new_dest` if the destination matches `old_dest`.
281278
/// Does nothing if called with a non-jump or non-branch instruction.
282279
pub fn rewrite_branch_destination(&mut self, inst: Inst, old_dest: Block, new_dest: Block) {
283-
match self.dfg.analyze_branch(inst) {
284-
BranchInfo::SingleDest(dest) => {
280+
match self.dfg.insts[inst] {
281+
InstructionData::Jump {
282+
destination: dest, ..
283+
} => {
285284
if dest.block(&self.dfg.value_lists) == old_dest {
286285
for block in self.dfg.insts[inst].branch_destination_mut() {
287286
block.set_block(new_dest, &mut self.dfg.value_lists)
288287
}
289288
}
290289
}
291290

292-
BranchInfo::Conditional(block_then, block_else) => {
291+
InstructionData::Brif {
292+
blocks: [block_then, block_else],
293+
..
294+
} => {
293295
if block_then.block(&self.dfg.value_lists) == old_dest {
294296
if let InstructionData::Brif {
295297
blocks: [block_then, _],
@@ -315,7 +317,11 @@ impl FunctionStencil {
315317
}
316318
}
317319

318-
BranchInfo::Table(table, default_dest) => {
320+
InstructionData::BranchTable {
321+
table,
322+
destination: default_dest,
323+
..
324+
} => {
319325
self.jump_tables[table].iter_mut().for_each(|entry| {
320326
if *entry == old_dest {
321327
*entry = new_dest;
@@ -335,7 +341,7 @@ impl FunctionStencil {
335341
}
336342
}
337343

338-
BranchInfo::NotABranch => {}
344+
_ => {}
339345
}
340346
}
341347

‎cranelift/codegen/src/ir/instructions.rs‎

Lines changed: 1 addition & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use crate::ir::{
2020
self,
2121
condcodes::{FloatCC, IntCC},
2222
trapcode::TrapCode,
23-
types, Block, FuncRef, JumpTable, MemFlags, SigRef, StackSlot, Type, Value,
23+
types, Block, FuncRef, MemFlags, SigRef, StackSlot, Type, Value,
2424
};
2525

2626
/// Some instructions use an external list of argument values because there is not enough space in
@@ -292,27 +292,6 @@ impl Default for VariableArgs {
292292
/// Avoid large matches on instruction formats by using the methods defined here to examine
293293
/// instructions.
294294
impl InstructionData {
295-
/// Return information about the destination of a branch or jump instruction.
296-
///
297-
/// Any instruction that can transfer control to another block reveals its possible destinations
298-
/// here.
299-
pub fn analyze_branch(&self) -> BranchInfo {
300-
match *self {
301-
Self::Jump { destination, .. } => BranchInfo::SingleDest(destination),
302-
Self::Brif {
303-
blocks: [block_then, block_else],
304-
..
305-
} => BranchInfo::Conditional(block_then, block_else),
306-
Self::BranchTable {
307-
table, destination, ..
308-
} => BranchInfo::Table(table, destination),
309-
_ => {
310-
debug_assert!(!self.opcode().is_branch());
311-
BranchInfo::NotABranch
312-
}
313-
}
314-
}
315-
316295
/// Get the destinations of this instruction, if it's a branch.
317296
///
318297
/// `br_table` returns the empty slice.
@@ -476,23 +455,6 @@ impl InstructionData {
476455
}
477456
}
478457

479-
/// Information about branch and jump instructions.
480-
pub enum BranchInfo {
481-
/// This is not a branch or jump instruction.
482-
/// This instruction will not transfer control to another block in the function, but it may still
483-
/// affect control flow by returning or trapping.
484-
NotABranch,
485-
486-
/// This is a branch or jump to a single destination block, possibly taking value arguments.
487-
SingleDest(BlockCall),
488-
489-
/// This is a conditional branch
490-
Conditional(BlockCall, BlockCall),
491-
492-
/// This is a jump table branch which can have many destination blocks and one default block.
493-
Table(JumpTable, Block),
494-
}
495-
496458
/// Information about call instructions.
497459
pub enum CallInfo<'a> {
498460
/// This is not a call instruction.

‎cranelift/codegen/src/machinst/lower.rs‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ use crate::entity::SecondaryMap;
99
use crate::fx::{FxHashMap, FxHashSet};
1010
use crate::inst_predicates::{has_lowering_side_effect, is_constant_64bit};
1111
use crate::ir::{
12-
instructions, ArgumentPurpose, Block, Constant, ConstantData, DataFlowGraph, ExternalName,
13-
Function, GlobalValue, GlobalValueData, Immediate, Inst, InstructionData, MemFlags, Opcode,
14-
RelSourceLoc, Type, Value, ValueDef, ValueLabelAssignments, ValueLabelStart,
12+
ArgumentPurpose, Block, Constant, ConstantData, DataFlowGraph, ExternalName, Function,
13+
GlobalValue, GlobalValueData, Immediate, Inst, InstructionData, MemFlags, Opcode, RelSourceLoc,
14+
Type, Value, ValueDef, ValueLabelAssignments, ValueLabelStart,
1515
};
1616
use crate::machinst::{
1717
writable_value_regs, BlockIndex, BlockLoweringOrder, Callee, LoweredBlock, MachLabel, Reg,
@@ -943,25 +943,27 @@ impl<'func, I: VCodeInst> Lower<'func, I> {
943943
let (inst, succ) = self.vcode.block_order().succ_indices(block)[succ_idx];
944944

945945
// Get branch args and convert to Regs.
946-
let branch_args = match self.f.dfg.analyze_branch(inst) {
947-
instructions::BranchInfo::NotABranch => unreachable!(),
948-
instructions::BranchInfo::SingleDest(block) => {
949-
block.args_slice(&self.f.dfg.value_lists)
950-
}
951-
instructions::BranchInfo::Conditional(then_block, else_block) => {
946+
let branch_args = match self.f.dfg.insts[inst] {
947+
InstructionData::Jump {
948+
destination: block, ..
949+
} => block.args_slice(&self.f.dfg.value_lists),
950+
InstructionData::Brif {
951+
blocks: [then_block, else_block],
952+
..
953+
} => {
952954
// NOTE: `succ_idx == 0` implying that we're traversing the `then_block` is
953955
// enforced by the traversal order defined in `visit_block_succs`. Eventually
954-
// we should traverse the `branch_destination` slice instead of the result of
955-
// analyze_branch there, which would simplify computing the branch args
956-
// significantly.
956+
// we should traverse the `branch_destination` slice there, which would
957+
// simplify computing the branch args significantly.
957958
if succ_idx == 0 {
958959
then_block.args_slice(&self.f.dfg.value_lists)
959960
} else {
960961
assert!(succ_idx == 1);
961962
else_block.args_slice(&self.f.dfg.value_lists)
962963
}
963964
}
964-
instructions::BranchInfo::Table(_, _) => &[],
965+
InstructionData::BranchTable { .. } => &[],
966+
_ => unreachable!(),
965967
};
966968
let mut branch_arg_vregs: SmallVec<[Reg; 16]> = smallvec![];
967969
for &arg in branch_args {

0 commit comments

Comments
 (0)