Implement RFC 2574, "SIMD vectors in FFI"

compiler/rustc_passes/src/check_attr.rs

@@ -368,11 +368,15 @@ impl CheckAttrVisitor<'tcx> {
         match target {
             Target::Fn
             | Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => true,
+
+            // Allow foreign functions for SIMD FFI.
+            Target::ForeignFn => true,
+
             // FIXME: #[target_feature] was previously erroneously allowed on statements and some
             // crates used this, so only emit a warning.
             Target::Statement => {
                 self.tcx.struct_span_lint_hir(UNUSED_ATTRIBUTES, hir_id, attr.span, |lint| {
-                    lint.build("attribute should be applied to a function")
+                    lint.build("`#[target_feature]` attribute should be applied to a function")
                         .warn(
                             "this was previously accepted by the compiler but is \
                              being phased out; it will become a hard error in \
@@ -394,7 +398,10 @@ impl CheckAttrVisitor<'tcx> {
             _ => {
                 self.tcx
                     .sess
-                    .struct_span_err(attr.span, "attribute should be applied to a function")
+                    .struct_span_err(
+                        attr.span,
+                        "`#[target_feature]` attribute should be applied to a function",
+                    )
                     .span_label(*span, "not a function")
                     .emit();
                 false

compiler/rustc_typeck/src/collect.rs

@@ -2479,43 +2479,202 @@ fn compute_sig_of_foreign_fn_decl<'tcx>(
         None,
     );
 
-    // Feature gate SIMD types in FFI, since I am not sure that the
-    // ABIs are handled at all correctly. -huonw
-    if abi != abi::Abi::RustIntrinsic
-        && abi != abi::Abi::PlatformIntrinsic
-        && !tcx.features().simd_ffi
-    {
-        let check = |ast_ty: &hir::Ty<'_>, ty: Ty<'_>| {
-            if ty.is_simd() {
-                let snip = tcx
-                    .sess
-                    .source_map()
-                    .span_to_snippet(ast_ty.span)
-                    .map_or_else(|_| String::new(), |s| format!(" `{}`", s));
-                tcx.sess
-                    .struct_span_err(
-                        ast_ty.span,
-                        &format!(
-                            "use of SIMD type{} in FFI is highly experimental and \
-                             may result in invalid code",
-                            snip
-                        ),
-                    )
-                    .help("add `#![feature(simd_ffi)]` to the crate attributes to enable")
-                    .emit();
-            }
-        };
+    // Using SIMD types in FFI signatures requires the signature
+    // to have appropriate `#[target_feature]`'s enabled.
+    if abi != abi::Abi::RustIntrinsic && abi != abi::Abi::PlatformIntrinsic {
         for (input, ty) in iter::zip(decl.inputs, fty.inputs().skip_binder()) {
-            check(&input, ty)
+            simd_ffi_check(tcx, def_id, &input, ty)
         }
         if let hir::FnRetTy::Return(ref ty) = decl.output {
-            check(&ty, fty.output().skip_binder())
+            simd_ffi_check(tcx, def_id, &ty, fty.output().skip_binder())
         }
     }
 
     fty
 }
 
+/// Returns `Ok()` if the target-feature allows using the SIMD type on C FFI.
+/// Otherwise, returns `Err(Some())` if the target_feature needs to be enabled or
+/// or `Err(None)` if it's unsupported.
+/// Some notes about arguments:
+/// - `simd_len`: A vector register size as octet, i.e. vN in the tests.
+/// - `simd_elem_width`: A width of each element as octet, e.g. v512 should be 16 on avx512.
+fn simd_ffi_feature_check(
+    target: &str,
+    simd_width: u64,
+    simd_elem_width: u64,
+    feature: Option<String>,
+) -> Result<(), Option<&'static str>> {
+    let feature = feature.unwrap_or_default();
+    match target {
+        t if t.contains("x86") => {
+            // FIXME: this needs to be architecture dependent and
+            // should probably belong somewhere else:
+            // * on mips: 16 => msa,
+            // * wasm: 16 => simd128
+            match simd_width {
+                8 if feature.contains("mmx")
+                    || feature.contains("sse")
+                    || feature.contains("ssse")
+                    || feature.contains("avx") =>
+                {
+                    Ok(())
+                }
+                8 => Err(None),
+                16 if feature.contains("sse") => Ok(()),
+                16 => Err(Some("sse")),
+                32 if feature.contains("avx") => Ok(()),
+                32 => Err(Some("avx")),
+                64 if feature.contains("avx512") => Ok(()),
+                64 => Err(Some("avx512")),
+                _ => Err(None),
+            }
+        }
+        t if t.contains("arm") => match simd_width {
+            8 | 16 | 32 => {
+                if feature.contains("neon") {
+                    Ok(())
+                } else {
+                    Err(Some("neon"))
+                }
+            }
+            _ => Err(None),
+        },
+        t if t.contains("aarch64") => match simd_width {
+            8 | 16 => {
+                if feature.contains("neon") {
+                    Ok(())
+                } else {
+                    Err(Some("neon"))
+                }
+            }
+            _ => Err(None),
+        },
+        t if t.contains("powerpc") => {
+            match simd_width {
+                // 64-bit wide elements are only available in VSX:
+                16 if simd_elem_width == 8 => {
+                    if feature.contains("vsx") {
+                        Ok(())
+                    } else {
+                        Err(Some("vsx"))
+                    }
+                }
+                16 if simd_elem_width < 8 => {
+                    if feature.contains("altivec") {
+                        Ok(())
+                    } else {
+                        Err(Some("altivec"))
+                    }
+                }
+                _ => Err(None),
+            }
+        }
+        t if t.contains("mips") => match simd_width {
+            16 => {
+                if feature.contains("msa") {
+                    Ok(())
+                } else {
+                    Err(Some("msa"))
+                }
+            }
+            _ => Err(None),
+        },
+        _ => Err(None),
+    }
+}
+
+fn simd_ffi_check<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, ast_ty: &hir::Ty<'_>, ty: Ty<'tcx>) {
+    if !ty.is_simd() {
+        return;
+    }
+
+    // The use of SIMD types in FFI is feature-gated:
+    if !tcx.features().simd_ffi {
+        let snip = tcx
+            .sess
+            .source_map()
+            .span_to_snippet(ast_ty.span)
+            .map_or_else(|_| String::new(), |s| format!("{}", s));
+        tcx.sess
+            .struct_span_err(
+                ast_ty.span,
+                &format!("use of SIMD type `{}` in FFI is unstable", snip),
+            )
+            .help("add `#![feature(simd_ffi)]` to the crate attributes to enable")
+            .emit();
+        return;
+    }
+
+    // If rustdoc, then we don't type check SIMD on FFI because rustdoc requires
+    // being able to compile a target, with features of other targets enabled
+    // (e.g. `x86+neon`, yikes).
+    if tcx.sess.opts.actually_rustdoc {
+        return;
+    }
+
+    let attrs = tcx.codegen_fn_attrs(def_id);
+
+    // Skip LLVM intrinsics.
+    if let Some(link_name) = attrs.link_name {
+        if link_name.to_ident_string().starts_with("llvm.") {
+            return;
+        }
+    }
+
+    let features = &attrs.target_features;
+    let simd_len = tcx
+        .layout_of(ty::ParamEnvAnd { param_env: ty::ParamEnv::empty(), value: ty })
+        .unwrap()
+        .layout
+        .size
+        .bytes();
+    let (simd_size, _) = ty.simd_size_and_type(tcx);
+    let simd_elem_width = simd_len / simd_size;
+    let target: &str = &tcx.sess.target.arch;
+
+    let type_str = tcx
+        .sess
+        .source_map()
+        .span_to_snippet(ast_ty.span)
+        .map_or_else(|_| String::new(), |s| format!("{}", s));
+
+    if features.is_empty() {
+        // Should **NOT** be `Ok()`.
+        let f = simd_ffi_feature_check(target, simd_len, simd_elem_width, None).unwrap_err();
+        let msg = if let Some(f) = f {
+            format!(
+                "use of SIMD type `{}` in FFI requires `#[target_feature(enable = \"{}\")]`",
+                type_str, f,
+            )
+        } else {
+            format!("use of SIMD type `{}` in FFI not supported by any target features", type_str)
+        };
+
+        tcx.sess.struct_span_err(ast_ty.span, &msg).emit();
+    }
+
+    for f in features {
+        if let Err(v) =
+            simd_ffi_feature_check(target, simd_len, simd_elem_width, Some(f.to_ident_string()))
+        {
+            let msg = if let Some(f) = v {
+                format!(
+                    "use of SIMD type `{}` in FFI requires `#[target_feature(enable = \"{}\")]`",
+                    type_str, f,
+                )
+            } else {
+                format!(
+                    "use of SIMD type `{}` in FFI not supported by any target features",
+                    type_str
+                )
+            };
+            tcx.sess.struct_span_err(ast_ty.span, &msg).emit();
+            return;
+        }
+    }
+}
+
 fn is_foreign_item(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
     match tcx.hir().get_if_local(def_id) {
         Some(Node::ForeignItem(..)) => true,
@@ -2799,7 +2958,10 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs {
                 codegen_fn_attrs.export_name = Some(s);
             }
         } else if tcx.sess.check_name(attr, sym::target_feature) {
-            if !tcx.is_closure(id) && tcx.fn_sig(id).unsafety() == hir::Unsafety::Normal {
+            if !tcx.is_closure(id)
+                && !tcx.is_foreign_item(id)
+                && tcx.fn_sig(id).unsafety() == hir::Unsafety::Normal
+            {
                 if tcx.sess.target.is_like_wasm || tcx.sess.opts.actually_rustdoc {
                     // The `#[target_feature]` attribute is allowed on
                     // WebAssembly targets on all functions, including safe

src/test/run-make-fulldeps/simd-ffi/simd.rs

@@ -4,53 +4,56 @@
 // cross-compiled standard libraries.
 #![feature(no_core, auto_traits)]
 #![no_core]
-#![feature(repr_simd, simd_ffi, link_llvm_intrinsics, lang_items, rustc_attrs)]
+#![feature(
+    mips_target_feature,
+    repr_simd,
+    simd_ffi,
+    link_llvm_intrinsics,
+    lang_items,
+    rustc_attrs
+)]
 
 #[derive(Copy)]
 #[repr(simd)]
-pub struct f32x4(f32, f32, f32, f32);
+pub struct F32x4(f32, f32, f32, f32);
 
 extern "C" {
     #[link_name = "llvm.sqrt.v4f32"]
-    fn vsqrt(x: f32x4) -> f32x4;
+    fn vsqrt(x: F32x4) -> F32x4;
 }
 
-pub fn foo(x: f32x4) -> f32x4 {
+pub fn foo(x: F32x4) -> F32x4 {
     unsafe { vsqrt(x) }
 }
 
 #[derive(Copy)]
 #[repr(simd)]
-pub struct i32x4(i32, i32, i32, i32);
+pub struct I32x4(i32, i32, i32, i32);
 
 extern "C" {
     // _mm_sll_epi32
-    #[cfg(any(target_arch = "x86", target_arch = "x86-64"))]
+    #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
     #[link_name = "llvm.x86.sse2.psll.d"]
-    fn integer(a: i32x4, b: i32x4) -> i32x4;
+    fn integer(a: I32x4, b: I32x4) -> I32x4;
 
     // vmaxq_s32
     #[cfg(target_arch = "arm")]
     #[link_name = "llvm.arm.neon.vmaxs.v4i32"]
-    fn integer(a: i32x4, b: i32x4) -> i32x4;
+    fn integer(a: I32x4, b: I32x4) -> I32x4;
     // vmaxq_s32
     #[cfg(target_arch = "aarch64")]
     #[link_name = "llvm.aarch64.neon.maxs.v4i32"]
-    fn integer(a: i32x4, b: i32x4) -> i32x4;
+    fn integer(a: I32x4, b: I32x4) -> I32x4;
 
     // just some substitute foreign symbol, not an LLVM intrinsic; so
     // we still get type checking, but not as detailed as (ab)using
     // LLVM.
-    #[cfg(not(any(
-        target_arch = "x86",
-        target_arch = "x86-64",
-        target_arch = "arm",
-        target_arch = "aarch64"
-    )))]
-    fn integer(a: i32x4, b: i32x4) -> i32x4;
+    #[cfg(target_arch = "mips")]
+    #[target_feature(enable = "msa")]
+    fn integer(a: I32x4, b: I32x4) -> I32x4;
 }
 
-pub fn bar(a: i32x4, b: i32x4) -> i32x4 {
+pub fn bar(a: I32x4, b: I32x4) -> I32x4 {
     unsafe { integer(a, b) }
 }
 

src/test/ui/attributes/multiple-invalid.stderr

@@ -7,7 +7,7 @@ LL | #[inline]
 LL | const FOO: u8 = 0;
    | ------------------ not a function or closure
 
-error: attribute should be applied to a function
+error: `#[target_feature]` attribute should be applied to a function
   --> $DIR/multiple-invalid.rs:6:1
    |
 LL | #[target_feature(enable = "sse2")]

src/test/ui/feature-gates/feature-gate-simd-ffi.stderr

@@ -1,12 +1,12 @@
-error: use of SIMD type `LocalSimd` in FFI is highly experimental and may result in invalid code
+error: use of SIMD type `LocalSimd` in FFI is unstable
   --> $DIR/feature-gate-simd-ffi.rs:9:17
    |
 LL |     fn baz() -> LocalSimd;
    |                 ^^^^^^^^^
    |
    = help: add `#![feature(simd_ffi)]` to the crate attributes to enable
 
-error: use of SIMD type `LocalSimd` in FFI is highly experimental and may result in invalid code
+error: use of SIMD type `LocalSimd` in FFI is unstable
   --> $DIR/feature-gate-simd-ffi.rs:10:15
    |
 LL |     fn qux(x: LocalSimd);

src/test/ui/macros/issue-68060.stderr

@@ -1,4 +1,4 @@
-error: attribute should be applied to a function
+error: `#[target_feature]` attribute should be applied to a function
   --> $DIR/issue-68060.rs:4:13
    |
 LL |             #[target_feature(enable = "")]