aarch64: Fix miscompile lowering the extr instruction (#12907)

This commit fixes a miscompile in the lowering of the `extr` instruction
for the aarch64 backend where one of the shift operands is 0. In this
edge case the generated `extr` instruction did not match the input CLIF
semantics, calculating a different value. The fix here is to only use
the `extr` instruction when both immediates are larger than 0.

Author: alexcrichton

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

@@ -1455,13 +1455,22 @@
 ;; `bor`.
 ;;
 ;; (x << xs) | (y >> ys) if (xs + ys == widthof(ty)) => extr(x, y, ys)
+;;
+;; Note that both `xs` and `ys` must be larger than 0. If either one is 0 and
+;; they sum to the type width then it means that the shifts don't actually do
+;; anything CLIF-wise and this should compile down to a `bor` operation. Leave
+;; that edge case to the mid-end and only lower to `extr` here.
 (rule 5 (lower (has_type (ty_32_or_64 ty)
   (bor _ (ishl _ x (u8_from_iconst xs)) (ushr _ y (u8_from_iconst ys)))))
   (if-let true (u64_eq (ty_bits ty) (u64_wrapping_add xs ys)))
+  (if-let true (u64_gt xs 0))
+  (if-let true (u64_gt ys 0))
   (a64_extr ty x y (imm_shift_from_u8 ys)))
 (rule 5 (lower (has_type (ty_32_or_64 ty)
   (bor _ (ushr _ y (u8_from_iconst ys)) (ishl _ x (u8_from_iconst xs)))))
   (if-let true (u64_eq (ty_bits ty) (u64_wrapping_add xs ys)))
+  (if-let true (u64_gt xs 0))
+  (if-let true (u64_gt ys 0))
   (a64_extr ty x y (imm_shift_from_u8 ys)))
 
 ;;;; Rules for `bxor` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

cranelift/filetests/filetests/runtests/bitops.clif

@@ -224,8 +224,8 @@ block0(v0: i32, v1: i32):
 
 function %test_bxor_band_band(i32, i32, i32) -> i32 fast {
 block0(v0: i32, v1: i32, v2: i32):
-    v3 = band v0, v1  
-    v4 = band v0, v2 
+    v3 = band v0, v1
+    v4 = band v0, v2
     v5 = bxor v3, v4
     return v5
 }
@@ -731,4 +731,15 @@ block0(v0: i32, v1: i32):
 
 ; run: %test_bor_band_bnot(0, 0) == 0
 ; run: %test_bor_band_bnot(1, 0) == 1
-; run: %test_bor_band_bnot(0xf0, 0x0f) == 0xff
+; run: %test_bor_band_bnot(0xf0, 0x0f) == 0xff
+
+function %test_aarch64_extr(i32, i32) -> i32 fast {
+block0(v0: i32, v1: i32):
+    v2 = ishl_imm v0, 32
+    v3 = ushr_imm v1, 0
+    v4 = bor v2, v3
+    return v4
+}
+
+; run: %test_aarch64_extr(1, 0) == 1
+; run: %test_aarch64_extr(0, 1) == 1

tests/disas/aarch64-extr.wat

@@ -26,6 +26,14 @@
     i32.const 11
     i32.shr_u
     i32.or)
+  (func $i32_0 (param i32 i32) (result i32)
+    local.get 0
+    i32.const 32
+    i32.shl
+    local.get 1
+    i32.const 0
+    i32.shr_u
+    i32.or)
 
   (func $i64_21 (param i64 i64) (result i64)
     local.get 0
@@ -51,6 +59,14 @@
     i64.const 11
     i64.shr_u
     i64.or)
+  (func $i64_0 (param i64 i64) (result i64)
+    local.get 0
+    i64.const 64
+    i64.shl
+    local.get 1
+    i64.const 0
+    i64.shr_u
+    i64.or)
 )
 
 ;; wasm[0]::function[0]::i32_21:
@@ -74,23 +90,37 @@
 ;;       ldp     x29, x30, [sp], #0x10
 ;;       ret
 ;;
-;; wasm[0]::function[3]::i64_21:
+;; wasm[0]::function[3]::i32_0:
+;;       stp     x29, x30, [sp, #-0x10]!
+;;       mov     x29, sp
+;;       orr     w2, w4, w5
+;;       ldp     x29, x30, [sp], #0x10
+;;       ret
+;;
+;; wasm[0]::function[4]::i64_21:
 ;;       stp     x29, x30, [sp, #-0x10]!
 ;;       mov     x29, sp
 ;;       extr    x2, x4, x5, #0x15
 ;;       ldp     x29, x30, [sp], #0x10
 ;;       ret
 ;;
-;; wasm[0]::function[4]::i64_21_swapped:
+;; wasm[0]::function[5]::i64_21_swapped:
 ;;       stp     x29, x30, [sp, #-0x10]!
 ;;       mov     x29, sp
 ;;       extr    x2, x4, x5, #0x15
 ;;       ldp     x29, x30, [sp], #0x10
 ;;       ret
 ;;
-;; wasm[0]::function[5]::i64_11:
+;; wasm[0]::function[6]::i64_11:
 ;;       stp     x29, x30, [sp, #-0x10]!
 ;;       mov     x29, sp
 ;;       extr    x2, x4, x5, #0xb
 ;;       ldp     x29, x30, [sp], #0x10
 ;;       ret
+;;
+;; wasm[0]::function[7]::i64_0:
+;;       stp     x29, x30, [sp, #-0x10]!
+;;       mov     x29, sp
+;;       orr     x2, x4, x5
+;;       ldp     x29, x30, [sp], #0x10
+;;       ret

tests/misc_testsuite/aarch64-extr.wast

@@ -0,0 +1,11 @@
+(module
+  (memory 1)
+  (func (export "a64-extr") (param i32 i32) (result i32)
+    (i32.or
+      (i32.shl (local.get 0) (i32.const 32))
+      (i32.shr_u (local.get 1) (i32.const 0))
+    )
+  )
+)
+
+(assert_return (invoke "a64-extr" (i32.const 65536) (i32.const 0)) (i32.const 65536))