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.

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

Enable bulk-memory by default #22873

Merged
merged 37 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
96bf43a
toward always using bulkmem and lowering
dschuff Oct 30, 2024
d38642b
remove libbulkmem, fix for memory64, various fixes
dschuff Nov 6, 2024
40e4fec
more fixes
dschuff Nov 6, 2024
ffc87f7
add lowering pass, fix some tests
dschuff Nov 6, 2024
8aa1812
simplify return
dschuff Nov 6, 2024
c099418
require bulk memory
dschuff Nov 8, 2024
ec3483a
fix warnings
dschuff Nov 8, 2024
48c1f06
Merge branch 'main' into always-bulkmem
dschuff Nov 15, 2024
6a82b9b
Merge branch 'main' into always-bulkmem
dschuff Nov 15, 2024
aeabee7
Merge branch 'main' into always-bulkmem
dschuff Nov 21, 2024
51ffac7
Update safari version, update pass name, fixes
dschuff Nov 22, 2024
76eee63
add node versions
dschuff Nov 22, 2024
cd2d0af
rebaseline codesize tests
dschuff Nov 25, 2024
747201c
review suggestions
dschuff Nov 26, 2024
b4aba23
Merge branch 'main' into always-bulkmem
dschuff Nov 26, 2024
4714852
rebaseline code size
dschuff Nov 26, 2024
e140c7d
Merge branch 'main' into always-bulkmem
dschuff Nov 26, 2024
9da8ecc
add suggested comment
dschuff Nov 27, 2024
3ab9bab
Merge branch 'main' into always-bulkmem
dschuff Nov 27, 2024
ca1e0da
Merge branch 'main' into always-bulkmem
dschuff Nov 27, 2024
b0e13ca
remove accidental commit
dschuff Nov 27, 2024
e4d0e82
rebaseline
dschuff Nov 27, 2024
17b47b5
Merge branch 'main' into always-bulkmem
dschuff Dec 19, 2024
c668e90
review comments
dschuff Dec 19, 2024
7fc041b
tweak test_wasm_features
dschuff Dec 19, 2024
47a4908
Use MIN_NODE_VERSION in test-node-compat
dschuff Dec 19, 2024
8523375
Merge branch 'main' into always-bulkmem
dschuff Dec 19, 2024
22600c1
remove extra blank lines
dschuff Dec 20, 2024
c20eee9
enable printing all features in test_dwarf
dschuff Dec 20, 2024
216ea09
Add node version required for threads and mutable globals
dschuff Dec 20, 2024
679927b
Merge branch 'main' into always-bulkmem
dschuff Dec 20, 2024
8399076
move node environment from job to step
dschuff Dec 20, 2024
3caad8d
review suggestion
dschuff Dec 20, 2024
7bbbb67
Make CI param for extra cflags, so node version can be injected only …
dschuff Dec 20, 2024
c3c901b
actually fix the node config by moving the env var to the intended place
dschuff Dec 20, 2024
4a35ba2
Merge branch 'main' into always-bulkmem
dschuff Jan 4, 2025
5f62d73
Merge branch 'main' into always-bulkmem
dschuff Jan 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,10 @@ commands:
description: "Name of given test suite"
type: string
default: ""
extra-cflags:
description: "Extra EMCC_CFLAGS"
type: string
default: ""
steps:
- when:
# We only set EMTEST_RETRY_FLAKY on pull requests. When we run
Expand All @@ -250,6 +254,8 @@ commands:
- set-retry-flaky-tests
- run:
name: run tests (<< parameters.title >>)
environment:
EMCC_CFLAGS: << parameters.extra-cflags >>
command: |
env
./test/runner << parameters.test_targets >>
Expand Down Expand Up @@ -765,6 +771,7 @@ jobs:
node_version: "10.19.0"
- run-tests:
title: "node (oldest / 10.19.0)"
extra-cflags: "-sMIN_NODE_VERSION=101900"
test_targets: "
other.test_gen_struct_info
other.test_native_call_before_init
Expand Down
1 change: 0 additions & 1 deletion embuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@

