Fix review comments

cranelift/codegen/src/abi.rs

@@ -123,7 +123,8 @@ pub fn legalize_args<AA: ArgAssigner>(args: &[AbiParam], aa: &mut AA) -> Option<
                 args.to_mut()[argno].location = loc;
                 argno += 1;
             }
-            // Assign argument to a location, change type to `INVALID` and move on to the next one.
+            // Assign argument to a location, change type to the requested one and move on to the
+            // next one.
             ArgAction::AssignAndChangeType(loc, ty) => {
                 let arg = &mut args.to_mut()[argno];
                 arg.location = loc;

cranelift/codegen/src/legalizer/boundary.rs

@@ -114,8 +114,7 @@ fn legalize_entry_params(func: &mut Function, entry: Block) {
 
         let abi_type = pos.func.signature.params[abi_arg];
         let arg_type = pos.func.dfg.value_type(arg);
-        match abi_type.purpose {
+        if let ArgumentPurpose::StructArgument(size) = abi_type.purpose {
-            ArgumentPurpose::StructArgument(size) => {
             let offset = if let ArgumentLoc::Stack(offset) = abi_type.location {
                 offset
             } else {
@@ -132,8 +131,7 @@ fn legalize_entry_params(func: &mut Function, entry: Block) {
             abi_arg += 1;
             continue;
         }
-            _ => {}
+
-        }
         if arg_type == abi_type.value_type {
             // No value translation is necessary, this argument matches the ABI type.
             // Just use the original block argument value. This is the most common case.
@@ -1023,6 +1021,7 @@ fn spill_entry_params(func: &mut Function, entry: Block) {
         .zip(func.dfg.block_params(entry))
     {
         if let ArgumentPurpose::StructArgument(_) = abi.purpose {
+            // Location has already been assigned during legalization.
         } else if let ArgumentLoc::Stack(offset) = abi.location {
             let ss = func
                 .stack_slots
@@ -1102,6 +1101,8 @@ fn spill_call_arguments(pos: &mut FuncCursor, isa: &dyn TargetIsa) -> bool {
             let mut s = Signature::new(isa.default_call_conv());
             s.params.push(AbiParam::new(pointer_type));
             s.params.push(AbiParam::new(pointer_type));
+            // The last argument of `memcpy` is a `size_t`. This is the same size as a pointer on
+            // all architectures we are interested in.
             s.params.push(AbiParam::new(pointer_type));
             legalize_libcall_signature(&mut s, isa);
             func.import_signature(s)

cranelift/filetests/filetests/isa/x86/struct-arg.clif

@@ -23,7 +23,29 @@ block0(v0: i64):
 ; nextln:  [Op1ret#c3]                         return v1, v6
 ; nextln:  }
 
-function u0:1(i64) -> i8 system_v {
+function u0:1(i64, i64 sarg(64)) -> i8 system_v {
+block0(v0: i64, v1: i64):
+    v2 = load.i8 v1
+    return v2
+}
+
+; check: function u0:1(i64 [%rdi], sarg_t sarg(64) [0], i64 fp [%rbp]) -> i8 [%rax], i64 fp [%rbp] system_v {
+; nextln:      ss0 = incoming_arg 64, offset 0
+; nextln:      ss1 = incoming_arg 16, offset -16
+
+; check:                                  block0(v0: i64 [%rdi], v4: sarg_t [ss0], v6: i64 [%rbp]):
+; nextln: [RexOp1pushq#50]                    x86_push v6
+; nextln: [RexOp1copysp#8089]                 copy_special %rsp -> %rbp
+; nextln: [RexOp1spaddr_id#808d,%rax]         v3 = stack_addr.i64 ss0
+; nextln:                                     v1 -> v3
+; nextln: [RexOp2ld#4b6,%rax]                 v5 = uload8.i32 v3
+; nextln: [null#00,%rax]                      v2 = ireduce.i8 v5
+; nextln: [RexOp1popq#58,%rbp]                v7 = x86_pop.i64
+; nextln: [Op1ret#c3]                         return v2, v7
+; nextln: }
+
+
+function u0:2(i64) -> i8 system_v {
 fn1 = u0:0(i64 sarg(64)) -> i8 system_v
 
 block0(v0: i64):
@@ -31,7 +53,7 @@ block0(v0: i64):
     return v1
 }
 
-; check: function u0:1(i64 [%rdi], i64 fp [%rbp]) -> i8 [%rax], i64 fp [%rbp] system_v {
+; check: function u0:2(i64 [%rdi], i64 fp [%rbp]) -> i8 [%rax], i64 fp [%rbp] system_v {
 ; nextln:     ss0 = outgoing_arg 64, offset 0
 ; nextln:     ss1 = incoming_arg 16, offset -16
 ; nextln:     sig0 = (sarg_t sarg(64) [0]) -> i8 [%rax] system_v
@@ -55,3 +77,41 @@ block0(v0: i64):
 ; nextln: [RexOp1popq#58,%rbp]                v6 = x86_pop.i64
 ; nextln: [Op1ret#c3]                         return v1, v6
 ; nextln: }
+
+function u0:3(i64, i64) -> i8 system_v {
+fn1 = u0:0(i64, i64 sarg(64)) -> i8 system_v
+
+block0(v0: i64, v1: i64):
+    v2 = call fn1(v0, v1)
+    return v2
+}
+
+; check: function u0:3(i64 [%rdi], i64 [%rsi], i64 fp [%rbp], i64 csr [%r15]) -> i8 [%rax], i64 fp [%rbp], i64 csr [%r15] system_v {
+; nextln:     ss0 = outgoing_arg 64, offset 0
+; nextln:     ss1 = spill_slot 8, offset -32
+; nextln:     ss2 = incoming_arg 24, offset -24
+; nextln:     sig0 = (i64 [%rdi], sarg_t sarg(64) [0]) -> i8 [%rax] system_v
+; nextln:     sig1 = (i64 [%rdi], i64 [%rsi], i64 [%rdx]) system_v
+; nextln:     fn1 = u0:0 sig0
+; nextln:     fn2 = %Memcpy sig1
+
+; check:                                  block0(v6: i64 [%rdi], v1: i64 [%rsi], v8: i64 [%rbp], v9: i64 [%r15]):
+; nextln: [RexOp1pushq#50]                    x86_push v8
+; nextln: [RexOp1copysp#8089]                 copy_special %rsp -> %rbp
+; nextln: [RexOp1pushq#50]                    x86_push v9
+; nextln: [RexOp1adjustsp_ib#d083]            adjust_sp_down_imm 72
+; nextln: [RexOp1spillSib32#8089,ss1]         v0 = spill v6
+; nextln: [RexOp1spaddr_id#808d,%rax]         v3 = stack_addr.i64 ss0
+; nextln: [RexOp1pu_id#b8,%rcx]               v4 = iconst.i64 64
+; nextln: [RexOp1rmov#8089]                   regmove v3, %rax -> %rdi
+; nextln: [RexOp1rmov#8089]                   regmove v4, %rcx -> %rdx
+; nextln: [Op1call_plt_id#e8]                 call fn2(v3, v1, v4)
+; nextln: [dummy_sarg_t#00,ss0]               v5 = dummy_sarg_t
+; nextln: [RexOp1fillSib32#808b,%r15]         v7 = fill v0
+; nextln: [RexOp1rmov#8089]                   regmove v7, %r15 -> %rdi
+; nextln: [Op1call_plt_id#e8,%rax]            v2 = call fn1(v7, v5)
+; nextln: [RexOp1adjustsp_ib#8083]            adjust_sp_up_imm 72
+; nextln: [RexOp1popq#58,%r15]                v11 = x86_pop.i64
+; nextln: [RexOp1popq#58,%rbp]                v10 = x86_pop.i64
+; nextln: [Op1ret#c3]                         return v2, v10, v11
+; nextln: }