From 2c8425998bac42a18d89d088658b2db889a3a982 Mon Sep 17 00:00:00 2001 From: Trevor Elliott Date: Tue, 7 Feb 2023 13:29:42 -0800 Subject: [PATCH] Refactor matches that used to consume BranchInfo (#5734) Explicitly borrow the instruction data, and use a mutable borrow to avoid rematch. --- cranelift/codegen/src/dominator_tree.rs | 6 ++-- cranelift/codegen/src/flowgraph.rs | 6 ++-- cranelift/codegen/src/inst_predicates.rs | 6 ++-- cranelift/codegen/src/ir/function.rs | 40 +++++------------------- cranelift/codegen/src/machinst/lower.rs | 2 +- cranelift/codegen/src/verifier/mod.rs | 6 ++-- cranelift/frontend/src/frontend.rs | 6 ++-- cranelift/frontend/src/ssa.rs | 2 +- 8 files changed, 24 insertions(+), 50 deletions(-) diff --git a/cranelift/codegen/src/dominator_tree.rs b/cranelift/codegen/src/dominator_tree.rs index d70db21971..4f04b14ca1 100644 --- a/cranelift/codegen/src/dominator_tree.rs +++ b/cranelift/codegen/src/dominator_tree.rs @@ -351,7 +351,7 @@ 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.insts[inst] { + match &func.dfg.insts[inst] { ir::InstructionData::Jump { destination: succ, .. } => self.push_if_unseen(succ.block(&func.dfg.value_lists)), @@ -367,10 +367,10 @@ impl DominatorTree { destination: dest, .. } => { - for succ in func.jump_tables[jt].iter() { + for succ in func.jump_tables[*jt].iter() { self.push_if_unseen(*succ); } - self.push_if_unseen(dest); + self.push_if_unseen(*dest); } _ => {} } diff --git a/cranelift/codegen/src/flowgraph.rs b/cranelift/codegen/src/flowgraph.rs index 9b04a5abc6..5a5bd40aaf 100644 --- a/cranelift/codegen/src/flowgraph.rs +++ b/cranelift/codegen/src/flowgraph.rs @@ -117,7 +117,7 @@ impl ControlFlowGraph { fn compute_block(&mut self, func: &Function, block: Block) { if let Some(inst) = func.layout.last_inst(block) { - match func.dfg.insts[inst] { + match &func.dfg.insts[inst] { ir::InstructionData::Jump { destination: dest, .. } => { @@ -135,9 +135,9 @@ impl ControlFlowGraph { destination: dest, .. } => { - self.add_edge(block, inst, dest); + self.add_edge(block, inst, *dest); - for dest in func.jump_tables[jt].iter() { + for dest in func.jump_tables[*jt].iter() { self.add_edge(block, inst, *dest); } } diff --git a/cranelift/codegen/src/inst_predicates.rs b/cranelift/codegen/src/inst_predicates.rs index 9e9280675f..99ed3b6bdd 100644 --- a/cranelift/codegen/src/inst_predicates.rs +++ b/cranelift/codegen/src/inst_predicates.rs @@ -175,7 +175,7 @@ pub(crate) fn visit_block_succs( mut visit: F, ) { if let Some(inst) = f.layout.last_inst(block) { - match f.dfg.insts[inst] { + match &f.dfg.insts[inst] { ir::InstructionData::Jump { destination: dest, .. } => { @@ -197,9 +197,9 @@ pub(crate) fn visit_block_succs( } => { // The default block is reached via a direct conditional branch, // so it is not part of the table. - visit(inst, dest, false); + visit(inst, *dest, false); - for &dest in f.jump_tables[table].as_slice() { + for &dest in f.jump_tables[*table].as_slice() { visit(inst, dest, true); } } diff --git a/cranelift/codegen/src/ir/function.rs b/cranelift/codegen/src/ir/function.rs index b0a14d426a..4fbef01fc4 100644 --- a/cranelift/codegen/src/ir/function.rs +++ b/cranelift/codegen/src/ir/function.rs @@ -277,14 +277,12 @@ 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.insts[inst] { + match &mut 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) - } + dest.set_block(new_dest, &mut self.dfg.value_lists) } } @@ -293,27 +291,11 @@ impl FunctionStencil { .. } => { if block_then.block(&self.dfg.value_lists) == old_dest { - if let InstructionData::Brif { - blocks: [block_then, _], - .. - } = &mut self.dfg.insts[inst] - { - block_then.set_block(new_dest, &mut self.dfg.value_lists); - } else { - unreachable!(); - } + block_then.set_block(new_dest, &mut self.dfg.value_lists); } if block_else.block(&self.dfg.value_lists) == old_dest { - if let InstructionData::Brif { - blocks: [_, block_else], - .. - } = &mut self.dfg.insts[inst] - { - block_else.set_block(new_dest, &mut self.dfg.value_lists); - } else { - unreachable!(); - } + block_else.set_block(new_dest, &mut self.dfg.value_lists); } } @@ -322,22 +304,14 @@ impl FunctionStencil { destination: default_dest, .. } => { - self.jump_tables[table].iter_mut().for_each(|entry| { + self.jump_tables[*table].iter_mut().for_each(|entry| { if *entry == old_dest { *entry = new_dest; } }); - if default_dest == old_dest { - match &mut self.dfg.insts[inst] { - InstructionData::BranchTable { destination, .. } => { - *destination = new_dest; - } - _ => panic!( - "Unexpected instruction {} having default destination", - self.dfg.display_inst(inst) - ), - } + if *default_dest == old_dest { + *default_dest = new_dest; } } diff --git a/cranelift/codegen/src/machinst/lower.rs b/cranelift/codegen/src/machinst/lower.rs index 250bfa0783..636d2abe21 100644 --- a/cranelift/codegen/src/machinst/lower.rs +++ b/cranelift/codegen/src/machinst/lower.rs @@ -943,7 +943,7 @@ 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.insts[inst] { + let branch_args = match &self.f.dfg.insts[inst] { InstructionData::Jump { destination: block, .. } => block.args_slice(&self.f.dfg.value_lists), diff --git a/cranelift/codegen/src/verifier/mod.rs b/cranelift/codegen/src/verifier/mod.rs index 523e32d7f3..d4c95ca376 100644 --- a/cranelift/codegen/src/verifier/mod.rs +++ b/cranelift/codegen/src/verifier/mod.rs @@ -1293,7 +1293,7 @@ impl<'a> Verifier<'a> { inst: Inst, errors: &mut VerifierErrors, ) -> VerifierStepResult<()> { - match self.func.dfg.insts[inst] { + match &self.func.dfg.insts[inst] { ir::InstructionData::Jump { destination: block, .. } => { @@ -1333,7 +1333,7 @@ impl<'a> Verifier<'a> { destination: block, .. } => { - let arg_count = self.func.dfg.num_block_params(block); + let arg_count = self.func.dfg.num_block_params(*block); if arg_count != 0 { return errors.nonfatal(( inst, @@ -1344,7 +1344,7 @@ impl<'a> Verifier<'a> { ), )); } - for block in self.func.jump_tables[table].iter() { + for block in self.func.jump_tables[*table].iter() { let arg_count = self.func.dfg.num_block_params(*block); if arg_count != 0 { return errors.nonfatal(( diff --git a/cranelift/frontend/src/frontend.rs b/cranelift/frontend/src/frontend.rs index a0b5e2e295..a260913af1 100644 --- a/cranelift/frontend/src/frontend.rs +++ b/cranelift/frontend/src/frontend.rs @@ -106,7 +106,7 @@ impl<'short, 'long> InstBuilderBase<'short> for FuncInstBuilder<'short, 'long> { self.builder.func.set_srcloc(inst, self.builder.srcloc); } - match self.builder.func.dfg.insts[inst] { + match &self.builder.func.dfg.insts[inst] { ir::InstructionData::Jump { destination: dest, .. } => { @@ -140,7 +140,7 @@ impl<'short, 'long> InstBuilderBase<'short> for FuncInstBuilder<'short, 'long> { .builder .func .jump_tables - .get(table) + .get(*table) .expect("you are referencing an undeclared jump table") .iter() .filter(|&dest_block| unique.insert(*dest_block)) @@ -152,7 +152,7 @@ impl<'short, 'long> InstBuilderBase<'short> for FuncInstBuilder<'short, 'long> { .ssa .declare_block_predecessor(*dest_block, inst); } - self.builder.declare_successor(destination, inst); + self.builder.declare_successor(*destination, inst); } _ => {} diff --git a/cranelift/frontend/src/ssa.rs b/cranelift/frontend/src/ssa.rs index 0623ddc743..dd9676bd66 100644 --- a/cranelift/frontend/src/ssa.rs +++ b/cranelift/frontend/src/ssa.rs @@ -577,7 +577,7 @@ impl SSABuilder { dest_block: Block, val: Value, ) -> Option<(Block, Inst)> { - match func.dfg.insts[branch] { + match &func.dfg.insts[branch] { // For a single destination appending a jump argument to the instruction // is sufficient. InstructionData::Jump { .. } => {