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

wasm2js: Stop emitting nan and infinity #5391

Merged
merged 1 commit into from
Jan 4, 2023
Merged

wasm2js: Stop emitting nan and infinity #5391

merged 1 commit into from
Jan 4, 2023

Conversation

willcohen
Copy link
Contributor

As noted in #4739, legacy language emitting nan and infinity exists, with the observation that it can be removed once asm.js is no longer used and global NaN is available.

This commit removes that asm.js-specific code accordingly.

This is a replacement for the draft #4770 proposed earlier.

@willcohen
Copy link
Contributor Author

willcohen commented Jan 4, 2023 •
edited

Working on adjusting to match CI.

@kripken
Copy link
Member

kripken commented Jan 4, 2023

Code lgtm, but please check the CI error.

kripken
kripken reviewed Jan 4, 2023
View reviewed changes
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Why was test/spec/expected-output/func_ptrs.wast.log deleted?

@willcohen
Copy link
Contributor Author

Once I got the rest of it passing (I think I've finally got the local test suite lining up with CI, so I'll try to quiet the noisy force pushes now), I was going to ask you to review. With the .log file present, the test suite was failing on func_ptrs. There's still a few nans and infinities I need to track down...

.. func_ptrs.wast
executing:  /Users/wcohen/programming/c/binaryen/bin/wasm2js split.wast -all --disable-exception-handling
executing:  /Users/wcohen/programming/c/binaryen/bin/wasm2js split.wast -all --disable-exception-handling --allow-asserts
executing:  /Users/wcohen/programming/c/binaryen/bin/wasm2js split.wast -all --disable-exception-handling --allow-asserts
[PassRunner] running passes
[PassRunner]   running pass: autodrop...                          5.417e-06 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: legalize-js-interface...             1.1083e-05 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: remove-non-js-ops...                 1.2417e-05 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: flatten...                           7.083e-06 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: i64-to-i32-lowering...               2.8583e-05 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: alignment-lowering...                2.417e-06 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: flatten...                           9.292e-06 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: simplify-locals-notee-nostructure... 3.6583e-05 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: reorder-locals...                    1.2125e-05 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: vacuum...                            2.0583e-05 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: remove-unused-module-elements...     1.0459e-05 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: dce...                               4.167e-06 seconds.
[PassRunner]   (validating)
[PassRunner] passes took 0.000160209 seconds.
[PassRunner] (final validation)
executing:  /Users/wcohen/programming/c/binaryen/bin/wasm2js split.wast -all --disable-exception-handling
executing:  /Users/wcohen/programming/c/binaryen/bin/wasm2js split.wast -all --disable-exception-handling --allow-asserts
skipping ( assert_trap ( invoke callt ( i32.const 7 ) ) undefined element )
skipping ( assert_trap ( invoke callt ( i32.const 100 ) ) undefined element )
skipping ( assert_trap ( invoke callt ( i32.const -1 ) ) undefined element )
skipping ( assert_trap ( invoke callu ( i32.const 7 ) ) undefined element )
skipping ( assert_trap ( invoke callu ( i32.const 100 ) ) undefined element )
skipping ( assert_trap ( invoke callu ( i32.const -1 ) ) undefined element )
executing:  /Users/wcohen/programming/c/binaryen/bin/wasm2js split.wast -all --disable-exception-handling --allow-asserts
[PassRunner] running passes
[PassRunner]   running pass: autodrop...                          5.542e-06 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: legalize-js-interface...             7.542e-06 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: remove-non-js-ops...                 1.4375e-05 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: flatten...                           6.042e-06 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: i64-to-i32-lowering...               3.075e-05 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: alignment-lowering...                2.916e-06 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: flatten...                           7.333e-06 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: simplify-locals-notee-nostructure... 2.8542e-05 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: reorder-locals...                    7.875e-06 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: vacuum...                            2.1292e-05 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: remove-unused-module-elements...     1.3375e-05 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: dce...                               4.167e-06 seconds.
[PassRunner]   (validating)
[PassRunner] passes took 0.000149751 seconds.
[PassRunner] (final validation)
skipping ( assert_trap ( invoke callt ( i32.const 7 ) ) undefined element )
skipping ( assert_trap ( invoke callt ( i32.const 100 ) ) undefined element )
skipping ( assert_trap ( invoke callt ( i32.const -1 ) ) undefined element )
skipping ( assert_trap ( invoke callu ( i32.const 7 ) ) undefined element )
skipping ( assert_trap ( invoke callu ( i32.const 100 ) ) undefined element )
skipping ( assert_trap ( invoke callu ( i32.const -1 ) ) undefined element )
executing:  /Users/wcohen/programming/c/binaryen/bin/wasm2js split.wast -all --disable-exception-handling
executing:  /Users/wcohen/programming/c/binaryen/bin/wasm2js split.wast -all --disable-exception-handling --allow-asserts
executing:  /Users/wcohen/programming/c/binaryen/bin/wasm2js split.wast -all --disable-exception-handling --allow-asserts
[PassRunner] running passes
[PassRunner]   running pass: autodrop...                          3.917e-06 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: legalize-js-interface...             6.333e-06 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: remove-non-js-ops...                 7.125e-06 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: flatten...                           5.542e-06 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: i64-to-i32-lowering...               2.7292e-05 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: alignment-lowering...                2.25e-06 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: flatten...                           4.958e-06 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: simplify-locals-notee-nostructure... 2.8584e-05 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: reorder-locals...                    8.667e-06 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: vacuum...                            1.2709e-05 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: remove-unused-module-elements...     1.7084e-05 seconds.
[PassRunner]   (validating)
[PassRunner]   running pass: dce...                               2.792e-06 seconds.
[PassRunner]   (validating)
[PassRunner] passes took 0.000127253 seconds.
[PassRunner] (final validation)
incorrect output, diff:

