bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Windows x86: Change i128 to return via the vector ABI

Browse source 
Clang and GCC both return `i128` in xmm0 on windows-msvc and
windows-gnu. Currently, Rust returns the type on the stack. Add a
calling convention adjustment so we also return scalar `i128`s using the
vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its
`-msvc` targets (more at [1]). If this happens, the change here will
need to be adjusted to only affect MinGW.

Link: https://github.com/rust-lang/rust/issues/134288
This commit is contained in:
Trevor Gross 2024-12-14 00:32:39 +00:00
parent 581e0ac90c
commit a44a20ee4a
5 changed files with 44 additions and 67 deletions
Download patch file Download diff file Expand all files Collapse all files

22
compiler/rustc_codegen_cranelift/src/abi/mod.rs
View file

@ -122,7 +122,7 @@ impl<'tcx> FunctionCx<'_, '_, 'tcx> {
&mut self,
name: &str,
params: Vec<AbiParam>,
returns: Vec<AbiParam>,
mut returns: Vec<AbiParam>,
args: &[Value],
) -> Cow<'_, [Value]> {
// Pass i128 arguments by-ref on Windows.
@ -146,15 +146,19 @@ impl<'tcx> FunctionCx<'_, '_, 'tcx> {
(params, args.into())
};
// Return i128 using a return area pointer on Windows and s390x.
let adjust_ret_param =
if self.tcx.sess.target.is_like_windows || self.tcx.sess.target.arch == "s390x" {
returns.len() == 1 && returns[0].value_type == types::I128
} else {
false
};
let ret_single_i128 = returns.len() == 1 && returns[0].value_type == types::I128;
if ret_single_i128 && self.tcx.sess.target.is_like_windows {
// Return i128 using the vector ABI on Windows
returns[0].value_type = types::I64X2;
if adjust_ret_param {
let ret = self.lib_call_unadjusted(name, params, returns, &args)[0];
// FIXME(bytecodealliance/wasmtime#6104) use bitcast instead of store to get from i64x2 to i128
let ret_ptr = self.create_stack_slot(16, 16);
ret_ptr.store(self, ret, MemFlags::trusted());
Cow::Owned(vec![ret_ptr.load(self, types::I128, MemFlags::trusted())])
} else if ret_single_i128 && self.tcx.sess.target.arch == "s390x" {
// Return i128 using a return area pointer on s390x.
let mut params = params;
let mut args = args.to_vec();

22
compiler/rustc_codegen_cranelift/src/cast.rs
View file

@ -96,25 +96,9 @@ pub(crate) fn clif_int_or_float_cast(
},
);
if fx.tcx.sess.target.is_like_windows {
let ret = fx.lib_call(
&name,
vec![AbiParam::new(from_ty)],
vec![AbiParam::new(types::I64X2)],
&[from],
)[0];
// FIXME(bytecodealliance/wasmtime#6104) use bitcast instead of store to get from i64x2 to i128
let ret_ptr = fx.create_stack_slot(16, 16);
ret_ptr.store(fx, ret, MemFlags::trusted());
ret_ptr.load(fx, types::I128, MemFlags::trusted())
} else {
fx.lib_call(
&name,
vec![AbiParam::new(from_ty)],
vec![AbiParam::new(types::I128)],
&[from],
)[0]
}
fx.lib_call(&name, vec![AbiParam::new(from_ty)], vec![AbiParam::new(types::I128)], &[
from,
])[0]
} else if to_ty == types::I8 || to_ty == types::I16 {
// FIXME implement fcvt_to_*int_sat.i8/i16
let val = if to_signed {

14
compiler/rustc_codegen_cranelift/src/codegen_i128.rs
View file

@ -33,19 +33,6 @@ pub(crate) fn maybe_codegen<'tcx>(
(BinOp::Rem, true) => "__modti3",
_ => unreachable!(),
};
if fx.tcx.sess.target.is_like_windows {
let args = [lhs.load_scalar(fx), rhs.load_scalar(fx)];
let ret = fx.lib_call(
name,
vec![AbiParam::new(types::I128), AbiParam::new(types::I128)],
vec![AbiParam::new(types::I64X2)],
&args,
)[0];
// FIXME(bytecodealliance/wasmtime#6104) use bitcast instead of store to get from i64x2 to i128
let ret_place = CPlace::new_stack_slot(fx, lhs.layout());
ret_place.to_ptr().store(fx, ret, MemFlags::trusted());
Some(ret_place.to_cvalue(fx))
} else {
let args = [lhs.load_scalar(fx), rhs.load_scalar(fx)];
let ret_val = fx.lib_call(
name,
@ -55,7 +42,6 @@ pub(crate) fn maybe_codegen<'tcx>(
)[0];
Some(CValue::by_val(ret_val, lhs.layout()))
}
}
BinOp::Lt | BinOp::Le | BinOp::Eq | BinOp::Ge | BinOp::Gt | BinOp::Ne | BinOp::Cmp => None,
BinOp::Shl | BinOp::ShlUnchecked | BinOp::Shr | BinOp::ShrUnchecked => None,
BinOp::AddWithOverflow | BinOp::SubWithOverflow | BinOp::MulWithOverflow => unreachable!(),

