From 7cea73a81da4ac86b1188676a89b4d99a8349d04 Mon Sep 17 00:00:00 2001 From: Trevor Elliott Date: Wed, 18 Jan 2023 17:17:03 -0800 Subject: [PATCH] Refactor BranchInfo::Table to no longer have an optional default branch (#5593) --- cranelift/codegen/src/dominator_tree.rs | 4 +--- cranelift/codegen/src/flowgraph.rs | 5 ++--- cranelift/codegen/src/inst_predicates.rs | 11 +++++------ cranelift/codegen/src/ir/function.rs | 2 +- cranelift/codegen/src/ir/instructions.rs | 6 +++--- cranelift/codegen/src/verifier/mod.rs | 22 ++++++++++------------ 6 files changed, 22 insertions(+), 28 deletions(-) diff --git a/cranelift/codegen/src/dominator_tree.rs b/cranelift/codegen/src/dominator_tree.rs index 175dad5f8037..0d1200db7b61 100644 --- a/cranelift/codegen/src/dominator_tree.rs +++ b/cranelift/codegen/src/dominator_tree.rs @@ -360,9 +360,7 @@ impl DominatorTree { for succ in func.jump_tables[jt].iter() { self.push_if_unseen(*succ); } - if let Some(dest) = dest { - self.push_if_unseen(dest); - } + self.push_if_unseen(dest); } BranchInfo::NotABranch => {} } diff --git a/cranelift/codegen/src/flowgraph.rs b/cranelift/codegen/src/flowgraph.rs index 4fd06e3c6ad2..d5e61fca78d4 100644 --- a/cranelift/codegen/src/flowgraph.rs +++ b/cranelift/codegen/src/flowgraph.rs @@ -126,9 +126,8 @@ impl ControlFlowGraph { self.add_edge(block, inst, dest.block(&func.dfg.value_lists)); } BranchInfo::Table(jt, dest) => { - if let Some(dest) = dest { - self.add_edge(block, inst, dest); - } + self.add_edge(block, inst, dest); + 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 f5e4905769c0..8eea3e383a12 100644 --- a/cranelift/codegen/src/inst_predicates.rs +++ b/cranelift/codegen/src/inst_predicates.rs @@ -169,12 +169,11 @@ fn visit_branch_targets(f: &Function, inst: Inst, v BranchInfo::SingleDest(dest) => { visit(inst, dest.block(&f.dfg.value_lists), false); } - BranchInfo::Table(table, maybe_dest) => { - if let Some(dest) = maybe_dest { - // The default block is reached via a direct conditional branch, - // so it is not part of the table. - visit(inst, dest, false); - } + BranchInfo::Table(table, dest) => { + // The default block is reached via a direct conditional branch, + // so it is not part of the table. + visit(inst, dest, false); + 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 d315b58cb947..bf1e2fa75be3 100644 --- a/cranelift/codegen/src/ir/function.rs +++ b/cranelift/codegen/src/ir/function.rs @@ -308,7 +308,7 @@ impl FunctionStencil { } }); - if default_dest == Some(old_dest) { + if default_dest == old_dest { match &mut self.dfg.insts[inst] { InstructionData::BranchTable { destination, .. } => { *destination = new_dest; diff --git a/cranelift/codegen/src/ir/instructions.rs b/cranelift/codegen/src/ir/instructions.rs index a16ef5848d8b..930160352550 100644 --- a/cranelift/codegen/src/ir/instructions.rs +++ b/cranelift/codegen/src/ir/instructions.rs @@ -273,7 +273,7 @@ impl InstructionData { Self::Branch { destination, .. } => BranchInfo::SingleDest(destination), Self::BranchTable { table, destination, .. - } => BranchInfo::Table(table, Some(destination)), + } => BranchInfo::Table(table, destination), _ => { debug_assert!(!self.opcode().is_branch()); BranchInfo::NotABranch @@ -456,8 +456,8 @@ pub enum BranchInfo { /// This is a branch or jump to a single destination block, possibly taking value arguments. SingleDest(BlockCall), - /// This is a jump table branch which can have many destination blocks and maybe one default block. - Table(JumpTable, Option), + /// This is a jump table branch which can have many destination blocks and one default block. + Table(JumpTable, Block), } /// Information about call instructions. diff --git a/cranelift/codegen/src/verifier/mod.rs b/cranelift/codegen/src/verifier/mod.rs index ec790faaa581..396f3ff865ff 100644 --- a/cranelift/codegen/src/verifier/mod.rs +++ b/cranelift/codegen/src/verifier/mod.rs @@ -1304,18 +1304,16 @@ impl<'a> Verifier<'a> { self.typecheck_variable_args_iterator(inst, iter, args, errors)?; } BranchInfo::Table(table, block) => { - if let Some(block) = block { - let arg_count = self.func.dfg.num_block_params(block); - if arg_count != 0 { - return errors.nonfatal(( - inst, - self.context(inst), - format!( - "takes no arguments, but had target {} with {} arguments", - block, arg_count, - ), - )); - } + let arg_count = self.func.dfg.num_block_params(block); + if arg_count != 0 { + return errors.nonfatal(( + inst, + self.context(inst), + format!( + "takes no arguments, but had target {} with {} arguments", + block, arg_count, + ), + )); } for block in self.func.jump_tables[table].iter() { let arg_count = self.func.dfg.num_block_params(*block);