Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

#[target_feature(..)] no longer in effect in naked functions #136280

Open
tmiasko opened this issue Jan 30, 2025 · 5 comments · May be fixed by #137720
Open

#[target_feature(..)] no longer in effect in naked functions #136280

tmiasko opened this issue Jan 30, 2025 · 5 comments · May be fixed by #137720
Labels
A-inline-assembly Area: Inline assembly (`asm!(…)`) A-naked Area: `#[naked]`, prologue and epilogue-free, functions, https://git.io/vAzzS C-bug Category: This is a bug. F-naked_functions `#![feature(naked_functions)]`

Comments

@tmiasko
Copy link
Contributor

tmiasko commented Jan 30, 2025

Since #128004, target features enabled with #[target_feature(..)] are no longer in effect in naked functions. For example:

// --crate-typelib --target aarch64-unknown-linux-gnu
#![feature(naked_functions)]
use std::arch::{asm, naked_asm};

#[target_feature(enable = "aes")]
pub unsafe extern "C" fn f() {
    // ok
    asm!("aese.16b v0, v1");
}

#[target_feature(enable = "aes")]
#[naked]
pub unsafe extern "C" fn g() {
    // Before #128004: ok
    // After  #128004: error: instruction requires: aes
    naked_asm!("aese.16b v0, v1");
}

I haven't seen this discussed in the pull request. Opening the issue to give a chance to review the behaviour before stabilization.

cc @folkertdev, @Amanieu

@tmiasko tmiasko added A-inline-assembly Area: Inline assembly (`asm!(…)`) A-naked Area: `#[naked]`, prologue and epilogue-free, functions, https://git.io/vAzzS C-bug Category: This is a bug. F-naked_functions `#![feature(naked_functions)]` labels Jan 30, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 30, 2025
@folkertdev
Copy link
Contributor

This behavior is unfortunately expected, because of a bug in LLVM

#80608
llvm/llvm-project#61991

So basically LLVM just does not attach the target features to global assembly.

There are some target-specific fixes that use directives to enable the target features within the global assembly (e.g. this one for arm). That may work, but should not be needed in the future when LLVM, hopefully, fixes this issue.

@tmiasko tmiasko removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 30, 2025
@Amanieu
Copy link
Member

Amanieu commented Feb 26, 2025

This is a different issue than those you listed, which are related to global target feature (-Ctarget-feature). In the case of local target features we need to explicitly pass those into the assembly code since the naked function is never seen by LLVM.

@folkertdev
Copy link
Contributor

ah, right. Then yeah we can at least do better (e.g. with .arch_extension for arm/aarch64), and I'll have to figure out what to do for other ISAs

@taiki-e
Copy link
Member

taiki-e commented Feb 26, 2025

I'll have to figure out what to do for other ISAs

AFAIK, in x86/x86_64 all instructions available without such a directive. Also, in powerpc/powerpc64 LLVM currently actually ignores .machine and allows all instructions (see this fixme comment).

(Of these directives, I have actually used only aarch64 .arch_extension & .arch, arm .arch, riscv .option arch, and powerpc64 .machine.)

@folkertdev
Copy link
Contributor

That's extremely helpful, thanks!

I think for the tier 2 targets, the ones I'm still missing are

  • wasm: nothing jumps out to me in the LLVM asm parser, so does wasm just not have such a directive yet?
  • loongarch: again nothing obvious in LLMV

Wasm does emit an error when you use inline assembly that requires a target feature https://godbolt.org/z/KaPqqM8Ez, while for loongarch I've not yet found an example where the target feature seems to matter.

And then there is a bunch of tier 3 targets which I did not all check but I'd assume most are not (close) to stable inline assembly, so we can skip those on a first pass.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-inline-assembly Area: Inline assembly (`asm!(…)`) A-naked Area: `#[naked]`, prologue and epilogue-free, functions, https://git.io/vAzzS C-bug Category: This is a bug. F-naked_functions `#![feature(naked_functions)]`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants