Change CLIF shuffle to validate lane indices (#5995)

* Change CLIF `shuffle` to validate lane indices

Previously the CLIF `shuffle` instruction did not perform any validation
on the lane shuffle mask and specified that out-of-bounds lanes always
returned 0 as the value. This behavior though is not required by
WebAssembly which validates that lane indices are always in-bounds.
Additionally since these are static immediates even other code
generators should be able to verify that the immediates are in-bounds.

As a result this commit updates the definition of the `shuffle`
instruction to specify that all byte immediates must be in-bounds in the
range of [0, 32). The verifier has been updated and some test cases have
been removed that were testing this functionality.

Closes #5989

* Only generate valid shuffle immediates in fuzzer

cranelift/codegen/meta/src/shared/instructions.rs

@@ -1422,7 +1422,7 @@ pub(crate) fn define(
         Shuffle two vectors using the given immediate bytes. For each of the 16 bytes of the
         immediate, a value i of 0-15 selects the i-th element of the first vector and a value i of
         16-31 selects the (i-16)th element of the second vector. Immediate values outside of the
-        0-31 range place a 0 in the resulting vector lane.
+        0-31 range are not valid.
         "#,
             &formats.shuffle,
         )

cranelift/codegen/src/verifier/mod.rs

@@ -1685,6 +1685,28 @@ impl<'a> Verifier<'a> {
                     Ok(())
                 }
             }
+            ir::InstructionData::Shuffle {
+                opcode: ir::instructions::Opcode::Shuffle,
+                imm,
+                ..
+            } => {
+                let imm = self.func.dfg.immediates.get(imm).unwrap().as_slice();
+                if imm.len() != 16 {
+                    errors.fatal((
+                        inst,
+                        self.context(inst),
+                        format!("the shuffle immediate wasn't 16-bytes long"),
+                    ))
+                } else if let Some(i) = imm.iter().find(|i| **i >= 32) {
+                    errors.fatal((
+                        inst,
+                        self.context(inst),
+                        format!("shuffle immediate index {i} is larger than the maximum 31"),
+                    ))
+                } else {
+                    Ok(())
+                }
+            }
             _ => Ok(()),
         }
     }

cranelift/filetests/filetests/isa/s390x/vec-permute-le-lane.clif

@@ -70,37 +70,6 @@ block0(v0: i8x16, v1: i8x16):
 ;   vperm %v24, %v24, %v25, %v3
 ;   br %r14
 
