T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

cranelift: Remove redundant branch and select instructions (#5097)

Browse Source 
As discussed in the 2022/10/19 meeting, this PR removes many of the branch and select instructions that used iflags, in favor if using brz/brnz and select in their place. Additionally, it reworks selectif_spectre_guard to take an i8 input instead of an iflags input.

For reference, the removed instructions are: br_icmp, brif, brff, trueif, trueff, and selectif.
This commit is contained in:
Trevor Elliott
2022-10-24 16:14:35 -07:00
committed by GitHub
parent 30589170b4
commit ec12415b1f
40 changed files with 579 additions and 2527 deletions
Download Patch File Download Diff File Expand all files Collapse all files

86
cranelift/codegen/src/isa/aarch64/lower.isle
View File

@@ -1706,21 +1706,6 @@
(rule (lower (resumable_trap trap_code))
(side_effect (udf trap_code)))
;;;; Rules for `trueif` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Verification ensures the input is always a single-def ifcmp.
(rule (lower (has_type out_ty
(trueif cc insn @ (ifcmp x @ (value_type in_ty) y))))
(lower_icmp_into_reg cc x y in_ty out_ty))
;;;; Rules for `trueff` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Verification ensures the input is always a single-def ffcmp.
(rule (lower (trueff cc insn @ (ffcmp x @ (value_type in_ty) y)))
(with_flags_reg
(fpu_cmp (scalar_size in_ty) x y)
(materialize_bool_result (fp_cond_code cc))))
;;;; Rules for `select` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(rule (lower (has_type ty
@@ -1755,22 +1740,11 @@
(cmp (OperandSize.Size64) rcond (zero_reg))
(Cond.Ne) ty rn rm)))
;;;; Rules for `selectif` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Verification ensures that the input is always a single-def ifcmp.
(rule (lower (has_type ty
(selectif cc flags @ (ifcmp x @ (value_type in_ty) y)
if_true if_false)))
(let ((cond Cond (cond_code cc)))
(lower_select
(lower_icmp_into_flags cc x y in_ty)
cond ty if_true if_false)))
;;;; Rules for `selectif_spectre_guard` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;;; Rules for `select_spectre_guard` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(rule (lower (has_type ty
(selectif_spectre_guard cc
flags @ (ifcmp x @ (value_type in_ty) y) if_true if_false)))
(select_spectre_guard
(icmp cc x @ (value_type in_ty) y) if_true if_false)))
(let ((cond Cond (cond_code cc))
(dst ValueRegs (lower_select
(lower_icmp_into_flags cc x y in_ty)
@@ -2422,7 +2396,7 @@
(rule (lower (fvpromote_low val))
(vec_rr_long (VecRRLongOp.Fcvtl32) val $false))
;;; Rules for `brz`/`brnz`/`brif`/`brff`/`bricmp` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;; Rules for `brz`/`brnz` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; `brz` following `icmp`
(rule (lower_branch (brz (icmp cc x @ (value_type ty) y) _ _) targets)
@@ -2505,58 +2479,6 @@
(with_flags_side_effect flags
(cond_br taken not_taken (cond_br_not_zero rt))))))
;; `br_icmp`
(rule (lower_branch (br_icmp cc x @ (value_type ty) y _ _) targets)
(let ((cond Cond (cond_code cc))
(taken BranchTarget (branch_target targets 0))
(not_taken BranchTarget (branch_target targets 1)))
(side_effect
(with_flags_side_effect (lower_icmp_into_flags cc x y ty)
(cond_br taken not_taken
(cond_br_cond cond))))))
;; `brif`
(rule (lower_branch (brif cc (ifcmp x @ (value_type ty) y) _ _) targets)
(let ((cond Cond (cond_code cc))
(taken BranchTarget (branch_target targets 0))
(not_taken BranchTarget (branch_target targets 1)))
(side_effect
(with_flags_side_effect (lower_icmp_into_flags cc x y ty)
(cond_br taken not_taken
(cond_br_cond cond))))))
;; If the `ifcmp` result is actually placed in a register, we need to move it
;; back into the flags.
(rule -1 (lower_branch (brif cc f _ _) targets)
(let ((cond Cond (cond_code cc))
(rn Reg (put_in_reg f))
(taken BranchTarget (branch_target targets 0))
(not_taken BranchTarget (branch_target targets 1)))
(side_effect
(with_flags_side_effect (mov_to_nzcv rn)
(cond_br taken not_taken
(cond_br_cond cond))))))
;; `brff`
(rule (lower_branch (brff cc (ffcmp x @ (value_type ty) y) _ _) targets)
(let ((cond Cond (fp_cond_code cc))
(taken BranchTarget (branch_target targets 0))
(not_taken BranchTarget (branch_target targets 1)))
(side_effect
(with_flags_side_effect (fpu_cmp (scalar_size ty) x y)
(cond_br taken not_taken
(cond_br_cond cond))))))
;; If the `ffcmp` result is actually placed in a register, we need to move it
;; back into the flags.
(rule -1 (lower_branch (brff cc f _ _) targets)
(let ((cond Cond (fp_cond_code cc))
(rn Reg (put_in_reg f))
(taken BranchTarget (branch_target targets 0))
(not_taken BranchTarget (branch_target targets 1)))
(side_effect
(with_flags_side_effect (mov_to_nzcv rn)
(cond_br taken not_taken
(cond_br_cond cond))))))
;;; Rules for `jump` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(rule (lower_branch (jump _ _) targets)

14
cranelift/codegen/src/isa/aarch64/lower_inst.rs
View File

@@ -151,14 +151,10 @@ pub(crate) fn lower_insn_to_regs(
Opcode::Select => implemented_in_isle(ctx),
Opcode::Selectif | Opcode::SelectifSpectreGuard => implemented_in_isle(ctx),
Opcode::SelectSpectreGuard => implemented_in_isle(ctx),
Opcode::Bitselect | Opcode::Vselect => implemented_in_isle(ctx),
Opcode::Trueif => implemented_in_isle(ctx),
Opcode::Trueff => implemented_in_isle(ctx),
Opcode::IsNull | Opcode::IsInvalid => implemented_in_isle(ctx),
Opcode::Copy => implemented_in_isle(ctx),
@@ -205,13 +201,7 @@ pub(crate) fn lower_insn_to_regs(
Opcode::GetPinnedReg | Opcode::SetPinnedReg => implemented_in_isle(ctx),
Opcode::Jump
| Opcode::Brz
| Opcode::Brnz
| Opcode::BrIcmp
| Opcode::Brif
| Opcode::Brff
| Opcode::BrTable => {
Opcode::Jump | Opcode::Brz | Opcode::Brnz | Opcode::BrTable => {
panic!("Branch opcode reached non-branch lowering logic!");
}

23
cranelift/codegen/src/isa/riscv64/inst.isle
View File

@@ -1915,24 +1915,29 @@
(rule
(lower_branch (brz v @ (value_type ty) _ _) targets)
(lower_brz_or_nz (IntCC.Equal) (normalize_value ty v) targets ty))
(rule 1
(lower_branch (brz (icmp cc a @ (value_type ty) b) _ _) targets)
(lower_br_icmp (intcc_inverse cc) a b targets ty))
(rule 1
(lower_branch (brz (fcmp cc a @ (value_type ty) b) _ _) targets)
(lower_br_fcmp (floatcc_inverse cc) a b targets ty))
;;;;
(rule
(lower_branch (brnz v @ (value_type ty) _ _) targets)
(lower_brz_or_nz (IntCC.NotEqual) (normalize_value ty v) targets ty))
;;;
(rule
(lower_branch (br_icmp cc a @ (value_type ty) b _ _) targets)
(rule 1
(lower_branch (brnz (icmp cc a @ (value_type ty) b) _ _) targets)
(lower_br_icmp cc a b targets ty))
(rule
(lower_branch (brif cc (ifcmp a @ (value_type ty) b) _ _) targets)
(lower_br_icmp cc a b targets ty))
(rule
(lower_branch (brff cc (ffcmp a @ (value_type ty) b) _ _) targets)
(rule 1
(lower_branch (brnz (fcmp cc a @ (value_type ty) b) _ _) targets)
(lower_br_fcmp cc a b targets ty))
;;;
(decl lower_br_table (Reg VecMachLabel) InstOutput)
(extern constructor lower_br_table lower_br_table)

29
cranelift/codegen/src/isa/riscv64/lower.isle
View File

@@ -605,6 +605,11 @@
(gen_select ty (normalize_value cty c) x y)
)
(rule 1
(lower (has_type ty (select (icmp cc a b) x y)))
(gen_select_reg cc a b x y)
)
;;;;; Rules for `bitselect`;;;;;;;;;
(rule
@@ -832,35 +837,15 @@
(gen_float_round (FloatRoundOP.Nearest) x ty))
;;;;; Rules for `selectif`;;;;;;;;;
;;;;; Rules for `select_spectre_guard`;;;;;;;;;
(rule
(lower (has_type r_ty (selectif cc (ifcmp ca @ (value_type cty) cb) a b)))
(let
((dst VecWritableReg (alloc_vec_writable r_ty))
(r Reg (lower_icmp cc ca cb cty))
(_ Unit (emit (MInst.SelectIf $false (vec_writable_clone dst) r a b))))
(vec_writable_to_regs dst)))
;;;;; Rules for `selectif_spectre_guard`;;;;;;;;;
(rule
(lower (has_type r_ty (selectif_spectre_guard cc (ifcmp ca @ (value_type cty) cb) a b)))
(lower (has_type r_ty (select_spectre_guard (icmp cc ca @ (value_type cty) cb) a b)))
(let
((dst VecWritableReg (alloc_vec_writable r_ty))
(r Reg (lower_icmp cc ca cb cty))
(_ Unit (emit (MInst.SelectIf $true (vec_writable_clone dst) r a b))))
(vec_writable_to_regs dst)))
;;;;; Rules for `trueif`;;;;;;;;;
(rule
(lower (has_type ty (trueif cc (ifcmp ca @ (value_type cty) cb))))
(lower_icmp cc ca cb cty))
;;;;; Rules for `trueff`;;;;;;;;;
(rule
(lower (has_type ty (trueff cc (ffcmp ca @ (value_type cty) cb))))
(gen_fcmp cc ca cb cty))
;;;;; Rules for `trapif`;;;;;;;;;
(rule

17
cranelift/codegen/src/isa/s390x/lower.isle
View File

@@ -3726,15 +3726,15 @@
(put_in_reg val_true) (put_in_reg val_false)))
;;;; Rules for `selectif_spectre_guard` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;;; Rules for `select_spectre_guard` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; We do not support the `iflags` mechanism on our platform. However, common
;; code will unconditionally emit certain patterns using `iflags` which we
;; need to handle somehow. Note that only those specific patterns are
;; recognized by the code below, other uses will fail to lower.
(rule (lower (has_type ty (selectif_spectre_guard int_cc
(ifcmp x y) val_true val_false)))
(rule (lower (has_type ty (select_spectre_guard
(icmp int_cc x y) val_true val_false)))
(select_bool_reg ty (icmp_val $false int_cc x y)
(put_in_reg val_true) (put_in_reg val_false)))
@@ -3801,17 +3801,6 @@
(vec_element targets 1))))
;;;; Rules for `brif` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Similarly to `selectif_spectre_guard`, we only recognize specific patterns
;; generated by common code here. Others will fail to lower.
(rule (lower_branch (brif int_cc (ifcmp x y) _ _) targets)
(side_effect (cond_br_bool (icmp_val $false int_cc x y)
(vec_element targets 0)
(vec_element targets 1))))
;;;; Rules for `trap` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(rule (lower (trap trap_code))

17
cranelift/codegen/src/isa/s390x/lower.rs
View File

@@ -171,7 +171,7 @@ impl LowerBackend for S390xBackend {
| Opcode::IsNull
| Opcode::IsInvalid
| Opcode::Select
| Opcode::SelectifSpectreGuard
| Opcode::SelectSpectreGuard
| Opcode::Trap
| Opcode::ResumableTrap
| Opcode::Trapz
@@ -223,21 +223,10 @@ impl LowerBackend for S390xBackend {
Opcode::GlobalValue => {
panic!("global_value should have been removed by legalization!");
}
Opcode::Ifcmp
| Opcode::Ffcmp
| Opcode::Trapff
| Opcode::Trueif
| Opcode::Trueff
| Opcode::Selectif => {
Opcode::Ifcmp | Opcode::Ffcmp | Opcode::Trapff => {
panic!("Flags opcode should not be encountered.");
}
Opcode::Jump
| Opcode::Brz
| Opcode::Brnz
| Opcode::BrIcmp
| Opcode::Brif
| Opcode::Brff
| Opcode::BrTable => {
Opcode::Jump | Opcode::Brz | Opcode::Brnz | Opcode::BrTable => {
panic!("Branch opcode reached non-branch lowering logic!");
}
Opcode::IaddImm

6
cranelift/codegen/src/isa/x64/inst.isle
View File

@@ -1089,12 +1089,6 @@
(decl cc_invert (CC) CC)
(extern constructor cc_invert cc_invert)
(decl intcc_reverse (IntCC) IntCC)
(extern constructor intcc_reverse intcc_reverse)
(decl floatcc_inverse (FloatCC) FloatCC)
(extern constructor floatcc_inverse floatcc_inverse)
;; Fails if the argument is not either CC.NZ or CC.Z.
(decl cc_nz_or_z (CC) CC)
(extern extractor cc_nz_or_z cc_nz_or_z)

22
cranelift/codegen/src/isa/x64/lower.isle
View File

@@ -2760,16 +2760,6 @@
(rule (lower_branch (jump _ _) (single_target target))
(side_effect (jmp_known target)))
;; Rules for `brif` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(rule (lower_branch (brif cc (ifcmp a b) _ _) (two_targets taken not_taken))
(side_effect (jmp_cond_icmp (emit_cmp cc a b) taken not_taken)))
;; Rules for `brff` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(rule (lower_branch (brff cc (ffcmp a b) _ _) (two_targets taken not_taken))
(side_effect (jmp_cond_fcmp (emit_fcmp cc a b) taken not_taken)))
;; Rules for `brz` and `brnz` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(rule 2 (lower_branch (brz (icmp cc a b) _ _) (two_targets taken not_taken))
@@ -2828,22 +2818,14 @@
(src Gpr val))
(x64_test size src src)))
;; Rules for `bricmp` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(rule (lower_branch (br_icmp cc a b _ _) (two_targets taken not_taken))
(side_effect (jmp_cond_icmp (emit_cmp cc a b) taken not_taken)))
;; 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)))
;; Rules for `selectif` and `selectif_spectre_guard` ;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Rules for `select_spectre_guard` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(rule (lower (selectif cc (ifcmp a b) x y))
(select_icmp (emit_cmp cc a b) x y))
(rule (lower (selectif_spectre_guard cc (ifcmp a b) x y))
(rule (lower (select_spectre_guard (icmp cc a b) x y))
(select_icmp (emit_cmp cc a b) x y))
;; Rules for `fcvt_from_sint` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

13
cranelift/codegen/src/isa/x64/lower.rs
View File

@@ -419,8 +419,7 @@ fn lower_insn_to_regs(
| Opcode::GetStackPointer
| Opcode::GetReturnAddress
| Opcode::Select
| Opcode::Selectif
| Opcode::SelectifSpectreGuard
| Opcode::SelectSpectreGuard
| Opcode::FcvtFromSint
| Opcode::FcvtLowFromSint
| Opcode::FcvtFromUint
@@ -499,8 +498,6 @@ fn lower_insn_to_regs(
Opcode::Bmask => unimplemented!("Bmask not implemented"),
Opcode::Trueif | Opcode::Trueff => unimplemented!("trueif / trueff not implemented"),
Opcode::Vsplit | Opcode::Vconcat => {
unimplemented!("Vector split/concat ops not implemented.");
}
@@ -570,13 +567,7 @@ fn lower_insn_to_regs(
panic!("trapz / trapnz / resumable_trapnz should have been removed by legalization!");
}
Opcode::Jump
| Opcode::Brz
| Opcode::Brnz
| Opcode::BrIcmp
| Opcode::Brif
| Opcode::Brff
| Opcode::BrTable => {
Opcode::Jump | Opcode::Brz | Opcode::Brnz | Opcode::BrTable => {
panic!("Branch opcode reached non-branch lowering logic!");
}
}

10
cranelift/codegen/src/isa/x64/lower/isle.rs
View File

@@ -593,16 +593,6 @@ impl Context for IsleContext<'_, '_, MInst, Flags, IsaFlags, 6> {
}
}
#[inline]
fn intcc_reverse(&mut self, cc: &IntCC) -> IntCC {
cc.reverse()
}
#[inline]
fn floatcc_inverse(&mut self, cc: &FloatCC) -> FloatCC {
cc.inverse()
}
#[inline]
fn sum_extend_fits_in_32_bits(
&mut self,