Skip to content

add math.wideMul #2111

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

Merged
merged 1 commit into from
Mar 28, 2019
Merged

add math.wideMul #2111

merged 1 commit into from
Mar 28, 2019

Conversation

shawnl
Copy link
Contributor

@shawnl shawnl commented Mar 27, 2019

…mulWithOverflow

As discussed in #1350 and also std/math/big/int.zig
@mulWithOverflow() doesn't really reflect the way hardware multiply
works, or the way most architectures provide multiplication.

Also add a wideMul() that does what hardware usually provides.

@shawnl
Copy link
Contributor Author

shawnl commented Mar 27, 2019

This cannot do @mulWithOverflow(u128), however, on both aarch64 and x86_64 that is being done with a 4-way multiplication and just throwing away the high bits, so my case for removal still stands.

@andrewrk andrewrk added work in progress This pull request is not ready for review yet. and removed work in progress This pull request is not ready for review yet. labels Mar 28, 2019
@andrewrk andrewrk changed the title remove @mulWithOverflow from the language, add math.wideMul and math.… add math.wideMul Mar 28, 2019
// anyway to get the carry bits. The branch on the overflow case costs more than
// just computing them unconditionally and splitting.
//
// This could be a single x86 mul instruction, which stores the carry/lower in rdx:rax.
Copy link
Member

Choose a reason for hiding this comment

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

Did this comment become incorrect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is no longer relevant, because that is exactly what math.wideMul() does.

@andrewrk andrewrk merged commit 85d1885 into ziglang:master Mar 28, 2019
@shawnl shawnl deleted the mul branch March 29, 2019 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants