diff --git a/cranelift/codegen/src/isa/aarch64/inst.isle b/cranelift/codegen/src/isa/aarch64/inst.isle index 3dfbeab7ce..6e088c4ce4 100644 --- a/cranelift/codegen/src/isa/aarch64/inst.isle +++ b/cranelift/codegen/src/isa/aarch64/inst.isle @@ -3748,15 +3748,15 @@ (MInst.EmitIsland needed_space))) ;; Helper for emitting `br_table` sequences. -(decl br_table_impl (u64 Reg VecMachLabel) InstOutput) +(decl br_table_impl (u64 Reg VecMachLabel) Unit) (rule (br_table_impl (imm12_from_u64 jt_size) ridx targets) (let ((jt_info BoxJTSequenceInfo (targets_jt_info targets))) - (side_effect (with_flags_side_effect + (emit_side_effect (with_flags_side_effect (cmp_imm (OperandSize.Size32) ridx jt_size) (jt_sequence ridx jt_info))))) (rule -1 (br_table_impl jt_size ridx targets) (let ((jt_size Reg (imm $I64 (ImmExtend.Zero) jt_size)) (jt_info BoxJTSequenceInfo (targets_jt_info targets))) - (side_effect (with_flags_side_effect + (emit_side_effect (with_flags_side_effect (cmp (OperandSize.Size32) ridx jt_size) (jt_sequence ridx jt_info))))) diff --git a/cranelift/codegen/src/isa/aarch64/lower.isle b/cranelift/codegen/src/isa/aarch64/lower.isle index 87215d66a9..0e1e97e820 100644 --- a/cranelift/codegen/src/isa/aarch64/lower.isle +++ b/cranelift/codegen/src/isa/aarch64/lower.isle @@ -12,7 +12,7 @@ ;; blocks while we lower, the fallthrough in the new order is not (necessarily) ;; the same as the fallthrough in CLIF. So, we use the explicitly-provided ;; target. -(decl partial lower_branch (Inst VecMachLabel) InstOutput) +(decl partial lower_branch (Inst VecMachLabel) Unit) ;;;; Rules for `iconst` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -2422,7 +2422,7 @@ (cond Cond (invert_cond (cond_code (flags_and_cc_cc comparison)))) (taken BranchTarget (branch_target targets 0)) (not_taken BranchTarget (branch_target targets 1))) - (side_effect + (emit_side_effect (with_flags_side_effect (flags_and_cc_flags comparison) (cond_br taken not_taken @@ -2433,7 +2433,7 @@ (cond Cond (cond_code (flags_and_cc_cc comparison))) (taken BranchTarget (branch_target targets 0)) (not_taken BranchTarget (branch_target targets 1))) - (side_effect + (emit_side_effect (with_flags_side_effect (flags_and_cc_flags comparison) (cond_br taken not_taken @@ -2444,7 +2444,7 @@ (cond Cond (invert_cond cond)) ;; negate for `brz` (taken BranchTarget (branch_target targets 0)) (not_taken BranchTarget (branch_target targets 1))) - (side_effect + (emit_side_effect (with_flags_side_effect (fpu_cmp (scalar_size ty) x y) (cond_br taken not_taken (cond_br_cond cond)))))) @@ -2453,7 +2453,7 @@ (let ((cond Cond (fp_cond_code cc)) (taken BranchTarget (branch_target targets 0)) (not_taken BranchTarget (branch_target targets 1))) - (side_effect + (emit_side_effect (with_flags_side_effect (fpu_cmp (scalar_size ty) x y) (cond_br taken not_taken (cond_br_cond cond)))))) @@ -2466,7 +2466,7 @@ (rt Reg (orr $I64 c_lo c_hi)) (taken BranchTarget (branch_target targets 0)) (not_taken BranchTarget (branch_target targets 1))) - (side_effect + (emit_side_effect (with_flags_side_effect flags (cond_br taken not_taken (cond_br_zero rt)))))) (rule -2 (lower_branch (brz c @ (value_type ty) _ _) targets) @@ -2475,7 +2475,7 @@ (rt Reg (put_in_reg_zext64 c)) (taken BranchTarget (branch_target targets 0)) (not_taken BranchTarget (branch_target targets 1))) - (side_effect + (emit_side_effect (with_flags_side_effect flags (cond_br taken not_taken (cond_br_zero rt)))))) ;; standard `brnz` @@ -2487,7 +2487,7 @@ (rt Reg (orr $I64 c_lo c_hi)) (taken BranchTarget (branch_target targets 0)) (not_taken BranchTarget (branch_target targets 1))) - (side_effect + (emit_side_effect (with_flags_side_effect flags (cond_br taken not_taken (cond_br_not_zero rt)))))) (rule -2 (lower_branch (brnz c @ (value_type ty) _ _) targets) @@ -2496,14 +2496,14 @@ (rt Reg (put_in_reg_zext64 c)) (taken BranchTarget (branch_target targets 0)) (not_taken BranchTarget (branch_target targets 1))) - (side_effect + (emit_side_effect (with_flags_side_effect flags (cond_br taken not_taken (cond_br_not_zero rt)))))) ;;; Rules for `jump` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (rule (lower_branch (jump _ _) targets) - (side_effect (aarch64_jump (branch_target targets 0)))) + (emit_side_effect (aarch64_jump (branch_target targets 0)))) ;;; Rules for `br_table` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/cranelift/codegen/src/isa/aarch64/lower.rs b/cranelift/codegen/src/isa/aarch64/lower.rs index 2e4b487d62..d219451cfb 100644 --- a/cranelift/codegen/src/isa/aarch64/lower.rs +++ b/cranelift/codegen/src/isa/aarch64/lower.rs @@ -15,7 +15,6 @@ use crate::isa::aarch64::inst::*; use crate::isa::aarch64::AArch64Backend; use crate::machinst::lower::*; use crate::machinst::{Reg, Writable}; -use crate::CodegenResult; use crate::{machinst::*, trace}; use smallvec::{smallvec, SmallVec}; @@ -745,51 +744,17 @@ pub(crate) fn maybe_value_multi( impl LowerBackend for AArch64Backend { type MInst = Inst; - fn lower(&self, ctx: &mut Lower, ir_inst: IRInst) -> CodegenResult { - if let Some(temp_regs) = super::lower::isle::lower(ctx, self, ir_inst) { - return Ok(temp_regs); - } - - let ty = if ctx.num_outputs(ir_inst) > 0 { - Some(ctx.output_ty(ir_inst, 0)) - } else { - None - }; - - unreachable!( - "not implemented in ISLE: inst = `{}`, type = `{:?}`", - ctx.dfg().display_inst(ir_inst), - ty - ); + fn lower(&self, ctx: &mut Lower, ir_inst: IRInst) -> Option { + isle::lower(ctx, self, ir_inst) } - fn lower_branch_group( + fn lower_branch( &self, ctx: &mut Lower, - branches: &[IRInst], + ir_inst: IRInst, targets: &[MachLabel], - ) -> CodegenResult<()> { - // A block should end with at most two branches. The first may be a - // conditional branch; a conditional branch can be followed only by an - // unconditional branch or fallthrough. Otherwise, if only one branch, - // it may be an unconditional branch, a fallthrough, a return, or a - // trap. These conditions are verified by `is_ebb_basic()` during the - // verifier pass. - assert!(branches.len() <= 2); - if branches.len() == 2 { - let op1 = ctx.data(branches[1]).opcode(); - assert!(op1 == Opcode::Jump); - } - - if let Some(temp_regs) = super::lower::isle::lower_branch(ctx, self, branches[0], targets) { - assert!(temp_regs.len() == 0); - return Ok(()); - } - - unreachable!( - "not implemented in ISLE: branch = `{}`", - ctx.dfg().display_inst(branches[0]), - ); + ) -> Option<()> { + isle::lower_branch(ctx, self, ir_inst, targets) } fn maybe_pinned_reg(&self) -> Option { diff --git a/cranelift/codegen/src/isa/aarch64/lower/isle.rs b/cranelift/codegen/src/isa/aarch64/lower/isle.rs index 7efd33610a..263e6cc5b5 100644 --- a/cranelift/codegen/src/isa/aarch64/lower/isle.rs +++ b/cranelift/codegen/src/isa/aarch64/lower/isle.rs @@ -64,7 +64,7 @@ pub(crate) fn lower_branch( backend: &AArch64Backend, branch: Inst, targets: &[MachLabel], -) -> Option { +) -> Option<()> { // TODO: reuse the ISLE context across lowerings so we can reuse its // internal heap allocations. let mut isle_ctx = IsleContext { lower_ctx, backend }; diff --git a/cranelift/codegen/src/isa/riscv64/inst.isle b/cranelift/codegen/src/isa/riscv64/inst.isle index e838533f81..4c5df742a7 100644 --- a/cranelift/codegen/src/isa/riscv64/inst.isle +++ b/cranelift/codegen/src/isa/riscv64/inst.isle @@ -1865,22 +1865,22 @@ (decl vec_label_get (VecMachLabel u8) MachLabel ) (extern constructor vec_label_get vec_label_get) -(decl partial lower_branch (Inst VecMachLabel) InstOutput) +(decl partial lower_branch (Inst VecMachLabel) Unit) (rule (lower_branch (jump _ _) targets ) - (side_effect (SideEffectNoResult.Inst (gen_jump (vec_label_get targets 0))))) + (emit_side_effect (SideEffectNoResult.Inst (gen_jump (vec_label_get targets 0))))) ;;; cc a b targets Type -(decl lower_br_icmp (IntCC ValueRegs ValueRegs VecMachLabel Type) InstOutput) +(decl lower_br_icmp (IntCC ValueRegs ValueRegs VecMachLabel Type) Unit) (extern constructor lower_br_icmp lower_br_icmp) -(decl lower_br_fcmp (FloatCC Reg Reg VecMachLabel Type) InstOutput) +(decl lower_br_fcmp (FloatCC Reg Reg VecMachLabel Type) Unit) (extern constructor lower_br_fcmp lower_br_fcmp) ;; int scalar zero regs. (decl int_zero_reg (Type) ValueRegs) (extern constructor int_zero_reg int_zero_reg) -(decl lower_brz_or_nz (IntCC ValueRegs VecMachLabel Type) InstOutput) +(decl lower_brz_or_nz (IntCC ValueRegs VecMachLabel Type) Unit) (extern constructor lower_brz_or_nz lower_brz_or_nz) ;; Normalize a value for comparision. @@ -1940,7 +1940,7 @@ (lower_br_fcmp cc a b targets ty)) ;;; -(decl lower_br_table (Reg VecMachLabel) InstOutput) +(decl lower_br_table (Reg VecMachLabel) Unit) (extern constructor lower_br_table lower_br_table) (rule diff --git a/cranelift/codegen/src/isa/riscv64/lower.rs b/cranelift/codegen/src/isa/riscv64/lower.rs index ea714abf02..1477509f39 100644 --- a/cranelift/codegen/src/isa/riscv64/lower.rs +++ b/cranelift/codegen/src/isa/riscv64/lower.rs @@ -4,7 +4,6 @@ use crate::isa::riscv64::inst::*; use crate::isa::riscv64::Riscv64Backend; use crate::machinst::lower::*; use crate::machinst::*; -use crate::CodegenResult; pub mod isle; //============================================================================= @@ -13,53 +12,17 @@ pub mod isle; impl LowerBackend for Riscv64Backend { type MInst = Inst; - fn lower(&self, ctx: &mut Lower, ir_inst: IRInst) -> CodegenResult { - if let Some(temp_regs) = super::lower::isle::lower(ctx, self, ir_inst) { - return Ok(temp_regs); - } - - let ty = if ctx.num_outputs(ir_inst) > 0 { - Some(ctx.output_ty(ir_inst, 0)) - } else { - None - }; - - unreachable!( - "not implemented in ISLE: inst = `{}`, type = `{:?}`", - ctx.dfg().display_inst(ir_inst), - ty - ); + fn lower(&self, ctx: &mut Lower, ir_inst: IRInst) -> Option { + isle::lower(ctx, self, ir_inst) } - fn lower_branch_group( + fn lower_branch( &self, ctx: &mut Lower, - branches: &[IRInst], + ir_inst: IRInst, targets: &[MachLabel], - ) -> CodegenResult<()> { - // A block should end with at most two branches. The first may be a - // conditional branch; a conditional branch can be followed only by an - // unconditional branch or fallthrough. Otherwise, if only one branch, - // it may be an unconditional branch, a fallthrough, a return, or a - // trap. These conditions are verified by `is_ebb_basic()` during the - // verifier pass. - assert!(branches.len() <= 2); - if branches.len() == 2 { - let op1 = ctx.data(branches[1]).opcode(); - assert!(op1 == Opcode::Jump); - } - - // Lower the first branch in ISLE. This will automatically handle - // the second branch (if any) by emitting a two-way conditional branch. - if let Some(temp_regs) = super::lower::isle::lower_branch(ctx, self, branches[0], targets) { - assert!(temp_regs.len() == 0); - return Ok(()); - } - - unreachable!( - "not implemented in ISLE: branch = `{}`", - ctx.dfg().display_inst(branches[0]), - ); + ) -> Option<()> { + isle::lower_branch(ctx, self, ir_inst, targets) } fn maybe_pinned_reg(&self) -> Option { diff --git a/cranelift/codegen/src/isa/riscv64/lower/isle.rs b/cranelift/codegen/src/isa/riscv64/lower/isle.rs index 8b8eb1e11c..d008ff228d 100644 --- a/cranelift/codegen/src/isa/riscv64/lower/isle.rs +++ b/cranelift/codegen/src/isa/riscv64/lower/isle.rs @@ -68,7 +68,7 @@ impl generated_code::Context for IsleContext<'_, '_, MInst, Riscv64Backend> { b: Reg, targets: &VecMachLabel, ty: Type, - ) -> InstOutput { + ) -> Unit { let tmp = self.temp_writable_reg(I64); MInst::lower_br_fcmp( *cc, @@ -81,7 +81,6 @@ impl generated_code::Context for IsleContext<'_, '_, MInst, Riscv64Backend> { ) .iter() .for_each(|i| self.emit(i)); - InstOutput::default() } fn lower_brz_or_nz( @@ -90,7 +89,7 @@ impl generated_code::Context for IsleContext<'_, '_, MInst, Riscv64Backend> { a: ValueRegs, targets: &VecMachLabel, ty: Type, - ) -> InstOutput { + ) -> Unit { MInst::lower_br_icmp( *cc, a, @@ -101,7 +100,6 @@ impl generated_code::Context for IsleContext<'_, '_, MInst, Riscv64Backend> { ) .iter() .for_each(|i| self.emit(i)); - InstOutput::default() } fn lower_br_icmp( &mut self, @@ -110,7 +108,7 @@ impl generated_code::Context for IsleContext<'_, '_, MInst, Riscv64Backend> { b: ValueRegs, targets: &VecMachLabel, ty: Type, - ) -> InstOutput { + ) -> Unit { let test = generated_code::constructor_lower_icmp(self, cc, a, b, ty); self.emit(&MInst::CondBr { taken: BranchTarget::Label(targets[0]), @@ -121,7 +119,6 @@ impl generated_code::Context for IsleContext<'_, '_, MInst, Riscv64Backend> { rs2: zero_reg(), }, }); - InstOutput::default() } fn load_ra(&mut self) -> Reg { if self.backend.flags.preserve_frame_pointers() { @@ -397,7 +394,7 @@ impl generated_code::Context for IsleContext<'_, '_, MInst, Riscv64Backend> { tmp.to_reg() } - fn lower_br_table(&mut self, index: Reg, targets: &VecMachLabel) -> InstOutput { + fn lower_br_table(&mut self, index: Reg, targets: &VecMachLabel) -> Unit { let tmp1 = self.temp_writable_reg(I64); let targets: Vec = targets .into_iter() @@ -409,7 +406,6 @@ impl generated_code::Context for IsleContext<'_, '_, MInst, Riscv64Backend> { tmp1, targets, }); - InstOutput::default() } fn x_reg(&mut self, x: u8) -> Reg { x_reg(x as usize) @@ -446,7 +442,7 @@ pub(crate) fn lower_branch( backend: &Riscv64Backend, branch: Inst, targets: &[MachLabel], -) -> Option { +) -> Option<()> { // TODO: reuse the ISLE context across lowerings so we can reuse its // internal heap allocations. let mut isle_ctx = IsleContext { lower_ctx, backend }; diff --git a/cranelift/codegen/src/isa/s390x/lower.isle b/cranelift/codegen/src/isa/s390x/lower.isle index 9873e935cb..e97e08b65f 100644 --- a/cranelift/codegen/src/isa/s390x/lower.isle +++ b/cranelift/codegen/src/isa/s390x/lower.isle @@ -7,7 +7,7 @@ ;; A variant of the main lowering constructor term, used for branches. ;; The only difference is that it gets an extra argument holding a vector ;; of branch targets to be used. -(decl partial lower_branch (Inst VecMachLabel) InstOutput) +(decl partial lower_branch (Inst VecMachLabel) Unit) ;;;; Rules for `iconst` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -3715,7 +3715,7 @@ ;; Unconditional branch. The target is found as first (and only) element in ;; the list of the current block's branch targets passed as `targets`. (rule (lower_branch (jump _ _) targets) - (side_effect (jump_impl (vec_element targets 0)))) + (emit_side_effect (jump_impl (vec_element targets 0)))) ;;;; Rules for `br_table` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -3731,8 +3731,8 @@ (cond ProducesBool (bool (icmpu_uimm32 $I64 idx (vec_length_minus1 targets)) (intcc_as_cond (IntCC.UnsignedGreaterThanOrEqual)))) - (_ InstOutput (side_effect (oneway_cond_br_bool cond - (vec_element targets 0))))) + (_ Unit (emit_side_effect (oneway_cond_br_bool cond + (vec_element targets 0))))) ;; Scale the index by the element size, and then emit the ;; compound instruction that does: ;; @@ -3747,7 +3747,7 @@ ;; PC-rel offset to the jumptable would be incorrect. ;; (The alternative is to introduce a relocation pass ;; for inlined jumptables, which is much worse, IMHO.) - (side_effect (jt_sequence (lshl_imm $I64 idx 2) targets)))) + (emit_side_effect (jt_sequence (lshl_imm $I64 idx 2) targets)))) ;;;; Rules for `brz` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -3756,9 +3756,9 @@ ;; - element 0: target if the condition is true (i.e. value is zero) ;; - element 1: target if the condition is false (i.e. value is nonzero) (rule (lower_branch (brz val_cond _ _) targets) - (side_effect (cond_br_bool (invert_bool (value_nonzero val_cond)) - (vec_element targets 0) - (vec_element targets 1)))) + (emit_side_effect (cond_br_bool (invert_bool (value_nonzero val_cond)) + (vec_element targets 0) + (vec_element targets 1)))) ;;;; Rules for `brnz` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -3767,9 +3767,9 @@ ;; - 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 (brnz val_cond _ _) targets) - (side_effect (cond_br_bool (value_nonzero val_cond) - (vec_element targets 0) - (vec_element targets 1)))) + (emit_side_effect (cond_br_bool (value_nonzero val_cond) + (vec_element targets 0) + (vec_element targets 1)))) ;;;; Rules for `trap` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/cranelift/codegen/src/isa/s390x/lower.rs b/cranelift/codegen/src/isa/s390x/lower.rs index 9ba6447aed..f4db0a459d 100644 --- a/cranelift/codegen/src/isa/s390x/lower.rs +++ b/cranelift/codegen/src/isa/s390x/lower.rs @@ -1,11 +1,9 @@ //! Lowering rules for S390x. use crate::ir::Inst as IRInst; -use crate::ir::Opcode; use crate::isa::s390x::inst::Inst; use crate::isa::s390x::S390xBackend; use crate::machinst::{InstOutput, Lower, LowerBackend, MachLabel}; -use crate::CodegenResult; pub mod isle; @@ -15,52 +13,16 @@ pub mod isle; impl LowerBackend for S390xBackend { type MInst = Inst; - fn lower(&self, ctx: &mut Lower, ir_inst: IRInst) -> CodegenResult { - if let Some(temp_regs) = super::lower::isle::lower(ctx, self, ir_inst) { - return Ok(temp_regs); - } - - let ty = if ctx.num_outputs(ir_inst) > 0 { - Some(ctx.output_ty(ir_inst, 0)) - } else { - None - }; - - unreachable!( - "not implemented in ISLE: inst = `{}`, type = `{:?}`", - ctx.dfg().display_inst(ir_inst), - ty - ); + fn lower(&self, ctx: &mut Lower, ir_inst: IRInst) -> Option { + isle::lower(ctx, self, ir_inst) } - fn lower_branch_group( + fn lower_branch( &self, ctx: &mut Lower, - branches: &[IRInst], + ir_inst: IRInst, targets: &[MachLabel], - ) -> CodegenResult<()> { - // A block should end with at most two branches. The first may be a - // conditional branch; a conditional branch can be followed only by an - // unconditional branch or fallthrough. Otherwise, if only one branch, - // it may be an unconditional branch, a fallthrough, a return, or a - // trap. These conditions are verified by `is_ebb_basic()` during the - // verifier pass. - assert!(branches.len() <= 2); - if branches.len() == 2 { - let op1 = ctx.data(branches[1]).opcode(); - assert!(op1 == Opcode::Jump); - } - - // Lower the first branch in ISLE. This will automatically handle - // the second branch (if any) by emitting a two-way conditional branch. - if let Some(temp_regs) = super::lower::isle::lower_branch(ctx, self, branches[0], targets) { - assert!(temp_regs.len() == 0); - return Ok(()); - } - - unreachable!( - "not implemented in ISLE: branch = `{}`", - ctx.dfg().display_inst(branches[0]), - ); + ) -> Option<()> { + isle::lower_branch(ctx, self, ir_inst, targets) } } diff --git a/cranelift/codegen/src/isa/s390x/lower/isle.rs b/cranelift/codegen/src/isa/s390x/lower/isle.rs index 6c3d243741..5a1813e3ce 100644 --- a/cranelift/codegen/src/isa/s390x/lower/isle.rs +++ b/cranelift/codegen/src/isa/s390x/lower/isle.rs @@ -71,7 +71,7 @@ pub(crate) fn lower_branch( backend: &S390xBackend, branch: Inst, targets: &[MachLabel], -) -> Option { +) -> Option<()> { // TODO: reuse the ISLE context across lowerings so we can reuse its // internal heap allocations. let mut isle_ctx = IsleContext { lower_ctx, backend }; diff --git a/cranelift/codegen/src/isa/x64/lower.isle b/cranelift/codegen/src/isa/x64/lower.isle index 8340ec9e20..0d31fc08dc 100644 --- a/cranelift/codegen/src/isa/x64/lower.isle +++ b/cranelift/codegen/src/isa/x64/lower.isle @@ -7,7 +7,7 @@ ;; A variant of the main lowering constructor term, used for branches. ;; The only difference is that it gets an extra argument holding a vector ;; of branch targets to be used. -(decl partial lower_branch (Inst MachLabelSlice) InstOutput) +(decl partial lower_branch (Inst MachLabelSlice) Unit) ;;;; Rules for `iconst` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -2865,55 +2865,55 @@ ;; Rules for `jump` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (rule (lower_branch (jump _ _) (single_target target)) - (side_effect (jmp_known target))) + (emit_side_effect (jmp_known target))) ;; Rules for `brz` and `brnz` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (rule 2 (lower_branch (brz (icmp cc a b) _ _) (two_targets taken not_taken)) (let ((cmp IcmpCondResult (invert_icmp_cond_result (emit_cmp cc a b)))) - (side_effect (jmp_cond_icmp cmp taken not_taken)))) + (emit_side_effect (jmp_cond_icmp cmp taken not_taken)))) (rule 2 (lower_branch (brz (uextend (icmp cc a b)) _ _) (two_targets taken not_taken)) (let ((cmp IcmpCondResult (invert_icmp_cond_result (emit_cmp cc a b)))) - (side_effect (jmp_cond_icmp cmp taken not_taken)))) + (emit_side_effect (jmp_cond_icmp cmp taken not_taken)))) (rule 2 (lower_branch (brz (fcmp cc a b) _ _) (two_targets taken not_taken)) (let ((cmp FcmpCondResult (emit_fcmp (floatcc_inverse cc) a b))) - (side_effect (jmp_cond_fcmp cmp taken not_taken)))) + (emit_side_effect (jmp_cond_fcmp cmp taken not_taken)))) (rule 2 (lower_branch (brz (uextend (fcmp cc a b)) _ _) (two_targets taken not_taken)) (let ((cmp FcmpCondResult (emit_fcmp (floatcc_inverse cc) a b))) - (side_effect (jmp_cond_fcmp cmp taken not_taken)))) + (emit_side_effect (jmp_cond_fcmp cmp taken not_taken)))) (rule 1 (lower_branch (brz val @ (value_type $I128) _ _) (two_targets taken not_taken)) - (side_effect (jmp_cond_icmp (cmp_zero_i128 (CC.NZ) val) taken not_taken))) + (emit_side_effect (jmp_cond_icmp (cmp_zero_i128 (CC.NZ) val) taken not_taken))) (rule 0 (lower_branch (brz val @ (value_type (ty_int_bool_or_ref)) _ _) (two_targets taken not_taken)) - (side_effect + (emit_side_effect (with_flags_side_effect (cmp_zero_int_bool_ref val) (jmp_cond (CC.Z) taken not_taken)))) (rule 2 (lower_branch (brnz (icmp cc a b) _ _) (two_targets taken not_taken)) - (side_effect (jmp_cond_icmp (emit_cmp cc a b) taken not_taken))) + (emit_side_effect (jmp_cond_icmp (emit_cmp cc a b) taken not_taken))) (rule 2 (lower_branch (brnz (fcmp cc a b) _ _) (two_targets taken not_taken)) (let ((cmp FcmpCondResult (emit_fcmp cc a b))) - (side_effect (jmp_cond_fcmp cmp taken not_taken)))) + (emit_side_effect (jmp_cond_fcmp cmp taken not_taken)))) (rule 2 (lower_branch (brnz (uextend (icmp cc a b)) _ _) (two_targets taken not_taken)) - (side_effect (jmp_cond_icmp (emit_cmp cc a b) taken not_taken))) + (emit_side_effect (jmp_cond_icmp (emit_cmp cc a b) taken not_taken))) (rule 2 (lower_branch (brnz (uextend (fcmp cc a b)) _ _) (two_targets taken not_taken)) (let ((cmp FcmpCondResult (emit_fcmp cc a b))) - (side_effect (jmp_cond_fcmp cmp taken not_taken)))) + (emit_side_effect (jmp_cond_fcmp cmp taken not_taken)))) (rule 1 (lower_branch (brnz val @ (value_type $I128) _ _) (two_targets taken not_taken)) - (side_effect (jmp_cond_icmp (cmp_zero_i128 (CC.Z) val) taken not_taken))) + (emit_side_effect (jmp_cond_icmp (cmp_zero_i128 (CC.Z) val) taken not_taken))) (rule 0 (lower_branch (brnz val @ (value_type (ty_int_bool_or_ref)) _ _) (two_targets taken not_taken)) - (side_effect + (emit_side_effect (with_flags_side_effect (cmp_zero_int_bool_ref val) (jmp_cond (CC.NZ) taken not_taken)))) @@ -2944,7 +2944,7 @@ ;; Rules for `br_table` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (rule (lower_branch (br_table idx @ (value_type ty) _ _) (jump_table_targets default_target jt_targets)) - (side_effect (jmp_table_seq ty idx default_target jt_targets))) + (emit_side_effect (jmp_table_seq ty idx default_target jt_targets))) ;; Rules for `select_spectre_guard` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/cranelift/codegen/src/isa/x64/lower.rs b/cranelift/codegen/src/isa/x64/lower.rs index f55f085dbe..c294fad0b3 100644 --- a/cranelift/codegen/src/isa/x64/lower.rs +++ b/cranelift/codegen/src/isa/x64/lower.rs @@ -299,51 +299,17 @@ fn lower_to_amode(ctx: &mut Lower, spec: InsnInput, offset: i32) -> Amode impl LowerBackend for X64Backend { type MInst = Inst; - fn lower(&self, ctx: &mut Lower, ir_inst: IRInst) -> CodegenResult { - if let Some(temp_regs) = isle::lower(ctx, self, ir_inst) { - return Ok(temp_regs); - } - - let ty = if ctx.num_outputs(ir_inst) > 0 { - Some(ctx.output_ty(ir_inst, 0)) - } else { - None - }; - - unreachable!( - "not implemented in ISLE: inst = `{}`, type = `{:?}`", - ctx.dfg().display_inst(ir_inst), - ty - ); + fn lower(&self, ctx: &mut Lower, ir_inst: IRInst) -> Option { + isle::lower(ctx, self, ir_inst) } - fn lower_branch_group( + fn lower_branch( &self, ctx: &mut Lower, - branches: &[IRInst], + ir_inst: IRInst, targets: &[MachLabel], - ) -> CodegenResult<()> { - // A block should end with at most two branches. The first may be a - // conditional branch; a conditional branch can be followed only by an - // unconditional branch or fallthrough. Otherwise, if only one branch, - // it may be an unconditional branch, a fallthrough, a return, or a - // trap. These conditions are verified by `is_ebb_basic()` during the - // verifier pass. - assert!(branches.len() <= 2); - if branches.len() == 2 { - let op1 = ctx.data(branches[1]).opcode(); - assert!(op1 == Opcode::Jump); - } - - if let Some(temp_regs) = isle::lower_branch(ctx, self, branches[0], targets) { - assert!(temp_regs.len() == 0); - return Ok(()); - } - - unreachable!( - "not implemented in ISLE: branch = `{}`", - ctx.dfg().display_inst(branches[0]), - ); + ) -> Option<()> { + isle::lower_branch(ctx, self, ir_inst, targets) } fn maybe_pinned_reg(&self) -> Option { diff --git a/cranelift/codegen/src/isa/x64/lower/isle.rs b/cranelift/codegen/src/isa/x64/lower/isle.rs index a4f52e6969..b32eb3cb30 100644 --- a/cranelift/codegen/src/isa/x64/lower/isle.rs +++ b/cranelift/codegen/src/isa/x64/lower/isle.rs @@ -68,7 +68,7 @@ pub(crate) fn lower_branch( backend: &X64Backend, branch: Inst, targets: &[MachLabel], -) -> Option { +) -> Option<()> { // TODO: reuse the ISLE context across lowerings so we can reuse its // internal heap allocations. let mut isle_ctx = IsleContext { lower_ctx, backend }; diff --git a/cranelift/codegen/src/machinst/isle.rs b/cranelift/codegen/src/machinst/isle.rs index 8d861c8f6e..cd45bdbe85 100644 --- a/cranelift/codegen/src/machinst/isle.rs +++ b/cranelift/codegen/src/machinst/isle.rs @@ -140,7 +140,7 @@ macro_rules! isle_lower_prelude_methods { _ => None, }; if let Some(insn) = insn { - if let Ok(regs) = self.backend.lower(self.lower_ctx, insn) { + if let Some(regs) = self.backend.lower(self.lower_ctx, insn) { assert!(regs.len() == 1); return regs[0]; } diff --git a/cranelift/codegen/src/machinst/lower.rs b/cranelift/codegen/src/machinst/lower.rs index 9ffbd6f17e..6f6b68b921 100644 --- a/cranelift/codegen/src/machinst/lower.rs +++ b/cranelift/codegen/src/machinst/lower.rs @@ -122,18 +122,22 @@ pub trait LowerBackend { /// For a branch, this function should not generate the actual branch /// instruction. However, it must force any values it needs for the branch /// edge (block-param actuals) into registers, because the actual branch - /// generation (`lower_branch_group()`) happens *after* any possible merged + /// generation (`lower_branch()`) happens *after* any possible merged /// out-edge. - fn lower(&self, ctx: &mut Lower, inst: Inst) -> CodegenResult; + /// + /// Returns `None` if no lowering for the instruction was found. + fn lower(&self, ctx: &mut Lower, inst: Inst) -> Option; /// Lower a block-terminating group of branches (which together can be seen /// as one N-way branch), given a vcode MachLabel for each target. - fn lower_branch_group( + /// + /// Returns `None` if no lowering for the branch was found. + fn lower_branch( &self, ctx: &mut Lower, - insts: &[Inst], + inst: Inst, targets: &[MachLabel], - ) -> CodegenResult<()>; + ) -> Option<()>; /// A bit of a hack: give a fixed register that always holds the result of a /// `get_pinned_reg` instruction, if known. This allows elision of moves @@ -740,7 +744,18 @@ impl<'func, I: VCodeInst> Lower<'func, I> { // or any of its outputs its used. if has_side_effect || value_needed { trace!("lowering: inst {}: {:?}", inst, self.f.dfg[inst]); - let temp_regs = backend.lower(self, inst)?; + let temp_regs = backend.lower(self, inst).unwrap_or_else(|| { + let ty = if self.num_outputs(inst) > 0 { + Some(self.output_ty(inst, 0)) + } else { + None + }; + panic!( + "should be implemented in ISLE: inst = `{}`, type = `{:?}`", + self.f.dfg.display_inst(inst), + ty + ) + }); // The ISLE generated code emits its own registers to define the // instruction's lowered values in. However, other instructions @@ -895,10 +910,29 @@ impl<'func, I: VCodeInst> Lower<'func, I> { branches, targets, ); + // A block should end with at most two branches. The first may be a + // conditional branch; a conditional branch can be followed only by an + // unconditional branch or fallthrough. Otherwise, if only one branch, + // it may be an unconditional branch, a fallthrough, a return, or a + // trap. These conditions are verified by `is_ebb_basic()` during the + // verifier pass. + assert!(branches.len() <= 2); + if branches.len() == 2 { + assert!(self.data(branches[1]).opcode() == Opcode::Jump); + } // When considering code-motion opportunities, consider the current // program point to be the first branch. self.cur_inst = Some(branches[0]); - backend.lower_branch_group(self, branches, targets)?; + // Lower the first branch in ISLE. This will automatically handle + // the second branch (if any) by emitting a two-way conditional branch. + backend + .lower_branch(self, branches[0], targets) + .unwrap_or_else(|| { + panic!( + "should be implemented in ISLE: branch = `{}`", + self.f.dfg.display_inst(branches[0]), + ) + }); let loc = self.srcloc(branches[0]); self.finish_ir_inst(loc); // Add block param outputs for current block. diff --git a/cranelift/codegen/src/prelude_lower.isle b/cranelift/codegen/src/prelude_lower.isle index f9d968c759..330eb24760 100644 --- a/cranelift/codegen/src/prelude_lower.isle +++ b/cranelift/codegen/src/prelude_lower.isle @@ -282,20 +282,23 @@ (inst2 MInst) (inst3 MInst)))) +;; Emit given side-effectful instruction. +(decl emit_side_effect (SideEffectNoResult) Unit) +(rule (emit_side_effect (SideEffectNoResult.Inst inst)) + (emit inst)) +(rule (emit_side_effect (SideEffectNoResult.Inst2 inst1 inst2)) + (let ((_ Unit (emit inst1))) + (emit inst2))) +(rule (emit_side_effect (SideEffectNoResult.Inst3 inst1 inst2 inst3)) + (let ((_ Unit (emit inst1)) + (_ Unit (emit inst2))) + (emit inst3))) + ;; Create an empty `InstOutput`, but do emit the given side-effectful ;; instruction. (decl side_effect (SideEffectNoResult) InstOutput) -(rule (side_effect (SideEffectNoResult.Inst inst)) - (let ((_ Unit (emit inst))) - (output_none))) -(rule (side_effect (SideEffectNoResult.Inst2 inst1 inst2)) - (let ((_ Unit (emit inst1)) - (_ Unit (emit inst2))) - (output_none))) -(rule (side_effect (SideEffectNoResult.Inst3 inst1 inst2 inst3)) - (let ((_ Unit (emit inst1)) - (_ Unit (emit inst2)) - (_ Unit (emit inst3))) +(rule (side_effect inst) + (let ((_ Unit (emit_side_effect inst))) (output_none))) (decl side_effect_concat (SideEffectNoResult SideEffectNoResult) SideEffectNoResult)