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

[stdlib] Simplify Int.__floordiv__ and friends #3497

Open
wants to merge 1 commit into
base: nightly
Choose a base branch
from

Conversation

soraros
Copy link
Contributor

@soraros soraros commented Sep 18, 2024

No description provided.

@soraros soraros marked this pull request as ready for review September 18, 2024 17:17
@soraros soraros requested a review from a team as a code owner September 18, 2024 17:17
var mod = self - div * rhs
var divMod = select(((rhs < 0) ^ (self < 0)) & mod, div - 1, div)
div = select(self > 0 & rhs > 0, div, divMod)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The first branch was never taken anyways, since

  self > 0 & rhs > 0
=
  self > (0 & rhs) > 0
=
  self > 0 > 0
=
  False

@soraros soraros changed the title [stdlib] Fix Int.__floordiv__ and friends [stdlib] Simplify Int.__floordiv__ and friends Sep 18, 2024
@JoeLoser
Copy link
Collaborator

!sync

@modularbot modularbot added the imported-internally Signals that a given pull request has been imported internally. label Sep 18, 2024
if ((rhs < 0) ^ (self < 0)) & mod:
return div - 1, mod + rhs
return div, mod
return self // rhs, self % rhs
Copy link
Collaborator

Choose a reason for hiding this comment

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

Question By recomputing things, won't this regress performance a bit?

Copy link
Contributor Author

@soraros soraros Sep 19, 2024

Choose a reason for hiding this comment

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

Both // and % are marked as always_inline and they share most of the code; I just assume that the compiler can see through the defs and do CSE well. I can inline and do a manual dedup if you so prefer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As a ref point, UInt also does this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked and they generates identical assembly.

@soraros soraros force-pushed the fix-int-floordiv branch 2 times, most recently from 10a8a71 to 933cc33 Compare September 26, 2024 06:37
@soraros soraros force-pushed the fix-int-floordiv branch 3 times, most recently from 278467d to d64f143 Compare October 1, 2024 06:54
@soraros

This comment was marked as outdated.

Signed-off-by: Yiwu Chen <210at85@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
imported-internally Signals that a given pull request has been imported internally.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants