From 7188706c4fbbae660fa7eb6f2bf13130ddf1726a Mon Sep 17 00:00:00 2001 From: "A.J. Gardner" Date: Sat, 20 Jan 2018 14:32:33 -0600 Subject: Teach rustc about DW_AT_noreturn and a few more DIFlags --- src/test/codegen/noreturnflag.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 src/test/codegen/noreturnflag.rs (limited to 'src/test/codegen') diff --git a/src/test/codegen/noreturnflag.rs b/src/test/codegen/noreturnflag.rs new file mode 100644 index 00000000000..473fed8e046 --- /dev/null +++ b/src/test/codegen/noreturnflag.rs @@ -0,0 +1,29 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// ignore-tidy-linelength +// min-llvm-version 3.8 + +// compile-flags: -g -C no-prepopulate-passes + +// CHECK-LABEL: foo +// CHECK: {{.*}}DISubprogram{{.*}} name: "foo",{{.*}}DIFlagNoReturn{{.*}} + +#[no_mangle] +pub fn foo() -> ! { + loop {} +} + +// CHECK-LABEL: main +// CHECK: {{.*}}DISubprogram{{.*}}name: "main",{{.*}}DIFlagMainSubprogram{{.*}} + +pub fn main() { + foo(); +} -- cgit 1.4.1-3-g733a5 From f7f6598083f24191bbdb170b993dbf97abd1b5c9 Mon Sep 17 00:00:00 2001 From: "A.J. Gardner" Date: Sat, 20 Jan 2018 21:22:11 -0600 Subject: Simplify and fix test --- src/test/codegen/noreturnflag.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'src/test/codegen') diff --git a/src/test/codegen/noreturnflag.rs b/src/test/codegen/noreturnflag.rs index 473fed8e046..3fa7921b444 100644 --- a/src/test/codegen/noreturnflag.rs +++ b/src/test/codegen/noreturnflag.rs @@ -13,15 +13,12 @@ // compile-flags: -g -C no-prepopulate-passes -// CHECK-LABEL: foo -// CHECK: {{.*}}DISubprogram{{.*}} name: "foo",{{.*}}DIFlagNoReturn{{.*}} +// CHECK: {{.*}}DISubprogram{{.*}}name: "foo"{{.*}}DIFlagNoReturn -#[no_mangle] -pub fn foo() -> ! { +fn foo() -> ! { loop {} } -// CHECK-LABEL: main // CHECK: {{.*}}DISubprogram{{.*}}name: "main",{{.*}}DIFlagMainSubprogram{{.*}} pub fn main() { -- cgit 1.4.1-3-g733a5 From e0f9b26899ea16bb2b6b966266e46698ebad9c4a Mon Sep 17 00:00:00 2001 From: "A.J. Gardner" Date: Sun, 21 Jan 2018 12:36:25 -0600 Subject: Ensure test doesn't run with llvm 3.9 --- src/rustllvm/RustWrapper.cpp | 2 +- src/test/codegen/noreturnflag.rs | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) (limited to 'src/test/codegen') diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index 2e8207d1cbb..42ddf5663f9 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -548,7 +548,6 @@ static unsigned fromRust(LLVMRustDIFlags Flags) { if (isSet(Flags & LLVMRustDIFlags::FlagRValueReference)) { Result |= DINode::DIFlags::FlagRValueReference; } -#if LLVM_RUSTLLVM || LLVM_VERSION_GE(4, 0) if (isSet(Flags & LLVMRustDIFlags::FlagExternalTypeRef)) { Result |= DINode::DIFlags::FlagExternalTypeRef; } @@ -558,6 +557,7 @@ static unsigned fromRust(LLVMRustDIFlags Flags) { if (isSet(Flags & LLVMRustDIFlags::FlagBitField)) { Result |= DINode::DIFlags::FlagBitField; } +#if LLVM_RUSTLLVM || LLVM_VERSION_GE(4, 0) if (isSet(Flags & LLVMRustDIFlags::FlagNoReturn)) { Result |= DINode::DIFlags::FlagNoReturn; } diff --git a/src/test/codegen/noreturnflag.rs b/src/test/codegen/noreturnflag.rs index 3fa7921b444..24a5a4e44cb 100644 --- a/src/test/codegen/noreturnflag.rs +++ b/src/test/codegen/noreturnflag.rs @@ -9,7 +9,7 @@ // except according to those terms. // ignore-tidy-linelength -// min-llvm-version 3.8 +// min-llvm-version 4.0 // compile-flags: -g -C no-prepopulate-passes @@ -19,8 +19,6 @@ fn foo() -> ! { loop {} } -// CHECK: {{.*}}DISubprogram{{.*}}name: "main",{{.*}}DIFlagMainSubprogram{{.*}} - pub fn main() { foo(); } -- cgit 1.4.1-3-g733a5 From 502de01ff40d37d3e6b419c3931a23284ce1a4e4 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 25 Jan 2018 08:00:22 -0800 Subject: rustc: SIMD types use pointers in Rust's ABI This commit changes the ABI of SIMD types in the "Rust" ABI to unconditionally be passed via pointers instead of being passed as immediates. This should fix a longstanding issue, #44367, where SIMD-using programs ended up showing very odd behavior at runtime because the ABI between functions was mismatched. As a bit of a recap, this is sort of an LLVM bug and sort of an LLVM feature (today's behavior). LLVM will generate code for a function solely looking at the function it's generating, including calls to other functions. Let's then say you've got something that looks like: ```llvm define void @foo() { ; no target features enabled call void @bar( zeroinitializer) ret void } define void @bar() #0 { ; enables the AVX feature ... } ``` LLVM will codegen the call to `bar` *without* using AVX registers becauase `foo` doesn't have access to these registers. Instead it's generated with emulation that uses two 128-bit registers. The `bar` function, on the other hand, will expect its argument in an AVX register (as it has AVX enabled). This means we've got a codegen problem! Comments on #44367 have some more contexutal information but the crux of the issue is that if we want SIMD to work in general we'll need to ensure that whenever a function calls another they ABI of the arguments being passed is in agreement. One possible solution to this would be to insert "shim functions" where whenever a `target_feature` mismatch is detected the compiler inserts a shim function where you pass arguments via memory to the shim and then the shim loads the values and calls the target function (where the shim and the target have the same target features enabled). This unfortunately is quite nontrivial to implement in rustc today (especially when accounting for function pointers and such). This commit takes a different solution, *always* passing SIMD arguments through memory instead of passing as immediates. This strategy solves the problem at the LLVM layer because the ABI between two functions never uses SIMD registers. This also shouldn't be a hit to performance because SIMD performance is thought to often rely on inlining anyway, where a `call` instruction, even if using SIMD registers, would be disastrous to performance regardless. LLVM should then be more than capable of fixing all our memory usage to use registers instead after enough inlining has been performed. Note that there's a few caveats to this commit though: * The "platform intrinsic" ABI is omitted from "always pass via memory". This ABI is used to define intrinsics like `simd_shuffle4` where LLVM and rustc need to have the arguments as an immediate. * Additionally this commit does *not* fix the `extern` ("C") ABI. This means that the bug in #44367 can still happen when using non-Rust-ABI functions. My hope is that before stabilization we can ban and/or warn about SIMD types in these functions (as AFAIK there's not much motivation to belong there anyway), but I'll leave that for a later commit and if this is merged I'll file a follow-up issue. All in all this... Closes #44367 --- src/librustc_trans/abi.rs | 25 ++++ src/test/codegen/x86_mmx.rs | 4 +- src/test/run-pass/simd-target-feature-mixup.rs | 181 +++++++++++++++++++++++++ 3 files changed, 207 insertions(+), 3 deletions(-) create mode 100644 src/test/run-pass/simd-target-feature-mixup.rs (limited to 'src/test/codegen') diff --git a/src/librustc_trans/abi.rs b/src/librustc_trans/abi.rs index 5079ce77523..9cabd9356e9 100644 --- a/src/librustc_trans/abi.rs +++ b/src/librustc_trans/abi.rs @@ -871,6 +871,31 @@ impl<'a, 'tcx> FnType<'tcx> { match arg.layout.abi { layout::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 platform 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. + layout::Abi::Vector { .. } if abi != Abi::PlatformIntrinsic => { + arg.make_indirect(); + return + } + _ => return } diff --git a/src/test/codegen/x86_mmx.rs b/src/test/codegen/x86_mmx.rs index bedda63bbff..dc9f63c35db 100644 --- a/src/test/codegen/x86_mmx.rs +++ b/src/test/codegen/x86_mmx.rs @@ -22,9 +22,7 @@ pub struct i8x8(u64); #[no_mangle] pub fn a(a: &mut i8x8, b: i8x8) -> i8x8 { - // CHECK-LABEL: define x86_mmx @a(x86_mmx*{{.*}}, x86_mmx{{.*}}) - // CHECK: store x86_mmx %b, x86_mmx* %a - // CHECK: ret x86_mmx %b + // CHECK-LABEL: define void @a(x86_mmx*{{.*}}, x86_mmx*{{.*}}, x86_mmx*{{.*}}) *a = b; return b } diff --git a/src/test/run-pass/simd-target-feature-mixup.rs b/src/test/run-pass/simd-target-feature-mixup.rs new file mode 100644 index 00000000000..b60aec2b5c9 --- /dev/null +++ b/src/test/run-pass/simd-target-feature-mixup.rs @@ -0,0 +1,181 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(repr_simd, target_feature, cfg_target_feature)] + +use std::process::{Command, ExitStatus}; +use std::env; + +fn main() { + if let Some(level) = env::args().nth(1) { + return test::main(&level) + } + + let me = env::current_exe().unwrap(); + for level in ["sse", "avx", "avx512"].iter() { + let status = Command::new(&me).arg(level).status().unwrap(); + if status.success() { + println!("success with {}", level); + continue + } + + // We don't actually know if our computer has the requisite target features + // for the test below. Testing for that will get added to libstd later so + // for now just asume sigill means this is a machine that can't run this test. + if is_sigill(status) { + println!("sigill with {}, assuming spurious", level); + continue + } + panic!("invalid status at {}: {}", level, status); + } +} + +#[cfg(unix)] +fn is_sigill(status: ExitStatus) -> bool { + use std::os::unix::prelude::*; + status.signal() == Some(4) +} + +#[cfg(any(target_arch = "x86", target_arch = "x86_64"))] +#[allow(bad_style)] +mod test { + // An SSE type + #[repr(simd)] + #[derive(PartialEq, Debug, Clone, Copy)] + struct __m128i(u64, u64); + + // An AVX type + #[repr(simd)] + #[derive(PartialEq, Debug, Clone, Copy)] + struct __m256i(u64, u64, u64, u64); + + // An AVX-512 type + #[repr(simd)] + #[derive(PartialEq, Debug, Clone, Copy)] + struct __m512i(u64, u64, u64, u64, u64, u64, u64, u64); + + pub fn main(level: &str) { + unsafe { + main_normal(level); + main_sse(level); + if level == "sse" { + return + } + main_avx(level); + if level == "avx" { + return + } + main_avx512(level); + } + } + + macro_rules! mains { + ($( + $(#[$attr:meta])* + unsafe fn $main:ident(level: &str) { + ... + } + )*) => ($( + $(#[$attr])* + unsafe fn $main(level: &str) { + let m128 = __m128i(1, 2); + let m256 = __m256i(3, 4, 5, 6); + let m512 = __m512i(7, 8, 9, 10, 11, 12, 13, 14); + assert_eq!(id_sse_128(m128), m128); + assert_eq!(id_sse_256(m256), m256); + assert_eq!(id_sse_512(m512), m512); + + if level == "sse" { + return + } + assert_eq!(id_avx_128(m128), m128); + assert_eq!(id_avx_256(m256), m256); + assert_eq!(id_avx_512(m512), m512); + + if level == "avx" { + return + } + assert_eq!(id_avx512_128(m128), m128); + assert_eq!(id_avx512_256(m256), m256); + assert_eq!(id_avx512_512(m512), m512); + } + )*) + } + + mains! { + unsafe fn main_normal(level: &str) { ... } + #[target_feature(enable = "sse2")] + unsafe fn main_sse(level: &str) { ... } + #[target_feature(enable = "avx")] + unsafe fn main_avx(level: &str) { ... } + #[target_feature(enable = "avx512bw")] + unsafe fn main_avx512(level: &str) { ... } + } + + + #[target_feature(enable = "sse2")] + unsafe fn id_sse_128(a: __m128i) -> __m128i { + assert_eq!(a, __m128i(1, 2)); + a.clone() + } + + #[target_feature(enable = "sse2")] + unsafe fn id_sse_256(a: __m256i) -> __m256i { + assert_eq!(a, __m256i(3, 4, 5, 6)); + a.clone() + } + + #[target_feature(enable = "sse2")] + unsafe fn id_sse_512(a: __m512i) -> __m512i { + assert_eq!(a, __m512i(7, 8, 9, 10, 11, 12, 13, 14)); + a.clone() + } + + #[target_feature(enable = "avx")] + unsafe fn id_avx_128(a: __m128i) -> __m128i { + assert_eq!(a, __m128i(1, 2)); + a.clone() + } + + #[target_feature(enable = "avx")] + unsafe fn id_avx_256(a: __m256i) -> __m256i { + assert_eq!(a, __m256i(3, 4, 5, 6)); + a.clone() + } + + #[target_feature(enable = "avx")] + unsafe fn id_avx_512(a: __m512i) -> __m512i { + assert_eq!(a, __m512i(7, 8, 9, 10, 11, 12, 13, 14)); + a.clone() + } + + #[target_feature(enable = "avx512bw")] + unsafe fn id_avx512_128(a: __m128i) -> __m128i { + assert_eq!(a, __m128i(1, 2)); + a.clone() + } + + #[target_feature(enable = "avx512bw")] + unsafe fn id_avx512_256(a: __m256i) -> __m256i { + assert_eq!(a, __m256i(3, 4, 5, 6)); + a.clone() + } + + #[target_feature(enable = "avx512bw")] + unsafe fn id_avx512_512(a: __m512i) -> __m512i { + assert_eq!(a, __m512i(7, 8, 9, 10, 11, 12, 13, 14)); + a.clone() + } +} + +#[cfg(not(any(target_arch = "x86", target_arch = "x86_64")))] +mod test { + pub fn main(level: &str) {} +} -- cgit 1.4.1-3-g733a5