cranelift: Fix implicit pointer argument register use (#5301)

* Fix arg handling to write to VRegs instead of physical regs

* Make is_included_in_clobbers required, and handle Args on x64 and riscv64

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

@@ -1142,6 +1142,7 @@ impl MachInst for Inst {
         // See the note in [crate::isa::aarch64::abi::is_caller_save_reg] for
         // more information on this ABI-implementation hack.
         match self {
+            &Inst::Args { .. } => false,
             &Inst::Call { ref info } => info.caller_callconv != info.callee_callconv,
             &Inst::CallInd { ref info } => info.caller_callconv != info.callee_callconv,
             _ => true,

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

@@ -657,7 +657,10 @@ impl MachInst for Inst {
     }
 
     fn is_included_in_clobbers(&self) -> bool {
-        true
+        match self {
+            &Inst::Args { .. } => false,
+            _ => true,
+        }
     }
 
     fn is_args(&self) -> bool {

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

@@ -1109,6 +1109,7 @@ impl MachInst for Inst {
         // half-caller-save, half-callee-save SysV ABI for some vector
         // registers.
         match self {
+            &Inst::Args { .. } => false,
             &Inst::Call { ref info, .. } => info.caller_callconv != info.callee_callconv,
             &Inst::CallInd { ref info, .. } => info.caller_callconv != info.callee_callconv,
             _ => true,

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

@@ -2236,6 +2236,13 @@ impl MachInst for Inst {
         }
     }
 
+    fn is_included_in_clobbers(&self) -> bool {
+        match self {
+            &Inst::Args { .. } => false,
+            _ => true,
+        }
+    }
+
     fn is_args(&self) -> bool {
         match self {
             Self::Args { .. } => true,

cranelift/codegen/src/machinst/abi.rs

@@ -1396,6 +1396,7 @@ impl<M: ABIMachineSpec> Callee<M> {
         sigs: &SigSet,
         idx: usize,
         into_regs: ValueRegs<Writable<Reg>>,
+        vregs: &mut VRegAllocator<M::I>,
     ) -> SmallInstVec<M::I> {
         let mut insts = smallvec![];
         let mut copy_arg_slot_to_reg = |slot: &ABIArgSlot, into_reg: &Writable<Reg>| {
@@ -1470,7 +1471,14 @@ impl<M: ABIMachineSpec> Callee<M> {
                 let into_reg = into_regs.only_reg().unwrap();
                 // We need to dereference the pointer.
                 let base = match &pointer {
-                    &ABIArgSlot::Reg { reg, .. } => Reg::from(reg),
+                    &ABIArgSlot::Reg { reg, ty, .. } => {
+                        let tmp = vregs.alloc(ty).unwrap().only_reg().unwrap();
+                        self.reg_args.push(ArgPair {
+                            vreg: Writable::from_reg(tmp),
+                            preg: reg.into(),
+                        });
+                        tmp
+                    }
                     &ABIArgSlot::Stack { offset, ty, .. } => {
                         // In this case we need a temp register to hold the address.
                         // This was allocated in the `init` routine.
@@ -1606,12 +1614,17 @@ impl<M: ABIMachineSpec> Callee<M> {
     /// values or an otherwise large return value that must be passed on the
     /// stack; typically the ABI specifies an extra hidden argument that is a
     /// pointer to that memory.
-    pub fn gen_retval_area_setup(&mut self, sigs: &SigSet) -> Option<M::I> {
+    pub fn gen_retval_area_setup(
+        &mut self,
+        sigs: &SigSet,
+        vregs: &mut VRegAllocator<M::I>,
+    ) -> Option<M::I> {
         if let Some(i) = sigs[self.sig].stack_ret_arg {
             let insts = self.gen_copy_arg_to_regs(
                 sigs,
                 i.into(),
                 ValueRegs::one(self.ret_area_ptr.unwrap()),
+                vregs,
             );
             insts.into_iter().next().map(|inst| {
                 trace!(

cranelift/codegen/src/machinst/lower.rs

@@ -573,7 +573,7 @@ impl<'func, I: VCodeInst> Lower<'func, I> {
                     .vcode
                     .vcode
                     .abi
-                    .gen_copy_arg_to_regs(&self.vcode.vcode.sigs, i, regs)
+                    .gen_copy_arg_to_regs(&self.vcode.vcode.sigs, i, regs, &mut self.vregs)
                     .into_iter()
                 {
                     self.emit(insn);
@@ -601,7 +601,7 @@ impl<'func, I: VCodeInst> Lower<'func, I> {
                 .vcode
                 .vcode
                 .abi
-                .gen_retval_area_setup(&self.vcode.vcode.sigs)
+                .gen_retval_area_setup(&self.vcode.vcode.sigs, &mut self.vregs)
             {
                 self.emit(insn);
             }

cranelift/codegen/src/machinst/mod.rs

@@ -104,9 +104,7 @@ pub trait MachInst: Clone + Debug {
     fn is_args(&self) -> bool;
 
     /// Should this instruction be included in the clobber-set?
-    fn is_included_in_clobbers(&self) -> bool {
+    fn is_included_in_clobbers(&self) -> bool;
-        true
-    }
 
     /// Generate a move.
     fn gen_move(to_reg: Writable<Reg>, from_reg: Reg, ty: Type) -> Self;

cranelift/filetests/filetests/isa/s390x/arithmetic.clif

@@ -703,7 +703,7 @@ block0(v0: i128, v1: i128):
 
 ;   stmg %r7, %r15, 56(%r15)
 ; block0:
-;   lgr %r14, %r2
+;   lgr %r10, %r2
 ;   vl %v0, 0(%r3)
 ;   vl %v1, 0(%r4)
 ;   lgdr %r4, %f0
@@ -718,7 +718,7 @@ block0(v0: i128, v1: i128):
 ;   agrk %r4, %r2, %r8
 ;   agr %r5, %r4
 ;   vlvgp %v5, %r5, %r3
-;   lgr %r2, %r14
+;   lgr %r2, %r10
 ;   vst %v5, 0(%r2)
 ;   lmg %r7, %r15, 56(%r15)
 ;   br %r14

cranelift/filetests/filetests/isa/s390x/conversions.clif

@@ -348,8 +348,8 @@ block0(v0: i128):
 
 ; block0:
 ;   vl %v0, 0(%r2)
-;   vgbm %v2, 0
+;   vgbm %v3, 0
-;   vceqgs %v4, %v0, %v2
+;   vceqgs %v5, %v0, %v3
 ;   lghi %r2, 0
 ;   locghine %r2, -1
 ;   br %r14
@@ -362,8 +362,8 @@ block0(v0: i128):
 
 ; block0:
 ;   vl %v0, 0(%r2)
-;   vgbm %v2, 0
+;   vgbm %v3, 0
-;   vceqgs %v4, %v0, %v2
+;   vceqgs %v5, %v0, %v3
 ;   lhi %r2, 0
 ;   lochine %r2, -1
 ;   br %r14
@@ -376,8 +376,8 @@ block0(v0: i128):
 
 ; block0:
 ;   vl %v0, 0(%r2)
-;   vgbm %v2, 0
+;   vgbm %v3, 0
-;   vceqgs %v4, %v0, %v2
+;   vceqgs %v5, %v0, %v3
 ;   lhi %r2, 0
 ;   lochine %r2, -1
 ;   br %r14
@@ -390,8 +390,8 @@ block0(v0: i128):
 
 ; block0:
 ;   vl %v0, 0(%r2)
-;   vgbm %v2, 0
+;   vgbm %v3, 0
-;   vceqgs %v4, %v0, %v2
+;   vceqgs %v5, %v0, %v3
 ;   lhi %r2, 0
 ;   lochine %r2, -1
 ;   br %r14

cranelift/filetests/filetests/isa/s390x/icmp-i128.clif

@@ -10,7 +10,7 @@ block0(v0: i128, v1: i128):
 ; block0:
 ;   vl %v0, 0(%r2)
 ;   vl %v1, 0(%r3)
-;   vceqgs %v4, %v0, %v1
+;   vceqgs %v5, %v0, %v1
 ;   lhi %r2, 0
 ;   lochie %r2, 1
 ;   br %r14
@@ -24,7 +24,7 @@ block0(v0: i128, v1: i128):
 ; block0:
 ;   vl %v0, 0(%r2)
 ;   vl %v1, 0(%r3)
-;   vceqgs %v4, %v0, %v1
+;   vceqgs %v5, %v0, %v1
 ;   lhi %r2, 0
 ;   lochine %r2, 1
 ;   br %r14
@@ -38,7 +38,7 @@ block0(v0: i128, v1: i128):
 ; block0:
 ;   vl %v0, 0(%r2)
 ;   vl %v1, 0(%r3)
-;   vecg %v0, %v1 ; jne 10 ; vchlgs %v4, %v1, %v0
+;   vecg %v0, %v1 ; jne 10 ; vchlgs %v5, %v1, %v0
 ;   lhi %r2, 0
 ;   lochil %r2, 1
 ;   br %r14
@@ -52,7 +52,7 @@ block0(v0: i128, v1: i128):
 ; block0:
 ;   vl %v0, 0(%r2)
 ;   vl %v1, 0(%r3)
-;   vecg %v1, %v0 ; jne 10 ; vchlgs %v4, %v0, %v1
+;   vecg %v1, %v0 ; jne 10 ; vchlgs %v5, %v0, %v1
 ;   lhi %r2, 0
 ;   lochil %r2, 1
 ;   br %r14
@@ -66,7 +66,7 @@ block0(v0: i128, v1: i128):
 ; block0:
 ;   vl %v0, 0(%r2)
 ;   vl %v1, 0(%r3)
-;   vecg %v1, %v0 ; jne 10 ; vchlgs %v4, %v0, %v1
+;   vecg %v1, %v0 ; jne 10 ; vchlgs %v5, %v0, %v1
 ;   lhi %r2, 0
 ;   lochinl %r2, 1
 ;   br %r14
@@ -80,7 +80,7 @@ block0(v0: i128, v1: i128):
 ; block0:
 ;   vl %v0, 0(%r2)
 ;   vl %v1, 0(%r3)
-;   vecg %v0, %v1 ; jne 10 ; vchlgs %v4, %v1, %v0
+;   vecg %v0, %v1 ; jne 10 ; vchlgs %v5, %v1, %v0
 ;   lhi %r2, 0
 ;   lochinl %r2, 1
 ;   br %r14
@@ -94,7 +94,7 @@ block0(v0: i128, v1: i128):
 ; block0:
 ;   vl %v0, 0(%r2)
 ;   vl %v1, 0(%r3)
-;   veclg %v0, %v1 ; jne 10 ; vchlgs %v4, %v1, %v0
+;   veclg %v0, %v1 ; jne 10 ; vchlgs %v5, %v1, %v0
 ;   lhi %r2, 0
 ;   lochil %r2, 1
 ;   br %r14
@@ -108,7 +108,7 @@ block0(v0: i128, v1: i128):
 ; block0:
 ;   vl %v0, 0(%r2)
 ;   vl %v1, 0(%r3)
-;   veclg %v1, %v0 ; jne 10 ; vchlgs %v4, %v0, %v1
+;   veclg %v1, %v0 ; jne 10 ; vchlgs %v5, %v0, %v1
 ;   lhi %r2, 0
 ;   lochil %r2, 1
 ;   br %r14
@@ -122,7 +122,7 @@ block0(v0: i128, v1: i128):
 ; block0:
 ;   vl %v0, 0(%r2)
 ;   vl %v1, 0(%r3)
-;   veclg %v1, %v0 ; jne 10 ; vchlgs %v4, %v0, %v1
+;   veclg %v1, %v0 ; jne 10 ; vchlgs %v5, %v0, %v1
 ;   lhi %r2, 0
 ;   lochinl %r2, 1
 ;   br %r14
@@ -136,7 +136,7 @@ block0(v0: i128, v1: i128):
 ; block0:
 ;   vl %v0, 0(%r2)
 ;   vl %v1, 0(%r3)
-;   veclg %v0, %v1 ; jne 10 ; vchlgs %v4, %v1, %v0
+;   veclg %v0, %v1 ; jne 10 ; vchlgs %v5, %v1, %v0
 ;   lhi %r2, 0
 ;   lochinl %r2, 1
 ;   br %r14