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

byterun/interp: simplified implementation of LSRINT and ASRINT #1563

Merged
merged 2 commits into from Jan 14, 2018

Conversation

Projects
None yet
5 participants
@murmour
Copy link
Contributor

murmour commented Jan 7, 2018

Clearing the tag bit of the first argument is redundant.
The formulas are now the same as in cmmgen.

byterun/interp: simplified implementation of LSRINT and ASRINT
Clearing the tag bit of the first argument is redundant.
The formulas are now the same as in cmmgen.

@let-def let-def requested review from mshinwell and removed request for mshinwell Jan 8, 2018

@let-def

This comment has been minimized.

Copy link
Contributor

let-def commented Jan 8, 2018

Fwiw, it looks correct to me (... unless there is some C trap that I am not aware of).

The checks are failing because you didn't update the Changelog (is it worth an entry though? :)).

@murmour

This comment has been minimized.

Copy link
Contributor Author

murmour commented Jan 8, 2018

is it worth an entry though? :)

I don't think so.

@gasche

This comment has been minimized.

Copy link
Member

gasche commented Jan 8, 2018

@let-def: feel free to approve this PR and merge it.

@mshinwell

This comment has been minimized.

Copy link
Contributor

mshinwell commented Jan 9, 2018

I think this needs a Changes entry -- however harmless this may seem, there is still a material change to the interpreter.

@murmour

This comment has been minimized.

Copy link
Contributor Author

murmour commented Jan 10, 2018

Done.

@xavierleroy xavierleroy merged commit 150f8d2 into ocaml:trunk Jan 14, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.