x64: Finish migrating brz and brnz to ISLE (#4614)

elliottt · web-flow · commit dc8362ceec4c · 2022-08-04T12:58:43.000-07:00
#4614
diff --git a/cranelift/codegen/src/isa/x64/inst.isle b/cranelift/codegen/src/isa/x64/inst.isle
@@ -3274,6 +3274,12 @@
       ;; Same flags as `UnorderedOrLessThanOrEqual`.
       (FcmpCondResult.Condition (x64_ucomis a b) (CC.BE)))
 
+;;;; Type Guards ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+
+;; A type guard for matching ints and bools up to 64 bits, or 64 bit references.
+(decl ty_int_bool_or_ref () Type)
+(extern extractor ty_int_bool_or_ref ty_int_bool_or_ref)
+
 ;;;; Atomics ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
 (decl x64_mfence () SideEffectNoResult)
diff --git a/cranelift/codegen/src/isa/x64/inst/emit_tests.rs b/cranelift/codegen/src/isa/x64/inst/emit_tests.rs
@@ -3261,109 +3261,6 @@ fn test_x64_emit() {
         "cmpb    %r13b, %r14b",
     ));
 
-    // ========================================================
-    // TestRmiR
-    insns.push((
-        Inst::test_rmi_r(OperandSize::Size64, RegMemImm::reg(r15), rdx),
-        "4C85FA",
-        "testq   %r15, %rdx",
-    ));
-    insns.push((
-        Inst::test_rmi_r(
-            OperandSize::Size64,
-            RegMemImm::mem(Amode::imm_reg(99, rdi)),
-            rdx,
-        ),
-        "48855763",
-        "testq   99(%rdi), %rdx",
-    ));
-    insns.push((
-        Inst::test_rmi_r(OperandSize::Size64, RegMemImm::imm(127), rdx),
-        "48F7C27F000000",
-        "testq   $127, %rdx",
-    ));
-    insns.push((
-        Inst::test_rmi_r(OperandSize::Size64, RegMemImm::imm(76543210), rdx),
-        "48F7C2EAF48F04",
-        "testq   $76543210, %rdx",
-    ));
-    //
-    insns.push((
-        Inst::test_rmi_r(OperandSize::Size32, RegMemImm::reg(r15), rdx),
-        "4485FA",
-        "testl   %r15d, %edx",
-    ));
-    insns.push((
-        Inst::test_rmi_r(
-            OperandSize::Size32,
-            RegMemImm::mem(Amode::imm_reg(99, rdi)),
-            rdx,
-        ),
-        "855763",
-        "testl   99(%rdi), %edx",
-    ));
-    insns.push((
-        Inst::test_rmi_r(OperandSize::Size32, RegMemImm::imm(76543210), rdx),
-        "F7C2EAF48F04",
-        "testl   $76543210, %edx",
-    ));
-    //
-    insns.push((
-        Inst::test_rmi_r(OperandSize::Size16, RegMemImm::reg(r15), rdx),
-        "664485FA",
-        "testw   %r15w, %dx",
-    ));
-    insns.push((
-        Inst::test_rmi_r(
-            OperandSize::Size16,
-            RegMemImm::mem(Amode::imm_reg(99, rdi)),
-            rdx,
-        ),
-        "66855763",
-        "testw   99(%rdi), %dx",
-    ));
-    insns.push((
-        Inst::test_rmi_r(OperandSize::Size16, RegMemImm::imm(23210), rdx),
-        "66F7C2AA5A",
-        "testw   $23210, %dx",
-    ));
-    //
-    insns.push((
-        Inst::test_rmi_r(OperandSize::Size8, RegMemImm::reg(r15), rdx),
-        "4484FA",
-        "testb   %r15b, %dl",
-    ));
-    insns.push((
-        Inst::test_rmi_r(
-            OperandSize::Size8,
-            RegMemImm::mem(Amode::imm_reg(99, rdi)),
-            rdx,
-        ),
-        "845763",
-        "testb   99(%rdi), %dl",
-    ));
-    insns.push((
-        Inst::test_rmi_r(OperandSize::Size8, RegMemImm::imm(70), rdx),
-        "F6C246",
-        "testb   $70, %dl",
-    ));
-    // Extra byte-cases (paranoia!) for test_rmi_r for first operand = R
-    insns.push((
-        Inst::test_rmi_r(OperandSize::Size8, RegMemImm::reg(rax), rbx),
-        "84C3",
-        "testb   %al, %bl",
-    ));
-    insns.push((
-        Inst::test_rmi_r(OperandSize::Size8, RegMemImm::reg(rcx), rsi),
-        "4084CE",
-        "testb   %cl, %sil",
-    ));
-    insns.push((
-        Inst::test_rmi_r(OperandSize::Size8, RegMemImm::reg(rcx), r10),
-        "4184CA",
-        "testb   %cl, %r10b",
-    ));
-
     // ========================================================
     // SetCC
     insns.push((Inst::setcc(CC::O, w_rsi), "400F90C6", "seto    %sil"));
