-
Notifications
You must be signed in to change notification settings - Fork 211
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
fix division on SPARC #393
Conversation
Is the CI broken? |
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.
Thanks! I'm trying to fix CI in #394
src/int/udiv.rs
Outdated
#[win64_128bit_abi_hack] | ||
/// Returns `n / d` | ||
pub extern "C" fn __udivti3(n: u128, d: u128) -> u128 { | ||
u128_divide_sparc(n, d, &mut 0) |
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.
Could this #[cfg]
happen inside the previous intrinsics!
block to avoid duplicating the definition?
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 changed it to use #[cfg]
inside the previous block, but I don't know if it is beneficial because of all the duplicated #[cfg]
that is required to completely disable u128_div_rem
src/int/specialized_div_rem/mod.rs
Outdated
target_arch = "sparc64", | ||
target_arch = "riscv32", | ||
target_arch = "riscv64" | ||
)))] |
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.
Instead of conditional definitions of this constant could it use ||
and cfg!
?
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.
good idea
9481c53
to
78ea036
Compare
👍 |
fixes #390. I decided to make an entire configuration path for all SPARC architectures, because SPARC does not have any widening multiplications which makes
_delegate
insanely slow. I manually tested the newu128_divide_sparc
function, but plan on testing it later with the new test system in PR #384. This PR needs to be merged first because 32-bit SPARC will not build without this. I will rebase #384 afterwards to testu128_divide_sparc
properly.