Skip to content

Set signext or zeroext for integer arguments on RISC-V and LoongArch64 #131955

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions compiler/rustc_target/src/callconv/loongarch.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::abi::call::{ArgAbi, ArgExtension, CastTarget, FnAbi, PassMode, Reg, RegKind, Uniform};
use crate::abi::{self, Abi, FieldsShape, HasDataLayout, Size, TyAbiInterface, TyAndLayout};
use crate::spec::HasTargetSpec;
use crate::spec::abi::Abi as SpecAbi;

#[derive(Copy, Clone)]
enum RegPassKind {
Expand Down Expand Up @@ -359,3 +360,30 @@ where
);
}
}

pub(crate) fn compute_rust_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>, abi: SpecAbi)
where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout + HasTargetSpec,
{
if abi == SpecAbi::RustIntrinsic {
return;
}

let grlen = cx.data_layout().pointer_size.bits();

for arg in fn_abi.args.iter_mut() {
if arg.is_ignore() {
continue;
}

// LLVM integers types do not differentiate between signed or unsigned integers.
// Some LoongArch instructions do not have a `.w` suffix version, they use all the
// GRLEN bits. By explicitly setting the `signext` or `zeroext` attribute
// according to signedness to avoid unnecessary integer extending instructions.
//
// This is similar to the RISC-V case, see
// https://github.com/rust-lang/rust/issues/114508 for details.
extend_integer_width(arg, grlen);
}
}
119 changes: 117 additions & 2 deletions compiler/rustc_target/src/callconv/mod.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
use std::fmt;
use std::str::FromStr;
use std::{fmt, iter};

pub use rustc_abi::{Reg, RegKind};
use rustc_macros::HashStable_Generic;
use rustc_span::Symbol;

use crate::abi::{self, Abi, Align, HasDataLayout, Size, TyAbiInterface, TyAndLayout};
use crate::abi::{
self, Abi, AddressSpace, Align, HasDataLayout, Pointer, Size, TyAbiInterface, TyAndLayout,
};
use crate::spec::abi::Abi as SpecAbi;
use crate::spec::{self, HasTargetSpec, HasWasmCAbiOpt, HasX86AbiOpt, WasmCAbi};

mod aarch64;
Expand Down Expand Up @@ -720,6 +723,118 @@ impl<'a, Ty> FnAbi<'a, Ty> {

Ok(())
}

pub fn adjust_for_rust_abi<C>(&mut self, cx: &C, abi: SpecAbi)
where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout + HasTargetSpec,
{
let spec = cx.target_spec();
match &spec.arch[..] {
"x86" => x86::compute_rust_abi_info(cx, self, abi),
"riscv32" | "riscv64" => riscv::compute_rust_abi_info(cx, self, abi),
"loongarch64" => loongarch::compute_rust_abi_info(cx, self, abi),
_ => {}
};

for (arg_idx, arg) in self
.args
.iter_mut()
.enumerate()
.map(|(idx, arg)| (Some(idx), arg))
.chain(iter::once((None, &mut self.ret)))
{
if arg.is_ignore() {
continue;
}

if arg_idx.is_none() && arg.layout.size > Pointer(AddressSpace::DATA).size(cx) * 2 {
// Return values larger than 2 registers using a return area
// pointer. LLVM and Cranelift disagree about how to return
// values that don't fit in the registers designated for return
// values. LLVM will force the entire return value to be passed
// by return area pointer, while Cranelift will look at each IR level
// return value independently and decide to pass it in a
// register or not, which would result in the return value
// being passed partially in registers and partially through a
// return area pointer.
//
// While Cranelift may need to be fixed as the LLVM behavior is
// generally more correct with respect to the surface language,
// forcing this behavior in rustc itself makes it easier for
// other backends to conform to the Rust ABI and for the C ABI
// rustc already handles this behavior anyway.
//
// In addition LLVM's decision to pass the return value in
// registers or using a return area pointer depends on how
// exactly the return type is lowered to an LLVM IR type. For
// example `Option<u128>` can be lowered as `{ i128, i128 }`
// in which case the x86_64 backend would use a return area
// pointer, or it could be passed as `{ i32, i128 }` in which
// case the x86_64 backend would pass it in registers by taking
// advantage of an LLVM ABI extension that allows using 3
// registers for the x86_64 sysv call conv rather than the
// officially specified 2 registers.
//
// FIXME: Technically we should look at the amount of available
// return registers rather than guessing that there are 2
// registers for return values. In practice only a couple of
// architectures have less than 2 return registers. None of
// which supported by Cranelift.
//
// NOTE: This adjustment is only necessary for the Rust ABI as
// for other ABI's the calling convention implementations in
// rustc_target already ensure any return value which doesn't
// fit in the available amount of return registers is passed in
// the right way for the current target.
arg.make_indirect();
continue;
}

match arg.layout.abi {
Abi::Aggregate { .. } => {}

// This is a fun case! The gist of what this is doing is
// that we want callers and callees to always agree on the
// ABI of how they pass SIMD arguments. If we were to *not*
// make these arguments indirect then they'd be immediates
// in LLVM, which means that they'd used whatever the
// appropriate ABI is for the callee and the caller. That
// means, for example, if the caller doesn't have AVX
// enabled but the callee does, then passing an AVX argument
// across this boundary would cause corrupt data to show up.
//
// This problem is fixed by unconditionally passing SIMD
// arguments through memory between callers and callees
// which should get them all to agree on ABI regardless of
// target feature sets. Some more information about this
// issue can be found in #44367.
//
// Note that the intrinsic ABI is exempt here as
// that's how we connect up to LLVM and it's unstable
// anyway, we control all calls to it in libstd.
Abi::Vector { .. } if abi != SpecAbi::RustIntrinsic && spec.simd_types_indirect => {
arg.make_indirect();
continue;
}

_ => continue,
}
// Compute `Aggregate` ABI.

let is_indirect_not_on_stack =
matches!(arg.mode, PassMode::Indirect { on_stack: false, .. });
assert!(is_indirect_not_on_stack);

let size = arg.layout.size;
if !arg.layout.is_unsized() && size <= Pointer(AddressSpace::DATA).size(cx) {
// We want to pass small aggregates as immediates, but using
// an LLVM aggregate type for this leads to bad optimizations,
// so we pick an appropriately sized integer type instead.
arg.cast_to(Reg { kind: RegKind::Integer, size });
}
}
}
}

