perf(x64): optimize i8 multiplication by constants

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

@@ -1098,8 +1098,8 @@
 
 ;; 8-bit base case, needs a special instruction encoding and additionally
 ;; move sinkable loads to the right.
-(rule -8 (lower (imul $I8 x y)) (x64_mul8 false x y))
-(rule -7 (lower (imul $I8 (sinkable_load x) y)) (x64_mul8 false y x))
+(rule -10 (lower (imul $I8 x y)) (x64_mul8 false x y))
+(rule -9 (lower (imul $I8 (sinkable_load x) y)) (x64_mul8 false y x))
 
 ;; 16-to-64-bit base cases, same as above by moving sinkable loads to the right.
 (rule -6 (lower (imul (ty_int_ref_16_to_64 ty) x y))
@@ -1113,6 +1113,14 @@
 (rule -3 (lower (imul (ty_int_ref_16_to_64 ty) (i32_from_iconst x) y))
          (x64_imul_imm ty y x))
 
+;; i8 multiplication can use a 32-bit immediate multiply: only the low byte of
+;; each operand affects the low byte of the result. Force the other operand into
+;; a register so that a sinkable i8 load is never widened to a 32-bit load.
+(rule -8 (lower (imul $I8 x (i32_from_iconst y)))
+         (x64_imul_imm $I32 (put_in_gpr x) y))
+(rule -7 (lower (imul $I8 (i32_from_iconst x) y))
+         (x64_imul_imm $I32 (put_in_gpr y) x))
+
 ;; Special case widening multiplication from 8-to-16-bits with a single
 ;; instruction since the 8-bit-multiply places both the high and low halves in
 ;; the same register

cranelift/filetests/filetests/isa/x64/imul-i8-const.clif

@@ -0,0 +1,82 @@
+test compile
+set unwind_info=false
+target x86_64
+set opt_level=speed
+target x86_64
+
+;; Regression test for #14660: use a 32-bit immediate multiply for an i8
+;; constant on either side, without a byte multiply or constant-pool load.
+
+function %imul_i8_const_rhs(i8) -> i8 {
+block0(v0: i8):
+    v1 = iconst.i8 97
+    v2 = imul v0, v1
+    return v2
+}
+
+; not: mulb
+; not: %rip
+; check: block0:
+; nextln: imull $$0x61, %edi, %eax
+; nextln: movq %rbp, %rsp
+; nextln: popq %rbp
+; nextln: retq
+; not: mulb
+; not: %rip
+
+function %imul_i8_const_lhs(i8) -> i8 {
+block0(v0: i8):
+    v1 = iconst.i8 97
+    v2 = imul v1, v0
+    return v2
+}
+
+; not: mulb
+; not: %rip
+; check: block0:
+; nextln: imull $$0x61, %edi, %eax
+; nextln: movq %rbp, %rsp
+; nextln: popq %rbp
+; nextln: retq
+; not: mulb
+; not: %rip
+
+;; The i8 load must stay a byte load. In particular, do not fold it into
+;; a 32-bit memory operand of imull, which would read past the input byte.
+function %imul_i8_load_const_rhs(i64) -> i8 {
+block0(v0: i64):
+    v1 = load.i8 v0
+    v2 = iconst.i8 97
+    v3 = imul v1, v2
+    return v3
+}
+
+; not: mulb
+; not: %rip
+; check: block0:
+; nextln: movzb$(=q|l) (%rdi), %$(=[a-z0-9]+)
+; nextln: imull $$0x61, %$(=[a-z0-9]+), %eax
+; nextln: movq %rbp, %rsp
+; nextln: popq %rbp
+; nextln: retq
+; not: mulb
+; not: %rip
+
+function %imul_i8_load_const_lhs(i64) -> i8 {
+block0(v0: i64):
+    v1 = load.i8 v0
+    v2 = iconst.i8 97
+    v3 = imul v2, v1
+    return v3
+}
+
+; not: mulb
+; not: %rip
+; check: block0:
+; nextln: movzb$(=q|l) (%rdi), %$(=[a-z0-9]+)
+; nextln: imull $$0x61, %$(=[a-z0-9]+), %eax
+; nextln: movq %rbp, %rsp
+; nextln: popq %rbp
+; nextln: retq
+; not: mulb
+; not: %rip

cranelift/filetests/filetests/isa/x64/mul.clif

@@ -237,8 +237,7 @@ block0(v0: i8):
 ;   pushq %rbp
 ;   movq %rsp, %rbp
 ; block0:
-;   movq %rdi, %rax
-;   mulb (%rip) ;; implicit: %ax
+;   imull $0x61, %edi, %eax
 ;   movq %rbp, %rsp
 ;   popq %rbp
 ;   retq
@@ -248,17 +247,10 @@ block0(v0: i8):
 ;   pushq %rbp
 ;   movq %rsp, %rbp
 ; block1: ; offset 0x4
-;   movq %rdi, %rax
-;   mulb 0xa(%rip)
+;   imull $0x61, %edi, %eax
 ;   movq %rbp, %rsp
 ;   popq %rbp
 ;   retq
-;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   addb %ah, (%rcx)
-;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   addb %al, (%rax)
 
 function %imul_i16_const(i16) -> i16{
 block0(v0: i16):
@@ -699,4 +691,3 @@ block0:
 ;   andb (%rax), %al
 ;   addb %al, (%rax)
 ;   .byte 0x00
-

cranelift/filetests/filetests/runtests/imul-i8-const.clif

@@ -0,0 +1,71 @@
+test interpret
+test run
+target x86_64
+set opt_level=speed
+target x86_64
+
+;; Regression test for #14660: widening an i8 constant multiply to i32 must
+;; preserve wrapping i8 results, including negative and unsigned constants.
+
+function %imul_i8_const_rhs(i8) -> i8 {
+block0(v0: i8):
+    v1 = iconst.i8 97
+    v2 = imul v0, v1
+    return v2
+}
+; run: %imul_i8_const_rhs(0) == 0
+; run: %imul_i8_const_rhs(2) == 0xc2
+; run: %imul_i8_const_rhs(0x7f) == 0x1f
+; run: %imul_i8_const_rhs(0x80) == 0x80
+; run: %imul_i8_const_rhs(0xff) == 0x9f
+
+function %imul_i8_const_lhs(i8) -> i8 {
+block0(v0: i8):
+    v1 = iconst.i8 193
+    v2 = imul v1, v0
+    return v2
+}
+; run: %imul_i8_const_lhs(1) == 0xc1
+; run: %imul_i8_const_lhs(2) == 0x82
+; run: %imul_i8_const_lhs(0x7f) == 0xbf
+; run: %imul_i8_const_lhs(0xff) == 0x3f
+
+;; ireduce can leave junk in the register's high bits. Only the low byte of
+;; the product may survive the uextend, even when that byte has its sign bit set.
+function %imul_i8_ireduce_negative(i64) -> i32 {
+block0(v0: i64):
+    v1 = ireduce.i8 v0
+    v2 = iconst.i8 -97
+    v3 = imul v1, v2
+    v4 = uextend.i32 v3
+    return v4
+}
+; run: %imul_i8_ireduce_negative(0x1234_5678_9abc_de00) == 0
+; run: %imul_i8_ireduce_negative(0x1234_5678_9abc_de01) == 159
+; run: %imul_i8_ireduce_negative(0x1234_5678_9abc_de02) == 62
+; run: %imul_i8_ireduce_negative(0x1234_5678_9abc_de80) == 128
+; run: %imul_i8_ireduce_negative(-1) == 97
+
+function %imul_i8_loaded(i64) -> i8 {
+block0(v0: i64):
+    v1 = load.i8 v0
+    v2 = iconst.i8 193
+    v3 = imul v2, v1
+    return v3
+}
+
+;; Use a separate function so store-to-load forwarding cannot remove the load.
+;; The compile test also checks that the memory access is not widened.
+function %imul_i8_load(i8) -> i8 {
+    ss0 = explicit_slot 1
+    fn0 = %imul_i8_loaded(i64) -> i8
+
+block0(v0: i8):
+    v1 = stack_addr.i64 ss0
+    store v0, v1
+    v2 = call fn0(v1)
+    return v2
+}
+; run: %imul_i8_load(2) == 0x82
+; run: %imul_i8_load(0x7f) == 0xbf
+; run: %imul_i8_load(0xff) == 0x3f