Skip to content
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.

Sign up for GitHub

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

Solve the issue of not being able to override existing builtin functions under LTO. #728

Closed
wants to merge 4 commits into from
Closed

Solve the issue of not being able to override existing builtin functions under LTO. #728

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
90fa854
Add the `weak-intrinsics` feature to compiler_builtins.
DianQK Dec 17, 2023
99a0ebd
Add intrinsics support for the optional return value.
DianQK Dec 17, 2023
a215988
Add the `naked` attribute of intrinsics
DianQK Dec 17, 2023
36244e6
Change `division_function` to use the `naked` function
DianQK Dec 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rp2040-hal/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ targets = ["thumbv6m-none-eabi"]
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
compiler_builtins = { version = "0.1.104", features = ["weak-intrinsics"] }
cortex-m = "0.7.2"
embedded-hal = { version = "0.2.5", features = ["unproven"] }
eh1_0_alpha = { package = "embedded-hal", version = "=1.0.0-rc.1", optional = true }
Expand Down
83 changes: 53 additions & 30 deletions rp2040-hal/src/intrinsics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,46 +1,46 @@
/// Generate a series of aliases for an intrinsic function.
macro_rules! intrinsics_aliases {
(
extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) -> $ret:ty,
extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) $(-> $ret:ty)?,
) => {};
(
unsafe extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) -> $ret:ty,
unsafe extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) $(-> $ret:ty)?,
) => {};

