Skip to content
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

rust_for_linux: -Zreg-struct-return commandline flag for X86 (#116973) #130777

Merged
merged 1 commit into from
Dec 6, 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
5 changes: 4 additions & 1 deletion compiler/rustc_codegen_gcc/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,10 @@ impl<'gcc, 'tcx> HasWasmCAbiOpt for CodegenCx<'gcc, 'tcx> {

impl<'gcc, 'tcx> HasX86AbiOpt for CodegenCx<'gcc, 'tcx> {
fn x86_abi_opt(&self) -> X86Abi {
X86Abi { regparm: self.tcx.sess.opts.unstable_opts.regparm }
X86Abi {
regparm: self.tcx.sess.opts.unstable_opts.regparm,
reg_struct_return: self.tcx.sess.opts.unstable_opts.reg_struct_return,
}
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,7 @@ fn test_unstable_options_tracking_hash() {
tracked!(precise_enum_drop_elaboration, false);
tracked!(profile_sample_use, Some(PathBuf::from("abc")));
tracked!(profiler_runtime, "abc".to_string());
tracked!(reg_struct_return, true);
tracked!(regparm, Some(3));
tracked!(relax_elf_relocations, Some(true));
tracked!(remap_cwd_prefix, Some(PathBuf::from("abc")));
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,10 @@ impl<'tcx> HasWasmCAbiOpt for TyCtxt<'tcx> {

impl<'tcx> HasX86AbiOpt for TyCtxt<'tcx> {
fn x86_abi_opt(&self) -> X86Abi {
X86Abi { regparm: self.sess.opts.unstable_opts.regparm }
X86Abi {
regparm: self.sess.opts.unstable_opts.regparm,
reg_struct_return: self.sess.opts.unstable_opts.reg_struct_return,
}
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_session/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -135,5 +135,6 @@ session_unsupported_crate_type_for_target =

session_unsupported_dwarf_version = requested DWARF version {$dwarf_version} is greater than 5

session_unsupported_reg_struct_return_arch = `-Zreg-struct-return` is only supported on x86
session_unsupported_regparm = `-Zregparm={$regparm}` is unsupported (valid values 0-3)
session_unsupported_regparm_arch = `-Zregparm=N` is only supported on x86
4 changes: 4 additions & 0 deletions compiler/rustc_session/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,10 @@ pub(crate) struct UnsupportedRegparm {
#[diag(session_unsupported_regparm_arch)]
pub(crate) struct UnsupportedRegparmArch;

#[derive(Diagnostic)]
#[diag(session_unsupported_reg_struct_return_arch)]
pub(crate) struct UnsupportedRegStructReturnArch;

#[derive(Diagnostic)]
#[diag(session_failed_to_create_profiler)]
pub(crate) struct FailedToCreateProfiler {
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1985,6 +1985,9 @@ options! {
"enable queries of the dependency graph for regression testing (default: no)"),
randomize_layout: bool = (false, parse_bool, [TRACKED],
"randomize the layout of types (default: no)"),
reg_struct_return: bool = (false, parse_bool, [TRACKED],
"On x86-32 targets, it overrides the default ABI to return small structs in registers.
It is UNSOUND to link together crates that use different values for this flag!"),
regparm: Option<u32> = (None, parse_opt_number, [TRACKED],
"On x86-32 targets, setting this to N causes the compiler to pass N arguments \
in registers EAX, EDX, and ECX instead of on the stack for\
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1305,6 +1305,11 @@ fn validate_commandline_args_with_session_available(sess: &Session) {
sess.dcx().emit_err(errors::UnsupportedRegparmArch);
}
}
if sess.opts.unstable_opts.reg_struct_return {
if sess.target.arch != "x86" {
sess.dcx().emit_err(errors::UnsupportedRegStructReturnArch);
}
}

// The code model check applies to `thunk` and `thunk-extern`, but not `thunk-inline`, so it is
// kept as a `match` to force a change if new ones are added, even if we currently only support
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_target/src/callconv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,9 @@ impl<'a, Ty> FnAbi<'a, Ty> {
}
_ => (x86::Flavor::General, None),
};
x86::compute_abi_info(cx, self, x86::X86Options { flavor, regparm });
let reg_struct_return = cx.x86_abi_opt().reg_struct_return;
let opts = x86::X86Options { flavor, regparm, reg_struct_return };
x86::compute_abi_info(cx, self, opts);
}
"x86_64" => match abi {
spec::abi::Abi::SysV64 { .. } => x86_64::compute_abi_info(cx, self),
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_target/src/callconv/x86.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ pub(crate) enum Flavor {
pub(crate) struct X86Options {
pub flavor: Flavor,
pub regparm: Option<u32>,
pub reg_struct_return: bool,
}

pub(crate) fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>, opts: X86Options)
Expand All @@ -31,7 +32,7 @@ where
// https://www.angelcode.com/dev/callconv/callconv.html
// Clang's ABI handling is in lib/CodeGen/TargetInfo.cpp
let t = cx.target_spec();
if t.abi_return_struct_as_int {
if t.abi_return_struct_as_int || opts.reg_struct_return {
// According to Clang, everyone but MSVC returns single-element
// float aggregates directly in a floating-point register.
if !t.is_like_msvc && fn_abi.ret.layout.is_single_fp_element(cx) {
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_target/src/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2108,6 +2108,8 @@ pub struct X86Abi {
/// On x86-32 targets, the regparm N causes the compiler to pass arguments
/// in registers EAX, EDX, and ECX instead of on the stack.
pub regparm: Option<u32>,
/// Override the default ABI to return small structs in registers
pub reg_struct_return: bool,
}

pub trait HasX86AbiOpt {
Expand Down
15 changes: 15 additions & 0 deletions src/doc/unstable-book/src/compiler-flags/reg-struct-return.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# `reg-struct-return`

The tracking issue for this feature is: https://github.com/rust-lang/rust/issues/116973.

------------------------

Option -Zreg-struct-return causes the compiler to return small structs in registers
instead of on the stack for extern "C"-like functions.
It is UNSOUND to link together crates that use different values for this flag.
It is only supported on `x86`.

It is equivalent to [Clang]'s and [GCC]'s `-freg-struct-return`.

[Clang]: https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-freg-struct-return
[GCC]: https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html#index-freg-struct-return
206 changes: 206 additions & 0 deletions tests/codegen/reg-struct-return.rs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should test this slightly more exhaustively. I'd like to see

  • single-field structs, too
  • structs with fields that aren't valid for all bitpatterns, like, say, { bool, bool, i16 }... I am not aware of a reason it should differ in handling, mind.
  • oddly-shaped structs that don't exactly match the sizes we're looking for, but are under the 64-bit limit, like e.g. { char, bool, u8 }
  • larger-than-64-bits structs, which obviously shouldn't be changed to integers

workingjubilee marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
// Checks how `reg-struct-return` flag works with different calling conventions:
// Return struct with 8/16/32/64 bit size will be converted into i8/i16/i32/i64
// (like abi_return_struct_as_int target spec).
// x86 only.

//@ revisions: ENABLED DISABLED
//@ add-core-stubs
//@ compile-flags: --target i686-unknown-linux-gnu -O -C no-prepopulate-passes
//@ [ENABLED] compile-flags: -Zreg-struct-return
//@ needs-llvm-components: x86

#![crate_type = "lib"]
#![no_std]
#![no_core]
#![feature(no_core, lang_items)]

extern crate minicore;
use minicore::*;

#[repr(C)]
pub struct Foo {
x: u32,
y: u32,
}

#[repr(C)]
pub struct Foo1 {
x: u32,
}

#[repr(C)]
pub struct Foo2 {
x: bool,
y: bool,
z: i16,
}

#[repr(C)]
pub struct Foo3 {
x: i16,
y: bool,
z: bool,
}

#[repr(C)]
pub struct Foo4 {
x: char,
y: bool,
z: u8,
}

#[repr(C)]
pub struct Foo5 {
x: u32,
y: u16,
z: u8,
a: bool,
}

#[repr(C)]
pub struct FooOversize1 {
x: u32,
y: u32,
z: u32,
}

#[repr(C)]
pub struct FooOversize2 {
f0: u16,
f1: u16,
f2: u16,
f3: u16,
f4: u16,
}

#[repr(C)]
pub struct FooFloat1 {
x: f32,
y: f32,
}

#[repr(C)]
pub struct FooFloat2 {
x: f64,
}

#[repr(C)]
pub struct FooFloat3 {
x: f32,
}

pub mod tests {
use {
Foo, Foo1, Foo2, Foo3, Foo4, Foo5, FooFloat1, FooFloat2, FooFloat3, FooOversize1,
FooOversize2,
};

// ENABLED: i64 @f1()
// DISABLED: void @f1(ptr {{.*}}sret
#[no_mangle]
pub extern "fastcall" fn f1() -> Foo {
Foo { x: 1, y: 2 }
}

// CHECK: { i32, i32 } @f2()
#[no_mangle]
pub extern "Rust" fn f2() -> Foo {
Foo { x: 1, y: 2 }
}

// ENABLED: i64 @f3()
// DISABLED: void @f3(ptr {{.*}}sret
#[no_mangle]
pub extern "C" fn f3() -> Foo {
Foo { x: 1, y: 2 }
}

// ENABLED: i64 @f4()
// DISABLED: void @f4(ptr {{.*}}sret
#[no_mangle]
pub extern "cdecl" fn f4() -> Foo {
Foo { x: 1, y: 2 }
}

// ENABLED: i64 @f5()
// DISABLED: void @f5(ptr {{.*}}sret
#[no_mangle]
pub extern "stdcall" fn f5() -> Foo {
Foo { x: 1, y: 2 }
}

// ENABLED: i64 @f6()
// DISABLED: void @f6(ptr {{.*}}sret
#[no_mangle]
pub extern "thiscall" fn f6() -> Foo {
Foo { x: 1, y: 2 }
}

// ENABLED: i32 @f7()
// DISABLED: void @f7(ptr {{.*}}sret
#[no_mangle]
pub extern "C" fn f7() -> Foo1 {
Foo1 { x: 1 }
}

// ENABLED: i32 @f8()
// DISABLED: void @f8(ptr {{.*}}sret
#[no_mangle]
pub extern "C" fn f8() -> Foo2 {
Foo2 { x: true, y: false, z: 5 }
}

// ENABLED: i32 @f9()
// DISABLED: void @f9(ptr {{.*}}sret
#[no_mangle]
pub extern "C" fn f9() -> Foo3 {
Foo3 { x: 5, y: false, z: true }
}

// ENABLED: i64 @f10()
// DISABLED: void @f10(ptr {{.*}}sret
#[no_mangle]
pub extern "C" fn f10() -> Foo4 {
Foo4 { x: 'x', y: true, z: 170 }
}

// ENABLED: i64 @f11()
// DISABLED: void @f11(ptr {{.*}}sret
#[no_mangle]
pub extern "C" fn f11() -> Foo5 {
Foo5 { x: 1, y: 2, z: 3, a: true }
}

// CHECK: void @f12(ptr {{.*}}sret
#[no_mangle]
pub extern "C" fn f12() -> FooOversize1 {
FooOversize1 { x: 1, y: 2, z: 3 }
}

// CHECK: void @f13(ptr {{.*}}sret
#[no_mangle]
pub extern "C" fn f13() -> FooOversize2 {
FooOversize2 { f0: 1, f1: 2, f2: 3, f3: 4, f4: 5 }
}

// ENABLED: i64 @f14()
// DISABLED: void @f14(ptr {{.*}}sret
#[no_mangle]
pub extern "C" fn f14() -> FooFloat1 {
FooFloat1 { x: 1.0, y: 1.0 }
}

// ENABLED: double @f15()
// DISABLED: void @f15(ptr {{.*}}sret
#[no_mangle]
pub extern "C" fn f15() -> FooFloat2 {
FooFloat2 { x: 1.0 }
}

// ENABLED: float @f16()
// DISABLED: void @f16(ptr {{.*}}sret
#[no_mangle]
pub extern "C" fn f16() -> FooFloat3 {
FooFloat3 { x: 1.0 }
}
Comment on lines +193 to +205
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm. Tried to disable float features and the LLVMIR we emit for matching the C ABI on such a "soft float" target didn't undergo a substantial change.

I should probably take a closer look at that later.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
error: `-Zreg-struct-return` is only supported on x86

error: aborting due to 1 previous error

21 changes: 21 additions & 0 deletions tests/ui/invalid-compile-flags/reg-struct-return/requires-x86.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//@ revisions: x86 x86_64 aarch64

//@ compile-flags: -Zreg-struct-return

//@[x86] check-pass
//@[x86] needs-llvm-components: x86
//@[x86] compile-flags: --target i686-unknown-linux-gnu

//@[x86_64] check-fail
//@[x86_64] needs-llvm-components: x86
//@[x86_64] compile-flags: --target x86_64-unknown-linux-gnu
//@[x86_64] error-pattern: `-Zreg-struct-return` is only supported on x86

//@[aarch64] check-fail
//@[aarch64] needs-llvm-components: aarch64
//@[aarch64] compile-flags: --target aarch64-unknown-linux-gnu
//@[aarch64] error-pattern: `-Zreg-struct-return` is only supported on x86

#![feature(no_core)]
#![no_core]
#![no_main]
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
error: `-Zreg-struct-return` is only supported on x86

error: aborting due to 1 previous error

Loading