cranelift: Emit argument location uses eagerly in gen_arg

This reverts the key parts of e3a08d4
(#8151), because it turns out that we didn't need that abstraction.

Several changes in the last month have enabled this:

- #8292 and then #8316 allow us to refer to either incoming or outgoing
  argument areas in a (mostly) consistent way

- #8327, #8377, and #8383 demonstrate that we never need to delay
  writing stack arguments directly to their final location

prtest:full

Author: jameysharp

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

@@ -178,8 +178,7 @@ fn emit_vm_call(
     assert_eq!(inputs.len(), abi.num_args(ctx.sigs()));
 
     for (i, input) in inputs.iter().enumerate() {
-        let moves = abi.gen_arg(ctx, i, ValueRegs::one(*input));
-        abi.emit_arg_moves(ctx, moves);
+        abi.gen_arg(ctx, i, ValueRegs::one(*input));
     }
 
     let mut retval_insts: SmallInstVec<_> = smallvec![];

cranelift/codegen/src/machinst/abi.rs

@@ -2051,19 +2051,6 @@ impl<M: ABIMachineSpec> Callee<M> {
     }
 }
 
-/// The register or stack slot location of an argument.
-#[derive(Clone, Debug)]
-pub enum ArgLoc {
-    /// The physical register that the value will be passed through.
-    Reg(PReg),
-
-    /// The offset into the argument area where this value will be passed. It's up to the consumer
-    /// of the `ArgLoc::Stack` variant to decide how to find the argument area that the `offset`
-    /// value is relative to. Depending on the abi, this may end up being relative to SP or FP, for
-    /// example with a tail call where the frame is reused.
-    Stack { offset: i64, ty: ir::Type },
-}
-
 /// An input argument to a call instruction: the vreg that is used,
 /// and the preg it is constrained to (per the ABI).
 #[derive(Clone, Debug)]
@@ -2298,38 +2285,17 @@ impl<M: ABIMachineSpec> CallSite<M> {
         }
     }
 
-    /// Emit moves or uses for the moves list generated by [`Self::gen_arg`].
-    pub fn emit_arg_moves(&mut self, ctx: &mut Lower<M::I>, moves: SmallVec<[(VReg, ArgLoc); 2]>) {
-        for (vreg, loc) in moves {
-            let vreg = vreg.into();
-            match loc {
-                ArgLoc::Reg(preg) => self.uses.push(CallArgPair {
-                    vreg,
-                    preg: preg.into(),
-                }),
-                ArgLoc::Stack { offset, ty } => {
-                    let amode = if self.is_tail_call() {
-                        StackAMode::IncomingArg(offset, ctx.sigs()[self.sig].sized_stack_arg_space)
-                    } else {
-                        StackAMode::OutgoingArg(offset)
-                    };
-                    ctx.emit(M::gen_store_stack(amode, vreg, ty))
-                }
-            }
-        }
-    }
-
     /// Add a constraint for an argument value from a source register.
     /// For large arguments with associated stack buffer, this may
     /// load the address of the buffer into the argument register, if
     /// required by the ABI.
