support #[target_feature(enable = ...)] on #[naked] functions
#137720
Conversation
rustbot
added
S-waiting-on-review
| // FIXME share code with `create_object_file` | ||
| fn parse_architecture( |
There was a problem hiding this comment.
this there some standard way of getting the architecture that I'm missing? I'd like the rest of the code to operate on an enum rather than doing a bunch of manual string matching.
folkertdev
force-pushed
the
naked-function-target-feature
branch
from
89d44c1 to
4626bcc
Compare
| // no action is needed, all instructions are accepted regardless of target feature | ||
| } | ||
|
|
||
| Architecture::Aarch64 | Architecture::Aarch64_Ilp32 | Architecture::Arm => { |
There was a problem hiding this comment.
As for Arm (AArch32), some target features like v8, neon, don't seem to work with .arch_extension: https://godbolt.org/z/GTK3vqfrh
As for v* features, they work with .arch armv*: https://godbolt.org/z/7oj9njWG6
There was a problem hiding this comment.
the docs list only three features for .arch_extension on aarch32: crc, fp16 and ras. I could see how the architectures are special, but even dotprod does not seem to work?!
also for the arch: because there is no push/pop, we can't get back to the original arch if we set it, right? I did figure out that you can .fpu neon for neon, but like arch, there is no way to reset the fpu once you set it.
There was a problem hiding this comment.
I did some further work categorizing the features that rust/llvm supports
There was a problem hiding this comment.
so, good news, I think there is a way forward here.
global_asm! being stateful (as mentioned here #137720 (comment)) cannot be relied on by users because of this rule in the reference
https://doc.rust-lang.org/reference/inline-assembly.html#r-asm.rules.not-successive
Therefore, we just need to make sure that at the end of a naked function, the settings are as configured via the global target features. It's less elegant that the push/pop mechanism , and for arm that will still be a bit of a mess, but it seems workable.
There was a problem hiding this comment.
well, after looking at it a bit, I still think it's possible, but hard. I have some partial code here https://gist.github.com/folkertdev/fe99874c466e598d0fb2dadf13b91b6f but I think this really needs someone more familiar with the arm target features to do a good job on.
(or, like, can we get LLVM to add a push/pop mechanism for us?)
So, in combination with target features being unstable on arm anyway, I would prefer to skip that logic for now, keep this PR straightforward, and get target features working on targets with stable asm support first.
|
I think this is as far as I'm going to get on my own (well with a bunch of help from taiki too, thanks!). Questions I don't have a good answer to:
r? @Amanieu |
fixes #136280
Instructions that are part of a target feature require a special directive on some targets. This PR adds those for the most common targets.
This is very WIP, but I'm hoping to collect some feedback on what is (not) supported and how to report that to users.
r? @ghost
cc @taiki-e @Amanieu