riscv64: Move is_null/is_invalid to ISLE (#5874)

* riscv64: Move `is_null`/`is_invalid` to ISLE

* riscv64: Fix `is_invalid` codegen

* Implement review suggestions

Thanks!

Co-authored-by: Jamey Sharp <jamey@minilop.net>

---------

Co-authored-by: Jamey Sharp <jamey@minilop.net>

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

@@ -205,11 +205,6 @@
       (x ValueRegs)
       (y ValueRegs))
 
-    (ReferenceCheck
-      (rd WritableReg)
-      (op ReferenceCheckOP)
-      (x Reg))
-
     (BrTable
       (index Reg)
       (tmp1 WritableReg)
@@ -370,11 +365,6 @@
   (Umin)
 ))
 
-(type ReferenceCheckOP (enum
-  (IsNull)
-  (IsInvalid)
-))
-
 (type AtomicOP (enum
   (LrW)
   (ScW)
@@ -879,6 +869,17 @@
 (rule (select_addi (fits_in_64 ty)) (AluOPRRI.Addi))
 
 
+;; Helper for emiting the `sltiu` instruction
+(decl sltiu (Reg Imm12) Reg)
+(rule (sltiu r imm)
+  (alu_rr_imm12 (AluOPRRI.SltiU) r imm))
+
+;; Helper for emiting the `seqz` mnemonic
+(decl seqz (Reg) Reg)
+(rule (seqz r)
+  (sltiu r (imm12_const 1)))
+
+
 (decl bnot_128 (ValueRegs) ValueRegs)
 (rule
   (bnot_128 val)
@@ -1699,14 +1700,6 @@
 (decl gen_moves (ValueRegs Type Type) ValueRegs)
 (extern constructor gen_moves gen_moves)
 
-;;
-(decl gen_reference_check (ReferenceCheckOP Reg) Reg)
-(rule
-  (gen_reference_check op r)
-  (let
-    ((tmp WritableReg (temp_writable_reg $I64))
-      (_ Unit (emit (MInst.ReferenceCheck tmp op r))))
-    tmp))
 
 ;;
 (decl gen_select (Type Reg ValueRegs ValueRegs) ValueRegs)

cranelift/codegen/src/isa/riscv64/inst/args.rs

@@ -1639,23 +1639,6 @@ impl IntSelectOP {
     }
 }
 
-impl ReferenceCheckOP {
-    pub(crate) fn op_name(self) -> &'static str {
-        match self {
-            ReferenceCheckOP::IsNull => "is_null",
-            ReferenceCheckOP::IsInvalid => "is_invalid",
-        }
-    }
-    #[inline]
-    pub(crate) fn from_ir_op(op: crate::ir::Opcode) -> Self {
-        match op {
-            crate::ir::Opcode::IsInvalid => Self::IsInvalid,
-            crate::ir::Opcode::IsNull => Self::IsNull,
-            _ => unreachable!(),
-        }
-    }
-}
-
 #[derive(Clone, Copy)]
 pub enum CsrAddress {
     Fcsr = 0x3,

cranelift/codegen/src/isa/riscv64/inst/emit.rs

@@ -680,57 +680,6 @@ impl MachInstEmit for Inst {
                         .for_each(|inst| inst.emit(&[], sink, emit_info, state));
                 }
             }
-
-            &Inst::ReferenceCheck { rd, op, x } => {
-                let x = allocs.next(x);
-                let rd = allocs.next_writable(rd);
-                let mut insts = SmallInstVec::new();
-                match op {
-                    ReferenceCheckOP::IsNull => {
-                        insts.push(Inst::CondBr {
-                            taken: BranchTarget::ResolvedOffset(Inst::INSTRUCTION_SIZE * 3),
-                            not_taken: BranchTarget::zero(),
-                            kind: IntegerCompare {
-                                kind: IntCC::Equal,
-                                rs1: zero_reg(),
-                                rs2: x,
-                            },
-                        });
-                        // here is false
-                        insts.push(Inst::load_imm12(rd, Imm12::FALSE));
-                        insts.push(Inst::Jal {
-                            dest: BranchTarget::ResolvedOffset(Inst::INSTRUCTION_SIZE * 2),
-                        });
-                        // here is true
-                        insts.push(Inst::load_imm12(rd, Imm12::TRUE));
-                    }
-
-                    ReferenceCheckOP::IsInvalid => {
-                        // todo:: right now just check if it is null
-                        // null is a valid reference??????
-                        insts.push(Inst::CondBr {
-                            taken: BranchTarget::ResolvedOffset(Inst::INSTRUCTION_SIZE * 3),
-                            not_taken: BranchTarget::zero(),
-                            kind: IntegerCompare {
-                                kind: IntCC::Equal,
-                                rs1: zero_reg(),
-                                rs2: x,
-                            },
-                        });
-                        // here is false
-                        insts.push(Inst::load_imm12(rd, Imm12::FALSE));
-                        insts.push(Inst::Jal {
-                            dest: BranchTarget::ResolvedOffset(Inst::INSTRUCTION_SIZE * 2),
-                        });
-                        // here is true
-                        insts.push(Inst::load_imm12(rd, Imm12::TRUE));
-                    }
-                }
-
-                insts
-                    .into_iter()
-                    .for_each(|i| i.emit(&[], sink, emit_info, state));
-            }
             &Inst::Args { .. } => {
                 // Nothing: this is a pseudoinstruction that serves
                 // only to constrain registers at a certain point.

cranelift/codegen/src/isa/riscv64/inst/mod.rs

@@ -51,8 +51,7 @@ pub(crate) type VecWritableReg = Vec<Writable<Reg>>;
 use crate::isa::riscv64::lower::isle::generated_code::MInst;
 pub use crate::isa::riscv64::lower::isle::generated_code::{
     AluOPRRI, AluOPRRR, AtomicOP, CsrOP, FClassResult, FFlagsException, FenceFm, FloatRoundOP,
-    FloatSelectOP, FpuOPRR, FpuOPRRR, FpuOPRRRR, IntSelectOP, LoadOP, MInst as Inst,
+    FloatSelectOP, FpuOPRR, FpuOPRRR, FpuOPRRRR, IntSelectOP, LoadOP, MInst as Inst, StoreOP, FRM,
-    ReferenceCheckOP, StoreOP, FRM,
 };
 
 type BoxCallInfo = Box<CallInfo>;
@@ -471,10 +470,6 @@ fn riscv64_get_operands<F: Fn(VReg) -> VReg>(inst: &Inst, collector: &mut Operan
                 collector.reg_early_def(d.clone());
             }
         }
-        &Inst::ReferenceCheck { rd, x, .. } => {
-            collector.reg_use(x);
-            collector.reg_def(rd);
-        }
         &Inst::AtomicCas {
             offset,
             t0,
@@ -1185,12 +1180,6 @@ impl Inst {
                     imm.bits
                 )
             }
-
-            &Inst::ReferenceCheck { rd, op, x } => {
-                let x = format_reg(x, allocs);
-                let rd = format_reg(rd.to_reg(), allocs);
-                format!("{} {},{}", op.op_name(), rd, x)
-            }
             &Inst::Jalr { rd, base, offset } => {
                 let base = format_reg(base, allocs);
                 let rd = format_reg(rd.to_reg(), allocs);
@@ -1344,6 +1333,9 @@ impl Inst {
                     (AluOPRRI::Xori, _, imm12) if imm12.as_i16() == -1 => {
                         return format!("not {},{}", rd, rs_s);
                     }
+                    (AluOPRRI::SltiU, _, imm12) if imm12.as_i16() == 1 => {
+                        return format!("seqz {},{}", rd, rs_s);
+                    }
                     (alu_op, _, _) if alu_op.option_funct12().is_some() => {
                         format!("{} {},{}", alu_op.op_name(), rd, rs_s)
                     }

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

@@ -612,14 +612,16 @@
   (gen_stack_addr ss offset))
 
 ;;;;;  Rules for `is_null`;;;;;;;;;
-(rule
+
-  (lower (is_null v))
+;; Null references are represented by the constant value `0`.
-  (gen_reference_check (ReferenceCheckOP.IsNull) v))
+(rule (lower (is_null v))
+  (seqz v))
 
 ;;;;;  Rules for `is_invalid`;;;;;;;;;
-(rule
+
-  (lower (is_invalid v))
+;; Invalid references are represented by the constant value `-1`.
-  (gen_reference_check (ReferenceCheckOP.IsInvalid) v))
+(rule (lower (is_invalid v))
+  (seqz (alu_rr_imm12 (AluOPRRI.Addi) v (imm12_const 1))))
 
 ;;;;;  Rules for `select`;;;;;;;;;
 (rule

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

@@ -452,7 +452,7 @@ fn construct_dest<F: std::ops::FnMut(Type) -> WritableReg>(
     mut alloc: F,
     ty: Type,
 ) -> WritableValueRegs {
-    if ty.is_int() {
+    if ty.is_int() || ty.is_ref() {
         if ty.bits() == 128 {
             WritableValueRegs::two(alloc(I64), alloc(I64))
         } else {

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

@@ -2025,7 +2025,7 @@
 
 ;; Rules for `is_invalid` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
-;; Null references are represented by the constant value `-1`.
+;; Invalid references are represented by the constant value `-1`.
 (rule (lower (is_invalid src @ (value_type $R64)))
       (with_flags
        (x64_cmp_imm (OperandSize.Size64) 0xffffffff src)  ;; simm32 0xffff_ffff is sign-extended to -1.

cranelift/filetests/filetests/isa/riscv64/reftypes.clif

@@ -23,15 +23,12 @@ block0(v0: r64):
 
 ; VCode:
 ; block0:
-;   is_null a0,a0
+;   seqz a0,a0
 ;   ret
 ; 
 ; Disassembled:
 ; block0: ; offset 0x0
-;   beq zero, a0, 0xc
+;   seqz a0, a0
-;   mv a0, zero
-;   j 8
-;   addi a0, zero, 1
 ;   ret
 
 function %f2(r64) -> i8 {
@@ -42,15 +39,14 @@ block0(v0: r64):
 
 ; VCode:
 ; block0:
-;   is_invalid a0,a0
+;   addi t2,a0,1
+;   seqz a0,t2
 ;   ret
 ; 
 ; Disassembled:
 ; block0: ; offset 0x0
-;   beq zero, a0, 0xc
+;   addi t2, a0, 1
-;   mv a0, zero
+;   seqz a0, t2
-;   j 8
-;   addi a0, zero, 1
 ;   ret
 
 function %f3() -> r64 {

cranelift/filetests/filetests/runtests/bitcast-ref64.clif

@@ -2,6 +2,7 @@ test run
 target aarch64
 target x86_64
 target s390x
+target riscv64gc
 ; the interpreter does not support bitcasting to/from references
 
 function %bitcast_ir64(i64) -> i8 {

cranelift/filetests/filetests/runtests/ref64-invalid-null.clif

@@ -3,6 +3,7 @@ test run
 target aarch64
 target x86_64
 target s390x
+target riscv64gc
 
 function %is_null_true_r64() -> i8 {
 block0: