-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
add Thumbs to the compiler #36874
add Thumbs to the compiler #36874
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @arielb1 (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
👍 |
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.
Oh, I meant to left this comment earlier but didn't realize I clicked the start review thing. 🤦
@@ -698,6 +707,10 @@ impl ToJson for Target { | |||
target_option_val!(max_atomic_width); | |||
target_option_val!(panic_strategy); | |||
|
|||
if self.options.max_atomic_width.to_string() != self.target_pointer_width { |
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.
@alexcrichton this fixes the problem I was seeing where setting max_atomic_width
to 0
in the Target
declaration was being converted into 32
due to the JSON roundtrip. I've also added a rmake test for this.
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.
Woah can't we use Option
instead of magic 0
treatment or similar? Granted, I'm arriving late to this issue so apologies if that has already been ruled out for good reason.
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.
This was the simplest change necessary to make this work but using Option
here sounds better; I'll give it a try.
☔ The latest upstream changes (presumably #36339) made this pull request unmergeable. Please resolve the merge conflicts. |
this commit adds 4 new target definitions to the compiler for easier cross compilation to ARM Cortex-M devices. - `thumbv6m-none-eabi` - For the Cortex-M0, Cortex-M0+ and Cortex-M1 - This architecture doesn't have hardware support (instructions) for atomics. Hence, the `Atomic*` structs are not available for this target. - `thumbv7m-none-eabi` - For the Cortex-M3 - `thumbv7em-none-eabi` - For the FPU-less variants of the Cortex-M4 and Cortex-M7 - On this target, all the floating point operations will be lowered software routines (intrinsics) - `thumbv7em-none-eabihf` - For the variants of the Cortex-M4 and Cortex-M7 that do have a FPU. - On this target, all the floating point operations will be lowered to hardware instructions No binary releases of standard crates, like `core`, are planned for these targets because Cargo, in the future, will compile e.g. the `core` crate on the fly as part of the `cargo build` process. In the meantime, you'll have to compile the `core` crate yourself. [Xargo] is the easiest way to do that as in handles the compilation of `core` automatically and can be used just like Cargo: `xargo build --target thumbv6m-none-eabi` is all that's needed. [Xargo]: https://crates.io/crates/xargo
|
||
options: TargetOptions { | ||
// vfp4 lowest common denominator between the Cortex-M4 (vfp4) and the Cortex-M7 (vfp5) | ||
features: "+vfp4".to_string(), |
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 believe you also need +fp-only-sp
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.
Hmm, won't that preclude a user from enabling double precision hardware acceleration? Perhaps they'll have to pass -fp-only-sp
apart from the other feature that enables DP FP ops. (I'm speculating; I haven't tried any of this)
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 -fp-only-sp
would work to switch it back on. +d16
should probably be added here too.
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.
Done! Thank you @parched.
@japaric could you also add some comments to each target file in line with the RFC itself? Basically just some basic rationale for the target name, what it's intended to be used for, etc. |
to better express the idea that omitting this field defaults this value to target_pointer_width
📌 Commit 6136069 has been approved by |
add Thumbs to the compiler this commit adds 4 new target definitions to the compiler for easier cross compilation to ARM Cortex-M devices. - `thumbv6m-none-eabi` - For the Cortex-M0, Cortex-M0+ and Cortex-M1 - This architecture doesn't have hardware support (instructions) for atomics. Hence, the `Atomic*` structs are not available for this target. - `thumbv7m-none-eabi` - For the Cortex-M3 - `thumbv7em-none-eabi` - For the FPU-less variants of the Cortex-M4 and Cortex-M7 - On this target, all the floating point operations will be lowered software routines (intrinsics) - `thumbv7em-none-eabihf` - For the variants of the Cortex-M4 and Cortex-M7 that do have a FPU. - On this target, all the floating point operations will be lowered to hardware instructions No binary releases of standard crates, like `core`, are planned for these targets because Cargo, in the future, will compile e.g. the `core` crate on the fly as part of the `cargo build` process. In the meantime, you'll have to compile the `core` crate yourself. [Xargo] is the easiest way to do that as in handles the compilation of `core` automatically and can be used just like Cargo: `xargo build --target thumbv6m-none-eabi` is all that's needed. [Xargo]: https://crates.io/crates/xargo --- cc @brson @alexcrichton
this commit adds 4 new target definitions to the compiler for easier
cross compilation to ARM Cortex-M devices.
thumbv6m-none-eabi
atomics. Hence, the
Atomic*
structs are not available for thistarget.
thumbv7m-none-eabi
thumbv7em-none-eabi
software routines (intrinsics)
thumbv7em-none-eabihf
to hardware instructions
No binary releases of standard crates, like
core
, are planned forthese targets because Cargo, in the future, will compile e.g. the
core
crate on the fly as part of the
cargo build
process. In the meantime,you'll have to compile the
core
crate yourself. Xargo is the easiestway to do that as in handles the compilation of
core
automatically andcan be used just like Cargo:
xargo build --target thumbv6m-none-eabi
is all that's needed.
cc @brson @alexcrichton