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.

Sign up for GitHub

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

Jump to bottom

Use __builtin_rint(f) to implement rint(f) #9239

Merged
merged 5 commits into from
Aug 16, 2019
Merged

Use __builtin_rint(f) to implement rint(f) #9239

merged 5 commits into from
Aug 16, 2019

Conversation

quantum5
Copy link
Member

Fixes #9236.

@sbc100
Copy link
Collaborator

sbc100 commented Aug 14, 2019

Do we have a way to mark or record such changes so that they don't get lost when we next upgrade musl?

@kripken
Copy link
Member

kripken commented Aug 14, 2019 •
edited

We've marked changes in musl with // XXX EMSCRIPTEN comments, so that they are very obvious in merge diffs.

@quantum5 have you verified that those get lowered into the proper intrinsics (even with -fno-builtin)? They should but better to be sure.

@quantum5 quantum5 changed the title Use __builtin_rint(f) to implement lrint(f) Use __builtin_rint(f) to implement rint(f) Aug 14, 2019
@quantum5
Copy link
Member Author

Yes, after the latest commit, -fno-builtin will still result in code that does fxx.nearest, with (l)rint(f).

tlively
tlively approved these changes Aug 15, 2019
View reviewed changes
Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

Too bad we don't have codegen tests in Emscripten :/

@quantum5
Copy link
Member Author

Are we guaranteed to have llvm-lit around? If we are, we could do a lit test with wasm-dis...

@quantum5 quantum5 merged commit 7f9e788 into emscripten-core:incoming Aug 16, 2019
@sbc100 sbc100 mentioned this pull request Aug 19, 2019
Closed
belraquib pushed a commit to belraquib/emscripten that referenced this pull request Dec 23, 2020
@quantum5 @belraquib
Use __builtin_rint(f) to implement rint(f) (emscripten-core#9239)
db6d634 
Fixes emscripten-core#9236.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbc100 sbc100 sbc100 left review comments

@kripken kripken kripken approved these changes

@tlively tlively tlively approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rintf/lrintf slowness in wasm backend
4 participants
@quantum5 @sbc100 @kripken @tlively