Skip to content

Commit 9736634

Browse files
committed
Auto merge of #131955 - SpriteOvO:riscv-int-arg-attr, r=<try>
Set `signext` or `zeroext` for integer arguments on RISC-V and LoongArch64 Fixes #114508. This PR contains 3 commits: - the first one introduces a new function `adjust_for_rust_abi` in `rustc_target`, and moves the x86 specific adjustment code into it; - the second one adds RISC-V specific adjustment code into it, which sets `signext` or `zeroext` attribute for integer arguments. - **UPDATE**: added the 3rd commit for apply the same adjustment for LoongArch64. r? `@workingjubilee` CC `@coastalwhite` `@Urgau` `@topperc` `@michaelmaitland` try-job: dist-loongarch64-linux try-job: dist-riscv64-linux try-job: test-various try-job: i686-gnu-nopt try-job: i686-gnu
2 parents 86d69c7 + 0adeaf3 commit 9736634

20 files changed

+473
-173
lines changed

Diff for: compiler/rustc_target/src/callconv/loongarch.rs

+28
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use crate::abi::call::{ArgAbi, ArgExtension, CastTarget, FnAbi, PassMode, Reg, RegKind, Uniform};
22
use crate::abi::{self, Abi, FieldsShape, HasDataLayout, Size, TyAbiInterface, TyAndLayout};
33
use crate::spec::HasTargetSpec;
4+
use crate::spec::abi::Abi as SpecAbi;
45

56
#[derive(Copy, Clone)]
67
enum RegPassKind {
@@ -359,3 +360,30 @@ where
359360
);
360361
}
361362
}
363+
364+
pub(crate) fn compute_rust_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>, abi: SpecAbi)
365+
where
366+
Ty: TyAbiInterface<'a, C> + Copy,
367+
C: HasDataLayout + HasTargetSpec,
368+
{
369+
if abi == SpecAbi::RustIntrinsic {
370+
return;
371+
}
372+
373+
let grlen = cx.data_layout().pointer_size.bits();
374+
375+
for arg in fn_abi.args.iter_mut() {
376+
if arg.is_ignore() {
377+
continue;
378+
}
379+
380+
// LLVM integers types do not differentiate between signed or unsigned integers.
381+
// Some LoongArch instructions do not have a `.w` suffix version, they use all the
382+
// GRLEN bits. By explicitly setting the `signext` or `zeroext` attribute
383+
// according to signedness to avoid unnecessary integer extending instructions.
384+
//
385+
// This is similar to the RISC-V case, see
386+
// https://github.com/rust-lang/rust/issues/114508 for details.
387+
extend_integer_width(arg, grlen);
388+
}
389+
}

Diff for: compiler/rustc_target/src/callconv/mod.rs

+116-1
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
1-
use std::fmt;
21
use std::str::FromStr;
2+
use std::{fmt, iter};
33

4+
use rustc_abi::AddressSpace;
5+
use rustc_abi::Primitive::Pointer;
46
pub use rustc_abi::{Reg, RegKind};
57
use rustc_macros::HashStable_Generic;
68
use rustc_span::Symbol;
79

810
use crate::abi::{self, Abi, Align, HasDataLayout, Size, TyAbiInterface, TyAndLayout};
11+
use crate::spec::abi::Abi as SpecAbi;
912
use crate::spec::{self, HasTargetSpec, HasWasmCAbiOpt, HasX86AbiOpt, WasmCAbi};
1013

