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

Support calling functions with SIMD vectors that couldn't be used in the caller #132865

Open
RalfJung opened this issue Nov 10, 2024 · 0 comments
Labels
A-ABI Area: Concerning the application binary interface (ABI) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-SIMD Area: SIMD (Single Instruction Multiple Data) C-feature-request Category: A feature request, i.e: not implemented / a PR. T-opsem Relevant to the opsem team WG-llvm Working group: LLVM backend code generation

Comments

@RalfJung
Copy link
Member

RalfJung commented Nov 10, 2024

We now lint and will eventually error on this program:

use std::mem::transmute;
#[cfg(target_arch = "x86")]
use std::arch::x86::*;
#[cfg(target_arch = "x86_64")]
use std::arch::x86_64::*;

#[target_feature(enable = "avx")]
#[allow(improper_ctypes_definitions)]
unsafe extern "C" fn with_target_feature(x: __m256) {
    let val = unsafe { transmute::<_, [u32; 8]>(x) };
    dbg!(val);
}

fn main() {
    assert!(is_x86_feature_detected!("avx"));
    // SAFETY: we checked that the `avx` feature is present.
    unsafe {
        with_target_feature(transmute([1; 8])); //~ ERROR: missing `avx` target feature
    }
}
warning: ABI error: this function call uses a vector type that requires the `avx` target feature, which is not enabled in the caller
  --> test.rs:18:9
   |
18 |         with_target_feature(transmute([1; 8]));
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function called here
   |
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #116558 <https://github.com/rust-lang/rust/issues/116558>
   = help: consider enabling it globally (`-C target-feature=+avx`) or locally (`#[target_feature(enable="avx")]`)
   = note: `#[warn(abi_unsupported_vector_types)]` on by default

The lint is necessary because the way we codegen this function would be unsound (and indeed, if you run this on the playground you can see that the argument value gets corrupted). See #116558 for more context.

However, there's no fundamental reason that we couldn't compile this code! We "just" need to generate the call to with_target_feature using its proper ABI, i.e., using the AVX registers. This is sound because the function anyway requires that target feature, so the caller must have already ensured that this target feature is available.

The problem is that LLVM currently simply has no way to express such a call. So we have three options:

  • error (the easiest one, and what we are currently working towards)
  • fix this in LLVM (also see Inlining loses ABI-relevant target feature information at call operations llvm/llvm-project#70563) -- I am told this is quite hard
  • generate a shim that uses the Rust ABI (so it is not affected by these ABI shenanigans), and has the avx feature gate, and calls the actual callee -- not a pretty solution since the extra function call is bad for performance, and performance is the reason people manually write SIMD code to begin with

Lucky enough, this only affects non-Rust ABIs, so users should only rarely run into this.

Cc @rust-lang/wg-llvm @rust-lang/opsem @chorman0773 @veluca93

@RalfJung RalfJung added A-ABI Area: Concerning the application binary interface (ABI) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-SIMD Area: SIMD (Single Instruction Multiple Data) C-feature-request Category: A feature request, i.e: not implemented / a PR. T-opsem Relevant to the opsem team WG-llvm Working group: LLVM backend code generation labels Nov 10, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 10, 2024
@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ABI Area: Concerning the application binary interface (ABI) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-SIMD Area: SIMD (Single Instruction Multiple Data) C-feature-request Category: A feature request, i.e: not implemented / a PR. T-opsem Relevant to the opsem team WG-llvm Working group: LLVM backend code generation
Projects
None yet
Development

No branches or pull requests

3 participants