diff --git a/cranelift/codegen/src/isa/x64/inst/mod.rs b/cranelift/codegen/src/isa/x64/inst/mod.rs
@@ -619,18 +619,6 @@ impl Inst {
         }
     }
 
-    /// Does a comparison of dst & src for operands of size `size`.
-    pub(crate) fn test_rmi_r(size: OperandSize, src: RegMemImm, dst: Reg) -> Inst {
-        src.assert_regclass_is(RegClass::Int);
-        debug_assert_eq!(dst.class(), RegClass::Int);
-        Inst::CmpRmiR {
-            size,
-            src: GprMemImm::new(src).unwrap(),
-            dst: Gpr::new(dst).unwrap(),
-            opcode: CmpOpcode::Test,
-        }
-    }
-
     pub(crate) fn trap(trap_code: TrapCode) -> Inst {
         Inst::Ud2 { trap_code }
     }
@@ -729,14 +717,6 @@ impl Inst {
         Inst::JmpKnown { dst }
     }
 
-    pub(crate) fn jmp_cond(cc: CC, taken: MachLabel, not_taken: MachLabel) -> Inst {
-        Inst::JmpCond {
-            cc,
-            taken,
-            not_taken,
-        }
-    }
-
     pub(crate) fn jmp_unknown(target: RegMem) -> Inst {
         target.assert_regclass_is(RegClass::Int);
         Inst::JmpUnknown { target }
diff --git a/cranelift/codegen/src/isa/x64/lower.isle b/cranelift/codegen/src/isa/x64/lower.isle
@@ -2890,6 +2890,12 @@
 (rule (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)))
 
+(rule (lower_branch (brz val @ (value_type (ty_int_bool_or_ref)) _ _) (two_targets taken not_taken))
+      (side_effect
+        (with_flags_side_effect (cmp_zero_int_bool_ref val)
+                                (jmp_cond (CC.Z) taken not_taken))))
+
+
 (rule (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)))
 
@@ -2900,6 +2906,12 @@
 (rule (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)))
 
+(rule (lower_branch (brnz val @ (value_type (ty_int_bool_or_ref)) _ _) (two_targets taken not_taken))
+      (side_effect
+        (with_flags_side_effect (cmp_zero_int_bool_ref val)
+                                (jmp_cond (CC.NZ) taken not_taken))))
+
+
 ;; Compare an I128 value to zero, returning a flags result suitable for making a
 ;; jump decision. The comparison is implemented as `(hi == 0) && (low == 0)`,
 ;; and the result can be interpreted as follows
@@ -2916,6 +2928,14 @@
                                       (x64_setcc (CC.Z)))))
           (icmp_cond_result (x64_test (OperandSize.Size8) lo_z hi_z) cc)))
 
+
+(decl cmp_zero_int_bool_ref (Value) ProducesFlags)
+(rule (cmp_zero_int_bool_ref val @ (value_type $B1))
+      (x64_test (OperandSize.Size8) (RegMemImm.Imm 1) val))
+(rule (cmp_zero_int_bool_ref val @ (value_type ty))
+      (let ((size OperandSize (raw_operand_size_of_type ty)))
+        (x64_test size val val)))
+
 ;; Rules for `bricmp` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
 (rule (lower_branch (br_icmp cc a b _ _) (two_targets taken not_taken))
diff --git a/cranelift/codegen/src/isa/x64/lower.rs b/cranelift/codegen/src/isa/x64/lower.rs
@@ -13,9 +13,9 @@ use crate::isa::x64::inst::args::*;
 use crate::isa::x64::inst::*;
 use crate::isa::{x64::settings as x64_settings, x64::X64Backend, CallConv};
 use crate::machinst::lower::*;
+use crate::machinst::*;
 use crate::result::CodegenResult;
 use crate::settings::{Flags, TlsModel};
-use crate::{machinst::*, trace};
 use alloc::boxed::Box;
 use smallvec::SmallVec;
 use std::convert::TryFrom;
@@ -33,14 +33,6 @@ fn is_int_or_ref_ty(ty: Type) -> bool {
     }
 }
 
