-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
Add Tier 3 Thumb-mode targets for Armv7-A, Armv7-R and Armv8-R #150556
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
base: main
Are you sure you want to change the base?
Add Tier 3 Thumb-mode targets for Armv7-A, Armv7-R and Armv8-R #150556
Conversation
|
Some changes occurred in src/doc/rustc/src/platform-support cc @Noratrieb These commits modify compiler targets. |
|
r? @fee1-dead rustbot has assigned @fee1-dead. Use |
|
@rustbot reroll |
This comment was marked as resolved.
This comment was marked as resolved.
a338091 to
a556210
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
wooops, sry @bors compiler |
|
Unknown command "compiler". Run |
|
@rustbot reroll |
| llvm_floatabi: Some(FloatAbi::Soft), | ||
| linker_flavor: LinkerFlavor::Gnu(Cc::No, Lld::Yes), | ||
| linker: Some("rust-lld".into()), | ||
| features: "+v7,+thumb2,+soft-float,-neon,+strict-align".into(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+v7,+thumb2 was lost, is this intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The feature flags were redundant as they are implied by the LLVM target name. The —print cfg output should be unchanged.
| linker: Some("rust-lld".into()), | ||
| features: "+v7,+thumb2,+soft-float,-neon,+strict-align".into(), | ||
| relocation_model: RelocModel::Static, | ||
| disable_redzone: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
disable_redzone was changed to false, is this intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t think red zones apply to this architecture, so I made the setting the same as the other AArch32 targets.
| linker: Some("rust-lld".into()), | ||
| features: "+v7,+vfp3d16,+thumb2,-neon,+strict-align".into(), | ||
| relocation_model: RelocModel::Static, | ||
| disable_redzone: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, features, disable_redzone and frame_pointer are different now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took out redundant flags and rationalised the options to match all the other targets.
| features: "+soft-float,-neon,+strict-align".into(), | ||
| max_atomic_width: Some(64), | ||
| has_thumb_interworking: true, | ||
| ..base::arm_none::opts() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, frame_pointer is now set to FramePointer::Always, is this intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Other AArch32 targets have frame pointers always on.
| c_enum_min_bits: Some(8), | ||
| has_thumb_interworking: true, | ||
| ..Default::default() | ||
| ..base::arm_none::opts() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
frame_pointer changes here and in the other R targets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Other AArch32 targets have frame pointers always on.
| llvm_floatabi: Some(FloatAbi::Soft), | ||
| features: "+soft-float,-neon,+strict-align".into(), | ||
| max_atomic_width: Some(64), | ||
| has_thumb_interworking: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does has_thumb_interworking make sense on thumb targets?
Most of them don't have it set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It enables the instruction_set attribute and AFAIK should be enabled on every target that supports both Arm mode and Thumb mode regardless of which is the default. The only AArch32 targets it shouldn’t apply to are the M-profile targets because they are Thumb-only.
| if !cx.sess.target.has_thumb_interworking { |
This comment has been minimized.
This comment has been minimized.
809a1a5 to
fb05cac
Compare
75afffe to
fa526c4
Compare
|
@petrochenkov - I've updated the docs as requested. |
We currently have targets for bare-metal Armv7-R, Armv7-A and Armv8-R, but only in Arm mode. This PR adds five new targets enabling bare-metal support on these architectures in Thumb mode.
This has been tested using https://github.com/rust-embedded/aarch32/compare/main...thejpster:aarch32:support-thumb-mode-v7-v8?expand=1 and they all seem to work as expected.
However, I wasn't sure what to do with the maintainer lists as these are five new targets, but they share the docs page with the existing Arm versions. I can ask the Embedded Devices WG Arm Team about taking on these ones too, but whether Arm themselves want to take them on I guess is a bigger question.