ISLE rule cleanups (#5389)

* cranelift-codegen: Use ISLE matching, not same_value

The `same_value` function just wrapped an equality test into an external
constructor, but we can do that with ISLE's equality constraints
instead.

* riscv64: Remove custom condition-code tests

The `lower_icmp` term exists solely to decide whether to sign-extend or
zero-extend the comparison operands, based on whether the condition code
requires a signed comparison. It additionally tested whether the
condition code was == or !=, but produced the same result as for other
unsigned comparisons.

We already have `signed_cond_code` in the ISLE prelude, which classifies
the total-ordering condition codes according to whether they're signed.
It also lumps == and != in the "unsigned" camp, as desired.

So this commit uses the existing method from the prelude instead of
riscv64-local definitions.

Because this version has no constraints on the left-hand side of the
rule in the unsigned case, ISLE generates Rust that always returns
`Some`. That shows that the current use of `unwrap` is justified, at the
only Rust-side call-site of `constructor_lower_icmp`, which is in
cranelift/codegen/src/isa/riscv64/lower/isle.rs.

* ISLE prelude: make offset32 infallible

This extractor always returns `Some`, so it doesn't need to be fallible.

Author: jameysharp

cranelift/codegen/src/isa/aarch64/lower.isle

@@ -189,21 +189,17 @@
 
 ;;;; Rules for `iadd_pairwise` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
-(rule (lower (has_type $I16X8 (iadd_pairwise (swiden_low x) (swiden_high y))))
-      (if-let z (same_value x y))
-      (saddlp8 z))
+(rule (lower (has_type $I16X8 (iadd_pairwise (swiden_low x) (swiden_high x))))
+      (saddlp8 x))
 
-(rule (lower (has_type $I32X4 (iadd_pairwise (swiden_low x) (swiden_high y))))
-      (if-let z (same_value x y))
-      (saddlp16 z))
+(rule (lower (has_type $I32X4 (iadd_pairwise (swiden_low x) (swiden_high x))))
+      (saddlp16 x))
 
-(rule (lower (has_type $I16X8 (iadd_pairwise (uwiden_low x) (uwiden_high y))))
-      (if-let z (same_value x y))
-      (uaddlp8 z))
+(rule (lower (has_type $I16X8 (iadd_pairwise (uwiden_low x) (uwiden_high x))))
+      (uaddlp8 x))
 
-(rule (lower (has_type $I32X4 (iadd_pairwise (uwiden_low x) (uwiden_high y))))
-      (if-let z (same_value x y))
-      (uaddlp16 z))
+(rule (lower (has_type $I32X4 (iadd_pairwise (uwiden_low x) (uwiden_high x))))
+      (uaddlp16 x))
 
 (rule -1 (lower (has_type ty (iadd_pairwise x y)))
       (addp x y (vector_size ty)))

cranelift/codegen/src/isa/riscv64/inst.isle

@@ -1766,29 +1766,13 @@
   (value_regs_get x 0))
 (convert ValueRegs Reg convert_valueregs_reg)
 
-;;; intcc is not equal nor ne.
-;;; intcc is >= <= ... 
-;;; return alongside with if signed.
-(decl intcc_is_gt_etc (IntCC bool) IntCC)
-(extern extractor intcc_is_gt_etc intcc_is_gt_etc)
-
-(decl intcc_is_eq_or_ne (IntCC) IntCC)
-(extern extractor intcc_is_eq_or_ne intcc_is_eq_or_ne)
-
 ;;; lower icmp 
 (decl lower_icmp (IntCC ValueRegs ValueRegs Type) Reg)
-;;; eq or ne.
-(rule -1
-  (lower_icmp (intcc_is_eq_or_ne cc) x y ty)
-  (gen_icmp cc (ext_int_if_need $false x ty) (ext_int_if_need $false y ty) ty))
-;;;; singed >= ... 
-(rule
-  (lower_icmp (intcc_is_gt_etc cc $true) x y ty)
+(rule 1 (lower_icmp cc x y ty)
+  (if (signed_cond_code cc))
   (gen_icmp cc (ext_int_if_need $true x ty) (ext_int_if_need $true y ty) ty))
-;;;; unsigned >= ... 
-(rule
-  (lower_icmp (intcc_is_gt_etc cc $false) x y ty)
-  (gen_icmp cc (ext_int_if_need $false x ty ) (ext_int_if_need $false y ty) ty))
+(rule (lower_icmp cc x y ty)
+  (gen_icmp cc (ext_int_if_need $false x ty) (ext_int_if_need $false y ty) ty))
 
 (decl lower_icmp_over_flow (ValueRegs ValueRegs Type) Reg)
 

cranelift/codegen/src/isa/riscv64/lower/isle.rs

@@ -406,29 +406,6 @@ impl generated_code::Context for IsleContext<'_, '_, MInst, Flags, IsaFlags, 6>
         tmp.to_reg()
     }
 
-    fn intcc_is_gt_etc(&mut self, cc: &IntCC) -> Option<(IntCC, bool)> {
-        let cc = *cc;
-        match cc {
-            IntCC::SignedLessThan => Some((cc, true)),
-            IntCC::SignedGreaterThanOrEqual => Some((cc, true)),
-            IntCC::SignedGreaterThan => Some((cc, true)),
-            IntCC::SignedLessThanOrEqual => Some((cc, true)),
-            //
-            IntCC::UnsignedLessThan => Some((cc, false)),
-            IntCC::UnsignedGreaterThanOrEqual => Some((cc, false)),
-            IntCC::UnsignedGreaterThan => Some((cc, false)),
-            IntCC::UnsignedLessThanOrEqual => Some((cc, false)),
-            _ => None,
-        }
-    }
-    fn intcc_is_eq_or_ne(&mut self, cc: &IntCC) -> Option<IntCC> {
-        let cc = *cc;
-        if cc == IntCC::Equal || cc == IntCC::NotEqual {
-            Some(cc)
-        } else {
-            None
-        }
-    }
     fn lower_br_table(&mut self, index: Reg, targets: &VecMachLabel) -> InstOutput {
         let tmp1 = self.temp_writable_reg(I64);
         let targets: Vec<BranchTarget> = targets

cranelift/codegen/src/isle_prelude.rs

@@ -524,9 +524,9 @@ macro_rules! isle_common_prelude_methods {
         }
 
         #[inline]
-        fn offset32(&mut self, x: Offset32) -> Option<u32> {
+        fn offset32(&mut self, x: Offset32) -> u32 {
             let x: i32 = x.into();
-            Some(x as u32)
+            x as u32
         }
 
         #[inline]

cranelift/codegen/src/machinst/isle.rs

@@ -46,15 +46,6 @@ macro_rules! isle_lower_prelude_methods {
     () => {
         isle_common_prelude_methods!();
 
-        #[inline]
-        fn same_value(&mut self, a: Value, b: Value) -> Option<Value> {
-            if a == b {
-                Some(a)
-            } else {
-                None
-            }
-        }
-
         #[inline]
         fn value_type(&mut self, val: Value) -> Type {
             self.lower_ctx.dfg().value_type(val)

cranelift/codegen/src/prelude.isle

@@ -57,7 +57,7 @@
 ;; Extractor that pulls apart an Offset32 into a u32 with the raw
 ;; signed-32-bit twos-complement bits.
 (decl offset32 (u32) Offset32)
-(extern extractor offset32 offset32)
+(extern extractor infallible offset32 offset32)
 
 ;; Pure/fallible constructor that tests if one u32 is less than or
 ;; equal to another.

cranelift/codegen/src/prelude_lower.isle

@@ -189,10 +189,6 @@
 (extractor (unwrap_head_value_list_2 head1 head2 tail)
            (value_list_slice (value_slice_unwrap head1 (value_slice_unwrap head2 tail))))
 
-;; Constructor to test whether two values are same.
-(decl pure same_value (Value Value) Value)
-(extern constructor same_value same_value)
-
 ;; Turn a `Writable<Reg>` into a `Reg` via `Writable::to_reg`.
 (decl writable_reg_to_reg (WritableReg) Reg)
 (extern constructor writable_reg_to_reg writable_reg_to_reg)