Swap to just-stabilised asm!() and global_asm!() macros#423
Merged
Swap to just-stabilised asm!() and global_asm!() macros#423
Conversation
|
(rust-highfive has picked a reviewer for you, use r? to override) |
thejpster
reviewed
Feb 22, 2022
| extern crate cortex_m_rt_macros as macros; | ||
|
|
||
| use core::fmt; | ||
| use core::arch::global_asm; |
Contributor
There was a problem hiding this comment.
I hope this clippy lint goes away in 1.59.
thejpster
reviewed
Feb 22, 2022
Contributor
|
Looks OK to me. Very excited to see this land! |
5a1dabe to
21837dc
Compare
21837dc to
c39dfb3
Compare
Member
Author
|
I've fixed that uppercase BXNS, set rust-version in the Cargo.toml (except the unchanged panic-itm), bumped those Cargos to edition 2021 while we're at it, and removed outdated references to old Rust versions from cortex-m-rt docs (since we now require 1.59 anyway, and they were already outdated with 1.42). |
…low checking code that uses it on native platform.
f3a4818 to
db8bea2
Compare
Contributor
|
What happened to the CI? |
…l asm, add inline to most cortex_m::asm methods
dcbcbef to
3e8a5be
Compare
Dirbaio
reviewed
Feb 25, 2022
Member
Author
|
Whew, a few snags from the CI but all green now. |
…to depend on cortex-m 0.8
e58d223 to
ac2a836
Compare
thejpster
approved these changes
Feb 26, 2022
Member
Author
|
bors r=thejpster |
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Once Rust 1.59 is released in a couple of days, the
asm!()andglobal_asm!()macros will become stable, and we will no longer require the various precompiled binaries we've used until now in cortex-m and cortex-m-rt. cc #420.This PR uses
asm!()in cortex-m, and removes the inline-asm feature, since I anticipate this going into cortex-m 0.8 and so we don't need to leave it behind for compatibility. In various places the previous version would call an extern C method when built for the native target, which would only fail at link time; to preserve the ability to build on x86 I've either made the whole method require thecortex_mconfiguration, or where appropriate/convenient simply skipped theasm!()call.This PR replaces the old gcc-preprocessed
asm.Sin cortex-m-rt with use ofglobal_asm!(), although since you can't normally use#[cfg(...)]attributes withglobal_asm!(), there's also a slightly scary macro modified from one invented by @Dirbaio for a similar purpose. I considered putting the initialisation of LR behind an armv6m flag, but since we want to restore it after calling__pre_initit seemed better to just leave it the same on both targets. I added Cargo features to optionally set SP and VTOR at startup, which has been variously requested but would previously have required multiplicatively more pre-built binaries. Now: no problem. Relevant issues:I've tested these on a couple of targets (and updated the CI): on the whole there's a small improvement in code size due to everyone getting inlined asm, especially in
cortex_m::interrupt::free().The major downside is we bump our MSRV from 1.42 (March 2020) to 1.59 (Feb 2022). For cortex-m, I propose putting these changes in the upcoming 0.8 release (which is technically what the master branch is already on) and not backporting. For cortex-m-rt I'm not sure: we don't have any other pending breaking changes, so we could consider a patch release. Anyway, this PR doesn't commit to any particular releases, so we can decide that later.
For cortex-m-semihosting/panic-semihosting I think a patch release would be ideal, especially since we had to yank the last c-m-sh release due to conflicting prebuilt binaries (a problem that should now vanish).
Also tagging these issues that I think might also benefit from new inline asm:
HardFaulttrampoline opt-in #406