emscripten-core / emscripten

Emscripten: An LLVM-to-WebAssembly Compiler
Other
25.35k stars 3.25k forks source link

Use C/C++ bool type to implement EM_BOOL #22157

Closed sbc100 closed 2 days ago

sbc100 commented 3 days ago

This reduces the size of several structs and can result in code size savings in some cases. The reason the code size savings don't show up in trivial examples is (I believe) because this change also increases the use of HEAP8 (where previously some examples only depended on HEAP32).

This change is split of from a larger change I have planned to remove the use of EM_BOOL completely: #22155.

sbc100 commented 3 days ago

I think we should probably do a release containing this change before we land part 2 so that we can separate any possible fallout.

sbc100 commented 3 days ago

I had to update a bunch of places where we were accessing these fields (u8 vs u32 access). PTAL.

sbc100 commented 3 days ago

I used the following commands to fine and fix all locations in the JS library code that was treating these values as 32 bit:

# Find all EM_BOOL struct members
$ git grep "^\s*EM_BOOL[^(]*$" system/include/ | awk '{ print $3 }' | sed "s/;//" | sort | uniq > bools.txt
# Find all places where these members are used in HEAP32/HEAPU32 accesses
$ for bool in `cat bools.txt`; do git grep "HEAPU\?32.*$bool"; done
# Find all the places they are used in `i32`/`u32` `makeGetValue`/`makeSetValue`
$ for bool in `cat bools.txt`; do git grep "make[GS]etValue.*\.$bool\>.*32"; done