impl FromStr for Conv {
Expand Down
27 changes: 27 additions & 0 deletions compiler/rustc_target/src/callconv/riscv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use crate::abi::call::{ArgAbi, ArgExtension, CastTarget, FnAbi, PassMode, Reg, RegKind, Uniform};
use crate::abi::{self, Abi, FieldsShape, HasDataLayout, Size, TyAbiInterface, TyAndLayout};
use crate::spec::HasTargetSpec;
use crate::spec::abi::Abi as SpecAbi;

#[derive(Copy, Clone)]
enum RegPassKind {
Expand Down Expand Up @@ -365,3 +366,29 @@ where
);
}
}

pub(crate) fn compute_rust_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>, abi: SpecAbi)
where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout + HasTargetSpec,
{
if abi == SpecAbi::RustIntrinsic {
return;
}

let xlen = cx.data_layout().pointer_size.bits();

for arg in fn_abi.args.iter_mut() {
if arg.is_ignore() {
continue;
}

// LLVM integers types do not differentiate between signed or unsigned integers.
// Some RISC-V instructions do not have a `.w` suffix version, they use all the
// XLEN bits. By explicitly setting the `signext` or `zeroext` attribute
// according to signedness to avoid unnecessary integer extending instructions.
//
// See https://github.com/rust-lang/rust/issues/114508 for details.
extend_integer_width(arg, xlen);
}
}
37 changes: 36 additions & 1 deletion compiler/rustc_target/src/callconv/x86.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use crate::abi::call::{ArgAttribute, FnAbi, PassMode, Reg, RegKind};
use crate::abi::{Abi, Align, HasDataLayout, TyAbiInterface, TyAndLayout};
use crate::abi::{
Abi, AddressSpace, Align, Float, HasDataLayout, Pointer, TyAbiInterface, TyAndLayout,
};
use crate::spec::HasTargetSpec;
use crate::spec::abi::Abi as SpecAbi;

#[derive(PartialEq)]
pub(crate) enum Flavor {
Expand Down Expand Up @@ -207,3 +210,35 @@ pub(crate) fn fill_inregs<'a, Ty, C>(
}
}
}

pub(crate) fn compute_rust_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>, abi: SpecAbi)
where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout + HasTargetSpec,
{
// Avoid returning floats in x87 registers on x86 as loading and storing from x87
// registers will quiet signalling NaNs. Also avoid using SSE registers since they
// are not always available (depending on target features).
if !fn_abi.ret.is_ignore()
// Intrinsics themselves are not actual "real" functions, so theres no need to change their ABIs.
&& abi != SpecAbi::RustIntrinsic
{
let has_float = match fn_abi.ret.layout.abi {
Abi::Scalar(s) => matches!(s.primitive(), Float(_)),
Abi::ScalarPair(s1, s2) => {
matches!(s1.primitive(), Float(_)) || matches!(s2.primitive(), Float(_))
}
_ => false, // anyway not passed via registers on x86
};
if has_float {
if fn_abi.ret.layout.size <= Pointer(AddressSpace::DATA).size(cx) {
// Same size or smaller than pointer, return in a register.
fn_abi.ret.cast_to(Reg { kind: RegKind::Integer, size: fn_abi.ret.layout.size });
} else {
// Larger than a pointer, return indirectly.
fn_abi.ret.make_indirect();
}
return;
}
}
}
Loading
Loading