diff --git a/cranelift/codegen/meta/src/cdsl/formats.rs b/cranelift/codegen/meta/src/cdsl/formats.rs index 876fb7702f..10d804a98a 100644 --- a/cranelift/codegen/meta/src/cdsl/formats.rs +++ b/cranelift/codegen/meta/src/cdsl/formats.rs @@ -38,6 +38,8 @@ pub(crate) struct InstructionFormat { pub imm_fields: Vec, + pub num_block_operands: usize, + /// Index of the value input operand that is used to infer the controlling type variable. By /// default, this is `0`, the first `value` operand. The index is relative to the values only, /// ignoring immediate operands. @@ -49,6 +51,7 @@ pub(crate) struct InstructionFormat { pub(crate) struct FormatStructure { pub num_value_operands: usize, pub has_value_list: bool, + pub num_block_operands: usize, /// Tuples of (Rust field name / Rust type) for each immediate field. pub imm_field_names: Vec<(&'static str, &'static str)>, } @@ -62,8 +65,8 @@ impl fmt::Display for InstructionFormat { .collect::>() .join(", "); fmt.write_fmt(format_args!( - "{}(imms=({}), vals={})", - self.name, imm_args, self.num_value_operands + "{}(imms=({}), vals={}, blocks={})", + self.name, imm_args, self.num_value_operands, self.num_block_operands, ))?; Ok(()) } @@ -75,6 +78,7 @@ impl InstructionFormat { FormatStructure { num_value_operands: self.num_value_operands, has_value_list: self.has_value_list, + num_block_operands: self.num_block_operands, imm_field_names: self .imm_fields .iter() @@ -92,6 +96,7 @@ impl InstructionFormatBuilder { name, num_value_operands: 0, has_value_list: false, + num_block_operands: 0, imm_fields: Vec::new(), typevar_operand: None, }) @@ -107,6 +112,11 @@ impl InstructionFormatBuilder { self } + pub fn block(mut self) -> Self { + self.0.num_block_operands += 1; + self + } + pub fn imm(mut self, operand_kind: &OperandKind) -> Self { let field = FormatField { kind: operand_kind.clone(), diff --git a/cranelift/codegen/meta/src/gen_inst.rs b/cranelift/codegen/meta/src/gen_inst.rs index 85bee66abe..25b413c7d2 100644 --- a/cranelift/codegen/meta/src/gen_inst.rs +++ b/cranelift/codegen/meta/src/gen_inst.rs @@ -82,6 +82,18 @@ fn gen_instruction_data(formats: &[&InstructionFormat], fmt: &mut Formatter) { } else if format.num_value_operands > 0 { fmtln!(fmt, "args: [Value; {}],", format.num_value_operands); } + + match format.num_block_operands { + 0 => (), + 1 => fmt.line("destination: ir::BlockCall,"), + 2 => fmtln!( + fmt, + "blocks: [ir::BlockCall; {}],", + format.num_block_operands + ), + n => panic!("Too many block operands in instruction: {}", n), + } + for field in &format.imm_fields { fmtln!(fmt, "{}: {},", field.member, field.kind.rust_type); } @@ -248,6 +260,18 @@ fn gen_instruction_data_impl(formats: &[&InstructionFormat], fmt: &mut Formatter None }; + let blocks_eq = match format.num_block_operands { + 0 => None, + 1 => { + members.push("destination"); + Some("destination1 == destination2") + }, + _ => { + members.push("blocks"); + Some("blocks1.iter().zip(blocks2.iter()).all(|(a, b)| a.block(pool) == b.block(pool))") + } + }; + for field in &format.imm_fields { members.push(field.member); } @@ -263,6 +287,9 @@ fn gen_instruction_data_impl(formats: &[&InstructionFormat], fmt: &mut Formatter if let Some(args_eq) = args_eq { fmtln!(fmt, "&& {}", args_eq); } + if let Some(blocks_eq) = blocks_eq { + fmtln!(fmt, "&& {}", blocks_eq); + } }); fmtln!(fmt, "}"); } @@ -304,6 +331,18 @@ fn gen_instruction_data_impl(formats: &[&InstructionFormat], fmt: &mut Formatter ("&[]", "0") }; + let blocks = match format.num_block_operands { + 0 => None, + 1 => { + members.push("ref destination"); + Some(("std::slice::from_ref(destination)", "1")) + } + _ => { + members.push("ref blocks"); + Some(("blocks", "blocks.len()")) + } + }; + for field in &format.imm_fields { members.push(field.member); } @@ -323,6 +362,21 @@ fn gen_instruction_data_impl(formats: &[&InstructionFormat], fmt: &mut Formatter fmtln!(fmt, "::core::hash::Hash::hash(&arg, state);"); }); fmtln!(fmt, "}"); + + if let Some((blocks, len)) = blocks { + fmtln!(fmt, "::core::hash::Hash::hash(&{}, state);", len); + fmtln!(fmt, "for &block in {} {{", blocks); + fmt.indent(|fmt| { + fmtln!(fmt, "::core::hash::Hash::hash(&block.block(pool), state);"); + fmtln!(fmt, "for &arg in block.args_slice(pool) {"); + fmt.indent(|fmt| { + fmtln!(fmt, "let arg = mapper(arg);"); + fmtln!(fmt, "::core::hash::Hash::hash(&arg, state);"); + }); + fmtln!(fmt, "}"); + }); + fmtln!(fmt, "}"); + } }); fmtln!(fmt, "}"); } @@ -774,6 +828,19 @@ fn gen_member_inits(format: &InstructionFormat, fmt: &mut Formatter) { } fmtln!(fmt, "args: [{}],", args.join(", ")); } + + // Block operands + match format.num_block_operands { + 0 => (), + 1 => fmt.line("destination: block0"), + n => { + let mut blocks = Vec::new(); + for i in 0..n { + blocks.push(format!("block{}", i)); + } + fmtln!(fmt, "blocks: [{}],", blocks.join(", ")); + } + } } /// Emit a method for creating and inserting an instruction format. @@ -793,6 +860,9 @@ fn gen_format_constructor(format: &InstructionFormat, fmt: &mut Formatter) { args.push(format!("{}: {}", f.member, f.kind.rust_type)); } + // Then the block operands. + args.extend((0..format.num_block_operands).map(|i| format!("block{}: ir::BlockCall", i))); + // Then the value operands. if format.has_value_list { // Take all value arguments as a finished value list. The value lists @@ -1152,6 +1222,41 @@ fn gen_common_isle( fmt.empty_line(); } + // Generate all of the block arrays we need for `InstructionData` as well as + // the constructors and extractors for them. + fmt.line(";;;; Block Arrays ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;"); + fmt.empty_line(); + let block_array_arities: BTreeSet<_> = formats + .iter() + .filter(|f| f.num_block_operands > 1) + .map(|f| f.num_block_operands) + .collect(); + for n in block_array_arities { + fmtln!(fmt, ";; ISLE representation of `[BlockCall; {}]`.", n); + fmtln!(fmt, "(type BlockArray{} extern (enum))", n); + fmt.empty_line(); + + fmtln!( + fmt, + "(decl block_array_{0} ({1}) BlockArray{0})", + n, + (0..n).map(|_| "BlockCall").collect::>().join(" ") + ); + + fmtln!( + fmt, + "(extern constructor block_array_{0} pack_block_array_{0})", + n + ); + + fmtln!( + fmt, + "(extern extractor infallible block_array_{0} unpack_block_array_{0})", + n + ); + fmt.empty_line(); + } + // Generate the extern type declaration for `Opcode`. fmt.line(";;;; `Opcode` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;"); fmt.empty_line(); @@ -1187,6 +1292,13 @@ fn gen_common_isle( } else if format.num_value_operands > 1 { write!(&mut s, " (args ValueArray{})", format.num_value_operands).unwrap(); } + + match format.num_block_operands { + 0 => (), + 1 => write!(&mut s, " (destination BlockCall)").unwrap(), + n => write!(&mut s, " (blocks BlockArray{})", n).unwrap(), + } + for field in &format.imm_fields { write!( &mut s, @@ -1328,13 +1440,37 @@ fn gen_common_isle( let imm_operands: Vec<_> = inst .operands_in .iter() - .filter(|o| !o.is_value() && !o.is_varargs()) + .filter(|o| !o.is_value() && !o.is_varargs() && !o.kind.is_block()) .collect(); - assert_eq!(imm_operands.len(), inst.format.imm_fields.len()); + assert_eq!(imm_operands.len(), inst.format.imm_fields.len(),); for op in imm_operands { write!(&mut s, " {}", op.name).unwrap(); } + // Blocks. + let block_operands: Vec<_> = inst + .operands_in + .iter() + .filter(|o| o.kind.is_block()) + .collect(); + assert_eq!(block_operands.len(), inst.format.num_block_operands); + assert!(block_operands.len() <= 2); + + if !block_operands.is_empty() { + if block_operands.len() == 1 { + write!(&mut s, " {}", block_operands[0].name).unwrap(); + } else { + let blocks: Vec<_> = block_operands.iter().map(|o| o.name).collect(); + let blocks = blocks.join(" "); + write!( + &mut s, + " (block_array_{} {})", + inst.format.num_block_operands, blocks, + ) + .unwrap(); + } + } + s.push_str("))"); fmt.line(&s); }); @@ -1400,11 +1536,31 @@ fn gen_common_isle( .unwrap(); } + if inst.format.num_block_operands > 0 { + let blocks: Vec<_> = inst + .operands_in + .iter() + .filter(|o| o.kind.is_block()) + .map(|o| o.name) + .collect(); + if inst.format.num_block_operands == 1 { + write!(&mut s, " {}", blocks.first().unwrap(),).unwrap(); + } else { + write!( + &mut s, + " (block_array_{} {})", + inst.format.num_block_operands, + blocks.join(" ") + ) + .unwrap(); + } + } + // Immediates (non-value args). for o in inst .operands_in .iter() - .filter(|o| !o.is_value() && !o.is_varargs()) + .filter(|o| !o.is_value() && !o.is_varargs() && !o.kind.is_block()) { write!(&mut s, " {}", o.name).unwrap(); } diff --git a/cranelift/codegen/meta/src/shared/entities.rs b/cranelift/codegen/meta/src/shared/entities.rs index fdad737c7d..e713bc92ec 100644 --- a/cranelift/codegen/meta/src/shared/entities.rs +++ b/cranelift/codegen/meta/src/shared/entities.rs @@ -19,6 +19,14 @@ pub(crate) struct EntityRefs { /// This is primarily used in control flow instructions. pub(crate) label: OperandKind, + /// A reference to a basic block in the same function, with its arguments provided. + /// This is primarily used in control flow instructions. + pub(crate) block_then: OperandKind, + + /// A reference to a basic block in the same function, with its arguments provided. + /// This is primarily used in control flow instructions. + pub(crate) block_else: OperandKind, + /// A reference to a stack slot declared in the function preamble. pub(crate) stack_slot: OperandKind, @@ -61,6 +69,18 @@ impl EntityRefs { "a basic block in the same function.", ), + block_then: new( + "block_then", + "ir::BlockCall", + "a basic block in the same function, with its arguments provided.", + ), + + block_else: new( + "block_else", + "ir::BlockCall", + "a basic block in the same function, with its arguments provided.", + ), + stack_slot: new("stack_slot", "ir::StackSlot", "A stack slot"), dynamic_stack_slot: new( diff --git a/cranelift/codegen/meta/src/shared/formats.rs b/cranelift/codegen/meta/src/shared/formats.rs index c017836091..c7c70bcf23 100644 --- a/cranelift/codegen/meta/src/shared/formats.rs +++ b/cranelift/codegen/meta/src/shared/formats.rs @@ -8,6 +8,7 @@ pub(crate) struct Formats { pub(crate) binary: Rc, pub(crate) binary_imm8: Rc, pub(crate) binary_imm64: Rc, + pub(crate) brif: Rc, pub(crate) branch: Rc, pub(crate) branch_table: Rc, pub(crate) call: Rc, @@ -111,12 +112,11 @@ impl Formats { .value() .build(), - jump: Builder::new("Jump").imm(&entities.block_call).build(), + jump: Builder::new("Jump").block().build(), - branch: Builder::new("Branch") - .value() - .imm(&entities.block_call) - .build(), + branch: Builder::new("Branch").value().block().build(), + + brif: Builder::new("Brif").value().block().block().build(), branch_table: Builder::new("BranchTable") .value() diff --git a/cranelift/codegen/meta/src/shared/instructions.rs b/cranelift/codegen/meta/src/shared/instructions.rs index 6bc318b12e..a96f84861f 100755 --- a/cranelift/codegen/meta/src/shared/instructions.rs +++ b/cranelift/codegen/meta/src/shared/instructions.rs @@ -44,6 +44,27 @@ fn define_control_flow( TypeSetBuilder::new().ints(Interval::All).build(), ); + { + let c = &Operand::new("c", ScalarTruthy).with_doc("Controlling value to test"); + let block_then = &Operand::new("block_then", &entities.block_then).with_doc("Then block"); + let block_else = &Operand::new("block_else", &entities.block_else).with_doc("Else block"); + + ig.push( + Inst::new( + "brif", + r#" + Conditional branch when cond is non-zero. + + Take the ``then`` branch when ``c != 0``, and the ``else`` branch otherwise. + "#, + &formats.brif, + ) + .operands_in(vec![c, block_then, block_else]) + .is_terminator(true) + .is_branch(true), + ); + } + { let c = &Operand::new("c", ScalarTruthy).with_doc("Controlling value to test"); diff --git a/cranelift/codegen/src/dominator_tree.rs b/cranelift/codegen/src/dominator_tree.rs index 0d1200db7b..9c2e6e3627 100644 --- a/cranelift/codegen/src/dominator_tree.rs +++ b/cranelift/codegen/src/dominator_tree.rs @@ -356,6 +356,10 @@ impl DominatorTree { BranchInfo::SingleDest(succ) => { self.push_if_unseen(succ.block(&func.dfg.value_lists)) } + BranchInfo::Conditional(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) => { for succ in func.jump_tables[jt].iter() { self.push_if_unseen(*succ); diff --git a/cranelift/codegen/src/flowgraph.rs b/cranelift/codegen/src/flowgraph.rs index d5e61fca78..1c6c4fc563 100644 --- a/cranelift/codegen/src/flowgraph.rs +++ b/cranelift/codegen/src/flowgraph.rs @@ -125,6 +125,10 @@ impl ControlFlowGraph { BranchInfo::SingleDest(dest) => { self.add_edge(block, inst, dest.block(&func.dfg.value_lists)); } + BranchInfo::Conditional(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) => { self.add_edge(block, inst, dest); diff --git a/cranelift/codegen/src/inst_predicates.rs b/cranelift/codegen/src/inst_predicates.rs index 3c28899bd3..d18e4b80b4 100644 --- a/cranelift/codegen/src/inst_predicates.rs +++ b/cranelift/codegen/src/inst_predicates.rs @@ -176,25 +176,23 @@ pub(crate) fn visit_block_succs( mut visit: F, ) { for inst in f.layout.block_likely_branches(block) { - if f.dfg.insts[inst].opcode().is_branch() { - visit_branch_targets(f, inst, &mut visit); - } - } -} + match f.dfg.insts[inst].analyze_branch() { + BranchInfo::NotABranch => {} + BranchInfo::SingleDest(dest) => { + visit(inst, dest.block(&f.dfg.value_lists), false); + } + BranchInfo::Conditional(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) => { + // The default block is reached via a direct conditional branch, + // so it is not part of the table. + visit(inst, dest, false); -fn visit_branch_targets(f: &Function, inst: Inst, visit: &mut F) { - match f.dfg.insts[inst].analyze_branch() { - BranchInfo::NotABranch => {} - BranchInfo::SingleDest(dest) => { - visit(inst, dest.block(&f.dfg.value_lists), 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); + for &dest in f.jump_tables[table].as_slice() { + visit(inst, dest, true); + } } } } diff --git a/cranelift/codegen/src/ir/dfg.rs b/cranelift/codegen/src/ir/dfg.rs index 74058938bb..49790bda1c 100644 --- a/cranelift/codegen/src/ir/dfg.rs +++ b/cranelift/codegen/src/ir/dfg.rs @@ -691,9 +691,10 @@ impl DataFlowGraph { self.inst_args_mut(inst)[i] = body(self, arg); } - for mut block in self.insts[inst].branch_destination().into_iter() { + for block_ix in 0..self.insts[inst].branch_destination().len() { // We aren't changing the size of the args list, so we won't need to write the branch // back to the instruction. + let mut block = self.insts[inst].branch_destination()[block_ix]; for i in 0..block.args_slice(&self.value_lists).len() { let arg = block.args_slice(&self.value_lists)[i]; block.args_slice_mut(&mut self.value_lists)[i] = body(self, arg); @@ -712,7 +713,8 @@ impl DataFlowGraph { *arg = values.next().unwrap(); } - for mut block in self.insts[inst].branch_destination().into_iter() { + for block_ix in 0..self.insts[inst].branch_destination().len() { + let mut block = self.insts[inst].branch_destination()[block_ix]; for arg in block.args_slice_mut(&mut self.value_lists) { *arg = values.next().unwrap(); } diff --git a/cranelift/codegen/src/ir/function.rs b/cranelift/codegen/src/ir/function.rs index bf1e2fa75b..5414831076 100644 --- a/cranelift/codegen/src/ir/function.rs +++ b/cranelift/codegen/src/ir/function.rs @@ -277,27 +277,41 @@ impl FunctionStencil { self.dfg.collect_debug_info(); } - /// Changes the destination of a jump or branch instruction. - /// Does nothing if called with a non-jump or non-branch instruction. - /// - /// Note that this method ignores multi-destination branches like `br_table`. - pub fn change_branch_destination(&mut self, inst: Inst, new_dest: Block) { - match self.dfg.insts[inst].branch_destination_mut() { - None => (), - Some(inst_dest) => inst_dest.set_block(new_dest, &mut self.dfg.value_lists), - } - } - /// 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. - /// - /// Unlike [change_branch_destination](FunctionStencil::change_branch_destination), this method - /// rewrite the destinations of multi-destination branches like `br_table`. pub fn rewrite_branch_destination(&mut self, inst: Inst, old_dest: Block, new_dest: Block) { match self.dfg.analyze_branch(inst) { BranchInfo::SingleDest(dest) => { if dest.block(&self.dfg.value_lists) == old_dest { - self.change_branch_destination(inst, new_dest); + for block in self.dfg.insts[inst].branch_destination_mut() { + block.set_block(new_dest, &mut self.dfg.value_lists) + } + } + } + + BranchInfo::Conditional(block_then, block_else) => { + 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!(); + } + } + + 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!(); + } } } diff --git a/cranelift/codegen/src/ir/instructions.rs b/cranelift/codegen/src/ir/instructions.rs index 9301603525..e805aaca94 100644 --- a/cranelift/codegen/src/ir/instructions.rs +++ b/cranelift/codegen/src/ir/instructions.rs @@ -271,6 +271,10 @@ impl InstructionData { match *self { Self::Jump { destination, .. } => BranchInfo::SingleDest(destination), Self::Branch { 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), @@ -281,27 +285,31 @@ impl InstructionData { } } - /// Get the single destination of this branch instruction, if it is a single destination - /// branch or jump. + /// Get the destinations of this instruction, if it's a branch. /// - /// Multi-destination branches like `br_table` return `None`. - pub fn branch_destination(&self) -> Option { - match *self { - Self::Jump { destination, .. } | Self::Branch { destination, .. } => Some(destination), - Self::BranchTable { .. } => None, + /// `br_table` returns the empty slice. + pub fn branch_destination(&self) -> &[BlockCall] { + match self { + Self::Jump { + ref destination, .. + } + | Self::Branch { + ref destination, .. + } => std::slice::from_ref(destination), + Self::Brif { blocks, .. } => blocks, + Self::BranchTable { .. } => &[], _ => { debug_assert!(!self.opcode().is_branch()); - None + &[] } } } - /// Get a mutable reference to the single destination of this branch instruction, if it is a - /// single destination branch or jump. + /// Get a mutable slice of the destinations of this instruction, if it's a branch. /// - /// Multi-destination branches like `br_table` return `None`. - pub fn branch_destination_mut(&mut self) -> Option<&mut BlockCall> { - match *self { + /// `br_table` returns the empty slice. + pub fn branch_destination_mut(&mut self) -> &mut [BlockCall] { + match self { Self::Jump { ref mut destination, .. @@ -309,11 +317,12 @@ impl InstructionData { | Self::Branch { ref mut destination, .. - } => Some(destination), - Self::BranchTable { .. } => None, + } => std::slice::from_mut(destination), + Self::Brif { blocks, .. } => blocks, + Self::BranchTable { .. } => &mut [], _ => { debug_assert!(!self.opcode().is_branch()); - None + &mut [] } } } @@ -456,6 +465,9 @@ pub enum BranchInfo { /// 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), } diff --git a/cranelift/codegen/src/isa/aarch64/lower.isle b/cranelift/codegen/src/isa/aarch64/lower.isle index 2f2a2ecfa3..14b5c04044 100644 --- a/cranelift/codegen/src/isa/aarch64/lower.isle +++ b/cranelift/codegen/src/isa/aarch64/lower.isle @@ -2461,6 +2461,52 @@ (rule (lower (fvpromote_low val)) (vec_rr_long (VecRRLongOp.Fcvtl32) val $false)) +;;; Rules for `brif` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +;; `brif` following `icmp` +(rule (lower_branch (brif (maybe_uextend (icmp cc x @ (value_type ty) y)) _ _) targets) + (let ((comparison FlagsAndCC (lower_icmp_into_flags cc x y ty)) + (cond Cond (cond_code (flags_and_cc_cc comparison))) + (taken BranchTarget (branch_target targets 0)) + (not_taken BranchTarget (branch_target targets 1))) + (emit_side_effect + (with_flags_side_effect (flags_and_cc_flags comparison) + (cond_br taken + not_taken + (cond_br_cond cond)))))) + +;; `brif` following `fcmp` +(rule (lower_branch (brif (maybe_uextend (fcmp cc x @ (value_type (ty_scalar_float ty)) y)) _ _) targets) + (let ((cond Cond (fp_cond_code cc)) + (taken BranchTarget (branch_target targets 0)) + (not_taken BranchTarget (branch_target targets 1))) + (emit_side_effect + (with_flags_side_effect (fpu_cmp (scalar_size ty) x y) + (cond_br taken not_taken + (cond_br_cond cond)))))) + +;; standard `brif` +(rule -1 (lower_branch (brif c @ (value_type $I128) _ _) targets) + (let ((flags ProducesFlags (flags_to_producesflags c)) + (c ValueRegs (put_in_regs c)) + (c_lo Reg (value_regs_get c 0)) + (c_hi Reg (value_regs_get c 1)) + (rt Reg (orr $I64 c_lo c_hi)) + (taken BranchTarget (branch_target targets 0)) + (not_taken BranchTarget (branch_target targets 1))) + (emit_side_effect + (with_flags_side_effect flags + (cond_br taken not_taken (cond_br_not_zero rt)))))) +(rule -2 (lower_branch (brif c @ (value_type ty) _ _) targets) + (if (ty_int_ref_scalar_64 ty)) + (let ((flags ProducesFlags (flags_to_producesflags c)) + (rt Reg (put_in_reg_zext64 c)) + (taken BranchTarget (branch_target targets 0)) + (not_taken BranchTarget (branch_target targets 1))) + (emit_side_effect + (with_flags_side_effect flags + (cond_br taken not_taken (cond_br_not_zero rt)))))) + ;;; Rules for `brz`/`brnz` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; `brz` following `icmp` diff --git a/cranelift/codegen/src/isa/aarch64/lower/isle.rs b/cranelift/codegen/src/isa/aarch64/lower/isle.rs index 263e6cc5b5..69796c4bc6 100644 --- a/cranelift/codegen/src/isa/aarch64/lower/isle.rs +++ b/cranelift/codegen/src/isa/aarch64/lower/isle.rs @@ -24,8 +24,8 @@ use crate::machinst::{isle::*, InputSourceInst}; use crate::{ binemit::CodeOffset, ir::{ - immediates::*, types::*, AtomicRmwOp, ExternalName, Inst, InstructionData, MemFlags, - TrapCode, Value, ValueList, + immediates::*, types::*, AtomicRmwOp, BlockCall, ExternalName, Inst, InstructionData, + MemFlags, TrapCode, Value, ValueList, }, isa::aarch64::abi::AArch64Caller, isa::aarch64::inst::args::{ShiftOp, ShiftOpShiftImm}, diff --git a/cranelift/codegen/src/isa/riscv64/inst.isle b/cranelift/codegen/src/isa/riscv64/inst.isle index 2031cf924a..6dc185014d 100644 --- a/cranelift/codegen/src/isa/riscv64/inst.isle +++ b/cranelift/codegen/src/isa/riscv64/inst.isle @@ -1898,6 +1898,30 @@ (hi Reg (value_regs_get regs 1))) (alu_rrr (AluOPRRR.Or) lo hi))) +;; Default behavior for branching based on an input value. +(rule + (lower_branch (brif v @ (value_type ty) _ _) targets) + (lower_brz_or_nz (IntCC.NotEqual) (normalize_cmp_value ty v) targets ty)) + +;; Special case for SI128 to reify the comparison value and branch on it. +(rule 2 + (lower_branch (brif v @ (value_type $I128) _ _) targets) + (let ((zero ValueRegs (value_regs (zero_reg) (zero_reg))) + (cmp Reg (gen_icmp (IntCC.NotEqual) v zero $I128))) + (lower_brz_or_nz (IntCC.NotEqual) cmp targets $I64))) + +;; Branching on the result of an icmp +(rule 1 + (lower_branch (brif (icmp cc a @ (value_type ty) b) _ _) targets) + (lower_br_icmp cc a b targets ty)) + +;; Branching on the result of an fcmp +(rule 1 + (lower_branch (brif (fcmp cc a @ (value_type ty) b) _ _) targets) + (let ((then BranchTarget (label_to_br_target (vec_label_get targets 0))) + (else BranchTarget (label_to_br_target (vec_label_get targets 1)))) + (emit_side_effect (cond_br (emit_fcmp cc ty a b) then else)))) + ;;;;; (rule (lower_branch (brz v @ (value_type ty) _) targets) diff --git a/cranelift/codegen/src/isa/riscv64/lower/isle.rs b/cranelift/codegen/src/isa/riscv64/lower/isle.rs index c41469eff6..125f005ae7 100644 --- a/cranelift/codegen/src/isa/riscv64/lower/isle.rs +++ b/cranelift/codegen/src/isa/riscv64/lower/isle.rs @@ -14,8 +14,8 @@ use crate::machinst::{isle::*, MachInst, SmallInstVec}; use crate::machinst::{VCodeConstant, VCodeConstantData}; use crate::{ ir::{ - immediates::*, types::*, AtomicRmwOp, ExternalName, Inst, InstructionData, MemFlags, - StackSlot, TrapCode, Value, ValueList, + immediates::*, types::*, AtomicRmwOp, BlockCall, ExternalName, Inst, InstructionData, + MemFlags, StackSlot, TrapCode, Value, ValueList, }, isa::riscv64::inst::*, machinst::{ArgPair, InstOutput, Lower}, diff --git a/cranelift/codegen/src/isa/s390x/lower.isle b/cranelift/codegen/src/isa/s390x/lower.isle index 96bffdf350..1819d060f8 100644 --- a/cranelift/codegen/src/isa/s390x/lower.isle +++ b/cranelift/codegen/src/isa/s390x/lower.isle @@ -3750,6 +3750,17 @@ (emit_side_effect (jt_sequence (lshl_imm $I64 idx 2) targets)))) +;;;; Rules for `brif` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +;; Two-way conditional branch on nonzero. `targets` contains: +;; - element 0: target if the condition is true (i.e. value is nonzero) +;; - element 1: target if the condition is false (i.e. value is zero) +(rule (lower_branch (brif val_cond _ _) targets) + (emit_side_effect (cond_br_bool (value_nonzero val_cond) + (vec_element targets 0) + (vec_element targets 1)))) + + ;;;; Rules for `brz` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Two-way conditional branch on zero. `targets` contains: diff --git a/cranelift/codegen/src/isa/s390x/lower/isle.rs b/cranelift/codegen/src/isa/s390x/lower/isle.rs index df615b36c7..520cd5f371 100644 --- a/cranelift/codegen/src/isa/s390x/lower/isle.rs +++ b/cranelift/codegen/src/isa/s390x/lower/isle.rs @@ -16,8 +16,8 @@ use crate::machinst::isle::*; use crate::machinst::{MachLabel, Reg}; use crate::{ ir::{ - condcodes::*, immediates::*, types::*, ArgumentPurpose, AtomicRmwOp, Endianness, Inst, - InstructionData, KnownSymbol, LibCall, MemFlags, Opcode, TrapCode, Value, ValueList, + condcodes::*, immediates::*, types::*, ArgumentPurpose, AtomicRmwOp, BlockCall, Endianness, + Inst, InstructionData, KnownSymbol, LibCall, MemFlags, Opcode, TrapCode, Value, ValueList, }, isa::unwind::UnwindInst, isa::CallConv, diff --git a/cranelift/codegen/src/isa/x64/lower.isle b/cranelift/codegen/src/isa/x64/lower.isle index 191e703f8c..ec77a7fc78 100644 --- a/cranelift/codegen/src/isa/x64/lower.isle +++ b/cranelift/codegen/src/isa/x64/lower.isle @@ -2885,6 +2885,25 @@ (rule (lower_branch (jump _) (single_target target)) (emit_side_effect (jmp_known target))) +;; Rules for `brif` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(rule 2 (lower_branch (brif (maybe_uextend (icmp cc a b)) _ _) (two_targets then else)) + (emit_side_effect (jmp_cond_icmp (emit_cmp cc a b) then else))) + +(rule 2 (lower_branch (brif (maybe_uextend (fcmp cc a b)) _ _) (two_targets then else)) + (emit_side_effect (jmp_cond_fcmp (emit_fcmp cc a b) then else))) + +(rule 1 (lower_branch (brif val @ (value_type $I128) _ _) + (two_targets then else)) + (emit_side_effect (jmp_cond_icmp (cmp_zero_i128 (CC.Z) val) then else))) + +(rule (lower_branch (brif val @ (value_type (ty_int_bool_or_ref)) _ _) + (two_targets then else)) + (emit_side_effect (with_flags_side_effect + (cmp_zero_int_bool_ref val) + (jmp_cond (CC.NZ) then else)))) + + ;; Rules for `brz` and `brnz` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (rule 2 (lower_branch (brz (maybe_uextend (icmp cc a b)) _) (two_targets taken not_taken)) diff --git a/cranelift/codegen/src/isa/x64/lower/isle.rs b/cranelift/codegen/src/isa/x64/lower/isle.rs index b32eb3cb30..7d97d761ed 100644 --- a/cranelift/codegen/src/isa/x64/lower/isle.rs +++ b/cranelift/codegen/src/isa/x64/lower/isle.rs @@ -20,7 +20,7 @@ use crate::{ condcodes::{CondCode, FloatCC, IntCC}, immediates::*, types::*, - Inst, InstructionData, MemFlags, Opcode, TrapCode, Value, ValueList, + BlockCall, Inst, InstructionData, MemFlags, Opcode, TrapCode, Value, ValueList, }, isa::{ unwind::UnwindInst, diff --git a/cranelift/codegen/src/isle_prelude.rs b/cranelift/codegen/src/isle_prelude.rs index 58f42f4868..8ee2d707df 100644 --- a/cranelift/codegen/src/isle_prelude.rs +++ b/cranelift/codegen/src/isle_prelude.rs @@ -637,5 +637,16 @@ macro_rules! isle_common_prelude_methods { fn pack_value_array_3(&mut self, a: Value, b: Value, c: Value) -> ValueArray3 { [a, b, c] } + + #[inline] + fn unpack_block_array_2(&mut self, arr: &BlockArray2) -> (BlockCall, BlockCall) { + let [a, b] = *arr; + (a, b) + } + + #[inline] + fn pack_block_array_2(&mut self, a: BlockCall, b: BlockCall) -> BlockArray2 { + [a, b] + } }; } diff --git a/cranelift/codegen/src/machinst/isle.rs b/cranelift/codegen/src/machinst/isle.rs index 30fcd7c24b..70bcb7d12e 100644 --- a/cranelift/codegen/src/machinst/isle.rs +++ b/cranelift/codegen/src/machinst/isle.rs @@ -1,4 +1,4 @@ -use crate::ir::{Value, ValueList}; +use crate::ir::{BlockCall, Value, ValueList}; use alloc::boxed::Box; use alloc::vec::Vec; use smallvec::SmallVec; @@ -22,6 +22,7 @@ pub type Unit = (); pub type ValueSlice = (ValueList, usize); pub type ValueArray2 = [Value; 2]; pub type ValueArray3 = [Value; 3]; +pub type BlockArray2 = [BlockCall; 2]; pub type WritableReg = Writable; pub type VecRetPair = Vec; pub type VecMask = Vec; diff --git a/cranelift/codegen/src/machinst/lower.rs b/cranelift/codegen/src/machinst/lower.rs index 3bdce4abd8..ea29cb3468 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::{ - ArgumentPurpose, Block, Constant, ConstantData, DataFlowGraph, ExternalName, Function, - GlobalValue, GlobalValueData, Immediate, Inst, InstructionData, MemFlags, Opcode, RelSourceLoc, - Type, Value, ValueDef, ValueLabelAssignments, ValueLabelStart, + instructions, 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, @@ -941,12 +941,28 @@ impl<'func, I: VCodeInst> Lower<'func, I> { for succ_idx in 0..self.vcode.block_order().succ_indices(block).len() { // Avoid immutable borrow by explicitly indexing. let (inst, succ) = self.vcode.block_order().succ_indices(block)[succ_idx]; - // Get branch args and convert to Regs. - let branch_args = self.f.dfg.insts[inst] - .branch_destination() - .into_iter() - .flat_map(|block| block.args_slice(&self.f.dfg.value_lists)); + // 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) => { + // 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. + if succ_idx == 0 { + then_block.args_slice(&self.f.dfg.value_lists) + } else { + assert!(succ_idx == 1); + else_block.args_slice(&self.f.dfg.value_lists) + } + } + instructions::BranchInfo::Table(_, _) => &[], + }; let mut branch_arg_vregs: SmallVec<[Reg; 16]> = smallvec![]; for &arg in branch_args { let arg = self.f.dfg.resolve_aliases(arg); @@ -976,7 +992,10 @@ impl<'func, I: VCodeInst> Lower<'func, I> { if last_inst != Some(inst) { branches.push(inst); } else { - debug_assert!(self.f.dfg.insts[inst].opcode() == Opcode::BrTable); + debug_assert!( + self.f.dfg.insts[inst].opcode() == Opcode::BrTable + || self.f.dfg.insts[inst].opcode() == Opcode::Brif + ); debug_assert!(branches.len() == 1); } last_inst = Some(inst); diff --git a/cranelift/codegen/src/prelude.isle b/cranelift/codegen/src/prelude.isle index 5a0aa3376c..77f7ccddf6 100644 --- a/cranelift/codegen/src/prelude.isle +++ b/cranelift/codegen/src/prelude.isle @@ -34,6 +34,7 @@ (type Type (primitive Type)) (type Value (primitive Value)) (type ValueList (primitive ValueList)) +(type BlockCall (primitive BlockCall)) ;; ISLE representation of `&[Value]`. (type ValueSlice (primitive ValueSlice)) diff --git a/cranelift/codegen/src/remove_constant_phis.rs b/cranelift/codegen/src/remove_constant_phis.rs index 4f03c7d8ca..4a9ef40170 100644 --- a/cranelift/codegen/src/remove_constant_phis.rs +++ b/cranelift/codegen/src/remove_constant_phis.rs @@ -4,7 +4,6 @@ use crate::dominator_tree::DominatorTree; use crate::fx::FxHashMap; use crate::fx::FxHashSet; use crate::ir; -use crate::ir::instructions::BranchInfo; use crate::ir::Function; use crate::ir::{Block, BlockCall, Inst, Value}; use crate::timing; @@ -112,6 +111,9 @@ impl AbstractValue { struct OutEdge<'a> { /// An instruction that transfers control. inst: Inst, + /// The index into branch_destinations for this instruction that corresponds + /// to this edge. + branch_index: u32, /// The block that control is transferred to. block: Block, /// The arguments to that block. @@ -126,7 +128,13 @@ impl<'a> OutEdge<'a> { /// Returns `None` if this is an edge without any block arguments, which /// means we can ignore it for this analysis's purposes. #[inline] - fn new(bump: &'a Bump, dfg: &ir::DataFlowGraph, inst: Inst, block: BlockCall) -> Option { + fn new( + bump: &'a Bump, + dfg: &ir::DataFlowGraph, + inst: Inst, + branch_index: usize, + block: BlockCall, + ) -> Option { let inst_var_args = block.args_slice(&dfg.value_lists); // Skip edges without params. @@ -136,6 +144,7 @@ impl<'a> OutEdge<'a> { Some(OutEdge { inst, + branch_index: branch_index as u32, block: block.block(&dfg.value_lists), args: bump.alloc_slice_fill_iter( inst_var_args @@ -231,12 +240,8 @@ pub fn do_remove_constant_phis(func: &mut Function, domtree: &mut DominatorTree) let mut summary = BlockSummary::new(&bump, formals); for inst in func.layout.block_insts(b) { - let idetails = &func.dfg.insts[inst]; - // Note that multi-dest transfers (i.e., branch tables) don't - // carry parameters in our IR, so we only have to care about - // `SingleDest` here. - if let BranchInfo::SingleDest(dest) = idetails.analyze_branch() { - if let Some(edge) = OutEdge::new(&bump, &func.dfg, inst, dest) { + for (ix, dest) in func.dfg.insts[inst].branch_destination().iter().enumerate() { + if let Some(edge) = OutEdge::new(&bump, &func.dfg, inst, ix, *dest) { summary.dests.push(edge); } } @@ -378,7 +383,9 @@ pub fn do_remove_constant_phis(func: &mut Function, domtree: &mut DominatorTree) } let dfg = &mut func.dfg; - let block = dfg.insts[edge.inst].branch_destination_mut().unwrap(); + let block = + &mut dfg.insts[edge.inst].branch_destination_mut()[edge.branch_index as usize]; + old_actuals.extend(block.args_slice(&dfg.value_lists)); // Check that the numbers of arguments make sense. diff --git a/cranelift/codegen/src/verifier/mod.rs b/cranelift/codegen/src/verifier/mod.rs index 396f3ff865..444b45c168 100644 --- a/cranelift/codegen/src/verifier/mod.rs +++ b/cranelift/codegen/src/verifier/mod.rs @@ -587,6 +587,15 @@ impl<'a> Verifier<'a> { Jump { destination, .. } | Branch { destination, .. } => { self.verify_block(inst, destination.block(&self.func.dfg.value_lists), errors)?; } + Brif { + arg, + blocks: [block_then, block_else], + .. + } => { + self.verify_value(inst, arg, errors)?; + self.verify_block(inst, block_then.block(&self.func.dfg.value_lists), errors)?; + self.verify_block(inst, block_else.block(&self.func.dfg.value_lists), errors)?; + } BranchTable { table, destination, .. } => { @@ -1303,6 +1312,25 @@ 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) => { + let iter = self + .func + .dfg + .block_params(block_then.block(&self.func.dfg.value_lists)) + .iter() + .map(|&v| self.func.dfg.value_type(v)); + let args_then = block_then.args_slice(&self.func.dfg.value_lists); + self.typecheck_variable_args_iterator(inst, iter, args_then, errors)?; + + let iter = self + .func + .dfg + .block_params(block_else.block(&self.func.dfg.value_lists)) + .iter() + .map(|&v| self.func.dfg.value_type(v)); + 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) => { let arg_count = self.func.dfg.num_block_params(block); if arg_count != 0 { diff --git a/cranelift/codegen/src/write.rs b/cranelift/codegen/src/write.rs index 49e6e76d29..34600b25ef 100644 --- a/cranelift/codegen/src/write.rs +++ b/cranelift/codegen/src/write.rs @@ -418,6 +418,16 @@ pub fn write_operands(w: &mut dyn Write, dfg: &DataFlowGraph, inst: Inst) -> fmt write!(w, " {}", destination.block(pool))?; write_block_args(w, destination.args_slice(pool)) } + Brif { + arg, + blocks: [block_then, block_else], + .. + } => { + write!(w, " {}, {}", arg, block_then.block(pool))?; + write_block_args(w, block_then.args_slice(pool))?; + write!(w, ", {}", block_else.block(pool))?; + write_block_args(w, block_else.args_slice(pool)) + } Branch { arg, destination, .. } => { diff --git a/cranelift/filetests/filetests/isa/x64/branches.clif b/cranelift/filetests/filetests/isa/x64/branches.clif index 1276b74369..8ed504d6c7 100644 --- a/cranelift/filetests/filetests/isa/x64/branches.clif +++ b/cranelift/filetests/filetests/isa/x64/branches.clif @@ -394,3 +394,65 @@ block2: ; popq %rbp ; ret +function %brif_i8_icmp(i32, i32) -> i32 { +block0(v0: i32, v1: i32): + v2 = icmp eq v0, v1 + v3 = uextend.i32 v2 + brif v3, block1, block2 + +block1: + v4 = iconst.i32 1 + return v4 + +block2: + v5 = iconst.i32 2 + return v5 +} + +; pushq %rbp +; movq %rsp, %rbp +; block0: +; cmpl %esi, %edi +; jz label1; j label2 +; block1: +; movl $1, %eax +; movq %rbp, %rsp +; popq %rbp +; ret +; block2: +; movl $2, %eax +; movq %rbp, %rsp +; popq %rbp +; ret + +function %brif_i8_fcmp(f32, f32) -> i32 { +block0(v0: f32, v1: f32): + v2 = fcmp eq v0, v1 + v3 = uextend.i32 v2 + brif v3, block1, block2 + +block1: + v4 = iconst.i32 1 + return v4 + +block2: + v5 = iconst.i32 2 + return v5 +} + +; pushq %rbp +; movq %rsp, %rbp +; block0: +; ucomiss %xmm1, %xmm0 +; jp label2 +; jnz label2; j label1 +; block1: +; movl $1, %eax +; movq %rbp, %rsp +; popq %rbp +; ret +; block2: +; movl $2, %eax +; movq %rbp, %rsp +; popq %rbp +; ret diff --git a/cranelift/filetests/filetests/parser/branch.clif b/cranelift/filetests/filetests/parser/branch.clif index c9a71312d9..4c7cd70c2a 100644 --- a/cranelift/filetests/filetests/parser/branch.clif +++ b/cranelift/filetests/filetests/parser/branch.clif @@ -114,3 +114,24 @@ block50: ; nextln: block50: ; nextln: trap user1 ; nextln: } + +function %twoargs(i32, f32) { +block0(v90: i32, v91: f32): + brif v90, block1(v90, v91), block2(v91, v90) + +block1(v92: i32, v93: f32): + brif v90, block1(v90, v91), block2(v91, v90) + +block2(v94: f32, v95: i32): + brif v90, block1(v90, v91), block2(v91, v90) +} +; sameln: function %twoargs(i32, f32) fast { +; nextln: block0(v90: i32, v91: f32): +; nextln: brif v90, block1(v90, v91), block2(v91, v90) +; nextln: +; nextln: block1(v92: i32, v93: f32): +; nextln: brif.i32 v90, block1(v90, v91), block2(v91, v90) +; nextln: +; nextln: block2(v94: f32, v95: i32): +; nextln: brif.i32 v90, block1(v90, v91), block2(v91, v90) +; nextln: } diff --git a/cranelift/filetests/filetests/runtests/brif.clif b/cranelift/filetests/filetests/runtests/brif.clif new file mode 100644 index 0000000000..b751ca09e6 --- /dev/null +++ b/cranelift/filetests/filetests/runtests/brif.clif @@ -0,0 +1,195 @@ +test interpret +test run +target aarch64 +target s390x +target x86_64 +target riscv64 + +function %brif_value(i8) -> i64 { +block0(v0: i8): + brif v0, block1, block2 +block1: + v1 = uextend.i64 v0 + return v1 +block2: + v2 = iconst.i64 42 + return v2 +} + +; run: %brif_value(0) == 42 +; run: %brif_value(42) == 42 +; run: %brif_value(97) == 97 + +function %brif_ne_zero(i8) -> i64 { +block0(v0: i8): + v1 = iconst.i8 0 + v2 = icmp ne v0, v1 + brif v2, block1, block2 +block1: + v3 = uextend.i64 v0 + return v3 +block2: + v4 = iconst.i64 42 + return v4 +} + +; run: %brif_ne_zero(0) == 42 +; run: %brif_ne_zero(42) == 42 +; run: %brif_ne_zero(97) == 97 + +function %brif_ne_one(i8) -> i64 { +block0(v0: i8): + v1 = iconst.i8 1 + v2 = icmp ne v0, v1 + brif v2, block1, block2 +block1: + v3 = uextend.i64 v0 + return v3 +block2: + v4 = iconst.i64 42 + return v4 +} + +; run: %brif_ne_one(1) == 42 +; run: %brif_ne_one(0) == 0 +; run: %brif_ne_one(42) == 42 +; run: %brif_ne_one(97) == 97 + +function %brif_uextend_ne_one(i8) -> i64 { +block0(v0: i8): + v1 = iconst.i8 1 + v2 = icmp ne v0, v1 + v3 = uextend.i64 v2 + brif v3, block1, block2 +block1: + v4 = uextend.i64 v0 + return v4 +block2: + v5 = iconst.i64 42 + return v5 +} + +; run: %brif_uextend_ne_one(1) == 42 +; run: %brif_uextend_ne_one(0) == 0 +; run: %brif_uextend_ne_one(42) == 42 +; run: %brif_uextend_ne_one(97) == 97 + + +function %brif_i64(i64) -> i8 { +block0(v0: i64): + brif v0, block1, block2 + +block1: + v1 = iconst.i8 1 + return v1 + +block2: + v2 = iconst.i8 0 + return v2 +} +; run: %brif_i64(0) == 0 +; run: %brif_i64(1) == 1 +; run: %brif_i64(-1) == 1 + +function %brif_i32(i32) -> i8 { +block0(v0: i32): + brif v0, block1, block2 + +block1: + v1 = iconst.i8 1 + return v1 + +block2: + v2 = iconst.i8 0 + return v2 +} +; run: %brif_i32(0) == 0 +; run: %brif_i32(1) == 1 +; run: %brif_i32(-1) == 1 + +function %brif_i16(i16) -> i8 { +block0(v0: i16): + brif v0, block1, block2 + +block1: + v1 = iconst.i8 1 + return v1 + +block2: + v2 = iconst.i8 0 + return v2 +} +; run: %brif_i16(0) == 0 +; run: %brif_i16(1) == 1 +; run: %brif_i16(-1) == 1 + +function %brif_i8(i8) -> i8 { +block0(v0: i8): + brif v0, block1, block2 + +block1: + v1 = iconst.i8 1 + return v1 + +block2: + v2 = iconst.i8 0 + return v2 +} +; run: %brif_i8(0) == 0 +; run: %brif_i8(1) == 1 +; run: %brif_i8(-1) == 1 +; run: %brif_i8(97) == 1 + +function %brif_different_args(i8) -> i8 { +block0(v0: i8): + brif v0, block1(v0, v0), block2(v0) + +block1(v1: i8, v2: i8): + v3 = iadd v1, v2 + return v3 + +block2(v4: i8): + return v4 +} + +; run: %brif_different_args(0) == 0 +; run: %brif_different_args(1) == 2 +; run: %brif_different_args(8) == 16 +; run: %brif_different_args(128) == 0 + +function %fuzzgen_1() -> i8 system_v { +block0: + v1 = iconst.i8 35 + brif v1, block1(v1), block1(v1) ; v1 = 35 + +block1(v0: i8): + return v0 +} + +; run: %fuzzgen_1() == 35 + +function %fuzzgen_2(i16) -> i16, i16 system_v { +block0(v0: i16): + brnz v0, block1(v0, v0) + jump block2(v0, v0) + +block1(v1: i16, v2: i16): + brif v1, block2(v2, v2), block2(v2, v2) + +block2(v3: i16, v4: i16): + return v3, v4 +} + +; run: %fuzzgen_2(0) == [0, 0] + +function %fuzzgen_3(i8 sext) -> i8 system_v { +block0(v0: i8): + v1 = iconst.i8 -9 + brif v0, block1(v1), block1(v0) + +block1(v2: i8): + return v2 +} + +; run: %fuzzgen_3(-65) == -9 +; run: %fuzzgen_3(0) == 0 diff --git a/cranelift/filetests/filetests/verifier/type_check.clif b/cranelift/filetests/filetests/verifier/type_check.clif index 0663f8fd7e..02f0589df7 100644 --- a/cranelift/filetests/filetests/verifier/type_check.clif +++ b/cranelift/filetests/filetests/verifier/type_check.clif @@ -98,6 +98,22 @@ function %jump_args2() { return } +function %brif_args() { +block0: + v0 = iconst.i16 10 + v1 = iconst.i16 10 + brif v0, block1(v1), block2(v1) + ; error: arg 0 (v1) has type i16, expected i64 + ; error: mismatched argument count + ; error: arg 0 (v1) has type i16, expected f32 + +block1(v2: i64): + return + +block2(v3: f32, v4: i8): + return +} + function %bad_extend() { block0: v0 = iconst.i32 10 diff --git a/cranelift/frontend/src/frontend.rs b/cranelift/frontend/src/frontend.rs index f25e6c37b1..dbef386f89 100644 --- a/cranelift/frontend/src/frontend.rs +++ b/cranelift/frontend/src/frontend.rs @@ -106,46 +106,48 @@ impl<'short, 'long> InstBuilderBase<'short> for FuncInstBuilder<'short, 'long> { self.builder.func.set_srcloc(inst, self.builder.srcloc); } - if data.opcode().is_branch() { - match data.branch_destination() { - Some(dest_block) => { - // If the user has supplied jump arguments we must adapt the arguments of - // the destination block - self.builder.declare_successor( - dest_block.block(&self.builder.func.dfg.value_lists), - inst, - ); + match self.builder.func.dfg.analyze_branch(inst) { + ir::instructions::BranchInfo::NotABranch => (), + + ir::instructions::BranchInfo::SingleDest(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) => { + let block_then = branch_then.block(&self.builder.func.dfg.value_lists); + let block_else = branch_else.block(&self.builder.func.dfg.value_lists); + + self.builder.declare_successor(block_then, inst); + if block_then != block_else { + self.builder.declare_successor(block_else, inst); } - None => { - // branch_destination() doesn't detect jump_tables - // If jump table we declare all entries successor - if let InstructionData::BranchTable { - table, destination, .. - } = data - { - // Unlike all other jumps/branches, jump tables are - // capable of having the same successor appear - // multiple times, so we must deduplicate. - let mut unique = EntitySet::::new(); - for dest_block in self - .builder - .func - .jump_tables - .get(table) - .expect("you are referencing an undeclared jump table") - .iter() - .filter(|&dest_block| unique.insert(*dest_block)) - { - // Call `declare_block_predecessor` instead of `declare_successor` for - // avoiding the borrow checker. - self.builder - .func_ctx - .ssa - .declare_block_predecessor(*dest_block, inst); - } - self.builder.declare_successor(destination, inst); - } + } + + ir::instructions::BranchInfo::Table(table, destination) => { + // Unlike all other jumps/branches, jump tables are + // capable of having the same successor appear + // multiple times, so we must deduplicate. + let mut unique = EntitySet::::new(); + for dest_block in self + .builder + .func + .jump_tables + .get(table) + .expect("you are referencing an undeclared jump table") + .iter() + .filter(|&dest_block| unique.insert(*dest_block)) + { + // Call `declare_block_predecessor` instead of `declare_successor` for + // avoiding the borrow checker. + self.builder + .func_ctx + .ssa + .declare_block_predecessor(*dest_block, inst); } + self.builder.declare_successor(destination, inst); } } @@ -680,14 +682,13 @@ impl<'a> FunctionBuilder<'a> { /// other jump instructions. pub fn change_jump_destination(&mut self, inst: Inst, new_dest: Block) { let dfg = &mut self.func.dfg; - let old_dest = dfg.insts[inst] - .branch_destination_mut() - .expect("you want to change the jump destination of a non-jump instruction"); - self.func_ctx - .ssa - .remove_block_predecessor(old_dest.block(&dfg.value_lists), inst); - old_dest.set_block(new_dest, &mut dfg.value_lists); - self.func_ctx.ssa.declare_block_predecessor(new_dest, inst); + for old_dest in dfg.insts[inst].branch_destination_mut() { + self.func_ctx + .ssa + .remove_block_predecessor(old_dest.block(&dfg.value_lists), inst); + old_dest.set_block(new_dest, &mut dfg.value_lists); + self.func_ctx.ssa.declare_block_predecessor(new_dest, inst); + } } /// Returns `true` if and only if the current `Block` is sealed and has no predecessors declared. diff --git a/cranelift/frontend/src/ssa.rs b/cranelift/frontend/src/ssa.rs index 058c95d2b5..4e41f6346f 100644 --- a/cranelift/frontend/src/ssa.rs +++ b/cranelift/frontend/src/ssa.rs @@ -586,10 +586,18 @@ impl SSABuilder { // is sufficient. BranchInfo::SingleDest(_) => { let dfg = &mut func.dfg; - dfg.insts[branch] - .branch_destination_mut() - .unwrap() - .append_argument(val, &mut dfg.value_lists); + for dest in dfg.insts[branch].branch_destination_mut() { + dest.append_argument(val, &mut dfg.value_lists); + } + None + } + BranchInfo::Conditional(_, _) => { + let dfg = &mut func.dfg; + for block in dfg.insts[branch].branch_destination_mut() { + if block.block(&dfg.value_lists) == dest_block { + block.append_argument(val, &mut dfg.value_lists); + } + } None } BranchInfo::Table(mut jt, _default_block) => { diff --git a/cranelift/fuzzgen/src/function_generator.rs b/cranelift/fuzzgen/src/function_generator.rs index 1de8f15c61..23ba8f1de7 100644 --- a/cranelift/fuzzgen/src/function_generator.rs +++ b/cranelift/fuzzgen/src/function_generator.rs @@ -1615,12 +1615,25 @@ where let _type = *self.u.choose(&condbr_types[..])?; let val = builder.use_var(self.get_variable_of_type(_type)?); - if bool::arbitrary(self.u)? { - builder.ins().brz(val, left, &left_args[..]); - } else { - builder.ins().brnz(val, left, &left_args[..]); + match self.u.int_in_range(0..=2)? { + 0 => { + builder.ins().brnz(val, left, &left_args[..]); + builder.ins().jump(right, &right_args[..]); + } + + 1 => { + builder.ins().brz(val, left, &left_args[..]); + builder.ins().jump(right, &right_args[..]); + } + + 2 => { + builder + .ins() + .brif(val, left, &left_args[..], right, &right_args[..]); + } + + _ => unreachable!(), } - builder.ins().jump(right, &right_args[..]); } BlockTerminator::BrTable(default, targets) => { // Create jump tables on demand diff --git a/cranelift/interpreter/src/step.rs b/cranelift/interpreter/src/step.rs index 89a63bae7e..d791ee6f87 100644 --- a/cranelift/interpreter/src/step.rs +++ b/cranelift/interpreter/src/step.rs @@ -7,8 +7,8 @@ use crate::value::{Value, ValueConversionKind, ValueError, ValueResult}; use cranelift_codegen::data_value::DataValue; use cranelift_codegen::ir::condcodes::{FloatCC, IntCC}; use cranelift_codegen::ir::{ - types, AbiParam, Block, ExternalName, FuncRef, Function, InstructionData, Opcode, TrapCode, - Type, Value as ValueRef, + types, AbiParam, Block, BlockCall, ExternalName, FuncRef, Function, InstructionData, Opcode, + TrapCode, Type, Value as ValueRef, }; use log::trace; use smallvec::{smallvec, SmallVec}; @@ -236,8 +236,7 @@ where // Retrieve an instruction's branch destination; expects the instruction to be a branch. - let continue_at = || { - let block = inst.branch_destination().unwrap(); + let continue_at = |block: BlockCall| { let branch_args = state .collect_values(block.args_slice(&state.get_current_function().dfg.value_lists)) .map_err(|v| StepError::UnknownValue(v))?; @@ -248,9 +247,9 @@ where }; // Based on `condition`, indicate where to continue the control flow. - let branch_when = |condition: bool| -> Result, StepError> { + let branch_when = |condition: bool, block| -> Result, StepError> { if condition { - continue_at() + continue_at(block) } else { Ok(ControlFlow::Continue) } @@ -279,17 +278,48 @@ where // Interpret a Cranelift instruction. Ok(match inst.opcode() { - Opcode::Jump => continue_at()?, - Opcode::Brz => branch_when( - !arg(0)? - .convert(ValueConversionKind::ToBoolean)? - .into_bool()?, - )?, - Opcode::Brnz => branch_when( - arg(0)? - .convert(ValueConversionKind::ToBoolean)? - .into_bool()?, - )?, + Opcode::Jump => { + let block = inst.branch_destination()[0]; + continue_at(block)? + } + Opcode::Brif => { + if let InstructionData::Brif { + arg, + blocks: [block_then, block_else], + .. + } = inst + { + let arg = state.get_value(arg).ok_or(StepError::UnknownValue(arg))?; + + let condition = arg.convert(ValueConversionKind::ToBoolean)?.into_bool()?; + + if condition { + continue_at(block_then)? + } else { + continue_at(block_else)? + } + } else { + unreachable!() + } + } + Opcode::Brz => { + let block = inst.branch_destination()[0]; + branch_when( + !arg(0)? + .convert(ValueConversionKind::ToBoolean)? + .into_bool()?, + block, + )? + } + Opcode::Brnz => { + let block = inst.branch_destination()[0]; + branch_when( + arg(0)? + .convert(ValueConversionKind::ToBoolean)? + .into_bool()?, + block, + )? + } Opcode::BrTable => { if let InstructionData::BranchTable { table, destination, .. diff --git a/cranelift/reader/src/parser.rs b/cranelift/reader/src/parser.rs index 1a6b05d2c8..e55ba1bf85 100644 --- a/cranelift/reader/src/parser.rs +++ b/cranelift/reader/src/parser.rs @@ -2588,6 +2588,26 @@ impl<'a> Parser<'a> { destination, } } + InstructionFormat::Brif => { + let arg = self.match_value("expected SSA value control operand")?; + self.match_token(Token::Comma, "expected ',' between operands")?; + let block_then = { + let block_num = self.match_block("expected branch then block")?; + let args = self.parse_opt_value_list()?; + ctx.function.dfg.block_call(block_num, &args) + }; + self.match_token(Token::Comma, "expected ',' between operands")?; + let block_else = { + let block_num = self.match_block("expected branch else block")?; + let args = self.parse_opt_value_list()?; + ctx.function.dfg.block_call(block_num, &args) + }; + InstructionData::Brif { + opcode, + arg, + blocks: [block_then, block_else], + } + } InstructionFormat::Branch => { let arg = self.match_value("expected SSA value control operand")?; self.match_token(Token::Comma, "expected ',' between operands")?;