Pre-RFC: stabilization of target_feature

compiler
26

@gnzlbg Thanks so much for writing this up! Sorry it has taken me so long to respond, but I’m just starting to ramp up work again on SIMD.

I feel like I pretty much agree with the consensus that you and @alexcrichton reached. In particular, I like the idea of requiring a feature to use unstable target features in code. It does seem more consistent with Rust’s overall stabilization story. It’s true that some nightly-only crates will break, but the fix seems relatively simple to me and I think that kind of breakage is acceptable.

One thing that I’m still not quite sure on is #[target_feature]. In particular, why is it necessary to stabilize it in this RFC? It seems like it can’t actually be used on Rust stable without any stable feature names? I think #[target_feature] probably needs more work to help prevent not just monomorphization errors, but runtime errors that look very suspicious to me. Take this program, for example, using the stdsimd crate:

#![feature(target_feature)]

extern crate stdsimd;

use std::env;
use stdsimd::simd;

#[inline(never)]
#[target_feature = "-sse2"] // strange, perhaps nonsensical, but allowed!
fn myop(
    (x0, x1, x2, x3): (u64, u64, u64, u64),
    (y0, y1, y2, y3): (u64, u64, u64, u64),
) -> (u64, u64, u64, u64) {
    let x = simd::u64x4::new(x0, x1, x2, x3);
    let y = simd::u64x4::new(y0, y1, y2, y3);
    let r = x * y;
    (r.extract(0), r.extract(1), r.extract(2), r.extract(3))
}

fn main() {
    let x = env::args().nth(1).unwrap().parse().unwrap();
    let y = env::args().nth(2).unwrap().parse().unwrap();
    let r = myop((x, x, x, x), (y, y, y, y));
    println!("{:?}", r);
}

What is the expected output of this program? IMO, since I’m using LLVM’s support for cross platform SIMD vector types, I’d expect this to work. But specifically disabling SSE2 seems to cause things to go very wrong:

$ RUSTFLAGS="-C target-cpu=native" cargo run --release --example types 7 11
    Finished release [optimized + debuginfo] target(s) in 0.0 secs
     Running `target/release/examples/types 7 11`
21
thread 'main' panicked at 'assertion failed: idx < 4', src/v256.rs:10
note: Run with `RUST_BACKTRACE=1` for a backtrace.

The specific assert that’s failing is this: https://github.com/BurntSushi/stdsimd/blob/e5599e0cc7d9476267cc8c60b5b4ee8d713f4c24/src/macros.rs#L42 — The 21 printed in the output above is the result of adding println!("{:?}", idx); just before the assert. Given that constants of 0, 1, 2 and 3 are used, something is clearly pretty wrong. I don’t have the expertise to diagnose the problem though. But the fact that this is possible through no use of unsafe at all in my program probably means that this behavior is unacceptable.

If I use #[target_feature = "+avx2"] instead, then things work just fine:

$ RUSTFLAGS="-C target-cpu=native" cargo run --release --example wat 7 11
   Compiling stdsimd v0.0.1 (file:///home/andrew/data/projects/rust/stdsimd)
    Finished release [optimized + debuginfo] target(s) in 0.38 secs
     Running `target/release/examples/types 7 11`
0
1
2
3
(77, 77, 77, 77)

Hell, you don’t even need target_feature at all for this to be correct, since it’s using the cross platform vector API. But still, the point remains.

In any case… This might all be moot if you aren’t stabilizing any feature names, but I’m not sure what the gain is?