Auto merge of #162460 - Jamesbarford:feat/inline-always+target-feature-#2, r=nikic,traviscross

Feat - `#[inline(always)] + #[target_feature(enable = "....")]` #2





This is broadly a resurrection of `#[inline(always)] + #[target_feature(enable = "...")]`.

- Applies the `alwaysinline` attribute to the function definition.
- `version < LLVM23` downgrades to an `inline` hint.
- Remove the check for using [target features + inline always](compiler/rustc_codegen_ssa/src/codegen_attrs.rs)

This should now be sound thanks to
* llvm/llvm-project#205113
* llvm/llvm-project#209345

Therefore, it should be okay to just allow this combination. But we can also have a period of unstable experimentation if t-lang prefers.

Reference PR; rust-lang/reference#2391

r? @RalfJung

Author: bors

compiler/rustc_codegen_llvm/src/attributes.rs

@@ -68,6 +68,14 @@ pub(crate) fn inline_attr<'tcx, 'll>(
 
     match inline {
         InlineAttr::Hint => Some(AttributeKind::InlineHint.create_attr(cx.llcx)),
+        // LLVM 22 and older may inline a target-featured function into a caller
+        // that lacks those features. Keep the function inlineable, but do not force it.
+        InlineAttr::Always
+            if !codegen_fn_attrs.target_features.is_empty()
+                && llvm_util::get_version() < (23, 0, 0) =>
+        {
+            Some(AttributeKind::InlineHint.create_attr(cx.llcx))
+        }
         InlineAttr::Always | InlineAttr::Force { .. } => {
             Some(AttributeKind::AlwaysInline.create_attr(cx.llcx))
         }

compiler/rustc_codegen_ssa/src/codegen_attrs.rs

@@ -415,33 +415,6 @@ fn check_result(
     interesting_spans: InterestingAttributeDiagnosticSpans,
     codegen_fn_attrs: &CodegenFnAttrs,
 ) {
-    // If a function uses `#[target_feature]` it can't be inlined into general
-    // purpose functions as they wouldn't have the right target features
-    // enabled. For that reason we also forbid `#[inline(always)]` as it can't be
-    // respected.
-    //
-    // `#[rustc_force_inline]` doesn't need to be prohibited here, only
-    // `#[inline(always)]`, as forced inlining is implemented entirely within
-    // rustc (and so the MIR inliner can do any necessary checks for compatible target
-    // features).
-    //
-    // This sidesteps the LLVM blockers in enabling `target_features` +
-    // `inline(always)` to be used together (see rust-lang/rust#116573 and
-    // llvm/llvm-project#70563).
-    if !codegen_fn_attrs.target_features.is_empty()
-        && matches!(codegen_fn_attrs.inline, InlineAttr::Always)
-        && let Some(span) = interesting_spans.inline
-    {
-        let mut diag = tcx
-            .dcx()
-            .struct_span_err(span, "cannot use `#[inline(always)]` with `#[target_feature]`");
-        diag.note(
-            "See this issue for full discussion: \
-            https://github.com/rust-lang/rust/issues/145574",
-        );
-        diag.emit();
-    }
-
     // warn that inline has no effect when no_sanitize is present
     if codegen_fn_attrs.sanitizers != SanitizerFnAttrs::default()
         && codegen_fn_attrs.inline.always()

compiler/rustc_lint/src/lib.rs

@@ -698,7 +698,9 @@ fn register_builtins(store: &mut LintStore) {
     store.register_removed("soft_unstable", "the general soft-unstable mechanism has been removed");
     store.register_removed(
         "inline_always_mismatching_target_features",
-        "replaced by a hard error for `#[inline(always)]` with `#[target_feature]`",
+        "replaced by a hard error for `#[inline(always)]` with `#[target_feature]`. \
+        This was prior to LLVM23 where combining these could be unsound. See \
+        <https://github.com/rust-lang/rust/issues/145574> for more information",
     );
     store.register_removed(
         "repr_transparent_external_private_fields",

tests/codegen-llvm/inline-always-target-feature-mismatch.rs

@@ -0,0 +1,25 @@
+//@ only-x86_64
+//@ min-llvm-version: 23
+//@ compile-flags: -Copt-level=3
+
+#![crate_type = "lib"]
+#![allow(unused_unsafe)]
+
+use std::arch::x86_64::_bzhi_u64;
+
+#[inline(always)]
+#[target_feature(enable = "bmi2")]
+#[unsafe(no_mangle)]
+pub unsafe fn callee_requires_bmi2() -> u64 {
+    // black box this as `_bzhi_u64(1, 2)` can evaluate to `1` at compile time and LLVM is smart
+    // enough to see that it then can ignore `bmi2` as this returns a constant. Which is safe to
+    // do.
+    std::hint::black_box(_bzhi_u64(1, 2))
+}
+
+#[unsafe(no_mangle)]
+// CHECK-LABEL: define{{.*}} @caller_only()
+// CHECK: [[TMP:%.+]] = tail call noundef i64 @callee_requires_bmi2()
+pub unsafe fn caller_only() {
+    let _x = callee_requires_bmi2();
+}

tests/codegen-llvm/inline-always.rs

@@ -0,0 +1,36 @@
+//@ add-minicore
+//@ compile-flags: --target aarch64-unknown-linux-gnu -Zinline-mir=no -C no-prepopulate-passes -Copt-level=3
+//@ needs-llvm-components: aarch64
+//@ min-llvm-version: 23
+
+#![crate_type = "lib"]
+#![feature(no_core, lang_items)]
+#![no_core]
+
+extern crate minicore;
+use minicore::*;
+
+#[inline(always)]
+#[target_feature(enable = "neon")]
+#[no_mangle]
+pub fn single_target_feature() -> i32 {
+    42
+}
+
+#[inline(always)]
+#[target_feature(enable = "neon,i8mm")]
+#[no_mangle]
+// CHECK: define{{( noundef)?}} i32 @multiple_target_features() unnamed_addr #1{{( !guid ![0-9]+)?}} {
+pub fn multiple_target_features() -> i32 {
+    // CHECK: %_0 = call{{( noundef)?}} i32 @single_target_feature() #3
+    single_target_feature()
+}
+
+#[no_mangle]
+// CHECK: define{{( noundef)?}} i32 @inherits_from_global() unnamed_addr #2{{( !guid ![0-9]+)?}} {
+pub fn inherits_from_global() -> i32 {
+    unsafe {
+        // CHECK: %_0 = call{{( noundef)?}} i32 @single_target_feature() #3
+        single_target_feature()
+    }
+}

tests/ui/target-feature/invalid-attribute.rs

@@ -66,12 +66,6 @@ type Uwu = ();
 //~^ ERROR attribute cannot be used on
 trait Baz {}
 
-#[inline(always)]
-//~^ ERROR: cannot use `#[inline(always)]`
-//~| NOTE: See this issue for full discussion: https://github.com/rust-lang/rust/issues/145574
-#[target_feature(enable = "sse2")]
-unsafe fn test() {}
-
 #[target_feature(enable = "sse2")]
 //~^ ERROR attribute cannot be used on
 static A: () = ();

tests/ui/target-feature/invalid-attribute.stderr

@@ -121,39 +121,39 @@ LL | #[target_feature(enable = "sse2")]
    = help: the `target_feature` attribute can only be applied to functions
 
 error: the `target_feature` attribute cannot be used on statics
-  --> $DIR/invalid-attribute.rs:75:3
+  --> $DIR/invalid-attribute.rs:69:3
    |
 LL | #[target_feature(enable = "sse2")]
    |   ^^^^^^^^^^^^^^
    |
    = help: the `target_feature` attribute can only be applied to functions
 
 error: the `target_feature` attribute cannot be used on trait impl blocks
-  --> $DIR/invalid-attribute.rs:79:3
+  --> $DIR/invalid-attribute.rs:73:3
    |
 LL | #[target_feature(enable = "sse2")]
    |   ^^^^^^^^^^^^^^
    |
    = help: the `target_feature` attribute can only be applied to functions
 
 error: the `target_feature` attribute cannot be used on inherent impl blocks
-  --> $DIR/invalid-attribute.rs:85:3
+  --> $DIR/invalid-attribute.rs:79:3
    |
 LL | #[target_feature(enable = "sse2")]
    |   ^^^^^^^^^^^^^^
    |
    = help: the `target_feature` attribute can only be applied to functions
 
 error: the `target_feature` attribute cannot be used on expressions
-  --> $DIR/invalid-attribute.rs:106:7
+  --> $DIR/invalid-attribute.rs:100:7
    |
 LL |     #[target_feature(enable = "sse2")]
    |       ^^^^^^^^^^^^^^
    |
    = help: the `target_feature` attribute can only be applied to functions
 
 error: the `target_feature` attribute cannot be used on closures
-  --> $DIR/invalid-attribute.rs:112:7
+  --> $DIR/invalid-attribute.rs:106:7
    |
 LL |     #[target_feature(enable = "sse2")]
    |       ^^^^^^^^^^^^^^
@@ -168,16 +168,8 @@ LL | #[target_feature(enable = "foo")]
    |
    = help: valid names are: `fma`, `xop`, `adx`, `aes`, and `avx` and X more
 
-error: cannot use `#[inline(always)]` with `#[target_feature]`
-  --> $DIR/invalid-attribute.rs:69:1
-   |
-LL | #[inline(always)]
-   | ^^^^^^^^^^^^^^^^^
-   |
-   = note: See this issue for full discussion: https://github.com/rust-lang/rust/issues/145574
-
 error[E0046]: not all trait items implemented, missing: `foo`
-  --> $DIR/invalid-attribute.rs:81:1
+  --> $DIR/invalid-attribute.rs:75:1
    |
 LL | impl Quux for u8 {}
    | ^^^^^^^^^^^^^^^^ missing `foo` in implementation
@@ -186,7 +178,7 @@ LL |     fn foo();
    |     --------- `foo` from trait
 
 error: `#[target_feature(..)]` cannot be applied to safe trait method
-  --> $DIR/invalid-attribute.rs:95:5
+  --> $DIR/invalid-attribute.rs:89:5
    |
 LL |     #[target_feature(enable = "sse2")]
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot be applied to safe trait method
@@ -195,21 +187,21 @@ LL |     fn foo() {}
    |     -------- not an `unsafe` function
 
 error[E0053]: method `foo` has an incompatible type for trait
-  --> $DIR/invalid-attribute.rs:98:5
+  --> $DIR/invalid-attribute.rs:92:5
    |
 LL |     fn foo() {}
    |     ^^^^^^^^ expected safe fn, found unsafe fn
    |
 note: type in trait
-  --> $DIR/invalid-attribute.rs:90:5
+  --> $DIR/invalid-attribute.rs:84:5
    |
 LL |     fn foo();
    |     ^^^^^^^^^
    = note: expected signature `fn()`
               found signature `#[target_feature(..)] fn()`
 
 error: the feature named `+sse2` is not valid for this target
-  --> $DIR/invalid-attribute.rs:117:18
+  --> $DIR/invalid-attribute.rs:111:18
    |
 LL | #[target_feature(enable = "+sse2")]
    |                  ^^^^^^^^^^^^^^^^ `+sse2` is not valid for this target
@@ -221,22 +213,22 @@ LL + #[target_feature(enable = "sse2")]
    |
 
 error: the feature named `sse5` is not valid for this target
-  --> $DIR/invalid-attribute.rs:122:18
+  --> $DIR/invalid-attribute.rs:116:18
    |
 LL | #[target_feature(enable = "sse5")]
    |                  ^^^^^^^^^^^^^^^ `sse5` is not valid for this target
    |
    = help: valid names are: `sse`, `sse2`, `sse3`, `sse4a`, and `ssse3` and X more
 
 error: the feature named `avx512` is not valid for this target
-  --> $DIR/invalid-attribute.rs:127:18
+  --> $DIR/invalid-attribute.rs:121:18
    |
 LL | #[target_feature(enable = "avx512")]
    |                  ^^^^^^^^^^^^^^^^^ `avx512` is not valid for this target
    |
    = help: valid names are: `avx512f`, `avx2`, `avx512bw`, `avx512cd`, and `avx512dq` and X more
 
-error: aborting due to 26 previous errors
+error: aborting due to 25 previous errors
 
 Some errors have detailed explanations: E0046, E0053, E0539.
 For more information about an error, try `rustc --explain E0046`.