From 759cc3e75196ca3efe8efda0ad642b18f9acfde0 Mon Sep 17 00:00:00 2001 From: teapotd Date: Mon, 25 May 2020 20:34:50 +0200 Subject: [PATCH] Implement passing arguments by ref for win64 ABI --- cranelift/codegen/src/abi.rs | 31 +++++++++++++++++-- cranelift/codegen/src/ir/extfunc.rs | 10 ++++++ cranelift/codegen/src/isa/x86/abi.rs | 26 ++++++++++++---- cranelift/codegen/src/legalizer/boundary.rs | 26 +++++++++++++++- .../isa/x86/windows_fastcall_x64.clif | 14 +++++++++ .../filetests/legalizer/pass_by_ref.clif | 31 +++++++++++++++++++ 6 files changed, 128 insertions(+), 10 deletions(-) create mode 100644 cranelift/filetests/filetests/legalizer/pass_by_ref.clif diff --git a/cranelift/codegen/src/abi.rs b/cranelift/codegen/src/abi.rs index 3a3ed7a53b..0da415b5ce 100644 --- a/cranelift/codegen/src/abi.rs +++ b/cranelift/codegen/src/abi.rs @@ -54,6 +54,9 @@ pub enum ValueConversion { /// Unsigned zero-extend value to the required type. Uext(Type), + + /// Pass value by pointer of given integer type. + Pointer(Type), } impl ValueConversion { @@ -63,7 +66,7 @@ impl ValueConversion { Self::IntSplit => ty.half_width().expect("Integer type too small to split"), Self::VectorSplit => ty.half_vector().expect("Not a vector"), Self::IntBits => Type::int(ty.bits()).expect("Bad integer size"), - Self::Sext(nty) | Self::Uext(nty) => nty, + Self::Sext(nty) | Self::Uext(nty) | Self::Pointer(nty) => nty, } } @@ -74,6 +77,11 @@ impl ValueConversion { _ => false, } } + + /// Is this a conversion to pointer? + pub fn is_pointer(self) -> bool { + matches!(self, Self::Pointer(_)) + } } /// Common trait for assigning arguments to registers or stack locations. @@ -110,10 +118,16 @@ pub fn legalize_args(args: &[AbiParam], aa: &mut AA) -> Option< } // Split this argument into two smaller ones. Then revisit both. ArgAction::Convert(conv) => { + debug_assert!( + !arg.legalized_to_pointer, + "No more conversions allowed after conversion to pointer" + ); let value_type = conv.apply(arg.value_type); - let new_arg = AbiParam { value_type, ..arg }; args.to_mut()[argno].value_type = value_type; - if conv.is_split() { + if conv.is_pointer() { + args.to_mut()[argno].legalized_to_pointer = true; + } else if conv.is_split() { + let new_arg = AbiParam { value_type, ..arg }; args.to_mut().insert(argno + 1, new_arg); } } @@ -152,6 +166,10 @@ pub fn legalize_abi_value(have: Type, arg: &AbiParam) -> ValueConversion { let have_bits = have.bits(); let arg_bits = arg.value_type.bits(); + if arg.legalized_to_pointer { + return ValueConversion::Pointer(arg.value_type); + } + match have_bits.cmp(&arg_bits) { // We have fewer bits than the ABI argument. Ordering::Less => { @@ -226,5 +244,12 @@ mod tests { legalize_abi_value(types::F64, &arg), ValueConversion::IntBits ); + + // Value is passed by reference + arg.legalized_to_pointer = true; + assert_eq!( + legalize_abi_value(types::F64, &arg), + ValueConversion::Pointer(types::I32) + ); } } diff --git a/cranelift/codegen/src/ir/extfunc.rs b/cranelift/codegen/src/ir/extfunc.rs index 0a34974c9d..e2f2bb984b 100644 --- a/cranelift/codegen/src/ir/extfunc.rs +++ b/cranelift/codegen/src/ir/extfunc.rs @@ -156,6 +156,8 @@ pub struct AbiParam { /// ABI-specific location of this argument, or `Unassigned` for arguments that have not yet /// been legalized. pub location: ArgumentLoc, + /// Was the argument converted to pointer during legalization? + pub legalized_to_pointer: bool, } impl AbiParam { @@ -166,6 +168,7 @@ impl AbiParam { extension: ArgumentExtension::None, purpose: ArgumentPurpose::Normal, location: Default::default(), + legalized_to_pointer: false, } } @@ -176,6 +179,7 @@ impl AbiParam { extension: ArgumentExtension::None, purpose, location: Default::default(), + legalized_to_pointer: false, } } @@ -186,6 +190,7 @@ impl AbiParam { extension: ArgumentExtension::None, purpose, location: ArgumentLoc::Reg(regunit), + legalized_to_pointer: false, } } @@ -219,6 +224,9 @@ pub struct DisplayAbiParam<'a>(&'a AbiParam, Option<&'a RegInfo>); impl<'a> fmt::Display for DisplayAbiParam<'a> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{}", self.0.value_type)?; + if self.0.legalized_to_pointer { + write!(f, " ptr")?; + } match self.0.extension { ArgumentExtension::None => {} ArgumentExtension::Uext => write!(f, " uext")?, @@ -415,6 +423,8 @@ mod tests { assert_eq!(t.sext().to_string(), "i32 sext"); t.purpose = ArgumentPurpose::StructReturn; assert_eq!(t.to_string(), "i32 uext sret"); + t.legalized_to_pointer = true; + assert_eq!(t.to_string(), "i32 ptr uext sret"); } #[test] diff --git a/cranelift/codegen/src/isa/x86/abi.rs b/cranelift/codegen/src/isa/x86/abi.rs index d0308f73e8..02358ba89b 100644 --- a/cranelift/codegen/src/isa/x86/abi.rs +++ b/cranelift/codegen/src/isa/x86/abi.rs @@ -70,6 +70,7 @@ struct Args { shared_flags: shared_settings::Flags, #[allow(dead_code)] isa_flags: isa_settings::Flags, + assigning_returns: bool, } impl Args { @@ -80,6 +81,7 @@ impl Args { call_conv: CallConv, shared_flags: &shared_settings::Flags, isa_flags: &isa_settings::Flags, + assigning_returns: bool, ) -> Self { let offset = if call_conv.extends_windows_fastcall() { WIN_SHADOW_STACK_SPACE @@ -99,6 +101,7 @@ impl Args { call_conv, shared_flags: shared_flags.clone(), isa_flags: isa_flags.clone(), + assigning_returns, } } } @@ -107,6 +110,17 @@ impl ArgAssigner for Args { fn assign(&mut self, arg: &AbiParam) -> ArgAction { let ty = arg.value_type; + if ty.bits() > u16::from(self.pointer_bits) { + if !self.assigning_returns && self.call_conv.extends_windows_fastcall() { + // "Any argument that doesn't fit in 8 bytes, or isn't + // 1, 2, 4, or 8 bytes, must be passed by reference" + return ValueConversion::Pointer(self.pointer_type).into(); + } else if !ty.is_vector() && !ty.is_float() { + // On SystemV large integers and booleans are broken down to fit in a register. + return ValueConversion::IntSplit.into(); + } + } + // Vectors should stay in vector registers unless SIMD is not enabled--then they are split if ty.is_vector() { if self.shared_flags.enable_simd() { @@ -117,11 +131,6 @@ impl ArgAssigner for Args { return ValueConversion::VectorSplit.into(); } - // Large integers and booleans are broken down to fit in a register. - if !ty.is_float() && ty.bits() > u16::from(self.pointer_bits) { - return ValueConversion::IntSplit.into(); - } - // Small integers are extended to the size of a pointer register. if ty.is_int() && ty.bits() < u16::from(self.pointer_bits) { match arg.extension { @@ -203,7 +212,7 @@ pub fn legalize_signature( PointerWidth::U16 => panic!(), PointerWidth::U32 => { bits = 32; - args = Args::new(bits, &[], 0, sig.call_conv, shared_flags, isa_flags); + args = Args::new(bits, &[], 0, sig.call_conv, shared_flags, isa_flags, false); } PointerWidth::U64 => { bits = 64; @@ -215,6 +224,7 @@ pub fn legalize_signature( sig.call_conv, shared_flags, isa_flags, + false, ) } else { Args::new( @@ -224,6 +234,7 @@ pub fn legalize_signature( sig.call_conv, shared_flags, isa_flags, + false, ) }; } @@ -243,6 +254,7 @@ pub fn legalize_signature( sig.call_conv, shared_flags, isa_flags, + true, ); // If we don't have enough available return registers @@ -267,6 +279,7 @@ pub fn legalize_signature( purpose: ArgumentPurpose::StructReturn, extension: ArgumentExtension::None, location: ArgumentLoc::Unassigned, + legalized_to_pointer: false, }; match args.assign(&ret_ptr_param) { ArgAction::Assign(ArgumentLoc::Reg(reg)) => { @@ -283,6 +296,7 @@ pub fn legalize_signature( purpose: ArgumentPurpose::StructReturn, extension: ArgumentExtension::None, location: ArgumentLoc::Unassigned, + legalized_to_pointer: false, }; match backup_rets.assign(&ret_ptr_return) { ArgAction::Assign(ArgumentLoc::Reg(reg)) => { diff --git a/cranelift/codegen/src/legalizer/boundary.rs b/cranelift/codegen/src/legalizer/boundary.rs index 185e4c74fa..aba52a8c2b 100644 --- a/cranelift/codegen/src/legalizer/boundary.rs +++ b/cranelift/codegen/src/legalizer/boundary.rs @@ -504,6 +504,13 @@ where // this value. pos.ins().with_results([into_result]).ireduce(ty, arg) } + // ABI argument is a pointer to the value we want. + ValueConversion::Pointer(abi_ty) => { + let arg = convert_from_abi(pos, abi_ty, None, get_arg); + pos.ins() + .with_results([into_result]) + .load(ty, MemFlags::new(), arg, 0) + } } } @@ -563,6 +570,18 @@ fn convert_to_abi( let arg = pos.ins().uextend(abi_ty, value); convert_to_abi(pos, cfg, arg, put_arg); } + ValueConversion::Pointer(abi_ty) => { + // Note: This conversion can only happen for call arguments, + // so we can allocate the value on stack safely. + let stack_slot = pos.func.create_stack_slot(StackSlotData { + kind: StackSlotKind::ExplicitSlot, + size: ty.bytes(), + offset: None, + }); + let arg = pos.ins().stack_addr(abi_ty, stack_slot, 0); + pos.ins().store(MemFlags::new(), value, arg, 0); + convert_to_abi(pos, cfg, arg, put_arg); + } } } @@ -815,7 +834,12 @@ pub fn handle_return_abi(inst: Inst, func: &mut Function, cfg: &ControlFlowGraph pos.use_srcloc(inst); legalize_inst_arguments(pos, cfg, abi_args, |func, abi_arg| { - func.signature.returns[abi_arg] + let arg = func.signature.returns[abi_arg]; + debug_assert!( + !arg.legalized_to_pointer, + "Return value cannot be legalized to pointer" + ); + arg }); // Append special return arguments for any `sret`, `link`, and `vmctx` return values added to // the legalized signature. These values should simply be propagated from the entry block diff --git a/cranelift/filetests/filetests/isa/x86/windows_fastcall_x64.clif b/cranelift/filetests/filetests/isa/x86/windows_fastcall_x64.clif index cb1856ca3d..27106d7d98 100644 --- a/cranelift/filetests/filetests/isa/x86/windows_fastcall_x64.clif +++ b/cranelift/filetests/filetests/isa/x86/windows_fastcall_x64.clif @@ -117,6 +117,20 @@ block0(v0: i64, v1: i64): } ; check: function %ret_val_i128(i64 [%rdx], i64 [%r8], i64 sret [%rcx], i64 fp [%rbp]) -> i64 sret [%rax], i64 fp [%rbp] windows_fastcall { +; check if i128 is passed by reference +function %i128_arg(i128) windows_fastcall { +block0(v0: i128): + return +} +; check: function %i128_arg(i64 ptr [%rcx], i64 fp [%rbp]) -> i64 fp [%rbp] windows_fastcall { + +; check if vector types are passed by reference +function %i32x4_arg(i32x4) windows_fastcall { +block0(v0: i32x4): + return +} +; check: function %i32x4_arg(i64 ptr [%rcx], i64 fp [%rbp]) -> i64 fp [%rbp] windows_fastcall { + function %internal_stack_arg_function_call(i64) -> i64 windows_fastcall { fn0 = %foo(i64, i64, i64, i64) -> i64 windows_fastcall fn1 = %foo2(i64, i64, i64, i64) -> i64 windows_fastcall diff --git a/cranelift/filetests/filetests/legalizer/pass_by_ref.clif b/cranelift/filetests/filetests/legalizer/pass_by_ref.clif new file mode 100644 index 0000000000..5cdfc92105 --- /dev/null +++ b/cranelift/filetests/filetests/legalizer/pass_by_ref.clif @@ -0,0 +1,31 @@ +test legalizer +target x86_64 + +function %legalize_entry(i128) -> i64 windows_fastcall { +block0(v0: i128): + v1, v2 = isplit v0 + return v2 +} +; check: function %legalize_entry(i64 ptr [%rcx]) -> i64 [%rax] windows_fastcall { +; nextln: block0(v3: i64): +; nextln: v4 = load.i64 v3 +; nextln: v1 -> v4 +; nextln: v5 = load.i64 v3+8 +; nextln: v2 -> v5 +; nextln: v0 = iconcat v4, v5 +; nextln: return v2 + +function %legalize_call() { + fn0 = %foo(i32x4) windows_fastcall +block0: + v0 = vconst.i32x4 [1 2 3 4] + call fn0(v0) + return +} +; check: ss0 = explicit_slot 16 +; check: sig0 = (i64 ptr [%rcx]) windows_fastcall +; check: v0 = vconst.i32x4 const0 +; nextln: v1 = stack_addr.i64 ss0 +; nextln: store v0, v1 +; nextln: v2 = func_addr.i64 fn0 +; nextln: call_indirect sig0, v2(v1)