Skip to content

x86: on targets that requires SSE, use those registers for ABI #161583

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.

Sign up for GitHub

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

Open
RalfJung wants to merge 1 commit into
base: main
Choose a base branch
from
Open

x86: on targets that requires SSE, use those registers for ABI #161583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 24 additions & 11 deletions compiler/rustc_target/src/callconv/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_abi::{
use rustc_macros::StableHash;

pub use crate::spec::AbiMap;
use crate::spec::{Arch, HasTargetSpec, HasX86AbiOpt};
use crate::spec::{Arch, HasTargetSpec, HasX86AbiOpt, RustcAbi};

mod aarch64;
mod amdgpu;
Expand Down Expand Up @@ -744,6 +744,23 @@ impl<'a, Ty> FnAbi<'a, Ty> {
_ => {}
};

// Decides whether we can pass the given SIMD argument via `PassMode::Direct`.
// May only return `true` if the target will always pass those arguments the same way,
// no matter what the user does with `-Ctarget-feature`! In other words, whatever
// target features are required to pass a SIMD value in registers must be listed in
// the `abi_required_features` for the current target and ABI.
let can_pass_simd_directly = |arg: &ArgAbi<'_, Ty>| match &spec.arch {
// On x86, if we have SSE2 (which we have by default for x86_64), we can always pass up
// to 128-bit-sized vectors.
Arch::X86 if spec.rustc_abi == Some(RustcAbi::X86Sse2) => arg.layout.size.bits() <= 128,
Arch::X86_64 if spec.rustc_abi != Some(RustcAbi::Softfloat) => {
// x86-64 non-softfloat targets all require SSE2 so we can use SSE registers.
arg.layout.size.bits() <= 128
}
// So far, we haven't implemented this logic for any other target.
_ => false,
};

