about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-07-23 00:16:03 +0000
committerbors <bors@rust-lang.org>2023-07-23 00:16:03 +0000
commit1c44af9b791c16557b5bf606707bb76df979134a (patch)
treec1b61598822646a529fd6ba2f96bedfc5f84aefc
parent98179ad634b1039e5db084f545c59c58c7f96222 (diff)
parentcdb9de7e8bd70c7e3580ec28b37af879e555f45d (diff)
downloadrust-1c44af9b791c16557b5bf606707bb76df979134a.tar.gz
rust-1c44af9b791c16557b5bf606707bb76df979134a.zip
Auto merge of #111836 - calebzulawski:target-feature-closure, r=workingjubilee
Fix #[inline(always)] on closures with target feature 1.1

Fixes #108655.  I think this is the most obvious solution that isn't overly complicated.  The comment includes more justification, but I think this is likely better than demoting the `#[inline(always)]` to `#[inline]`, since existing code is unaffected.
-rw-r--r--compiler/rustc_codegen_ssa/src/codegen_attrs.rs17
-rw-r--r--tests/codegen/target-feature-inline-closure.rs33
-rw-r--r--tests/ui/rfcs/rfc-2396-target_feature-11/issue-108655-inline-always-closure.rs18
3 files changed, 67 insertions, 1 deletions
diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs
index 0c7b8a79612..92792ab6477 100644
--- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs
+++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs
@@ -501,7 +501,22 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
     });
 
     // #73631: closures inherit `#[target_feature]` annotations
-    if tcx.features().target_feature_11 && tcx.is_closure(did.to_def_id()) {
+    //
+    // If this closure is marked `#[inline(always)]`, simply skip adding `#[target_feature]`.
+    //
+    // At this point, `unsafe` has already been checked and `#[target_feature]` only affects codegen.
+    // Emitting both `#[inline(always)]` and `#[target_feature]` can potentially result in an
+    // ICE, because LLVM errors when the function fails to be inlined due to a target feature
+    // mismatch.
+    //
+    // Using `#[inline(always)]` implies that this closure will most likely be inlined into
+    // its parent function, which effectively inherits the features anyway. Boxing this closure
+    // would result in this closure being compiled without the inherited target features, but this
+    // is probably a poor usage of `#[inline(always)]` and easily avoided by not using the attribute.
+    if tcx.features().target_feature_11
+        && tcx.is_closure(did.to_def_id())
+        && codegen_fn_attrs.inline != InlineAttr::Always
+    {
         let owner_id = tcx.parent(did.to_def_id());
         if tcx.def_kind(owner_id).has_codegen_attrs() {
             codegen_fn_attrs
diff --git a/tests/codegen/target-feature-inline-closure.rs b/tests/codegen/target-feature-inline-closure.rs
new file mode 100644
index 00000000000..d075706173f
--- /dev/null
+++ b/tests/codegen/target-feature-inline-closure.rs
@@ -0,0 +1,33 @@
+// only-x86_64
+// compile-flags: -Copt-level=3
+
+#![crate_type = "lib"]
+#![feature(target_feature_11)]
+
+#[cfg(target_arch = "x86_64")]
+use std::arch::x86_64::*;
+
+// CHECK-LABEL: @with_avx
+#[no_mangle]
+#[cfg(target_arch = "x86_64")]
+#[target_feature(enable = "avx")]
+fn with_avx(x: __m256) -> __m256 {
+    // CHECK: fadd
+    let add = {
+        #[inline(always)]
+        |x, y| unsafe { _mm256_add_ps(x, y) }
+    };
+    add(x, x)
+}
+
+// CHECK-LABEL: @without_avx
+#[no_mangle]
+#[cfg(target_arch = "x86_64")]
+unsafe fn without_avx(x: __m256) -> __m256 {
+    // CHECK-NOT: fadd
+    let add = {
+        #[inline(always)]
+        |x, y| unsafe { _mm256_add_ps(x, y) }
+    };
+    add(x, x)
+}
diff --git a/tests/ui/rfcs/rfc-2396-target_feature-11/issue-108655-inline-always-closure.rs b/tests/ui/rfcs/rfc-2396-target_feature-11/issue-108655-inline-always-closure.rs
new file mode 100644
index 00000000000..bc886400099
--- /dev/null
+++ b/tests/ui/rfcs/rfc-2396-target_feature-11/issue-108655-inline-always-closure.rs
@@ -0,0 +1,18 @@
+// Tests #108655: closures in `#[target_feature]` functions can still be marked #[inline(always)]
+
+// check-pass
+// revisions: mir thir
+// [thir]compile-flags: -Z thir-unsafeck
+// only-x86_64
+
+#![feature(target_feature_11)]
+
+#[target_feature(enable = "avx")]
+pub unsafe fn test() {
+    ({
+        #[inline(always)]
+        move || {}
+    })();
+}
+
+fn main() {}