x64: Mask shift amounts for small types (#4752)

* x64: Mask shift amounts for small types

* cranelift: Disable i128 shifts in fuzzer again

They are fixed. But we had a bunch of fuzzgen issues come in, and we don't want to accidentaly mark them as fixed

* cranelift: Avoid masking shifts for 32 and 64 bit cases

* cranelift: Add const shift tests and fix them

* cranelift: Remove const `rotl` cases

Now that `put_masked_in_imm8_gpr` works properly we can simplify rotl/rotr

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

@@ -1057,7 +1057,12 @@
 ;;
 ;; This is used when lowering various shifts and rotates.
 (decl put_masked_in_imm8_gpr (Value Type) Imm8Gpr)
-(extern constructor put_masked_in_imm8_gpr put_masked_in_imm8_gpr)
+(rule (put_masked_in_imm8_gpr (u64_from_iconst amt) ty)
+      (const_to_type_masked_imm8 amt ty))
+(rule (put_masked_in_imm8_gpr amt (fits_in_16 ty))
+      (x64_and $I64 (value_regs_get_gpr amt 0) (RegMemImm.Imm (shift_mask ty))))
+(rule (put_masked_in_imm8_gpr amt ty)
+      (value_regs_get_gpr amt 0))
 
 ;; Condition codes
 (type CC extern

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

@@ -793,11 +793,6 @@
 (rule (lower (has_type (fits_in_64 ty) (rotl src amt)))
         (x64_rotl ty src (put_masked_in_imm8_gpr amt ty)))
 
-(rule (lower (has_type (fits_in_64 ty)
-                       (rotl src (u64_from_iconst amt))))
-      (x64_rotl ty src
-                (const_to_type_masked_imm8 amt ty)))
-
 
 ;; `i128`.
 
@@ -819,11 +814,6 @@
 (rule (lower (has_type (fits_in_64 ty) (rotr src amt)))
         (x64_rotr ty src (put_masked_in_imm8_gpr amt ty)))
 
-(rule (lower (has_type (fits_in_64 ty)
-                       (rotr src (u64_from_iconst amt))))
-      (x64_rotr ty src
-                (const_to_type_masked_imm8 amt ty)))
-
 
 ;; `i128`.
 

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

@@ -154,23 +154,6 @@ impl Context for IsleContext<'_, '_, MInst, Flags, IsaFlags, 6> {
         RegMem::reg(self.put_in_reg(val))
     }
 
-    fn put_masked_in_imm8_gpr(&mut self, val: Value, ty: Type) -> Imm8Gpr {
-        let inputs = self.lower_ctx.get_value_as_source_or_const(val);
-
-        if let Some(c) = inputs.constant {
-            let mask = 1_u64.checked_shl(ty.bits()).map_or(u64::MAX, |x| x - 1);
-            return Imm8Gpr::new(Imm8Reg::Imm8 {
-                imm: (c & mask) as u8,
-            })
-            .unwrap();
-        }
-
-        Imm8Gpr::new(Imm8Reg::Reg {
-            reg: self.put_in_regs(val).regs()[0],
-        })
-        .unwrap()
-    }
-
     #[inline]
     fn encode_fcmp_imm(&mut self, imm: &FcmpImm) -> u8 {
         imm.encode()
@@ -272,7 +255,7 @@ impl Context for IsleContext<'_, '_, MInst, Flags, IsaFlags, 6> {
 
     #[inline]
     fn const_to_type_masked_imm8(&mut self, c: u64, ty: Type) -> Imm8Gpr {
-        let mask = 1_u64.checked_shl(ty.bits()).map_or(u64::MAX, |x| x - 1);
+        let mask = self.shift_mask(ty) as u64;
         Imm8Gpr::new(Imm8Reg::Imm8 {
             imm: (c & mask) as u8,
         })
@@ -281,6 +264,8 @@ impl Context for IsleContext<'_, '_, MInst, Flags, IsaFlags, 6> {
 
     #[inline]
     fn shift_mask(&mut self, ty: Type) -> u32 {
+        debug_assert!(ty.lane_bits().is_power_of_two());
+
         ty.lane_bits() - 1
     }
 

cranelift/filetests/filetests/isa/x64/i128.clif

@@ -867,8 +867,8 @@ block0(v0: i8, v1: i128):
 ;   pushq   %rbp
 ;   movq    %rsp, %rbp
 ; block0:
-;   movq    %rsi, %r9
-;   movq    %r9, %rcx
+;   movq    %rsi, %rcx
+;   andq    %rcx, $7, %rcx
 ;   shlb    %cl, %dil, %dil
 ;   movq    %rdi, %rax
 ;   movq    %rbp, %rsp