-    pub fn gen_arg(
-        &mut self,
-        ctx: &mut Lower<M::I>,
-        idx: usize,
-        from_regs: ValueRegs<Reg>,
-    ) -> SmallVec<[(VReg, ArgLoc); 2]> {
-        let mut locs = smallvec![];
+    pub fn gen_arg(&mut self, ctx: &mut Lower<M::I>, idx: usize, from_regs: ValueRegs<Reg>) {
+        let stack_arg_space = ctx.sigs()[self.sig].sized_stack_arg_space;
+        let stack_arg = if self.is_tail_call() {
+            StackAMode::IncomingArg
+        } else {
+            |offset, _| StackAMode::OutgoingArg(offset)
+        };
         let word_rc = M::word_reg_class();
         let word_bits = M::word_bits() as usize;
 
@@ -2342,7 +2308,9 @@ impl<M: ABIMachineSpec> CallSite<M> {
                             reg, ty, extension, ..
                         } => {
                             let ext = M::get_ext_mode(ctx.sigs()[self.sig].call_conv, extension);
-                            if ext != ir::ArgumentExtension::None && ty_bits(ty) < word_bits {
+                            let vreg = if ext != ir::ArgumentExtension::None
+                                && ty_bits(ty) < word_bits
+                            {
                                 assert_eq!(word_rc, reg.class());
                                 let signed = match ext {
                                     ir::ArgumentExtension::Uext => false,
@@ -2358,7 +2326,7 @@ impl<M: ABIMachineSpec> CallSite<M> {
                                     ty_bits(ty) as u8,
                                     word_bits as u8,
                                 ));
-                                locs.push((extend_result.to_reg().into(), ArgLoc::Reg(reg.into())));
+                                extend_result.to_reg()
                             } else if ty.is_ref() {
                                 // Reference-typed args need to be
                                 // passed as a copy; the original vreg
@@ -2367,10 +2335,13 @@ impl<M: ABIMachineSpec> CallSite<M> {
                                 let ref_copy = ctx.alloc_tmp(M::word_type()).only_reg().unwrap();
                                 ctx.emit(M::gen_move(ref_copy, *from_reg, M::word_type()));
 
-                                locs.push((ref_copy.to_reg().into(), ArgLoc::Reg(reg.into())));
+                                ref_copy.to_reg()
                             } else {
-                                locs.push((from_reg.into(), ArgLoc::Reg(reg.into())));
-                            }
+                                *from_reg
+                            };
+
+                            let preg = reg.into();
+                            self.uses.push(CallArgPair { vreg, preg });
                         }
                         &ABIArgSlot::Stack {
                             offset,
@@ -2401,7 +2372,11 @@ impl<M: ABIMachineSpec> CallSite<M> {
                                 } else {
                                     (*from_reg, ty)
                                 };
-                            locs.push((data.into(), ArgLoc::Stack { offset, ty }));
+                            ctx.emit(M::gen_store_stack(
+                                stack_arg(offset, stack_arg_space),
+                                data,
+                                ty,
+                            ));
                         }
                     }
                 }
@@ -2422,18 +2397,19 @@ impl<M: ABIMachineSpec> CallSite<M> {
                 ctx.emit(M::gen_get_stack_addr(amode, tmp));
                 let tmp = tmp.to_reg();
                 ctx.emit(M::gen_store_base_offset(tmp, 0, vreg, ty));
-                let loc = match pointer {
-                    ABIArgSlot::Reg { reg, .. } => ArgLoc::Reg(reg.into()),
-                    ABIArgSlot::Stack { offset, .. } => {
-                        let ty = M::word_type();
-                        ArgLoc::Stack { offset, ty }
-                    }
-                };
-                locs.push((tmp.into(), loc));
+                match pointer {
+                    ABIArgSlot::Reg { reg, .. } => self.uses.push(CallArgPair {
+                        vreg: tmp,
+                        preg: reg.into(),
+                    }),
+                    ABIArgSlot::Stack { offset, .. } => ctx.emit(M::gen_store_stack(
+                        stack_arg(offset, stack_arg_space),
+                        tmp,
+                        M::word_type(),
+                    )),
+                }
             }
         }
-
-        locs
     }
 
     /// Call `gen_arg` for each non-hidden argument and emit all instructions
@@ -2451,8 +2427,7 @@ impl<M: ABIMachineSpec> CallSite<M> {
             self.emit_copy_regs_to_buffer(ctx, i, *arg_regs);
         }
         for (i, value_regs) in arg_value_regs.iter().enumerate() {
-            let moves = self.gen_arg(ctx, i, *value_regs);
-            self.emit_arg_moves(ctx, moves);
+            self.gen_arg(ctx, i, *value_regs);
         }
     }
 
@@ -2464,8 +2439,7 @@ impl<M: ABIMachineSpec> CallSite<M> {
                 "if the tail callee has a return pointer, then the tail caller \
                  must as well",
             );
-            let moves = self.gen_arg(ctx, i.into(), ValueRegs::one(ret_area_ptr.to_reg()));
-            self.emit_arg_moves(ctx, moves);
+            self.gen_arg(ctx, i.into(), ValueRegs::one(ret_area_ptr.to_reg()));
         }
     }
 
@@ -2564,8 +2538,7 @@ impl<M: ABIMachineSpec> CallSite<M> {
                 StackAMode::OutgoingArg(ret_area_base),
                 rd,
             ));
-            let moves = self.gen_arg(ctx, i.into(), ValueRegs::one(rd.to_reg()));
-            self.emit_arg_moves(ctx, moves);
+            self.gen_arg(ctx, i.into(), ValueRegs::one(rd.to_reg()));
         }
 
         let (uses, defs) = (

cranelift/codegen/src/machinst/isle.rs

@@ -826,8 +826,7 @@ macro_rules! isle_prelude_method_helpers {
                 call_site.emit_copy_regs_to_buffer(self.lower_ctx, i, *arg_regs);
             }
             for (i, arg_regs) in arg_regs.iter().enumerate() {
-                let moves = call_site.gen_arg(self.lower_ctx, i, *arg_regs);
-                call_site.emit_arg_moves(self.lower_ctx, moves);
+                call_site.gen_arg(self.lower_ctx, i, *arg_regs);
             }
         }