20
compiler/rustc_target/src/callconv/x86_win64.rs
View file

@ -1,4 +1,4 @@
use rustc_abi::{BackendRepr, Float, Primitive};
use rustc_abi::{BackendRepr, Float, Integer, Primitive, RegKind, Size};
use crate::abi::call::{ArgAbi, FnAbi, Reg};
use crate::spec::HasTargetSpec;
@ -6,7 +6,7 @@ use crate::spec::HasTargetSpec;
// Win64 ABI: https://docs.microsoft.com/en-us/cpp/build/parameter-passing
pub(crate) fn compute_abi_info<Ty>(_cx: &impl HasTargetSpec, fn_abi: &mut FnAbi<'_, Ty>) {
let fixup = |a: &mut ArgAbi<'_, Ty>| {
let fixup = |a: &mut ArgAbi<'_, Ty>, is_ret: bool| {
match a.layout.backend_repr {
BackendRepr::Uninhabited | BackendRepr::Memory { sized: false } => {}
BackendRepr::ScalarPair(..) | BackendRepr::Memory { sized: true } => {
@ -23,11 +23,16 @@ pub(crate) fn compute_abi_info<Ty>(_cx: &impl HasTargetSpec, fn_abi: &mut FnAbi<
// (probably what clang calls "illegal vectors").
}
BackendRepr::Scalar(scalar) => {
// Match what LLVM does for `f128` so that `compiler-builtins` builtins match up
// with what LLVM expects.
if a.layout.size.bytes() > 8
if is_ret && matches!(scalar.primitive(), Primitive::Int(Integer::I128, _)) {
// `i128` is returned in xmm0 by Clang and GCC
// FIXME(#134288): This may change for the `-msvc` targets in the future.
let reg = Reg { kind: RegKind::Vector, size: Size::from_bits(128) };
a.cast_to(reg);
} else if a.layout.size.bytes() > 8
&& !matches!(scalar.primitive(), Primitive::Float(Float::F128))
{
// Match what LLVM does for `f128` so that `compiler-builtins` builtins match up
// with what LLVM expects.
a.make_indirect();
} else {
a.extend_integer_width_to(32);
@ -37,8 +42,9 @@ pub(crate) fn compute_abi_info<Ty>(_cx: &impl HasTargetSpec, fn_abi: &mut FnAbi<
};
if !fn_abi.ret.is_ignore() {
fixup(&mut fn_abi.ret);
fixup(&mut fn_abi.ret, true);
}
for arg in fn_abi.args.iter_mut() {
if arg.is_ignore() && arg.layout.is_zst() {
// Windows ABIs do not talk about ZST since such types do not exist in MSVC.
@ -49,7 +55,7 @@ pub(crate) fn compute_abi_info<Ty>(_cx: &impl HasTargetSpec, fn_abi: &mut FnAbi<
arg.make_indirect_from_ignore();
continue;
}
fixup(arg);
fixup(arg, false);
}
// FIXME: We should likely also do something about ZST return types, similar to above.
// However, that's non-trivial due to `()`.

17
tests/codegen/i128-x86-callconv.rs
View file

@ -41,12 +41,11 @@ pub extern "C" fn pass(_arg0: u32, arg1: i128) {
#[no_mangle]
pub extern "C" fn ret(_arg0: u32, arg1: i128) -> i128 {
// CHECK-LABEL: @ret(
// i128 is returned on the stack on Windows.
// FIXME: this ABI does not agree with Clang or MinGW GCC
// WIN-SAME: ptr{{.*}} sret([16 x i8]){{.*}} [[RET:%_[0-9]+]], i32{{.*}} %_arg0, ptr{{.*}} %arg1)
// WIN: [[LOADED:%[0-9]+]] = load i128, ptr %arg1
// WIN: store i128 [[LOADED]], ptr [[RET]]
// WIN: ret void
// i128 is returned in xmm0 on Windows
// FIXME(#134288): This may change for the `-msvc` targets in the future.
// WIN-SAME: i32{{.*}} %_arg0, ptr{{.*}} %arg1)
// WIN: [[LOADED:%[_0-9]+]] = load <16 x i8>, ptr %arg1
// WIN-NEXT: ret <16 x i8> [[LOADED]]
arg1
}
@ -55,10 +54,8 @@ pub extern "C" fn ret(_arg0: u32, arg1: i128) -> i128 {
pub extern "C" fn forward(dst: *mut i128) {
// CHECK-LABEL: @forward
// WIN-SAME: ptr{{.*}} %dst)
// WIN: [[RETURNED:%[_0-9]+]] = alloca [16 x i8], align 16
// WIN: call void @extern_ret({{.*}} [[RETURNED]])
// WIN: [[TMP:%[_0-9]+]] = load i128, ptr [[RETURNED]]
// WIN: store i128 [[TMP]], ptr %dst
// WIN: [[RETURNED:%[_0-9]+]] = tail call <16 x i8> @extern_ret()
// WIN: store <16 x i8> [[RETURNED]], ptr %dst
// WIN: ret void
unsafe { *dst = extern_ret() };
}