-function %shuffle_2(i8x16, i8x16) -> i8x16 wasmtime_system_v {
-block0(v0: i8x16, v1: i8x16):
-    v2 = shuffle v0, v1, [0 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47]
-    return v2
-}
-
-; VCode:
-; block0:
-;   vgbm %v3, 1
-;   bras %r1, 20 ; data.u128 0x8080808080808080808080808080800f ; vl %v5, 0(%r1)
-;   vperm %v7, %v24, %v25, %v5
-;   vn %v24, %v3, %v7
-;   br %r14
-; 
-; Disassembled:
-; block0: ; offset 0x0
-;   vgbm %v3, 1
-;   bras %r1, 0x1a
-;   .byte 0x80, 0x80
-;   .byte 0x80, 0x80
-;   .byte 0x80, 0x80
-;   .byte 0x80, 0x80
-;   .byte 0x80, 0x80
-;   .byte 0x80, 0x80
-;   .byte 0x80, 0x80
-;   .byte 0x80, 0x0f
-;   vl %v5, 0(%r1)
-;   vperm %v7, %v24, %v25, %v5
-;   vn %v24, %v3, %v7
-;   br %r14
-
 function %shuffle_vmrhg_xy(i8x16, i8x16) -> i8x16 wasmtime_system_v {
 block0(v0: i8x16, v1: i8x16):
     v2 = shuffle v0, v1, [24 25 26 27 28 29 30 31 8 9 10 11 12 13 14 15]

cranelift/filetests/filetests/isa/s390x/vec-permute.clif

@@ -68,37 +68,6 @@ block0(v0: i8x16, v1: i8x16):
 ;   vperm %v24, %v24, %v25, %v3
 ;   br %r14
 
-function %shuffle_2(i8x16, i8x16) -> i8x16 {
-block0(v0: i8x16, v1: i8x16):
-    v2 = shuffle v0, v1, [0 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47]
-    return v2
-}
-
-; VCode:
-; block0:
-;   vgbm %v3, 32768
-;   bras %r1, 20 ; data.u128 0x00808080808080808080808080808080 ; vl %v5, 0(%r1)
-;   vperm %v7, %v24, %v25, %v5
-;   vn %v24, %v3, %v7
-;   br %r14
-; 
-; Disassembled:
-; block0: ; offset 0x0
-;   vgbm %v3, 0x8000
-;   bras %r1, 0x1a
-;   .byte 0x00, 0x80
-;   .byte 0x80, 0x80
-;   .byte 0x80, 0x80
-;   .byte 0x80, 0x80
-;   .byte 0x80, 0x80
-;   .byte 0x80, 0x80
-;   .byte 0x80, 0x80
-;   .byte 0x80, 0x80
-;   vl %v5, 0(%r1)
-;   vperm %v7, %v24, %v25, %v5
-;   vn %v24, %v3, %v7
-;   br %r14
-
 function %shuffle_vmrhg_xy(i8x16, i8x16) -> i8x16 {
 block0(v0: i8x16, v1: i8x16):
     v2 = shuffle v0, v1, [0 1 2 3 4 5 6 7 16 17 18 19 20 21 22 23]

cranelift/filetests/filetests/isa/x64/shuffle-avx512.clif

@@ -42,55 +42,6 @@ block0(v0: i8x16, v1: i8x16):
 ;   addb %al, (%rax)
 ;   addb %al, (%rax)
 
-function %shuffle_out_of_bounds(i8x16, i8x16) -> i8x16 {
-block0(v0: i8x16, v1: i8x16):
-    ;; pick zero for the first lane, the rest use first lane of v0
-    ;; This should introduce two constants, one for the permutation and one to
-    ;; mask the non-zero values for lanes 1-15
-    v2 = shuffle v0, v1, 0x80000000000000000000000000000000
-    return v2
-}
-
-; VCode:
-;   pushq   %rbp
-;   movq    %rsp, %rbp
-; block0:
-;   movdqa  %xmm0, %xmm6
-;   movdqu  const(0), %xmm0
-;   movdqa  %xmm6, %xmm7
-;   vpermi2b %xmm1, %xmm7, %xmm0, %xmm0
-;   andps   %xmm0, const(1), %xmm0
-;   movq    %rbp, %rsp
-;   popq    %rbp
-;   ret
-; 
-; Disassembled:
-; block0: ; offset 0x0
-;   pushq %rbp
-;   movq %rsp, %rbp
-; block1: ; offset 0x4
-;   movdqa %xmm0, %xmm6
-;   movdqu 0x20(%rip), %xmm0
-;   movdqa %xmm6, %xmm7
-;   vpermi2b %xmm1, %xmm7, %xmm0
-;   andps 0x1f(%rip), %xmm0
-;   movq %rbp, %rsp
-;   popq %rbp
-;   retq
-;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   addb %al, -1(%rax)
-
 function %f3(i8x16, i8x16) -> i8x16 {
 block0(v0: i8x16, v1: i8x16):
     v2 = shuffle v0, v1, [3 0 31 26 4 6 12 11 23 13 24 4 2 15 17 5]

cranelift/filetests/filetests/runtests/simd-shuffle.clif

@@ -14,13 +14,6 @@ block0(v0: i8x16, v1: i8x16):
 }
 ; run: %shuffle_i8x16([1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16], [17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32]) == [4 1 32 27 5 7 13 12 24 14 25 5 3 16 18 6]
 
-function %shuffle_zeros(i8x16, i8x16) -> i8x16 {
-block0(v0: i8x16, v1: i8x16):
-    v2 = shuffle v0, v1, [3 0 32 255 4 6 12 11 23 13 24 4 2 97 17 5]
-    return v2
-}
-; run: %shuffle_zeros([1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16], [17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32]) == [4 1 0 0 5 7 13 12 24 14 25 5 3 0 18 6]
-
 function %shuffle1(i8x16) -> i8x16 {
 block0(v0: i8x16):
     v1 = shuffle v0, v0, [8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23]

cranelift/filetests/filetests/verifier/type_check.clif

@@ -123,3 +123,9 @@ block0:
     v1 = ireduce.i64 v0 ; error: input i32 must be larger than output i64
     return
 }
+
+function %bad_shuffle(i8x16, i8x16) {
+block0(v0: i8x16, v1: i8x16):
+    v2 = shuffle v0, v1, [0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 32] ; error: shuffle immediate index 32 is larger than the maximum 31
+    return
+}

cranelift/fuzzgen/src/function_generator.rs

@@ -339,7 +339,10 @@ fn insert_shuffle(
     let rhs = builder.use_var(fgen.get_variable_of_type(ctrl_type)?);
 
     let mask = {
-        let lanes = fgen.u.arbitrary::<[u8; 16]>()?;
+        let mut lanes = [0u8; 16];
+        for lane in lanes.iter_mut() {
+            *lane = fgen.u.int_in_range(0..=31)?;
+        }
         let lanes = ConstantData::from(lanes.as_ref());
         builder.func.dfg.immediates.push(lanes)
     };