for (arg_idx, arg) in self
.args
.iter_mut()
Expand Down Expand Up @@ -861,16 +878,12 @@ impl<'a, Ty> FnAbi<'a, Ty> {
// 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.
//
// We *could* do better in some cases, e.g. on x86_64 targets where SSE2 is
// required. However, it turns out that that makes LLVM worse at optimizing this
// code, so we pass things indirectly even there. See #139029 for more on that.
if spec.simd_types_indirect {
// This problem is fixed by passing most SIMD arguments through memory between
// callers and callees which should get them all to agree on ABI regardless of
// target feature sets, except for those that rely on target features that we
// know to be always available. Some more information about this issue can be
// found in #44367 and the comment on `can_pass_simd_directly`.
if spec.simd_types_indirect && !can_pass_simd_directly(arg) {
arg.make_indirect();
}
}
Expand Down
13 changes: 12 additions & 1 deletion tests/codegen-llvm/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ revisions annotation, like so:

```rust
// revisions: aaa bbb
// [bbb] compile-flags: --flags-for-bbb
// [bbb] compile-flags: -Csomething=true
```

After specifying those variations, you can write different expected, or
Expand All @@ -22,3 +22,14 @@ like so:
// bbb-NOT: emitted-only-for-aaa
// bbb-SAME: emitted-only-for-bbb
```

If multiple test revisions want to share some annotations, you can use a setup like this:

```rust
// revisions: aaa bbb ccc
// [bbb] filecheck-flags: --check-prefix=both
// [ccc] filecheck-flags: --check-prefix=both
```

Now `aaa:`, `bbb:`, `ccc:` annotations are only check for those specific revisions,
and `both:` annotations are checked for revisions `bbb` and `ccc`.
5 changes: 2 additions & 3 deletions tests/codegen-llvm/abi-x86-sse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,8 @@ trait Copy {}
#[repr(simd)]
pub struct Sse([f32; 4]);

// FIXME: due to #139029 we are passing them all indirectly.
// x86-64: void @sse_id(ptr{{( [^,]*)?}} sret([16 x i8]){{( .*)?}}, ptr{{( [^,]*)?}})
// x86-32: void @sse_id(ptr{{( [^,]*)?}} sret([16 x i8]){{( .*)?}}, ptr{{( [^,]*)?}})
// x86-64: <4 x float> @sse_id(<4 x float> {{[^,]*}})
// x86-32: <4 x float> @sse_id(<4 x float> {{[^,]*}})
// x86-32-nosse: void @sse_id(ptr{{( [^,]*)?}} sret([16 x i8]){{( .*)?}}, ptr{{( [^,]*)?}})
#[no_mangle]
pub fn sse_id(x: Sse) -> Sse {
Expand Down
12 changes: 6 additions & 6 deletions tests/codegen-llvm/debuginfo-dse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ struct Aggregate_4xi8(i8, i8, i8, i8); // scalar(i32)

// The pass mode is indirect and the backend represent is simd vector.
#[repr(simd)]
struct Simd_i32x4([i32; 4]);
struct Simd_i32x8([i32; 8]);

unsafe extern "Rust" {
#[rustc_nounwind]
Expand Down Expand Up @@ -169,10 +169,10 @@ fn indirect(
tuple_sliceref_scalar: Tuple_SliceRef_Scalar,
array: Array,
typle_i32_i64_i8: Typle_i32_i64_i8,
simd_i32x4: Simd_i32x4,
simd_i32x8: Simd_i32x8,
) {
// CHECK-LABEL: define{{( dso_local)?}} void @indirect
// CHECK-SAME: (ptr{{.*}} %tuple_sliceref_scalar, ptr{{.*}} %array, ptr{{.*}} %typle_i32_i64_i8, ptr{{.*}} %simd_i32x4)
// CHECK-SAME: (ptr{{.*}} %tuple_sliceref_scalar, ptr{{.*}} %array, ptr{{.*}} %typle_i32_i64_i8, ptr{{.*}} %simd_i32x8)
// CHECK: call void @opaque_fn()
opaque_fn();
// CHECK-NEXT: #dbg_value(ptr %tuple_sliceref_scalar, [[ref_tuple_sliceref_scalar:![0-9]+]], !DIExpression()
Expand All @@ -183,8 +183,8 @@ fn indirect(
let ref_1_array = &array[1];
// CHECK-NEXT: #dbg_value(ptr %typle_i32_i64_i8, [[ref_1_typle_i32_i64_i8:![0-9]+]], !DIExpression()
let ref_1_typle_i32_i64_i8 = &typle_i32_i64_i8.1;
// CHECK-NEXT: #dbg_value(ptr %simd_i32x4, [[ref_simd_i32x4:![0-9]+]], !DIExpression()
let ref_simd_i32x4 = &simd_i32x4;
// CHECK-NEXT: #dbg_value(ptr %simd_i32x8, [[ref_simd_i32x8:![0-9]+]], !DIExpression()
let ref_simd_i32x8 = &simd_i32x8;
// CHECK: call void @opaque_fn()
opaque_fn();
}
Expand Down Expand Up @@ -334,7 +334,7 @@ fn index(slice: &[i32; 4], idx: usize) -> i32 {
// CHECK-DAG: [[ref_1_tuple_sliceref_scalar]] = !DILocalVariable(name: "ref_1_tuple_sliceref_scalar"
// CHECK-DAG: [[ref_1_array]] = !DILocalVariable(name: "ref_1_array"
// CHECK-DAG: [[ref_1_typle_i32_i64_i8]] = !DILocalVariable(name: "ref_1_typle_i32_i64_i8"
// CHECK-DAG: [[ref_simd_i32x4]] = !DILocalVariable(name: "ref_simd_i32x4"
// CHECK-DAG: [[ref_simd_i32x8]] = !DILocalVariable(name: "ref_simd_i32x8"

// CHECK-DAG: [[ref_direct_ref]] = !DILocalVariable(name: "ref_direct_ref"
// CHECK-DAG: [[ref_1_direct_ref]] = !DILocalVariable(name: "ref_1_direct_ref"
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen-llvm/pclmulqdq-target-feature-inlining.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use std::arch::x86_64 as arch;

// CHECK-LABEL: @reduce128_caller
// CHECK-NEXT: start
// CHECK-COUNT-3: load
// CHECK-NOT: load
// CHECK-NEXT: call <2 x i64> @llvm.x86.pclmulqdq
// CHECK-NEXT: call <2 x i64> @llvm.x86.pclmulqdq
#[target_feature(enable = "pclmulqdq", enable = "sse2", enable = "sse4.1")]
Expand Down
Loading
Loading