riscv64: Refactor implementation of {u,s}{div,rem} (bytecodeallianc…

…e#7136)

* riscv64: Refactor implementation of `{u,s}{div,rem}`

This commit's goal is to remove the usage of `gen_icmp` with
division-related instructions in the riscv64 backend. I've opted for a
slightly more verbose approach than what was present prior to make
sign/zero extensions more precise and additionally enable some
immediate-related optimizations.

Divison/remainder by an immediate will no longer emit checks to see if a
trap needs to be emitted. Instead only the raw `div`/`rem` instructions
are emitted. Additionally a few minor sign extensions are now avoided
such as the dividend in 32-bit remainders/division because only the
low 32-bits are inspected.

Finally, while I was here, I went ahead an added `has_m` guards to all
these lowering rules. The M extension is always required with riscv64
right now and won't work if it's turned off, but I figure it's not too
bad to add in terms of completeness for now.

As to the meat of the commit, the check for trapping value as part of
division now happens without `gen_icmp` but instead `gen_trapif` which
enables avoiding materializing the comparison's value into a register.

* Fix tests

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

@@ -1090,6 +1090,9 @@
 
 ;; ISA Extension helpers
 
+(decl pure has_m () bool)
+(extern constructor has_m has_m)
+
 (decl pure has_v () bool)
 (extern constructor has_v has_v)
 
@@ -2789,25 +2792,6 @@
 (rule (gen_trapz test trap_code)
  (gen_trapif (IntCC.Equal) test (zero_reg) trap_code))
 
-
-(decl shift_int_to_most_significant (XReg Type) XReg)
-(extern constructor shift_int_to_most_significant shift_int_to_most_significant)
-
-;;; generate div overflow.
-(decl gen_div_overflow (XReg XReg Type) InstOutput)
-(rule (gen_div_overflow rs1 rs2 ty)
- (let ((r_const_neg_1 XReg (imm $I64 (i64_as_u64 -1)))
- (r_const_min XReg (rv_slli (imm $I64 1) (imm12_const 63)))
- (tmp_rs1 XReg (shift_int_to_most_significant rs1 ty))
- (t1 XReg (gen_icmp (IntCC.Equal) r_const_neg_1 rs2 ty))
- (t2 XReg (gen_icmp (IntCC.Equal) r_const_min tmp_rs1 ty))
- (test XReg (rv_and t1 t2)))
- (gen_trapnz test (TrapCode.IntegerOverflow))))
-
-(decl gen_div_by_zero (XReg) InstOutput)
-(rule (gen_div_by_zero r)
- (gen_trapz r (TrapCode.IntegerDivisionByZero)))
-
 ;;;; Helpers for Emitting Calls ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
 (decl gen_call (SigRef ExternalName RelocDistance ValueSlice) InstOutput)

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

@@ -505,68 +505,147 @@
 (rule 4 (lower (has_type (ty_vec_fits_in_register ty) (umulhi x (splat y))))
  (rv_vmulhu_vx x y (unmasked) ty))
 
-;;;; Rules for `div` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-
-(rule -1 (lower (has_type (fits_in_32 ty) (udiv x y)))
- (let
- ((y2 XReg (zext y))
- (_ InstOutput (gen_div_by_zero y2)))
- (rv_divuw (zext x) y2)))
-
-(rule -1 (lower (has_type (fits_in_32 ty) (sdiv x y)))
- (let
- ((a XReg (sext x))
- (b XReg (sext y))
- (_ InstOutput (gen_div_overflow a b ty))
- (_ InstOutput (gen_div_by_zero b)))
- (rv_divw a b)))
-
-(rule (lower (has_type $I64 (sdiv x y)))
- (let
- ((_ InstOutput (gen_div_overflow x y $I64))
- (_ InstOutput (gen_div_by_zero y)) )
- (rv_div x y)))
-
-(rule (lower (has_type $I64 (udiv x y)))
- (let
- ((_ InstOutput (gen_div_by_zero y)))
- (rv_divu x y)))
-
-;;;; Rules for `rem` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-
-(rule -1 (lower (has_type (fits_in_16 ty) (urem x y)))
- (let
- ((y2 XReg (zext y))
- (_ InstOutput (gen_div_by_zero y2)))
- (rv_remuw (zext x) y2)))
-
-(rule -1 (lower (has_type (fits_in_16 ty) (srem x y)))
- (let
- ((y2 XReg (sext y))
- (_ InstOutput (gen_div_by_zero y2)))
- (rv_remw (sext x) y2)))
-
-(rule (lower (has_type $I32 (srem x y)))
- (let
- ((y2 XReg (sext y))
- (_ InstOutput (gen_div_by_zero y2)))
- (rv_remw x y2)))
-
-(rule (lower (has_type $I32 (urem x y)))
- (let
- ((y2 XReg (zext y))
- (_ InstOutput (gen_div_by_zero y2)))
- (rv_remuw x y2)))
-
-(rule (lower (has_type $I64 (srem x y)))
- (let
- ((_ InstOutput (gen_div_by_zero y)))
- (rv_rem x y)))
-
-(rule (lower (has_type $I64 (urem x y)))
- (let
- ((_ InstOutput (gen_div_by_zero y)))
- (rv_remu x y)))
+;;;; Rules for `udiv` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+
+(rule 0 (lower (has_type (fits_in_16 ty) (udiv x y)))
+ (if-let $true (has_m))
+ (rv_divuw (zext x) (nonzero_divisor (zext y))))
+
+(rule 1 (lower (has_type (fits_in_16 ty) (udiv x y @ (iconst imm))))
+ (if-let $true (has_m))
+ (if (safe_divisor_from_imm64 ty imm))
+ (rv_divuw (zext x) (zext y)))
+
+(rule 2 (lower (has_type $I32 (udiv x y)))
+ (if-let $true (has_m))
+ (rv_divuw x (nonzero_divisor (zext y))))
+
+(rule 3 (lower (has_type $I32 (udiv x y @ (iconst imm))))
+ (if-let $true (has_m))
+ (if (safe_divisor_from_imm64 $I32 imm))
+ (rv_divuw x y))
+
+(rule 2 (lower (has_type $I64 (udiv x y)))
+ (if-let $true (has_m))
+ (rv_divu x (nonzero_divisor y)))
+
+(rule 3 (lower (has_type $I64 (udiv x y @ (iconst imm))))
+ (if-let $true (has_m))
+ (if (safe_divisor_from_imm64 $I64 imm))
+ (rv_divu x y))
+
+;; Traps if the input register is zero, otherwise returns the same register.
+(decl nonzero_divisor (XReg) XReg)
+(rule (nonzero_divisor val)
+ (let ((_ InstOutput (gen_trapif (IntCC.Equal) val (zero_reg) (TrapCode.IntegerDivisionByZero))))
+ val))
+
+;;;; Rules for `sdiv` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+
+(rule 0 (lower (has_type (fits_in_16 ty) (sdiv x y)))
+ (if-let $true (has_m))
+ (let ((x XReg (sext x)))
+ (rv_divw x (safe_sdiv_divisor ty x (sext y)))))
+
+(rule 1 (lower (has_type (fits_in_16 ty) (sdiv x y @ (iconst imm))))
+ (if-let $true (has_m))
+ (if (safe_divisor_from_imm64 ty imm))
+ (rv_divw (sext x) y))
+
+(rule 2 (lower (has_type $I32 (sdiv x y)))
+ (if-let $true (has_m))
+ (let ((x XReg (sext x)))
+ (rv_divw x (safe_sdiv_divisor $I32 x (sext y)))))
+
+(rule 3 (lower (has_type $I32 (sdiv x y @ (iconst imm))))
+ (if-let $true (has_m))
+ (if (safe_divisor_from_imm64 $I32 imm))
+ (rv_divw x y))
+
+(rule 2 (lower (has_type $I64 (sdiv x y)))
+ (if-let $true (has_m))
+ (rv_div x (safe_sdiv_divisor $I64 x y)))
+
+(rule 3 (lower (has_type $I64 (sdiv x y @ (iconst imm))))
+ (if-let $true (has_m))
+ (if (safe_divisor_from_imm64 $I64 imm))
+ (rv_div x y))
+
+;; Check for two trapping conditions:
+;;
+;; * the divisor is 0, or...
+;; * the divisor is -1 and the dividend is $ty::MIN
+(decl safe_sdiv_divisor (Type XReg XReg) XReg)
+(rule (safe_sdiv_divisor ty x y)
+ (let (
+ (y XReg (nonzero_divisor y))
+ (min XReg (imm $I64 (u64_shl 0xffffffff_ffffffff (u64_sub (ty_bits ty) 1))))
+ (x_is_not_min XReg (rv_xor x min))
+ (y_is_not_neg_one XReg (rv_not y))
+ (no_int_overflow XReg (rv_or x_is_not_min y_is_not_neg_one))
+ (_ InstOutput (gen_trapif
+ (IntCC.Equal)
+ no_int_overflow (zero_reg)
+ (TrapCode.IntegerOverflow))))
+ y))
+
+;;;; Rules for `urem` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+
+(rule 0 (lower (has_type (fits_in_16 ty) (urem x y)))
+ (if-let $true (has_m))
+ (rv_remuw (zext x) (nonzero_divisor (zext y))))
+
+(rule 1 (lower (has_type (fits_in_16 ty) (urem x y @ (iconst imm))))
+ (if-let $true (has_m))
+ (if (safe_divisor_from_imm64 ty imm))
+ (rv_remuw (zext x) y))
+
+(rule 2 (lower (has_type $I32 (urem x y)))
+ (if-let $true (has_m))
+ (rv_remuw x (nonzero_divisor (zext y))))
+
+(rule 3 (lower (has_type $I32 (urem x y @ (iconst imm))))
+ (if-let $true (has_m))
+ (if (safe_divisor_from_imm64 $I32 imm))
+ (rv_remuw x y))
+
+(rule 2 (lower (has_type $I64 (urem x y)))
+ (if-let $true (has_m))
+ (rv_remu x (nonzero_divisor y)))
+
+(rule 3 (lower (has_type $I64 (urem x y @ (iconst imm))))
+ (if-let $true (has_m))
+ (if (safe_divisor_from_imm64 $I64 imm))
+ (rv_remu x y))
+
+;;;; Rules for `srem` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+
+(rule 0 (lower (has_type (fits_in_16 ty) (srem x y)))
+ (if-let $true (has_m))
+ (rv_remw (sext x) (nonzero_divisor (sext y))))
+
+(rule 1 (lower (has_type (fits_in_16 ty) (srem x y @ (iconst imm))))
+ (if-let $true (has_m))
+ (if (safe_divisor_from_imm64 ty imm))
+ (rv_remw (sext x) y))
+
+(rule 2 (lower (has_type $I32 (srem x y)))
+ (if-let $true (has_m))
+ (rv_remw x (nonzero_divisor (sext y))))
+
+(rule 3 (lower (has_type $I32 (srem x y @ (iconst imm))))
+ (if-let $true (has_m))
+ (if (safe_divisor_from_imm64 $I32 imm))
+ (rv_remw x y))
+
+(rule 2 (lower (has_type $I64 (srem x y)))
+ (if-let $true (has_m))
+ (rv_rem x (nonzero_divisor y)))
+
+(rule 3 (lower (has_type $I64 (srem x y @ (iconst imm))))
+ (if-let $true (has_m))
+ (if (safe_divisor_from_imm64 $I64 imm))
+ (rv_rem x y))
 
 ;;;; Rules for `and` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 (rule 0 (lower (has_type (ty_int ty) (band x y)))

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

@@ -392,6 +392,10 @@ impl generated_code::Context for RV64IsleContext<'_, '_, MInst, Riscv64Backend>
  self.backend.isa_flags.has_v()
  }
 
+ fn has_m(&mut self) -> bool {
+ self.backend.isa_flags.has_m()
+ }
+
  fn has_zbkb(&mut self) -> bool {
  self.backend.isa_flags.has_zbkb()
  }
@@ -501,22 +505,6 @@ impl generated_code::Context for RV64IsleContext<'_, '_, MInst, Riscv64Backend>
  px_reg(2)
  }
 
- fn shift_int_to_most_significant(&mut self, v: XReg, ty: Type) -> XReg {
- assert!(ty.is_int() && ty.bits() <= 64);
- if ty == I64 {
- return v;
- }
- let tmp = self.temp_writable_reg(I64);
- self.emit(&MInst::AluRRImm12 {
- alu_op: AluOPRRI::Slli,
- rd: tmp,
- rs: v.to_reg(),
- imm12: Imm12::from_i16((64 - ty.bits()) as i16),
- });
-
- self.xreg_new(tmp.to_reg())
- }
-
  #[inline]
  fn int_compare(&mut self, kind: &IntCC, rs1: XReg, rs2: XReg) -> IntegerCompare {
  IntegerCompare {