-
Notifications
You must be signed in to change notification settings - Fork 250
fix division on SPARC #393
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
Conversation
|
Is the CI broken? |
alexcrichton
left a comment
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
_delegateinsanely slow. I manually tested the newu128_divide_sparcfunction, 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_sparcproperly.