# Minimal subset of targets used by CI systems to build enough to be useful
MINIMAL_TASKS = [
'libbulkmemory',
'libcompiler_rt',
'libcompiler_rt-wasm-sjlj',
'libcompiler_rt-ww',
Expand Down
8 changes: 0 additions & 8 deletions emcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -385,14 +385,6 @@ def get_clang_flags(user_args):
if '-mbulk-memory' not in user_args:
flags.append('-mbulk-memory')

# In emscripten we currently disable bulk memory by default.
# This should be removed/updated when we als update the default browser targets.
if '-mbulk-memory' not in user_args and '-mno-bulk-memory' not in user_args:
# Bulk memory may be enabled via threads or directly via -s.
if not settings.BULK_MEMORY:
flags.append('-mno-bulk-memory')
flags.append('-mno-bulk-memory-opt')

if settings.RELOCATABLE and '-fPIC' not in user_args:
flags.append('-fPIC')

Expand Down
32 changes: 0 additions & 32 deletions src/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -405,38 +405,6 @@ addToLibrary({
// the initial values of the environment accessible by getenv.
$ENV: {},

// In -Oz builds, we replace memcpy() altogether with a non-unrolled wasm
// variant, so we should never emit _emscripten_memcpy_js() in the build.
// In STANDALONE_WASM we avoid the _emscripten_memcpy_js dependency so keep
// the wasm file standalone.
// In BULK_MEMORY mode we include native versions of these functions based
// on memory.fill and memory.copy.
// In MAIN_MODULE=1 or EMCC_FORCE_STDLIBS mode all of libc is force included
// so we cannot override parts of it, and therefore cannot use libc_optz.
#if (SHRINK_LEVEL < 2 || LINKABLE || process.env.EMCC_FORCE_STDLIBS) && !STANDALONE_WASM && !BULK_MEMORY

#if MIN_CHROME_VERSION < 45 || MIN_FIREFOX_VERSION < 34 || MIN_SAFARI_VERSION < 100101
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/copyWithin lists browsers that support TypedArray.prototype.copyWithin, but it
// has outdated information for Safari, saying it would not support it.
// https://github.com/WebKit/webkit/commit/24a800eea4d82d6d595cdfec69d0f68e733b5c52#diff-c484911d8df319ba75fce0d8e7296333R1 suggests support was added on Aug 28, 2015.
// Manual testing suggests:
// Safari/601.1 Version/9.0 on iPhone 4s with iOS 9.3.6 (released September 30, 2015) does not support copyWithin.
// but the following systems do:
// AppleWebKit/602.2.14 Safari/602.1 Version/10.0 Mobile/14B100 iPhone OS 10_1_1 on iPhone 5s with iOS 10.1.1 (released October 31, 2016)
// AppleWebKit/603.3.8 Safari/602.1 Version/10.0 on iPhone 5 with iOS 10.3.4 (released July 22, 2019)
// AppleWebKit/605.1.15 iPhone OS 12_3_1 Version/12.1.1 Safari/604.1 on iPhone SE with iOS 12.3.1
// AppleWebKit/605.1.15 Safari/604.1 Version/13.0.4 iPhone OS 13_3 on iPhone 6s with iOS 13.3
// AppleWebKit/605.1.15 Version/13.0.3 Intel Mac OS X 10_15_1 on Safari 13.0.3 (15608.3.10.1.4) on macOS Catalina 10.15.1
// Hence the support status of .copyWithin() for Safari version range [10.0.0, 10.1.0] is unknown.
_emscripten_memcpy_js: `= Uint8Array.prototype.copyWithin
? (dest, src, num) => HEAPU8.copyWithin(dest, src, src + num)
: (dest, src, num) => HEAPU8.set(HEAPU8.subarray(src, src+num), dest)`,
#else
_emscripten_memcpy_js: (dest, src, num) => HEAPU8.copyWithin(dest, src, src + num),
#endif

#endif

#if !STANDALONE_WASM
// ==========================================================================
// assert.h
Expand Down
1 change: 0 additions & 1 deletion src/library_sigs.js
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,6 @@ sigs = {
_emscripten_get_progname__sig: 'vpi',
_emscripten_init_main_thread_js__sig: 'vp',
_emscripten_lookup_name__sig: 'ip',
_emscripten_memcpy_js__sig: 'vppp',
_emscripten_notify_mailbox_postmessage__sig: 'vpp',
_emscripten_push_main_loop_blocker__sig: 'vppp',
_emscripten_push_uncounted_main_loop_blocker__sig: 'vppp',
Expand Down
6 changes: 0 additions & 6 deletions system/lib/libc/emscripten_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,6 @@ _Noreturn void _abort_js(void);

void setThrew(uintptr_t threw, int value);

// An external JS implementation that is efficient for very large copies, using
// HEAPU8.set()
void _emscripten_memcpy_js(void* __restrict__ dest,
const void* __restrict__ src,
size_t n) EM_IMPORT(_emscripten_memcpy_js);

void* _emscripten_memcpy_bulkmem(void* __restrict__ dest,
const void* __restrict__ src,
size_t n);
Expand Down
14 changes: 6 additions & 8 deletions system/lib/libc/emscripten_memcpy.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,20 @@
#include "emscripten_internal.h"

// Use the simple/naive version of memcpy when building with asan
#if defined(EMSCRIPTEN_OPTIMIZE_FOR_OZ) || __has_feature(address_sanitizer)
#if __has_feature(address_sanitizer)

static void *__memcpy(void *dest, const void *src, size_t n) {
unsigned char *d = (unsigned char *)dest;
const unsigned char *s = (const unsigned char *)src;
#pragma clang loop unroll(disable)
while(n--) *d++ = *s++;
return dest;
}

#elif defined(__wasm_bulk_memory__)
#elif defined(EMSCRIPTEN_OPTIMIZE_FOR_OZ)

static void *__memcpy(void *restrict dest, const void *restrict src, size_t n) {
return n ? _emscripten_memcpy_bulkmem(dest, src, n) : dest;
// TODO: Ensure this is inlined with Binaryen or inline asm
return _emscripten_memcpy_bulkmem(dest, src, n);
}

#else
Expand All @@ -35,12 +35,10 @@ static void *__memcpy(void *restrict dest, const void *restrict src, size_t n) {
unsigned char *block_aligned_d_end;
unsigned char *d_end;

#if !defined(EMSCRIPTEN_STANDALONE_WASM)
if (n >= 512) {
_emscripten_memcpy_js(dest, src, n);
return dest;
// TODO: Re-investigate the size threshold to enable this
return _emscripten_memcpy_bulkmem(dest, src, n);
}
#endif

d_end = d + n;
if ((((uintptr_t)d) & 3) == (((uintptr_t)s) & 3)) {
Expand Down
13 changes: 10 additions & 3 deletions system/lib/libc/emscripten_memcpy_bulkmem.S
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,17 @@
.globl _emscripten_memcpy_bulkmem
_emscripten_memcpy_bulkmem:
.functype _emscripten_memcpy_bulkmem (PTR, PTR, PTR) -> (PTR)
local.get 0
local.get 1
local.get 2
memory.copy 0, 0
#ifdef __wasm64__
i32.wrap_i64
#endif
// memory.copy traps on OOB zero-length copies, but memcpy must not.
if
local.get 0
local.get 1
local.get 2
dschuff marked this conversation as resolved.
Show resolved Hide resolved
memory.copy 0, 0
end_if
sbc100 marked this conversation as resolved.
Show resolved Hide resolved
local.get 0
end_function

Expand Down
11 changes: 1 addition & 10 deletions system/lib/libc/emscripten_memset.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,7 @@ __attribute__((no_sanitize("address"))) void *__memset(void *str, int c, size_t
__attribute__((__weak__)) void *__musl_memset(void *str, int c, size_t n);
__attribute__((__weak__)) void *__memset(void *str, int c, size_t n);

#ifdef EMSCRIPTEN_OPTIMIZE_FOR_OZ

void *__memset(void *str, int c, size_t n) {
unsigned char *s = (unsigned char *)str;
#pragma clang loop unroll(disable)
while(n--) *s++ = c;
return str;
}

#elif defined(__wasm_bulk_memory__)
#if defined(EMSCRIPTEN_OPTIMIZE_FOR_OZ)

void *__memset(void *str, int c, size_t n) {
return _emscripten_memset_bulkmem(str, c, n);
Expand Down
13 changes: 10 additions & 3 deletions system/lib/libc/emscripten_memset_bulkmem.S
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,17 @@
.globl _emscripten_memset_bulkmem
_emscripten_memset_bulkmem:
.functype _emscripten_memset_bulkmem (PTR, i32, PTR) -> (PTR)
local.get 0
local.get 1
local.get 2
memory.fill 0
#ifdef __wasm64__
i32.wrap_i64
#endif
// memory.fill traps on OOB zero-length sets, but memset must not.
if
local.get 0
local.get 1
local.get 2
memory.fill 0
end_if
local.get 0
end_function

Expand Down
2 changes: 1 addition & 1 deletion test/browser/test_small_js_flags.js.size
Original file line number Diff line number Diff line change
@@ -1 +1 @@
4374
4341
8 changes: 4 additions & 4 deletions test/code_size/embind_hello_wasm.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
"a.html.gz": 380,
"a.js": 9879,
"a.js.gz": 4288,
"a.wasm": 7479,
"a.wasm.gz": 3441,
"total": 17910,
"total_gz": 8109
"a.wasm": 7348,
"a.wasm.gz": 3375,
"total": 17779,
"total_gz": 8043
}
8 changes: 4 additions & 4 deletions test/code_size/embind_val_wasm.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
"a.html.gz": 380,
"a.js": 7153,
"a.js.gz": 3042,
"a.wasm": 9233,
"a.wasm.gz": 4742,
"total": 16938,
"total_gz": 8164
"a.wasm": 9119,
"a.wasm.gz": 4701,
"total": 16824,
"total_gz": 8123
}
8 changes: 4 additions & 4 deletions test/code_size/hello_webgl2_wasm.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
"a.html.gz": 328,
"a.js": 4538,
"a.js.gz": 2320,
"a.wasm": 10206,
"a.wasm.gz": 6663,
"total": 15198,
"total_gz": 9311
"a.wasm": 10148,
"a.wasm.gz": 6641,
"total": 15140,
"total_gz": 9289
}
4 changes: 2 additions & 2 deletions test/code_size/hello_webgl2_wasm2js.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"a.html": 346,
"a.html.gz": 262,
"a.js": 22202,
"a.js.gz": 11604,
"a.js.gz": 11636,
"total": 22548,
"total_gz": 11866
"total_gz": 11898
}
8 changes: 4 additions & 4 deletions test/code_size/hello_webgl_wasm.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
"a.html.gz": 328,
"a.js": 4076,
"a.js.gz": 2163,
"a.wasm": 10206,
"a.wasm.gz": 6663,
"total": 14736,
"total_gz": 9154
"a.wasm": 10148,
"a.wasm.gz": 6641,
"total": 14678,
"total_gz": 9132
}
4 changes: 2 additions & 2 deletions test/code_size/hello_webgl_wasm2js.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"a.html": 346,
"a.html.gz": 262,
"a.js": 21728,
"a.js.gz": 11435,
"a.js.gz": 11470,
"total": 22074,
"total_gz": 11697
"total_gz": 11732
}
4 changes: 2 additions & 2 deletions test/code_size/hello_world_wasm.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
"a.html.gz": 380,
"a.js": 283,
"a.js.gz": 244,
"a.wasm": 103,
"a.wasm": 106,
"a.wasm.gz": 113,
"total": 938,
"total": 941,
"total_gz": 737
}
1 change: 1 addition & 0 deletions test/code_size/hello_world_wasm2js.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ function e(b) {
for (var v, p = 0, t = a, w = f.length, y = a + (3 * w >> 2) - ("=" == f[w - 2]) - ("=" == f[w - 1]); p < w; p += 4) a = m[f.charCodeAt(p + 1)],
v = m[f.charCodeAt(p + 2)], c[t++] = m[f.charCodeAt(p)] << 2 | a >> 4, t < y && (c[t++] = a << 4 | v >> 2),
t < y && (c[t++] = v << 6 | m[f.charCodeAt(p + 3)]);
return c;
Copy link
Member

Choose a reason for hiding this comment

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

Any idea what this is? Looks odd.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, that does look odd. Hard to know why without knowing which function this actually is.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's plausible that this is some function that in some way uses bulk memory.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, based on the charCodeAt and << 6 etc., this looks like base64Decode:

function base64Decode(b64) {
#if ENVIRONMENT_MAY_BE_NODE
if (typeof ENVIRONMENT_IS_NODE != 'undefined' && ENVIRONMENT_IS_NODE) {
var buf = Buffer.from(b64, 'base64');
return new Uint8Array(buf.buffer, buf.byteOffset, buf.byteLength);
}
#endif
#if ASSERTIONS
assert(b64.length % 4 == 0);
#endif
var b1, b2, i = 0, j = 0, bLength = b64.length, output = new Uint8Array((bLength*3>>2) - (b64[bLength-2] == '=') - (b64[bLength-1] == '='));
for (; i < bLength; i += 4, j += 3) {
b1 = base64ReverseLookup[b64.charCodeAt(i+1)];
b2 = base64ReverseLookup[b64.charCodeAt(i+2)];
output[j] = base64ReverseLookup[b64.charCodeAt(i)] << 2 | b1 >> 4;
output[j+1] = b1 << 4 | b2 >> 2;
output[j+2] = b2 << 6 | base64ReverseLookup[b64.charCodeAt(i+3)];
}
return output;
}

That doesn't use bulk memory AFAICT. Strange that it changed.

Copy link
Member

Choose a reason for hiding this comment

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

Anyhow, it must be a weird closure quirk, as x() is called once and the return value isn't used.

}
for (var q, m = new Uint8Array(123), n = 25; 0 <= n; --n) m[48 + n] = 52 + n, m[65 + n] = n,
m[97 + n] = 26 + n;
Expand Down
8 changes: 4 additions & 4 deletions test/code_size/hello_world_wasm2js.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"a.html": 323,
"a.html.gz": 253,
"a.js": 1052,
"a.js.gz": 630,
"total": 1375,
"total_gz": 883
"a.js": 1061,
"a.js.gz": 629,
"total": 1384,
"total_gz": 882
}
8 changes: 4 additions & 4 deletions test/code_size/math_wasm.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
"a.html.gz": 380,
"a.js": 110,
"a.js.gz": 125,
"a.wasm": 2695,
"a.wasm.gz": 1664,
"total": 3357,
"total_gz": 2169
"a.wasm": 2698,
"a.wasm.gz": 1666,
"total": 3360,
"total_gz": 2171
}
8 changes: 4 additions & 4 deletions test/code_size/random_printf_wasm.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"a.html": 12517,
"a.html.gz": 6871,
"total": 12517,
"total_gz": 6871
"a.html": 12449,
"a.html.gz": 6811,
"total": 12449,
"total_gz": 6811
}
8 changes: 4 additions & 4 deletions test/code_size/random_printf_wasm2js.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"a.html": 17195,
"a.html.gz": 7480,
"total": 17195,
"total_gz": 7480
"a.html": 17233,
"a.html.gz": 7535,
"total": 17233,
"total_gz": 7535
}
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_ctors1.gzsize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
8365
8346
1 change: 0 additions & 1 deletion test/other/codesize/test_codesize_cxx_ctors1.imports
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
env.__cxa_throw
env._abort_js
env._emscripten_memcpy_js
env._tzset_js
env.emscripten_resize_heap
wasi_snapshot_preview1.environ_get
Expand Down
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_ctors1.jssize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
20373
20320
1 change: 0 additions & 1 deletion test/other/codesize/test_codesize_cxx_ctors1.sent
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
__cxa_throw
_abort_js
_emscripten_memcpy_js
_tzset_js
emscripten_resize_heap
environ_get
Expand Down
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_ctors1.size
Original file line number Diff line number Diff line change
@@ -1 +1 @@
129570
129218
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_ctors2.gzsize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
8347
8328
1 change: 0 additions & 1 deletion test/other/codesize/test_codesize_cxx_ctors2.imports
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
env._abort_js
env._emscripten_memcpy_js
env._tzset_js
env.emscripten_resize_heap
wasi_snapshot_preview1.fd_close
Expand Down
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_ctors2.jssize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
20341
20288
1 change: 0 additions & 1 deletion test/other/codesize/test_codesize_cxx_ctors2.sent
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
__cxa_throw
_abort_js
_emscripten_memcpy_js
_tzset_js
emscripten_resize_heap
environ_get
Expand Down
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_ctors2.size
Original file line number Diff line number Diff line change
@@ -1 +1 @@
129019
128630
Loading
Loading