1114
mod aarch64;
@@ -720,6 +723,118 @@ impl<'a, Ty> FnAbi<'a, Ty> {
720723

721724
Ok(())
722725
}
726+
727+
pub fn adjust_for_rust_abi<C>(&mut self, cx: &C, abi: SpecAbi)
728+
where
729+
Ty: TyAbiInterface<'a, C> + Copy,
730+
C: HasDataLayout + HasTargetSpec,
731+
{
732+
let spec = cx.target_spec();
733+
match &spec.arch[..] {
734+
"x86" => x86::compute_rust_abi_info(cx, self, abi),
735+
"riscv32" | "riscv64" => riscv::compute_rust_abi_info(cx, self, abi),
736+
"loongarch64" => loongarch::compute_rust_abi_info(cx, self, abi),
737+
_ => {}
738+
};
739+
740+
for (arg_idx, arg) in self
741+
.args
742+
.iter_mut()
743+
.enumerate()
744+
.map(|(idx, arg)| (Some(idx), arg))
745+
.chain(iter::once((None, &mut self.ret)))
746+
{
747+
if arg.is_ignore() {
748+
continue;
749+
}
750+
751+
if arg_idx.is_none() && arg.layout.size > Pointer(AddressSpace::DATA).size(cx) * 2 {
752+
// Return values larger than 2 registers using a return area
753+
// pointer. LLVM and Cranelift disagree about how to return
754+
// values that don't fit in the registers designated for return
755+
// values. LLVM will force the entire return value to be passed
756+
// by return area pointer, while Cranelift will look at each IR level
757+
// return value independently and decide to pass it in a
758+
// register or not, which would result in the return value
759+
// being passed partially in registers and partially through a
760+
// return area pointer.
761+
//
762+
// While Cranelift may need to be fixed as the LLVM behavior is
763+
// generally more correct with respect to the surface language,
764+
// forcing this behavior in rustc itself makes it easier for
765+
// other backends to conform to the Rust ABI and for the C ABI
766+
// rustc already handles this behavior anyway.
767+
//
768+
// In addition LLVM's decision to pass the return value in
769+
// registers or using a return area pointer depends on how
770+
// exactly the return type is lowered to an LLVM IR type. For
771+
// example `Option<u128>` can be lowered as `{ i128, i128 }`
772+
// in which case the x86_64 backend would use a return area
773+
// pointer, or it could be passed as `{ i32, i128 }` in which
774+
// case the x86_64 backend would pass it in registers by taking
775+
// advantage of an LLVM ABI extension that allows using 3
776+
// registers for the x86_64 sysv call conv rather than the
777+
// officially specified 2 registers.
778+
//
779+
// FIXME: Technically we should look at the amount of available
780+
// return registers rather than guessing that there are 2
781+
// registers for return values. In practice only a couple of
782+
// architectures have less than 2 return registers. None of
783+
// which supported by Cranelift.
784+
//
785+
// NOTE: This adjustment is only necessary for the Rust ABI as
786+
// for other ABI's the calling convention implementations in
787+
// rustc_target already ensure any return value which doesn't
788+
// fit in the available amount of return registers is passed in
789+
// the right way for the current target.
790+
arg.make_indirect();
791+
continue;
792+
}
793+
794+
match arg.layout.abi {
795+
Abi::Aggregate { .. } => {}
796+
797+
// This is a fun case! The gist of what this is doing is
798+
// that we want callers and callees to always agree on the
799+
// ABI of how they pass SIMD arguments. If we were to *not*
800+
// make these arguments indirect then they'd be immediates
801+
// in LLVM, which means that they'd used whatever the
802+
// appropriate ABI is for the callee and the caller. That
803+
// means, for example, if the caller doesn't have AVX
804+
// enabled but the callee does, then passing an AVX argument
805+
// across this boundary would cause corrupt data to show up.
806+
//
807+
// This problem is fixed by unconditionally passing SIMD
808+
// arguments through memory between callers and callees
809+
// which should get them all to agree on ABI regardless of
810+
// target feature sets. Some more information about this
811+
// issue can be found in #44367.
812+
//
813+
// Note that the intrinsic ABI is exempt here as
814+
// that's how we connect up to LLVM and it's unstable
815+
// anyway, we control all calls to it in libstd.
816+
Abi::Vector { .. } if abi != SpecAbi::RustIntrinsic && spec.simd_types_indirect => {
817+
arg.make_indirect();
818+
continue;
819+
}
820+
821+
_ => continue,
822+
}
823+
// Compute `Aggregate` ABI.
824+
825+
let is_indirect_not_on_stack =
826+
matches!(arg.mode, PassMode::Indirect { on_stack: false, .. });
827+
assert!(is_indirect_not_on_stack);
828+
829+
let size = arg.layout.size;
830+
if !arg.layout.is_unsized() && size <= Pointer(AddressSpace::DATA).size(cx) {
831+
// We want to pass small aggregates as immediates, but using
832+
// an LLVM aggregate type for this leads to bad optimizations,
833+
// so we pick an appropriately sized integer type instead.
834+
arg.cast_to(Reg { kind: RegKind::Integer, size });
835+
}
836+
}
837+
}
723838
}
724839

