From 5c5a30f76c35e15697fc150fb00c4b86be621d66 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Thu, 16 Jul 2020 18:22:05 +0200 Subject: [PATCH] Fix review comments --- cranelift/codegen/src/abi.rs | 3 +- cranelift/codegen/src/legalizer/boundary.rs | 39 +++++------ .../filetests/isa/x86/struct-arg.clif | 64 ++++++++++++++++++- 3 files changed, 84 insertions(+), 22 deletions(-) diff --git a/cranelift/codegen/src/abi.rs b/cranelift/codegen/src/abi.rs index a281413e56..883ec546e4 100644 --- a/cranelift/codegen/src/abi.rs +++ b/cranelift/codegen/src/abi.rs @@ -123,7 +123,8 @@ pub fn legalize_args(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; diff --git a/cranelift/codegen/src/legalizer/boundary.rs b/cranelift/codegen/src/legalizer/boundary.rs index 12eb7b2b7c..43d4f99113 100644 --- a/cranelift/codegen/src/legalizer/boundary.rs +++ b/cranelift/codegen/src/legalizer/boundary.rs @@ -114,26 +114,24 @@ 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 { - ArgumentPurpose::StructArgument(size) => { - let offset = if let ArgumentLoc::Stack(offset) = abi_type.location { - offset - } else { - unreachable!("StructArgument must already have a Stack ArgumentLoc assigned"); - }; - let ss = pos.func.stack_slots.make_incoming_arg(size, offset); - let struct_arg = pos.ins().stack_addr(arg_type, ss, 0); - pos.func.dfg.change_to_alias(arg, struct_arg); - let dummy = pos - .func - .dfg - .append_block_param(entry, crate::ir::types::SARG_T); - pos.func.locations[dummy] = ValueLoc::Stack(ss); - abi_arg += 1; - continue; - } - _ => {} + if let ArgumentPurpose::StructArgument(size) = abi_type.purpose { + let offset = if let ArgumentLoc::Stack(offset) = abi_type.location { + offset + } else { + unreachable!("StructArgument must already have a Stack ArgumentLoc assigned"); + }; + let ss = pos.func.stack_slots.make_incoming_arg(size, offset); + let struct_arg = pos.ins().stack_addr(arg_type, ss, 0); + pos.func.dfg.change_to_alias(arg, struct_arg); + let dummy = pos + .func + .dfg + .append_block_param(entry, crate::ir::types::SARG_T); + pos.func.locations[dummy] = ValueLoc::Stack(ss); + 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) diff --git a/cranelift/filetests/filetests/isa/x86/struct-arg.clif b/cranelift/filetests/filetests/isa/x86/struct-arg.clif index cd85fc026f..2043ac4979 100644 --- a/cranelift/filetests/filetests/isa/x86/struct-arg.clif +++ b/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: }