Use passive segments instead of mem files #8974
Conversation
kripken
left a comment
kripken
left a comment
There was a problem hiding this comment.
Is passive segments part of bulk memory, or pthreads, or a separate feature? (Can we always assume it is present when using pthreads?)
|
Passive segments is a bulk memory feature, but after https://reviews.llvm.org/D64586 rolls we will be able to assume that all pthreads builds have bulk memory enabled. We will also get a nice linker error about missing features if this is somehow screwed up. |
|
I also don't think this is going to work with WASM2JS+pthreads until wasm2js explicitly supports translating passive segments. |
kripken
left a comment
kripken
left a comment
There was a problem hiding this comment.
Thanks for the explanation, lgtm with those comments fixed.
|
@kripken Does the Firefox update LGTY? |
ChangeLog.md
Outdated
|
|
||
| - Add support for [address sanitizer](https://clang.llvm.org/docs/AddressSanitizer.html). (#8884) | ||
| - Currently, only supports one thread without dynamic linking. | ||
| - LLVM backend pthread builds no longer use external memory initialization files |
There was a problem hiding this comment.
please add and depend on passive segments or such
|
Test failures look real now. |
|
Yep 😭 will investigate tomorrow |
|
@kripken failing tests are test_clientside_vertex_arrays_es3 on fastcomp and test_glgears_deriv on upstream. I'll rerun those particular jobs, but would you say this is safe to land? |
| # we will include the mem init data in the wasm, when we don't need the | ||
| # mem init file to be loadable by itself | ||
| shared.Settings.MEM_INIT_IN_WASM = not shared.Settings.USE_PTHREADS | ||
| shared.Settings.MEM_INIT_IN_WASM = True if shared.Settings.WASM_BACKEND else not shared.Settings.USE_PTHREADS |
There was a problem hiding this comment.
This is still confusing. I guess it will get simpler once fastcomp is gone, but why should options.memory_init_file not be the inverse of shared.Settings.MEM_INIT_IN_WASM?
There was a problem hiding this comment.
This is a great question and I don’t know the answer. @kripken did tell me it was complicated, though.
There was a problem hiding this comment.
I tried to simplify this but ran into fastcomp issues with how those are used (in bad ways). it seems more efficient to defer a cleanup til after fastcomp is removed.
There was a problem hiding this comment.
SGTM. This logic was confusing when I first added wasm backend support originally and I made it even worse then.
4 participants
For pthread builds.