-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ARM] Some processors appear to have optional features enabled by default #113008
Comments
@llvm/issue-subscribers-backend-arm Author: Alex Rønne Petersen (alexrp)
See this Zig pull request: https://github.com/ziglang/zig/pull/18498 (specifically, [this hunk](https://github.com/ziglang/zig/pull/18498/files#diff-88edb24013ad7e5428ff29aac0ebba01f919136cfcfe17d806b1892b2af7d065R382-R404))
It seems that LLVM is enabling features for these processors that are actually optional, per Arm documentation. We're currently working around this in Zig by omitting these features by default in the Zig CPU model/feature data which is sourced from LLVM's. Would there be interest in a patch to similarly disable these features by default on the LLVM side? (Note: Please ignore the addition of |
cc @davemgreen maybe you have thoughts on this? |
I think @smithp35 is away at the moment but might have more to say. @tmatheson-arm and @pratlucas too for the target-parser parts. AFAIU, the rule in clang, that was agreed with gcc is that:
There are exceptions and some places where that is not followed exactly. It was done this way so that most people get a good default, and people can disable the features they don't have. That isn't to say that llvm needs to follow the same rules, so long as the interface for clang doesn't change. The features in https://github.com/ziglang/zig/pull/18498/files#diff-88edb24013ad7e5428ff29aac0ebba01f919136cfcfe17d806b1892b2af7d065R382-R404 are either
pacbti is considered a benign architecture feature, as it only enables assembly/disassembly. The actual codegen changes are controlled with other options. fp is complicated in clang/gcc by the -mfpu option and -mfloat-abi depending on the abi. In clang it essentially defaults to -mcpu=auto from gcc. GCC docs are available at https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html. I guess I would recommend the same scheme for other frontends. Needless differentiation between the frontends does not sound ideal. |
I'm only concerned with the processor definitions in LLVM specifically. If we can change those to only imply the minimum set of features there, while leaving them unchanged at the Clang level, I agree that would be ideal. |
I agree with @davemgreen on the rules of thumb for CPU features. I'm hesitant to recommend changing the LLVM defaults, even if clang remains stable, without wider consultation via an RFC. For example it would affect other front-ends built on top of LLVM like Rust as well. One possible solution, and I'd need to defer to @tmatheson-arm and @pratlucas for how practical it would be, is for us separate out what feature is optional for a CPU or not, and whether LLVM enables it by default for a CPU. That way LLVM can act as a machine readable technical reference manual (TRM) for the CPU, without affecting the default option it presents to front-ends. |
FWIW, Rust ran into the same issue, previously reported at #90365 and also discussed at rust-lang/rust#125033. As far as I understand it, the preference on the Rust side would be for LLVM to use the minimum guaranteed set of target features. |
Frontends that would like to have sounder defaults for their CLI options, and are strongly asked by many of their library developers and target maintainers to do so, are not engaging in "needless" differentiation. I do not expect you to agree on what is "necessary" versus "unnecessary", but the conversations I've had with Rust's embedded ARM devs and maintainers strongly suggest that they are considering PRing a workaround to this issue on rustc's level. By the sounds of it, this might be primarily by completely abandoning the |
See this Zig pull request: ziglang/zig#18498 (specifically, this hunk)
It seems that LLVM is enabling features for these processors that are actually optional, per Arm documentation. We're currently working around this in Zig by omitting these features by default in the Zig CPU model/feature data which is sourced from LLVM's.
Would there be interest in a patch to similarly disable these features by default on the LLVM side?
(Note: Please ignore the addition of
trustzone
tocortex-m85
; as discussed in #109770, this was based on a misunderstanding and has since been reverted in Zig.)The text was updated successfully, but these errors were encountered: