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

[wasm2js] Enables table exports. #1786

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@yurydelendik
Contributor

yurydelendik commented Nov 29, 2018

Changes default behavior to create a single table and exports
an object with the length and get properties.

Old asm.js behavior is accessible via "--asmjs-tables" option.

Addresses #1781

@yurydelendik yurydelendik force-pushed the yurydelendik:table-export branch from 9f6bb4c to 44de5df Nov 30, 2018

@kripken

This comment has been minimized.

Member

kripken commented Nov 30, 2018

Is there a reason to keep around the old behavior? It may be simpler to just remove it?

@yurydelendik

This comment has been minimized.

Contributor

yurydelendik commented Nov 30, 2018

Is there a reason to keep around the old behavior? It may be simpler to just remove it?

Not sure about how friendly we needs to be with asm.js. I can remove that if needed.

@Boscop

This comment has been minimized.

Boscop commented Dec 1, 2018

I cloned your fork and ran cmake, and it generated some .vcxproj files but not for wasm2js, any idea why?

> dir

2018-12-01  15:06    <DIR>          .
2018-12-01  15:06    <DIR>          ..
2018-12-01  15:06            63,396 ALL_BUILD.vcxproj
2018-12-01  15:06               288 ALL_BUILD.vcxproj.filters
2018-12-01  15:06            74,661 asm2wasm.vcxproj
2018-12-01  15:06               733 asm2wasm.vcxproj.filters
2018-12-01  15:06            21,647 binaryen.sln
2018-12-01  15:06            74,262 binaryen.vcxproj
2018-12-01  15:06               726 binaryen.vcxproj.filters
2018-12-01  15:06            15,240 CMakeCache.txt
2018-12-01  15:06    <DIR>          CMakeFiles
2018-12-01  15:06            12,113 cmake_install.cmake
2018-12-01  15:06            11,482 INSTALL.vcxproj
2018-12-01  15:06               530 INSTALL.vcxproj.filters
2018-12-01  15:06            74,377 s2wasm.vcxproj
2018-12-01  15:06               867 s2wasm.vcxproj.filters
2018-12-01  15:06    <DIR>          src
2018-12-01  15:06            74,223 wasm-as.vcxproj
2018-12-01  15:06               592 wasm-as.vcxproj.filters
2018-12-01  15:06            74,241 wasm-dis.vcxproj
2018-12-01  15:06               593 wasm-dis.vcxproj.filters
2018-12-01  15:06            74,609 wasm-merge.vcxproj
2018-12-01  15:06               595 wasm-merge.vcxproj.filters
2018-12-01  15:06            74,573 wasm-opt.vcxproj
2018-12-01  15:06               593 wasm-opt.vcxproj.filters
2018-12-01  15:06            74,698 wasm-shell.vcxproj
2018-12-01  15:06               736 wasm-shell.vcxproj.filters
2018-12-01  15:06            46,380 ZERO_CHECK.vcxproj
2018-12-01  15:06               531 ZERO_CHECK.vcxproj.filters
              25 File(s)        772,686 bytes
               4 Dir(s)  171,630,764,032 bytes free
@Boscop

This comment has been minimized.

Boscop commented Dec 3, 2018

When I delete the build dir and start again, it still doesn't generate the wasm2js.vcxproj file that I get when building the upstream WebAssembly/binaryen.

This is the output from cmake:

D:\data\yurydelendik\binaryen\build> cmake ..
-- Building for: Visual Studio 14 2015
-- The C compiler identification is MSVC 19.0.24215.1
-- The CXX compiler identification is MSVC 19.0.24215.1
-- Check for working C compiler using: Visual Studio 14 2015
-- Check for working C compiler using: Visual Studio 14 2015 -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working CXX compiler using: Visual Studio 14 2015
-- Check for working CXX compiler using: Visual Studio 14 2015 -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- No build type selected, default to Release
-- Building with /wd4146
-- Building with /wd4267
-- Building with /wd4244
-- Building with /WX-
-- Building with /O2
-- Building with /D_CRT_SECURE_NO_WARNINGS
-- Building with /D_SCL_SECURE_NO_WARNINGS
-- Building with /UNDEBUG
-- Linking with /STACK:8388608
-- Configuring done
-- Generating done
-- Build files have been written to: D:/data/yurydelendik/binaryen/build

This is the side by side comparison of the ALL_BUILD.vcxproj files (upstream on the left):

image

Any idea why this is happening?

@kripken

This comment has been minimized.

Member

kripken commented Dec 3, 2018

Not sure about how friendly we needs to be with asm.js. I can remove that if needed.

I think we can remove the asm.js requirement myself - as more features are coming in to wasm, that would be less and less practical anyhow. It will also simplify the code.

yurydelendik added some commits Nov 29, 2018

[wasm2js] Enables table exports.
Changes default behavior to create a single table and exports
 an object with the length and get properties.

Old asm.js behavior is accessible via "--asmjs-tables" option.

@yurydelendik yurydelendik force-pushed the yurydelendik:table-export branch from 44de5df to 330a66a Dec 13, 2018

@yurydelendik

This comment has been minimized.

Contributor

yurydelendik commented Dec 18, 2018

it still doesn't generate the wasm2js.vcxproj file

@Boscop I verified on different Window machine, the wasm2js project present for me. Can you check the branch again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment