Skip to content

P is not a function: when compiling with -Os and ASM_JS=1 #11456

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

Closed
simon-jentzsch opened this issue Jun 21, 2020 · 3 comments
Closed

P is not a function: when compiling with -Os and ASM_JS=1 #11456

simon-jentzsch opened this issue Jun 21, 2020 · 3 comments
Assignees

Comments

@simon-jentzsch
Copy link

simon-jentzsch commented Jun 21, 2020

If I compile my project as release ( -O2) the js-file is about 1MB and all tests pass, but using -Os reduces the size to 600k, but when running the tests some function names git wrong and I get an exception `P is not a function``

the linkinig command uses those options:

emcc -DNDEBUG -Os  -s ALLOW_MEMORY_GROWTH=1  -s FINALIZE_ASM_JS=1 -s NODEJS_CATCH_REJECTION=0  -s SEPARATE_ASM=1 -s WASM=0 -s ASM_JS=1 -s EXPORT_NAME=in3w  -s FILESYSTEM=0 -s  'EXTRA_EXPORTED_RUNTIME_METHODS=["ccall", "cwrap"]' -s SINGLE_FILE=1 -s MALLOC=emmalloc @CMakeFiles/in3w.dir/objects1.rsp  -o bin/in3w.js 

as sdk I'm using latest-upstream (1.39.18 node-12.9.1-64bit)

To reproduce:

git clone --single-branch --branch develop https://github.com/slockit/in3-c.git
cd in3-c

mkdir build
cd build

emcmake cmake -DWASM=true -DASMJS=true -DWASM_EMMALLOC=true  -DCMAKE_BUILD_TYPE=MINSIZEREL .. 
make -j8 in3_wasm
make test

(building it with -DCMAKE_BUILD_TYPE=RELEASE works fine)

@kripken
Copy link
Member

kripken commented Jun 23, 2020

Thanks @simon-jentzsch ! This turned out to be a bug in binaryen, fix is in WebAssembly/binaryen#2926

What happens is when optimizing for size we do metadce. That removes getTempRet0 in this case. But wasm2js then legalizes some internal stuff that adds a new import, which then breaks. The fix removes creation of new imports at that stage, which we don't need.

kripken added a commit to WebAssembly/binaryen that referenced this issue Jun 23, 2020
That code originally used memory location 1024 to save 64 bits of
data (as that is what rust does apparently). We refactored it
manually to instead use a scratch memory helper, which is safer.
However, that 64-bit function ends up legalized, which actually
changes the interface between the module and the outside,
which is confusing and causes problems with optimizations
that can remove the getTempRet0 imports, see
emscripten-core/emscripten#11456

Instead, just use a global i64 to stash those bits. This requires
adding support for copying globals from the intrinsics module,
but otherwise seems simpler overall.
@kripken
Copy link
Member

kripken commented Jun 24, 2020

Fixed by that PR.

@kripken kripken closed this as completed Jun 24, 2020
@simon-jentzsch
Copy link
Author

thanks

kripken added a commit that referenced this issue Jul 7, 2020
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

No branches or pull requests

2 participants