--- expected
+++ actual
@@ -1,2 +1 @@
-83 : i32


Traceback (most recent call last):
  File "/Users/wcohen/programming/c/binaryen/./check.py", line 416, in <module>
    sys.exit(main())
  File "/Users/wcohen/programming/c/binaryen/./check.py", line 399, in main
    TEST_SUITES[test]()
  File "/Users/wcohen/programming/c/binaryen/scripts/test/wasm2js.py", line 146, in test_wasm2js
    test_wasm2js_output()
  File "/Users/wcohen/programming/c/binaryen/scripts/test/wasm2js.py", line 118, in test_wasm2js_output
    shared.fail_if_not_identical(all_out, expected_out)
  File "/Users/wcohen/programming/c/binaryen/scripts/test/shared.py", line 359, in fail_if_not_identical
    fail(actual, expected, fromfile=fromfile)
  File "/Users/wcohen/programming/c/binaryen/scripts/test/shared.py", line 354, in fail
    fail_with_error("incorrect output, diff:\n\n%s" % diff_str)
  File "/Users/wcohen/programming/c/binaryen/scripts/test/shared.py", line 342, in fail_with_error
    raise Exception(msg)
Exception: incorrect output, diff:

--- expected
+++ actual
@@ -1,2 +1 @@
-83 : i32

@willcohen willcohen marked this pull request as draft January 4, 2023 21:07
@willcohen
wasm2js: Stop emitting nan and infinity
17d4a23 
As noted in WebAssembly#4739, legacy language emitting nan and infinity
exists, with the observation that it can be removed once asm.js
is no longer used and global NaN is available.

This commit removes that asm.js-specific code accordingly.
@willcohen
Copy link
Contributor Author

Test suite builds locally. The error with the .log appeared to be transient, its deletion has been reverted.

@willcohen willcohen marked this pull request as ready for review January 4, 2023 22:31
@willcohen
Copy link
Contributor Author

Sorry for the noise -- CI now passes.

kripken
kripken approved these changes Jan 4, 2023
View reviewed changes
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Great!

@kripken kripken merged commit f92350d into WebAssembly:main Jan 4, 2023
@willcohen willcohen deleted the nan branch January 4, 2023 23:03
@willcohen
Copy link
Contributor Author

Thank you!

@willcohen willcohen mentioned this pull request Jan 5, 2023
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kripken kripken kripken 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.

None yet
2 participants
@willcohen @kripken