Skip to content

Conversation

@bradjc
Copy link
Contributor

@bradjc bradjc commented Jan 18, 2024

Pull Request Overview

This is a realization of the long-desired #1654 (at least for arm). Switches #[naked] to global_asm!().

It seems like no one cares to stabilize naked_functions. There was momentum 1.5 years ago, but that was derailed and it seems like projects looking to use stable are just using global_asm!().

I somewhat arbitrarily decided to switch hail to use stable as an example.

Testing Strategy

Running on nrf52840dk.

TODO or Help Wanted

One odd thing came up:

#[cfg(all(
    target_arch = "arm",
    target_feature = "v7",
    target_feature = "thumb-mode",
    target_os = "none"
))]

Doesn't seem to work the same way on stable. With that, even when building for a board the cfg doesn't match and the function/variable is removed. It's the target_feature part which doesn't seem to work the same way.

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make prepush.

@github-actions github-actions bot added arch/risc-v RISC-V architecture WG-OpenTitan In the purview of the OpenTitan working group. labels Jan 18, 2024
@hudson-ayers
Copy link
Contributor

Looks like cortex-m0 builds are broken due to some missing imports @bradjc

@bradjc
Copy link
Contributor Author

bradjc commented Jan 18, 2024

This is blocked on #3803

@bradjc bradjc added the blocked Waiting on something, like a different PR or a dependency. label Jan 18, 2024
@bradjc bradjc force-pushed the remove-naked branch 3 times, most recently from 5f35e72 to f7f7c22 Compare February 1, 2024 22:24
The compiler seems to be placing the global_asm code differently,
meaning a branch instruction no longer works. Load the address and
branch to the register instead.
lschuermann
lschuermann previously approved these changes Feb 8, 2024
@bradjc bradjc removed the blocked Waiting on something, like a different PR or a dependency. label Feb 8, 2024
@lschuermann
Copy link
Member

Seems like one unintended(?) consequence of this PR is that our script / Makefile hackery to generate alldocs also respects the toolchain override for the Hail board, and then fails to understand some of the flags we pass when generating docs. Presumably we need to work around this, perhaps with some more Makefile magic.

Only include flags which require nightly if we are compiling on nightly.
@alevy alevy added this pull request to the merge queue Feb 9, 2024
Merged via the queue into master with commit 27ee299 Feb 9, 2024
@alevy alevy deleted the remove-naked branch February 9, 2024 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch/risc-v RISC-V architecture WG-OpenTitan In the purview of the OpenTitan working group.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants