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.

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

Add divmod support #4437

Closed
kmsquire opened this issue Oct 5, 2013 · 15 comments
Closed

Add divmod support #4437

kmsquire opened this issue Oct 5, 2013 · 15 comments

Comments

@kmsquire
Copy link
Member

kmsquire commented Oct 5, 2013

Related discussion in #4427

@quinnj
Copy link
Member

quinnj commented Oct 5, 2013

This would be great. A few of the date conversion algorithms in Datetime utilize divmod.

@StefanKarpinski
Copy link
Sponsor Member

Technically this should be divrem or fldmod, which are unfortunately less enjoyable names.

@JeffBezanson
Copy link
Sponsor Member

We can implement any and all of those.

@fabianlischka
Copy link
Contributor

Should this be closed, given that divrem is implemented now?
#5738

@JeffBezanson
Copy link
Sponsor Member

We might want divmod or fldmod too, they just won't be as efficient.

@StefanKarpinski
Copy link
Sponsor Member

divrem and fldmod make sense but divmod doesn't. div and rem are naturally paired and fld and mod are naturally paired; div and mod, despite the catchiness of the name divmod, are a fundamentally mismatched operations.

@fabianlischka
Copy link
Contributor

Understood, because they wouldn't fulfil a nice invariant.
I just saw that divrem appears to be implemented as

divrem(x,y) = (div(x,y),rem(x,y))

Question: That does not seem faster than, well, computing the quotient using div, and the remainder using rem, i.e. essentially doing the work twice. Is there any trickery behind the scenes?

If not, it seems trivial to add

fldmod(x,y) = (fld(x,y),mod(x,y))

(Oh, except that there is a divrem implementation for BigInts, calling a gmp function, so that should be faster for BigInts)

@quinnj
Copy link
Member

quinnj commented Apr 15, 2014

I thought this was originally brought up because LLVM has a special instruction for this?

@StefanKarpinski
Copy link
Sponsor Member

It's actually the opposite – LLVM does not have a special instruction for this but x86 does both with idiv. LLVM is usually clever enough to only do one x86 idiv instruction when one div and rem with the same arguments near each other. Given all the other nonsense we're currently doing before div and rem to handle the possibility of the second argument being zero, however, it may be beneficial to explicitly consolidate that checking when doing both. The code generated for div and rem just makes me want to cry.

@JeffBezanson
Copy link
Sponsor Member

That is correct; the extra checks we need to do seem to prevent LLVM from using a single idiv. If we added our own divrem intrinsic this could probably be solved.

@StefanKarpinski
Copy link
Sponsor Member

Isn't there anything we can do to make div and rem less awful? These operations are so important.

@JeffBezanson
Copy link
Sponsor Member

Yes; lobby for and hope for an x86-div-with-trap intrinsic in LLVM.

@JeffBezanson
Copy link
Sponsor Member

Or we could potentially patch the bit of the LLVM optimizer that replaces undefined div and rem operations with random values.

@quinnj
Copy link
Member

quinnj commented Aug 4, 2014

The perennial division discussion aside, do we still want fldmod? Otherwise, I don't think there's anything else to keep this open.

@quinnj
Copy link
Member

quinnj commented Aug 21, 2014

Feel free to reopen if there's anything else desired here.

@quinnj quinnj closed this as completed Aug 21, 2014
garrison added a commit to garrison/julia that referenced this issue Feb 20, 2015
This was mentioned/planned in the discussion for JuliaLang#4437 but
was not implemented at the time.
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

No branches or pull requests

5 participants