From ddb16e2884cdfe3c1c7eff8a431d1165213f7842 Mon Sep 17 00:00:00 2001 From: Caleb Zulawski Date: Sun, 21 May 2023 21:51:57 -0400 Subject: [PATCH 1/2] Fix #[inline(always)] on closures with target feature 1.1 --- .../rustc_codegen_ssa/src/codegen_attrs.rs | 17 ++++++++++++++++- .../issue-108655-inline-always-closure.rs | 18 ++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 tests/ui/rfcs/rfc-2396-target_feature-11/issue-108655-inline-always-closure.rs diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index d6c2301276262..1fa4b8783f6ff 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -493,7 +493,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/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 0000000000000..bc886400099a5 --- /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() {} From cdb9de7e8bd70c7e3580ec28b37af879e555f45d Mon Sep 17 00:00:00 2001 From: Caleb Zulawski Date: Sun, 16 Jul 2023 21:25:46 -0400 Subject: [PATCH 2/2] Add codegen test ensuring always-inline closures don't bypass target features --- .../codegen/target-feature-inline-closure.rs | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 tests/codegen/target-feature-inline-closure.rs diff --git a/tests/codegen/target-feature-inline-closure.rs b/tests/codegen/target-feature-inline-closure.rs new file mode 100644 index 0000000000000..d075706173fd1 --- /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) +}