From 9397ea1abe1aadde08c1574dd68e1f34cc065427 Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Mon, 12 Dec 2022 17:13:34 -0800 Subject: [PATCH] Cranelift: implement general select_spectre_guard fallbacks. (#5420) When adding some optimization rules for `icmp` in the egraph infrastructure, we ended up creating a path to legal CLIF but with patterns unsupported by three of our four backends: specifically, `select_spectre_guard` with a general truthy input, rather than an `icmp`. In #5206 we discussed replacing `select_spectre_guard` with something more specific, and that could still be a long-term solution here, but doing so now would interfere with ongoing refactoring of heap access lowering, so I've opted not to do so. (In that issue I was concerned about complexity and didn't see the need but with this fuzzbug I'm starting to feel a bit differently; maybe we should remove this non-orthogonal op in the long run.) Fixes #5417. --- cranelift/codegen/src/isa/aarch64/lower.isle | 6 ++++++ cranelift/codegen/src/isa/riscv64/lower.isle | 3 +++ cranelift/codegen/src/isa/x64/lower.isle | 5 +++++ .../filetests/filetests/egraph/issue-5417.clif | 15 +++++++++++++++ 4 files changed, 29 insertions(+) create mode 100644 cranelift/filetests/filetests/egraph/issue-5417.clif diff --git a/cranelift/codegen/src/isa/aarch64/lower.isle b/cranelift/codegen/src/isa/aarch64/lower.isle index 2a867c437b..8f22ae599d 100644 --- a/cranelift/codegen/src/isa/aarch64/lower.isle +++ b/cranelift/codegen/src/isa/aarch64/lower.isle @@ -1746,6 +1746,12 @@ (_ InstOutput (side_effect (csdb)))) dst)) +(rule -1 (lower (has_type ty (select_spectre_guard rcond rn rm))) + (let ((rcond Reg (put_in_reg_zext64 rcond))) + (lower_select + (cmp (OperandSize.Size64) rcond (zero_reg)) + (Cond.Ne) ty rn rm))) + ;;;; Rules for `vconst` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (rule (lower (has_type (ty_vec128 _) (vconst (u128_from_constant x)))) diff --git a/cranelift/codegen/src/isa/riscv64/lower.isle b/cranelift/codegen/src/isa/riscv64/lower.isle index 634ea2e81b..b3a7ab3bd3 100644 --- a/cranelift/codegen/src/isa/riscv64/lower.isle +++ b/cranelift/codegen/src/isa/riscv64/lower.isle @@ -831,6 +831,9 @@ (_ Unit (emit (MInst.SelectIf $true (vec_writable_clone dst) r a b)))) (vec_writable_to_regs dst))) +(rule -1 + (lower (has_type ty (select_spectre_guard c @ (value_type cty) x y))) + (gen_select ty (normalize_cmp_value cty c) x y)) ;;;;; Rules for `bmask`;;;;;;;;; (rule diff --git a/cranelift/codegen/src/isa/x64/lower.isle b/cranelift/codegen/src/isa/x64/lower.isle index d6a6dbe225..e4edf44131 100644 --- a/cranelift/codegen/src/isa/x64/lower.isle +++ b/cranelift/codegen/src/isa/x64/lower.isle @@ -2948,6 +2948,11 @@ (rule (lower (select_spectre_guard (icmp cc a b) x y)) (select_icmp (emit_cmp cc a b) x y)) +(rule -1 (lower (has_type ty (select_spectre_guard c @ (value_type (fits_in_64 a_ty)) x y))) + (let ((size OperandSize (raw_operand_size_of_type a_ty)) + (gpr_c Gpr (put_in_gpr c))) + (with_flags (x64_test size gpr_c gpr_c) (cmove_from_values ty (CC.NZ) x y)))) + ;; Rules for `fcvt_from_sint` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (rule 2 (lower (has_type $F32 (fcvt_from_sint a @ (value_type $I8)))) diff --git a/cranelift/filetests/filetests/egraph/issue-5417.clif b/cranelift/filetests/filetests/egraph/issue-5417.clif new file mode 100644 index 0000000000..98cb16eac1 --- /dev/null +++ b/cranelift/filetests/filetests/egraph/issue-5417.clif @@ -0,0 +1,15 @@ +test compile +set opt_level=speed +set use_egraphs=true +target x86_64 +target aarch64 +target s390x +target riscv64 + +function %my_fn(i16) system_v { +block0(v0: i16): + v1 = icmp eq v0, v0 + v2 = select_spectre_guard v1, v1, v1 + return +} +; run: %my_fn(6330)