Skip to content

Commit

Permalink
Fix division on AVRs
Browse files Browse the repository at this point in the history
For division and modulo, AVR uses a custom calling convention that does
not match compiler_builtins' expectations, leading to non-working code¹.

Ideally we'd just use hand-written naked functions (as, afair, ARM
does), but that's a lot of code to port², so hopefully we'll be able to
do it gradually later.

For the time being, I'd suggest not compiling problematic functions for
AVR target - this causes avr-gcc (which is a mandatory part of Rust+AVR
toolchain anyway) to link hand-written assembly from libgcc, which is
confirmed to work.

I've tested the code locally on simavr and the patch seems to be working
correctly :-)

¹ rust-lang/rust#82242,
  rust-lang/rust#83281
² https://github.com/gcc-mirror/gcc/blob/31048012db98f5ec9c2ba537bfd850374bdd771f/libgcc/config/avr/lib1funcs.S

Closes rust-lang/rust#82242
Closes rust-lang/rust#83281
  • Loading branch information
Patryk27 committed May 15, 2022
1 parent 4117da3 commit fbf1586
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/int/sdiv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ macro_rules! sdivmod {
$($attr:tt),* // attributes
) => {
intrinsics! {
#[avr_skip]
$(
#[$attr]
)*
Expand Down
3 changes: 3 additions & 0 deletions src/int/udiv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ intrinsics! {
u32_div_rem(n, d).1
}

#[avr_skip]
#[maybe_use_optimized_c_shim]
/// Returns `n / d` and sets `*rem = n % d`
pub extern "C" fn __udivmodsi4(n: u32, d: u32, rem: Option<&mut u32>) -> u32 {
Expand All @@ -40,6 +41,7 @@ intrinsics! {
u64_div_rem(n, d).1
}

#[avr_skip]
#[maybe_use_optimized_c_shim]
/// Returns `n / d` and sets `*rem = n % d`
pub extern "C" fn __udivmoddi4(n: u64, d: u64, rem: Option<&mut u64>) -> u64 {
Expand Down Expand Up @@ -77,6 +79,7 @@ intrinsics! {
}
}

#[avr_skip]
#[win64_128bit_abi_hack]
/// Returns `n / d` and sets `*rem = n % d`
pub extern "C" fn __udivmodti4(n: u128, d: u128, rem: Option<&mut u128>) -> u128 {
Expand Down
31 changes: 30 additions & 1 deletion src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ macro_rules! intrinsics {

$($rest:tt)*
) => (

#[cfg($name = "optimized-c")]
pub extern $abi fn $name( $($argname: $ty),* ) $(-> $ret)? {
extern $abi {
Expand Down Expand Up @@ -304,6 +303,36 @@ macro_rules! intrinsics {
intrinsics!($($rest)*);
);

// For division and modulo, AVR uses a custom calling convention¹ that does
// not match our definitions here. Ideally we would just use hand-written
// naked functions, but that's quite a lot of code to port² - so for the
// time being we are just ignoring the problematic functions, letting
// avr-gcc (which is required to compile to AVR anyway) link them from
// libgcc.
//
// ¹ https://gcc.gnu.org/wiki/avr-gcc (see "Exceptions to the Calling
// Convention")
// ² https://github.com/gcc-mirror/gcc/blob/31048012db98f5ec9c2ba537bfd850374bdd771f/libgcc/config/avr/lib1funcs.S
(
#[avr_skip]
$(#[$($attr:tt)*])*
pub extern $abi:tt fn $name:ident( $($argname:ident: $ty:ty),* ) $(-> $ret:ty)? {
$($body:tt)*
}

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

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

// This is the final catch-all rule. At this point we generate an
// intrinsic with a conditional `#[no_mangle]` directive to avoid
// interfering with duplicate symbols and whatnot during testing.
Expand Down

0 comments on commit fbf1586

Please sign in to comment.