Assert that we only use virtual registers with moves (#5440)

Assert that we never see real registers as arguments to move instructions in VCodeBuilder::collect_operands.

Also fix a bug in the riscv64 backend that was discovered by these assertions: the lowerings of get_stack_pointer and get_frame_pointer were using physical registers 8 and 2 directly. The solution was similar to other backends: add a move instruction specifically for moving out of physical registers, whose source operand is opaque to regalloc2.

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

@@ -148,6 +148,13 @@
       (rm Reg)
       (ty Type))
 
+    ;; A MOV instruction, but where the source register is a non-allocatable
+    ;; PReg. It's important that the register be non-allocatable, as regalloc2
+    ;; will not see it as used.
+    (MovFromPReg
+      (rd WritableReg)
+      (rm PReg))
+
     (Fence
       (pred FenceReq)
       (succ FenceReq))
@@ -1958,9 +1965,6 @@
   (lower_branch (br_table index _ _) targets)
   (lower_br_table index targets))
 
-(decl x_reg (u8) Reg)
-(extern constructor x_reg x_reg)
-
 (decl load_ra () Reg)
 (extern constructor load_ra load_ra)
 
@@ -2142,3 +2146,20 @@
   (lower_bmask $I128 $I128 val)
   (let ((res ValueRegs (lower_bmask $I64 $I128 val)))
     (value_regs (value_regs_get res 0) (value_regs_get res 0))))
+
+
+;;;; Helpers for physical registers ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+
+(decl gen_mov_from_preg (PReg) Reg)
+
+(rule
+  (gen_mov_from_preg rm)
+  (let ((rd WritableReg (temp_writable_reg $I64))
+        (_ Unit (emit (MInst.MovFromPReg rd rm))))
+    rd))
+
+(decl fp_reg () PReg)
+(extern constructor fp_reg fp_reg)
+
+(decl sp_reg () PReg)
+(extern constructor sp_reg sp_reg)

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

@@ -1123,6 +1123,19 @@ impl MachInstEmit for Inst {
                     }
                 }
             }
+
+            &Inst::MovFromPReg { rd, rm } => {
+                debug_assert!([px_reg(2), px_reg(8)].contains(&rm));
+                let rd = allocs.next_writable(rd);
+                let x = Inst::AluRRImm12 {
+                    alu_op: AluOPRRI::Ori,
+                    rd,
+                    rs: Reg::from(rm),
+                    imm12: Imm12::zero(),
+                };
+                x.emit(&[], sink, emit_info, state);
+            }
+
             &Inst::BrTable {
                 index,
                 tmp1,

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

@@ -431,6 +431,10 @@ fn riscv64_get_operands<F: Fn(VReg) -> VReg>(inst: &Inst, collector: &mut Operan
             collector.reg_use(rm);
             collector.reg_def(rd);
         }
+        &Inst::MovFromPReg { rd, rm } => {
+            debug_assert!([px_reg(2), px_reg(8)].contains(&rm));
+            collector.reg_def(rd);
+        }
         &Inst::Fence { .. } => {}
         &Inst::FenceI => {}
         &Inst::ECall => {}
@@ -1530,6 +1534,12 @@ impl Inst {
                 };
                 format!("{} {},{}", v, rd, rm)
             }
+            &MInst::MovFromPReg { rd, rm } => {
+                let rd = format_reg(rd.to_reg(), allocs);
+                debug_assert!([px_reg(2), px_reg(8)].contains(&rm));
+                let rm = reg_name(Reg::from(rm));
+                format!("mv {},{}", rd, rm)
+            }
             &MInst::Fence { pred, succ } => {
                 format!(
                     "fence {},{}",

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

@@ -858,10 +858,10 @@
 ;;; Rules for `get_{frame,stack}_pointer` and `get_return_address` ;;;;;;;;;;;;;
 
 (rule (lower (get_frame_pointer))
-  (gen_move2 (x_reg 8) $I64 $I64))
+  (gen_mov_from_preg (fp_reg)))
 
 (rule (lower (get_stack_pointer))
-  (gen_move2 (x_reg 2) $I64 $I64))
+  (gen_mov_from_preg (sp_reg)))
 
 (rule (lower (get_return_address))
   (load_ra))

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

@@ -407,9 +407,15 @@ impl generated_code::Context for IsleContext<'_, '_, MInst, Riscv64Backend> {
             targets,
         });
     }
-    fn x_reg(&mut self, x: u8) -> Reg {
-        x_reg(x as usize)
+
+    fn fp_reg(&mut self) -> PReg {
+        px_reg(8)
     }
+
+    fn sp_reg(&mut self) -> PReg {
+        px_reg(2)
+    }
+
     fn shift_int_to_most_significant(&mut self, v: Reg, ty: Type) -> Reg {
         assert!(ty.is_int() && ty.bits() <= 64);
         if ty == I64 {

cranelift/codegen/src/machinst/vcode.rs

@@ -568,11 +568,25 @@ impl<I: VCodeInst> VCodeBuilder<I> {
             }
 
             if let Some((dst, src)) = insn.is_move() {
+                // We should never see non-virtual registers present in move
+                // instructions.
+                assert!(
+                    src.is_virtual(),
+                    "the real register {:?} was used as the source of a move instruction",
+                    src
+                );
+                assert!(
+                    dst.to_reg().is_virtual(),
+                    "the real register {:?} was used as the destination of a move instruction",
+                    dst.to_reg()
+                );
+
                 let src = Operand::reg_use(Self::resolve_vreg_alias_impl(vreg_aliases, src.into()));
                 let dst = Operand::reg_def(Self::resolve_vreg_alias_impl(
                     vreg_aliases,
                     dst.to_reg().into(),
                 ));
+
                 // Note that regalloc2 requires these in (src, dst) order.
                 self.vcode.is_move.insert(InsnIndex::new(i), (src, dst));
             }