725840
impl FromStr for Conv {

Diff for: compiler/rustc_target/src/callconv/riscv.rs

+27
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use crate::abi::call::{ArgAbi, ArgExtension, CastTarget, FnAbi, PassMode, Reg, RegKind, Uniform};
88
use crate::abi::{self, Abi, FieldsShape, HasDataLayout, Size, TyAbiInterface, TyAndLayout};
99
use crate::spec::HasTargetSpec;
10+
use crate::spec::abi::Abi as SpecAbi;
1011

1112
#[derive(Copy, Clone)]
1213
enum RegPassKind {
@@ -365,3 +366,29 @@ where
365366
);
366367
}
367368
}
369+
370+
pub(crate) fn compute_rust_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>, abi: SpecAbi)
371+
where
372+
Ty: TyAbiInterface<'a, C> + Copy,
373+
C: HasDataLayout + HasTargetSpec,
374+
{
375+
if abi == SpecAbi::RustIntrinsic {
376+
return;
377+
}
378+
379+
let xlen = cx.data_layout().pointer_size.bits();
380+
381+
for arg in fn_abi.args.iter_mut() {
382+
if arg.is_ignore() {
383+
continue;
384+
}
385+
386+
// LLVM integers types do not differentiate between signed or unsigned integers.
387+
// Some RISC-V instructions do not have a `.w` suffix version, they use all the
388+
// XLEN bits. By explicitly setting the `signext` or `zeroext` attribute
389+
// according to signedness to avoid unnecessary integer extending instructions.
390+
//
391+
// See https://github.com/rust-lang/rust/issues/114508 for details.
392+
extend_integer_width(arg, xlen);
393+
}
394+
}

Diff for: compiler/rustc_target/src/callconv/x86.rs

+39
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
1+
use rustc_abi::Float::*;
2+
use rustc_abi::Primitive::Float;
3+
14
use crate::abi::call::{ArgAttribute, FnAbi, PassMode, Reg, RegKind};
25
use crate::abi::{Abi, Align, HasDataLayout, TyAbiInterface, TyAndLayout};
36
use crate::spec::HasTargetSpec;
7+
use crate::spec::abi::Abi as SpecAbi;
48

59
#[derive(PartialEq)]
610
pub(crate) enum Flavor {
@@ -207,3 +211,38 @@ pub(crate) fn fill_inregs<'a, Ty, C>(
207211
}
208212
}
209213
}
214+
215+
pub(crate) fn compute_rust_abi_info<'a, Ty, C>(_cx: &C, fn_abi: &mut FnAbi<'a, Ty>, abi: SpecAbi)
216+
where
217+
Ty: TyAbiInterface<'a, C> + Copy,
218+
C: HasDataLayout + HasTargetSpec,
219+
{
220+
// Avoid returning floats in x87 registers on x86 as loading and storing from x87
221+
// registers will quiet signalling NaNs.
222+
if !fn_abi.ret.is_ignore()
223+
// Intrinsics themselves are not actual "real" functions, so theres no need to change their ABIs.
224+
&& abi != SpecAbi::RustIntrinsic
225+
{
226+
match fn_abi.ret.layout.abi {
227+
// Handle similar to the way arguments with an `Abi::Aggregate` abi are handled
228+
// in `adjust_for_rust_abi`, by returning arguments up to the size of a pointer (32 bits on x86)
229+
// cast to an appropriately sized integer.
230+
Abi::Scalar(s) if s.primitive() == Float(F32) => {
231+
// Same size as a pointer, return in a register.
232+
fn_abi.ret.cast_to(Reg::i32());
233+
}
234+
Abi::Scalar(s) if s.primitive() == Float(F64) => {
235+
// Larger than a pointer, return indirectly.
236+
fn_abi.ret.make_indirect();
237+
}
238+
Abi::ScalarPair(s1, s2)
239+
if matches!(s1.primitive(), Float(F32 | F64))
240+
|| matches!(s2.primitive(), Float(F32 | F64)) =>
241+
{
242+
// Larger than a pointer, return indirectly.
243+
fn_abi.ret.make_indirect();
244+
}
245+
_ => {}
246+
};
247+
}
248+
}

0 commit comments

Comments
 (0)