(
extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) -> $ret:ty,
extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) $(-> $ret:ty)?,
$alias:ident
$($rest:ident)*
) => {
#[cfg(all(target_arch = "arm", not(feature = "disable-intrinsics")))]
mod $alias {
#[no_mangle]
pub extern $abi fn $alias( $($argname: $ty),* ) -> $ret {
pub extern $abi fn $alias( $($argname: $ty),* ) $(-> $ret)? {
super::$name($($argname),*)
}
}

intrinsics_aliases! {
extern $abi fn $name( $($argname: $ty),* ) -> $ret,
extern $abi fn $name( $($argname: $ty),* ) $(-> $ret)?,
$($rest)*
}
};

(
unsafe extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) -> $ret:ty,
unsafe extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) $(-> $ret:ty)?,
$alias:ident
$($rest:ident)*
) => {
#[cfg(all(target_arch = "arm", not(feature = "disable-intrinsics")))]
mod $alias {
#[no_mangle]
unsafe extern $abi fn $alias( $($argname: $ty),* ) -> $ret {
unsafe extern $abi fn $alias( $($argname: $ty),* ) $(-> $ret)? {
super::$name($($argname),*)
}
}

intrinsics_aliases! {
unsafe extern $abi fn $name( $($argname: $ty),* ) -> $ret,
unsafe extern $abi fn $name( $($argname: $ty),* ) $(-> $ret)?,
$($rest)*
}
};
Expand Down Expand Up @@ -88,7 +88,7 @@ macro_rules! intrinsics {
(
#[slower_than_default]
$(#[$($attr:tt)*])*
extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) -> $ret:ty {
extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) $(-> $ret:ty)? {
$($body:tt)*
}

Expand All @@ -97,7 +97,7 @@ macro_rules! intrinsics {
// Not exported, but defined so the actual implementation is
// considered used
#[allow(dead_code)]
fn $name( $($argname: $ty),* ) -> $ret {
fn $name( $($argname: $ty),* ) $(-> $ret)? {
$($body)*
}

Expand All @@ -107,7 +107,7 @@ macro_rules! intrinsics {
(
#[bootrom_v2]
$(#[$($attr:tt)*])*
extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) -> $ret:ty {
extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) $(-> $ret:ty)? {
$($body:tt)*
}

Expand All @@ -117,14 +117,14 @@ macro_rules! intrinsics {
// considered used
#[cfg(not(feature = "rom-v2-intrinsics"))]
#[allow(dead_code)]
fn $name( $($argname: $ty),* ) -> $ret {
fn $name( $($argname: $ty),* ) $(-> $ret)? {
$($body)*
}

#[cfg(feature = "rom-v2-intrinsics")]
intrinsics! {
$(#[$($attr)*])*
extern $abi fn $name( $($argname: $ty),* ) -> $ret {
extern $abi fn $name( $($argname: $ty),* ) $(-> $ret)? {
$($body)*
}
}
Expand All @@ -135,21 +135,21 @@ macro_rules! intrinsics {
(
#[alias = $($alias:ident),*]
$(#[$($attr:tt)*])*
extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) -> $ret:ty {
extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) $(-> $ret:ty)? {
$($body:tt)*
}

$($rest:tt)*
) => {
intrinsics! {
$(#[$($attr)*])*
extern $abi fn $name( $($argname: $ty),* ) -> $ret {
extern $abi fn $name( $($argname: $ty),* ) $(-> $ret)? {
$($body)*
}
}

intrinsics_aliases! {
extern $abi fn $name( $($argname: $ty),* ) -> $ret,
extern $abi fn $name( $($argname: $ty),* ) $(-> $ret)?,
$($alias) *
}

Expand All @@ -159,21 +159,21 @@ macro_rules! intrinsics {
(
#[alias = $($alias:ident),*]
$(#[$($attr:tt)*])*
unsafe extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) -> $ret:ty {
unsafe extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) $(-> $ret:ty)? {
$($body:tt)*
}

$($rest:tt)*
) => {
intrinsics! {
$(#[$($attr)*])*
unsafe extern $abi fn $name( $($argname: $ty),* ) -> $ret {
unsafe extern $abi fn $name( $($argname: $ty),* ) $(-> $ret)? {
$($body)*
}
}

intrinsics_aliases! {
unsafe extern $abi fn $name( $($argname: $ty),* ) -> $ret,
unsafe extern $abi fn $name( $($argname: $ty),* ) $(-> $ret)?,
$($alias) *
}

Expand All @@ -183,21 +183,21 @@ macro_rules! intrinsics {
(
#[aeabi = $($alias:ident),*]
$(#[$($attr:tt)*])*
extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) -> $ret:ty {
extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) $(-> $ret:ty)? {
$($body:tt)*
}

$($rest:tt)*
) => {
intrinsics! {
$(#[$($attr)*])*
extern $abi fn $name( $($argname: $ty),* ) -> $ret {
extern $abi fn $name( $($argname: $ty),* ) $(-> $ret)? {
$($body)*
}
}

intrinsics_aliases! {
extern "aapcs" fn $name( $($argname: $ty),* ) -> $ret,
extern "aapcs" fn $name( $($argname: $ty),* ) $(-> $ret)?,
$($alias) *
}

Expand All @@ -206,23 +206,23 @@ macro_rules! intrinsics {

(
$(#[$($attr:tt)*])*
extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) -> $ret:ty {
extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) $(-> $ret:ty)? {
$($body:tt)*
}

$($rest:tt)*
) => {
#[cfg(all(target_arch = "arm", not(feature = "disable-intrinsics")))]
$(#[$($attr)*])*
extern $abi fn $name( $($argname: $ty),* ) -> $ret {
extern $abi fn $name( $($argname: $ty),* ) $(-> $ret)? {
$($body)*
}

#[cfg(all(target_arch = "arm", not(feature = "disable-intrinsics")))]
mod $name {
#[no_mangle]
$(#[$($attr)*])*
pub extern $abi fn $name( $($argname: $ty),* ) -> $ret {
pub extern $abi fn $name( $($argname: $ty),* ) $(-> $ret)? {
super::$name($($argname),*)
}
}
Expand All @@ -231,32 +231,55 @@ macro_rules! intrinsics {
// considered used
#[cfg(not(all(target_arch = "arm", not(feature = "disable-intrinsics"))))]
#[allow(dead_code)]
fn $name( $($argname: $ty),* ) -> $ret {
fn $name( $($argname: $ty),* ) $(-> $ret)? {
$($body)*
}

intrinsics!($($rest)*);
};

// Naked functions are special: we can't generate wrappers for them since
// they use a custom calling convention.
(
#[naked]
$(#[$($attr:tt)*])*
unsafe extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) -> $ret:ty {
unsafe extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) $(-> $ret:ty)? {
$($body:tt)*
}

$($rest:tt)*
) => (
mod $name {
#[naked]
$(#[$($attr)*])*
#[no_mangle]
pub unsafe extern $abi fn $name( $($argname: $ty),* ) $(-> $ret)? {
$($body)*
}
}

intrinsics!($($rest)*);
);

(
$(#[$($attr:tt)*])*
unsafe extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) $(-> $ret:ty)? {
$($body:tt)*
}

$($rest:tt)*
) => {
#[cfg(all(target_arch = "arm", not(feature = "disable-intrinsics")))]
$(#[$($attr)*])*
unsafe extern $abi fn $name( $($argname: $ty),* ) -> $ret {
unsafe extern $abi fn $name( $($argname: $ty),* ) $(-> $ret)? {
$($body)*
}

#[cfg(all(target_arch = "arm", not(feature = "disable-intrinsics")))]
mod $name {
#[no_mangle]
$(#[$($attr)*])*
pub unsafe extern $abi fn $name( $($argname: $ty),* ) -> $ret {
pub unsafe extern $abi fn $name( $($argname: $ty),* ) $(-> $ret)? {
super::$name($($argname),*)
}
}
Expand All @@ -265,7 +288,7 @@ macro_rules! intrinsics {
// considered used
#[cfg(not(all(target_arch = "arm", not(feature = "disable-intrinsics"))))]
#[allow(dead_code)]
unsafe fn $name( $($argname: $ty),* ) -> $ret {
unsafe fn $name( $($argname: $ty),* ) $(-> $ret)? {
$($body)*
}

Expand Down
2 changes: 2 additions & 0 deletions rp2040-hal/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@

#![warn(missing_docs)]
#![no_std]
#![feature(naked_functions)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is my understanding correct that this is not stable yet ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's only on nightly. Are we going to use it on stable too?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This library targets stable rust, yes. So nightly features can't be used.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that even stabilizing naked is not enough. Using compiler_builtins requires nightly.
But we can change it to default weak in rust. I think the key here is still to stabilize the naked first.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please help me verify that the current modification works?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, what can I do?
Currently, it doesn't build. Obviously not with stable, but I also get errors with nightly:

$ cargo +nightly build --release
[...]
   Compiling rp2040-hal v0.9.1 (/home/jan/rp2040/rp-rs/rp-hal/rp2040-hal)
error: invalid instruction
   --> rp2040-hal/src/sio.rs:299:22
    |
299 |                     "hwdivider_head",
    |                      ^
    |
note: instantiated into assembly here
   --> <inline asm>:1:2
    |
1   |     hwdivider_head
    |     ^
[...]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this error yet. But I was able to compile it successfully at https://github.com/rp-rs/rp2040-project-template. I also need add compiler_builtins = { version = "0.1.104", features = ["weak-intrinsics"] } to the rp2040-project-template.


#[doc(hidden)]
pub use paste;
Expand Down Expand Up @@ -94,6 +95,7 @@ pub use sio::Sio;
pub use spi::Spi;
pub use timer::Timer;
pub use watchdog::Watchdog;
extern crate compiler_builtins;
// Re-export crates used in rp2040-hal's public API
pub extern crate fugit;

Expand Down
39 changes: 21 additions & 18 deletions rp2040-hal/src/sio.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,29 +278,32 @@ core::arch::global_asm!(

macro_rules! division_function {
(
$name:ident $($intrinsic:ident)* ( $argty:ty ) {
$name:ident $intrinsic:ident $intrinsic_alias:ident ( $argty:ty ) {
$($begin:literal),+
}
) => {
#[cfg(all(target_arch = "arm", not(feature = "disable-intrinsics")))]
core::arch::global_asm!(
// Mangle the name slightly, since this is a global symbol.
concat!(".global _rphal_", stringify!($name)),
concat!(".type _rphal_", stringify!($name), ", %function"),
".align 2",
concat!("_rphal_", stringify!($name), ":"),
$(
concat!(".global ", stringify!($intrinsic)),
concat!(".type ", stringify!($intrinsic), ", %function"),
concat!(stringify!($intrinsic), ":"),
)*

"hwdivider_head",
$($begin),+ ,
"hwdivider_tail",
);
intrinsics! {
#[naked]
unsafe extern "C" fn $intrinsic() {
core::arch::asm!(
"hwdivider_head",
$($begin),+ ,
"hwdivider_tail",
options(noreturn)
);
}
#[naked]
unsafe extern "C" fn $intrinsic_alias() {
core::arch::asm!(
"hwdivider_head",
$($begin),+ ,
"hwdivider_tail",
options(noreturn)
);
}
}

#[cfg(all(target_arch = "arm", feature = "disable-intrinsics"))]
core::arch::global_asm!(
// Mangle the name slightly, since this is a global symbol.
concat!(".global _rphal_", stringify!($name)),
Expand Down
Loading