Cranelift: aarch64: lower umin.i64 and friends (bytecodealliance#5495)

* Cranelift: aarch64: lower umin.i64 and friends * fuzzgen: Enable integer-min/max for aarch64
ahaoboy · Dec 30, 2022 · fe992c2 · fe992c2
1 parent ff995d9
commit fe992c2
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 22 deletions.
diff --git a/cranelift/codegen/src/isa/aarch64/lower.isle b/cranelift/codegen/src/isa/aarch64/lower.isle
@@ -887,6 +887,36 @@
 
 ;;; Rules for integer min/max: umin, smin, umax, smax ;;;;;;;;;;;;;;;;;;;;;;;;;
 
+;; `i64` and smaller.
+
+;; cmp     $x, $y
+;; csel    .., $x, $y, $cc
+(decl cmp_and_choose (Type Cond bool Value Value) ValueRegs)
+(rule (cmp_and_choose (fits_in_64 ty) cc _ x y)
+      (let ((x Reg (put_in_reg x))
+            (y Reg (put_in_reg y)))
+       (with_flags_reg (cmp (operand_size ty) x y)
+                       (csel cc x y))))
+
+;; `i16` and `i8` min/max require sign extension as
+;; the comparison operates on (at least) 32 bits.
+(rule 1 (cmp_and_choose (fits_in_16 ty) cc signed x y)
+      (let ((x Reg (extend (put_in_reg x) signed (ty_bits ty) 32))
+            (y Reg (extend (put_in_reg y) signed (ty_bits ty) 32)))
+      (with_flags_reg (cmp (operand_size ty) x y)
+                      (csel cc x y))))
+
+(rule 2 (lower (has_type (and (fits_in_64 ty) (ty_int _)) (umin x y)))
+      (cmp_and_choose ty (Cond.Lo) $false x y))
+(rule 2 (lower (has_type (and (fits_in_64 ty) (ty_int _)) (smin x y)))
+      (cmp_and_choose ty (Cond.Lt) $true x y))
+(rule 2 (lower (has_type (and (fits_in_64 ty) (ty_int _)) (umax x y)))
+      (cmp_and_choose ty (Cond.Hi) $false x y))
+(rule 2 (lower (has_type (and (fits_in_64 ty) (ty_int _)) (smax x y)))
+      (cmp_and_choose ty (Cond.Gt) $true x y))
+
+;; Vector types.
+
 (rule (lower (has_type ty @ (not_i64x2) (smin x y)))
       (vec_rrr (VecALUOp.Smin) x y (vector_size ty)))
 

diff --git a/cranelift/filetests/filetests/runtests/integer-minmax.clif b/cranelift/filetests/filetests/runtests/integer-minmax.clif
@@ -1,6 +1,6 @@
 test interpret
 test run
-; target aarch64
+target aarch64
 ; target s390x
 target x86_64
 target riscv64
@@ -134,4 +134,19 @@ block0(v0: i64, v1: i64):
 ; run: %umin_max_i64(1, 2) == [1, 2]
 ; run: %umin_max_i64(2, 1) == [1, 2]
 ; run: %umin_max_i64(0x7fffffff, 0x80000000) == [0x7fffffff, 0x80000000]
-; run: %umin_max_i64(0x7fffffffffffffff, 0x8000000000000000) == [0x7fffffffffffffff, 0x8000000000000000]
+; run: %umin_max_i64(0x7fffffffffffffff, 0x8000000000000000) == [0x7fffffffffffffff, 0x8000000000000000]
+
+; make sure that upper/uninitialized parts of operand registers do not leak
+; inside of the comparison
+function %umin_max_i32_trunc_to_i8(i32, i32) -> i8, i8 {
+block0(v0: i32, v1: i32):
+    v2 = ireduce.i8 v0
+    v3 = ireduce.i8 v1
+    v4 = umin.i8 v2, v3
+    v5 = umax.i8 v2, v3
+    return v4, v5
+}
+; run: %umin_max_i32_trunc_to_i8(1, 2) == [1, 2]
+; run: %umin_max_i32_trunc_to_i8(0xBB01, 0xAA02) == [1, 2]
+; run: %umin_max_i32_trunc_to_i8(0xBB02, 0xABCD0001) == [1, 2]
+; run: %umin_max_i32_trunc_to_i8(0x1234567f, 0x12345680) == [0x7f, 0x80]
diff --git a/cranelift/fuzzgen/src/function_generator.rs b/cranelift/fuzzgen/src/function_generator.rs
@@ -365,58 +365,42 @@ const OPCODE_SIGNATURES: &'static [(
     #[cfg(not(any(target_arch = "x86_64", target_arch = "aarch64")))]
     (Opcode::Iabs, &[I128], &[I128], insert_opcode),
     // Smin
-    // smin not implemented in some backends:
+    // smin.i128 is not implemented in some backends:
     //   x64: https://github.com/bytecodealliance/wasmtime/issues/3370
     //   aarch64: https://github.com/bytecodealliance/wasmtime/issues/4313
-    #[cfg(not(target_arch = "aarch64"))]
     (Opcode::Smin, &[I8, I8], &[I8], insert_opcode),
-    #[cfg(not(target_arch = "aarch64"))]
     (Opcode::Smin, &[I16, I16], &[I16], insert_opcode),
-    #[cfg(not(target_arch = "aarch64"))]
     (Opcode::Smin, &[I32, I32], &[I32], insert_opcode),
-    #[cfg(not(target_arch = "aarch64"))]
     (Opcode::Smin, &[I64, I64], &[I64], insert_opcode),
     #[cfg(not(any(target_arch = "x86_64", target_arch = "aarch64")))]
     (Opcode::Smin, &[I128, I128], &[I128], insert_opcode),
     // Umin
-    // umin not implemented in some backends:
+    // umin.i128 is not implemented in some backends:
     //   x64: https://github.com/bytecodealliance/wasmtime/issues/3370
     //   aarch64: https://github.com/bytecodealliance/wasmtime/issues/4313
-    #[cfg(not(target_arch = "aarch64"))]
     (Opcode::Umin, &[I8, I8], &[I8], insert_opcode),
-    #[cfg(not(target_arch = "aarch64"))]
     (Opcode::Umin, &[I16, I16], &[I16], insert_opcode),
-    #[cfg(not(target_arch = "aarch64"))]
     (Opcode::Umin, &[I32, I32], &[I32], insert_opcode),
-    #[cfg(not(target_arch = "aarch64"))]
     (Opcode::Umin, &[I64, I64], &[I64], insert_opcode),
     #[cfg(not(any(target_arch = "x86_64", target_arch = "aarch64")))]
     (Opcode::Umin, &[I128, I128], &[I128], insert_opcode),
     // Smax
-    // smax not implemented in some backends:
+    // smax.i128 is not implemented in some backends:
     //   x64: https://github.com/bytecodealliance/wasmtime/issues/3370
     //   aarch64: https://github.com/bytecodealliance/wasmtime/issues/4313
-    #[cfg(not(target_arch = "aarch64"))]
     (Opcode::Smax, &[I8, I8], &[I8], insert_opcode),
-    #[cfg(not(target_arch = "aarch64"))]
     (Opcode::Smax, &[I16, I16], &[I16], insert_opcode),
-    #[cfg(not(target_arch = "aarch64"))]
     (Opcode::Smax, &[I32, I32], &[I32], insert_opcode),
-    #[cfg(not(target_arch = "aarch64"))]
     (Opcode::Smax, &[I64, I64], &[I64], insert_opcode),
     #[cfg(not(any(target_arch = "x86_64", target_arch = "aarch64")))]
     (Opcode::Smax, &[I128, I128], &[I128], insert_opcode),
     // Umax
-    // umax not implemented in some backends:
+    // umax.i128 is not implemented in some backends:
     //   x64: https://github.com/bytecodealliance/wasmtime/issues/3370
     //   aarch64: https://github.com/bytecodealliance/wasmtime/issues/4313
-    #[cfg(not(target_arch = "aarch64"))]
     (Opcode::Umax, &[I8, I8], &[I8], insert_opcode),
-    #[cfg(not(target_arch = "aarch64"))]
     (Opcode::Umax, &[I16, I16], &[I16], insert_opcode),
-    #[cfg(not(target_arch = "aarch64"))]
     (Opcode::Umax, &[I32, I32], &[I32], insert_opcode),
-    #[cfg(not(target_arch = "aarch64"))]
     (Opcode::Umax, &[I64, I64], &[I64], insert_opcode),
     #[cfg(not(any(target_arch = "x86_64", target_arch = "aarch64")))]
     (Opcode::Umax, &[I128, I128], &[I128], insert_opcode),