fix MIN / -1 cprop and add negative tests for things simplify_skeleton cannot handle yet

Author: fitzgen

cranelift/codegen/src/isle_prelude.rs

@@ -105,10 +105,25 @@ macro_rules! isle_common_prelude_methods {
         }
 
         #[inline]
-        fn u64_sdiv(&mut self, x: u64, y: u64) -> Option<u64> {
-            let x = x as i64;
-            let y = y as i64;
-            x.checked_div(y).map(|d| d as u64)
+        fn imm64_sdiv(&mut self, ty: Type, x: Imm64, y: Imm64) -> Option<Imm64> {
+            // Sign extend `x` and `y`.
+            let shift = u32::checked_sub(64, ty.bits()).unwrap_or(0);
+            let x = (x.bits() << shift) >> shift;
+            let y = (y.bits() << shift) >> shift;
+
+            // NB: We can't rely on `checked_div` to detect `ty::MIN / -1`
+            // (which overflows and should trap) because we are working with
+            // `i64` values here, and `i32::MIN != i64::MIN`, for
+            // example. Therefore, we have to explicitly check for this case
+            // ourselves.
+            let min = ((self.ty_smin(ty) as i64) << shift) >> shift;
+            if x == min && y == -1 {
+                return None;
+            }
+
+            let ty_mask = self.ty_mask(ty) as i64;
+            let result = x.checked_div(y)? & ty_mask;
+            Some(Imm64::new(result))
         }
 
         #[inline]

cranelift/codegen/src/opts/cprop.isle

@@ -19,10 +19,10 @@
       (subsume (iconst ty (imm64_masked ty (u64_mul k1 k2)))))
 
 (rule (simplify_skeleton
-       (sdiv (iconst_s ty k1)
-             (iconst_s ty k2)))
-      (if-let d (u64_sdiv (i64_as_u64 k1) (i64_as_u64 k2)))
-      (iconst ty (imm64_masked ty d)))
+       (sdiv (iconst ty k1)
+             (iconst _ k2)))
+      (if-let d (imm64_sdiv ty k1 k2))
+      (iconst ty d))
 
 (rule (simplify_skeleton
        (udiv (iconst_u ty k1)

cranelift/codegen/src/prelude.isle

@@ -228,9 +228,6 @@
 (decl pure u64_mul (u64 u64) u64)
 (extern constructor u64_mul u64_mul)
 
-(decl pure partial u64_sdiv (u64 u64) u64)
-(extern constructor u64_sdiv u64_sdiv)
-
 (decl pure partial u64_udiv (u64 u64) u64)
 (extern constructor u64_udiv u64_udiv)
 
@@ -246,6 +243,9 @@
 (decl pure u64_shl (u64 u64) u64)
 (extern constructor u64_shl u64_shl)
 
+(decl pure partial imm64_sdiv (Type Imm64 Imm64) Imm64)
+(extern constructor imm64_sdiv imm64_sdiv)
+
 (decl pure imm64_shl (Type Imm64 Imm64) Imm64)
 (extern constructor imm64_shl imm64_shl)
 

cranelift/filetests/filetests/egraph/skeleton.clif

@@ -89,3 +89,62 @@ block0(v0: i32):
     return v2
 }
 ; check: return v0
+
+;;;;;;;;;; Tests for `simplify_skeleton` TODOs ;;;;;;;;;;;;
+;;
+;; What follows are tests for patterns that `simplify_skeleton` *should* clean
+;; up, but is unable to at this moment in time.
+
+function %int_min_sdiv_neg_one() -> i32 {
+block0():
+    v0 = iconst.i32 0x80000000
+    v1 = iconst.i32 -1
+    v2 = sdiv v0, v1
+    return v2
+}
+; check:  v2 = sdiv v0, v1
+; nextln: return v2
+
+function %sdiv_zero(i32) -> i32 {
+block0(v0: i32):
+    v1 = iconst.i32 0
+    v2 = sdiv v0, v1
+    return v2
+}
+; check:  v2 = sdiv v0, v1
+; nextln: return v2
+
+function %udiv_zero(i32) -> i32 {
+block0(v0: i32):
+    v1 = iconst.i32 0
+    v2 = udiv v0, v1
+    return v2
+}
+; check:  v2 = udiv v0, v1
+; nextln: return v2
+
+function %always_trapping_trapz() -> i32 {
+block0:
+    v0 = iconst.i32 0
+    trapz v0, user42
+    return v0
+}
+; check: trapz v0, user42
+
+function %always_trapping_trapnz() -> i32 {
+block0:
+    v0 = iconst.i32 1
+    trapnz v0, user42
+    return v0
+}
+; check: trapnz v0, user42
+
+function %always_trapping_uadd_overflow_trap() -> i32 {
+block0:
+    v0 = iconst.i32 0xffffffff
+    v1 = iconst.i32 1
+    v2 = uadd_overflow_trap v0, v1, user42
+    return v2
+}
+; check:  v2 = uadd_overflow_trap v0, v1, user42
+; nextln: return v2