Default to -mno-bulk-memory-opt #23126
Conversation
This should fix the recent CI failures on the test-node-compat bot. Once emscripten-core#22873 lands this can, of course, be removed.
|
@sunfishcode, its maybe little annoying the users who have |
|
PTAL, to green the CI again |
| @@ -11,5 +11,6 @@ j (_wasmfs_get_preloaded_child_path) | |||
| k (_wasmfs_get_num_preloaded_files) | |||
| l (_wasmfs_get_num_preloaded_dirs) | |||
| m (_wasmfs_copy_preloaded_file_data) | |||
| n (_abort_js) | |||
| o (random_get) | |||
| n (_emscripten_memcpy_js) | |||
There was a problem hiding this comment.
Because without -mno-bulk-memory-opt (part of this change) we were defaulting to using the bulk memory instruction here (by mistake).
There was a problem hiding this comment.
The changes here originally made it in via a rebaseline that I did this morning: ea87423. I normally don't land re-baselines without review if they include symbol changes like this, but this one slipped through.
There was a problem hiding this comment.
Oh, I see, I didn't review the earlier rebaseline, so this seemed like a regression to me...
Maybe we should add some automation for the automatic rebaselining? Like a script that does it + does a check if it is "trivial". Anything aside from size changes within some range could show an error to the user and suggest investigation?
There was a problem hiding this comment.
I think that's a great idea.
Then contributors could run the script on their PRs too and we could maybe remove the slack on those tests?
This should fix the recent CI failures on the test-node-compat bot. Once emscripten-core#22873 lands this can, of course, be removed.
This should fix the recent CI failures on the test-node-compat bot. Once #22873 lands this can, of course, be removed.