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

Preserve signalling bit in NaNs #87

Merged
merged 2 commits into from
Apr 25, 2018

Conversation

eira-fransham
Copy link
Contributor

@eira-fransham eira-fransham commented Apr 24, 2018

This commit prevents using hardware floats for any operations that WASM specifies that we should preserve the signalling bit for. As far as I can tell, the only operations that require preservation of the signalling bit are conversion to/from int and binary negation. On x86_64 these are preserved by default since it does not distinguish between floats and integers in its calling convention, but in x87 floats are passed to and from functions on the x87 FP stack, which quietens NaNs as a result of its conversions to/from the internal 80-bit FP representation in x87.

Closes #43

Copy link
Collaborator

@pepyakin pepyakin left a comment

Choose a reason for hiding this comment

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

Seems good to me overall.

.gitmodules Outdated
@@ -1,3 +1,6 @@
[submodule "tests/spec/testsuite"]
path = tests/spec/testsuite
url = https://github.com/WebAssembly/testsuite.git
[submodule "nan-preserving-float"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this is still needed? AFAICS it's already added into the repo?

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's added as a submodule

EDIT: My mistake, I staged it wrong

Copy link
Collaborator

@pepyakin pepyakin Apr 24, 2018

Choose a reason for hiding this comment

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

Hm, I always thought that files from the repos added via submodules are not displayed as added in a PR.

@pepyakin
Copy link
Collaborator

Ah, almost forgot about it, let's remove feature opt-in-32bit from Cargo.toml!

@pepyakin
Copy link
Collaborator

@pepyakin pepyakin merged commit 3890dd3 into wasmi-labs:master Apr 25, 2018
@pepyakin
Copy link
Collaborator

Great!

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.

None yet

2 participants