-fn is_bool_ty(ty: Type) -> bool {
-    match ty {
-        types::B1 | types::B8 | types::B16 | types::B32 | types::B64 => true,
-        types::R32 => panic!("shouldn't have 32-bits refs on x64"),
-        _ => false,
-    }
-}
-
 /// Returns whether the given specified `input` is a result produced by an instruction with Opcode
 /// `op`.
 // TODO investigate failures with checking against the result index.
@@ -2758,75 +2750,7 @@ impl LowerBackend for X64Backend {
         };
 
         if branches.len() == 2 {
-            // Must be a conditional branch followed by an unconditional branch.
-            let op0 = ctx.data(branches[0]).opcode();
-            let op1 = ctx.data(branches[1]).opcode();
-
-            trace!(
-                "lowering two-branch group: opcodes are {:?} and {:?}",
-                op0,
-                op1
-            );
-            assert!(op1 == Opcode::Jump);
-
-            let taken = targets[0];
-            // not_taken target is the target of the second branch.
-            let not_taken = targets[1];
-
-            match op0 {
-                Opcode::Brz | Opcode::Brnz => {
-                    let flag_input = InsnInput {
-                        insn: branches[0],
-                        input: 0,
-                    };
-
-                    let src_ty = ctx.input_ty(branches[0], 0);
-
-                    if let Some(_icmp) = matches_input(ctx, flag_input, Opcode::Icmp) {
-                        implemented_in_isle(ctx)
-                    } else if let Some(_fcmp) = matches_input(ctx, flag_input, Opcode::Fcmp) {
-                        implemented_in_isle(ctx)
-                    } else if src_ty == types::I128 {
-                        implemented_in_isle(ctx);
-                    } else if is_int_or_ref_ty(src_ty) || is_bool_ty(src_ty) {
-                        let src = put_input_in_reg(
-                            ctx,
-                            InsnInput {
-                                insn: branches[0],
-                                input: 0,
-                            },
-                        );
-                        let cc = match op0 {
-                            Opcode::Brz => CC::Z,
-                            Opcode::Brnz => CC::NZ,
-                            _ => unreachable!(),
-                        };
-                        // See case for `Opcode::Select` above re: testing the
-                        // boolean input.
-                        let test_input = if src_ty == types::B1 {
-                            // test src, 1
-                            RegMemImm::imm(1)
-                        } else {
-                            assert!(!is_bool_ty(src_ty));
-                            // test src, src
-                            RegMemImm::reg(src)
-                        };
-
-                        ctx.emit(Inst::test_rmi_r(
-                            OperandSize::from_ty(src_ty),
-                            test_input,
-                            src,
-                        ));
-                        ctx.emit(Inst::jmp_cond(cc, taken, not_taken));
-                    } else {
-                        unimplemented!("brz/brnz with non-int type {:?}", src_ty);
-                    }
-                }
-
-                Opcode::BrIcmp | Opcode::Brif | Opcode::Brff => implemented_in_isle(ctx),
-
-                _ => panic!("unexpected branch opcode: {:?}", op0),
-            }
+            implemented_in_isle(ctx)
         } else {
             assert_eq!(branches.len(), 1);
 
diff --git a/cranelift/codegen/src/isa/x64/lower/isle.rs b/cranelift/codegen/src/isa/x64/lower/isle.rs
@@ -3,6 +3,7 @@
 // Pull in the ISLE generated code.
 pub(crate) mod generated_code;
 use crate::{
+    ir::types,
     ir::AtomicRmwOp,
     machinst::{InputSourceInst, Reg, Writable},
 };
@@ -561,6 +562,16 @@ where
         }
     }
 
+    #[inline]
+    fn ty_int_bool_or_ref(&mut self, ty: Type) -> Option<()> {
+        match ty {
+            types::I8 | types::I16 | types::I32 | types::I64 | types::R64 => Some(()),
+            types::B1 | types::B8 | types::B16 | types::B32 | types::B64 => Some(()),
+            types::R32 => panic!("shouldn't have 32-bits refs on x64"),
+            _ => None,
+        }
+    }
+
     #[inline]
     fn intcc_neq(&mut self, x: &IntCC, y: &IntCC) -> Option<IntCC> {
         if x != y {
