Skip to content

create Math_{min,max} variables for wasm2asm-generated JS#1476

Merged
kripken merged 1 commit into
WebAssembly:masterfrom
froydnj:fix-wasm2asm-minmax
Mar 16, 2018
Merged

create Math_{min,max} variables for wasm2asm-generated JS#1476
kripken merged 1 commit into
WebAssembly:masterfrom
froydnj:fix-wasm2asm-minmax

Conversation

@froydnj

@froydnj froydnj commented Mar 15, 2018

Copy link
Copy Markdown
Contributor

We were using Math_{min,max} in wasm2asm-generated files without
declaring said functions. This decision created problems for tests,
because Math_min (resp. max) would first be used on f32s, thus returning
f32, and then validation would fail when it was used on f64s.

The resulting changes make wasm2asm tests pass with MOZJS asm.js
validation, which moves #1443 forward.

@kripken

kripken commented Mar 15, 2018

Copy link
Copy Markdown
Member

Looks good, thanks. Just need to rebuild the js files and update the binaryen.js test (./build-js.sh, ./auto_update_tests.py) for that test failure.

@froydnj
create Math_{min,max} variables for wasm2asm-generated JS
d520b5e 
We were using Math_{min,max} in wasm2asm-generated files without
declaring said functions.  This decision created problems for tests,
because Math_min (resp. max) would first be used on f32s, thus returning
f32, and then validation would fail when it was used on f64s.

The resulting changes make wasm2asm tests pass with MOZJS asm.js
validation, which moves WebAssembly#1443 forward.
@froydnj froydnj force-pushed the fix-wasm2asm-minmax branch from fe76476 to d520b5e Compare March 16, 2018 15:43
@froydnj

froydnj commented Mar 16, 2018

Copy link
Copy Markdown
Contributor Author

Gah, always forget about the binaryen tests. Building binaryen.js worked out, but running ./auto_update_tests.py afterwards did not:

[ checking example testcases... ]

build:  gcc test/example/c-api-hello-world.c -c -o example.o -Isrc -g -L/home/froydnj/src/emsdk-portable/clang/e1.37.36_64bit/binaryen/lib -pthread
/home/froydnj/src/binaryen.git
   c-api-hello-world.c test/example/c-api-hello-world.c test/example/c-api-hello-world.txt
link:  g++ -std=c++11 example.o -lbinaryen -Wl,-rpath=/home/froydnj/src/emsdk-portable/clang/e1.37.36_64bit/binaryen/lib -Isrc -g -lasmjs -lsupport -L/home/froydnj/src/emsdk-portable/clang/e1.37.36_64bit/binaryen/lib -pthread -o bin/example
/usr/bin/ld: cannot find -lbinaryen
/usr/bin/ld: cannot find -lasmjs
/usr/bin/ld: cannot find -lsupport
collect2: error: ld returned 1 exit status
Traceback (most recent call last):
  File "./auto_update_tests.py", line 208, in <module>
    os.remove(output_file)
OSError: [Errno 2] No such file or directory: 'bin/example'

Patched things manually and ./travis-emcc-tests.sh seems to succeed now.

@kripken kripken merged commit 8fd854d into WebAssembly:master Mar 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@froydnj @kripken