emscripten
emscripten copied to clipboard
EMSCRIPTEN_VERSION macro in JS libraries
When working on wasm_webgpu bindings library, I realized that after #19097 which landed for Emscripten 3.1.35, I needed to add these directives
https://github.com/juj/wasm_webgpu/commit/a1cc82dc445512f3acc2bf044a699531b8e4d944
which broke the JS library from working on Emscripten < 3.1.35, so in order to make the JS library work for different versions, I had to a backwards compatibility item to the library, that looks like this:
https://github.com/juj/wasm_webgpu/commit/5460a0d8c71343b5a1984da11f46a606cec3c9e3
Maintaining that kind of backwards compatibility item is brittle and difficult, since if the upstream code changes in the future, the change won't appear for users who use the wasm_webgpu library.
So instead, what I would like to do is write something like
#if EMSCRIPTEN_VERSION >= 3.1.35
wgpu_object_get_label__deps: ['$stringToUTF8'],
#endif
to make the needed deps appear only for old Emscripten versions, or simply
#if EMSCRIPTEN_VERSION < 3.1.35
#error "lib_webgpu.js requires Emscripten 3.1.35 or newer"
#endif
to require developers to update to a newer version.
However looks like we don't have a construct that would allow implementing a EMSCRIPTEN_VERSION < 3.1.35 yet - seems like we should add one.
Any preferences on how the syntax would like to look?
Use the same kind of compacted version number like the MIN_xxx_VERSION fields?
#if EMSCRIPTEN_VERSION < 30135
or use the same names as C/C++ code gets as preprocessor:
#if __EMSCRIPTEN_major__*10000 + __EMSCRIPTEN_minor__*100 + __EMSCRIPTEN_tiny__ < 30135
or use function syntax?
#if EMSCRIPTEN_VERSION_LESS_THAN(3,1,35)
#if EMSCRIPTEN_VERSION_LESS_OR_EQUAL(3,1,35)
#if EMSCRIPTEN_VERSION_GREATER_THAN(3,1,35)
#if EMSCRIPTEN_VERSION_GREATER_OR_EQUAL(3,1,35)
or something else?
(I think I'd prefer the very first one)
It looks like EMSCRIPTEN_VERSION is already available to JS libraries, but in string form:
https://github.com/emscripten-core/emscripten/blob/5c27e79dd0a9c4e27ef2326841698cdd4f6b5784/src/settings_internal.js#L108
How one should do comparisons based on that string is still not clear. There was a recent discussion on this where somebody found way to make it work: https://github.com/emscripten-core/emscripten/discussions/19168#discussioncomment-5618047
The good thing about a solution that uses a local JS helper function is that it will work with all version of emscripten and doesn't require an upstream change.
Oh right, thanks! let me think about how to do that.. looks quite gnarly though :/
https://github.com/emscripten-core/emscripten/pull/21555 introduced a bidirectional breaking change that Emscripten 3.1.56 doesn't allow one to declare
foo__deps: ['$stackSave', '$stackAlloc', '$stackRestore']
but Emscripten 3.1.57 requires users must do so.
So I went and wrote
wgpuReportErrorCodeAndMessage__deps: ['$lengthBytesUTF8', '$stringToUTF8'
#if EMSCRIPTEN_VERSION.split('.').map(parseInt) >= [3,1,57]
, '$stackSave', '$stackAlloc', '$stackRestore'
#endif
],
based on the earlier conversation: https://github.com/emscripten-core/emscripten/discussions/19168#discussioncomment-10555971
However, now I realize that the whole construct is garbage.. First, parseInt doesn't work here (could be fixed by doing parseFloat).
Second, I had just assumed when the code snippet was presented, that the presented code had been tested so that JS arrays would have a lexicographic comparison ordering (which I thought to be neat), but it actually does not.. instead comparing e.g. [3,1,20] < [3,1,5] just coerces both sides back to strings, e.g. "3,1,20" < "3,1,5", which is what EMSCRIPTEN_VERSION started with before the whole .split() operation. Just garbage.
To get something that actually works, ended up with this: https://github.com/juj/wasm_webgpu/commit/622f5084d2f223989000993a22dd9b6329591903 .. But that is horrible as well.
So I think we really need something here.
Adding a new macro to parseTools.js for version comparison seems reasonable.
However, I also think it would be great if external projects could provide their own macros more easily, so that this could all be done out-of-tree.
Currently we have a mechanism for adding new macros in library JS files that is little ugly and has some limitations:
{{{
globalThis.newMacro = () = { .. }
}}}
A major drawback (IIUC) of doing this is that the macros are not available during preprocessing since macros expansion happens after preprocessing. So to use these macros in a preprocessor directrive one would need define them in a separate file that is loaded first. e.g. --js-library=my_macros.js --js-library=my_library.js Here the macros defined in my_macros.js would be available in my_library.js. Perhaps we can address this somehow?
One idea would be to allow my_library.js to somehow #include "my_macros.js" .. however that isn't currently how the #include directive works. Currently #include files are placed directly into the output file and don't have macros expanded, so we would need somekind of new #include mechanism.
I opened https://github.com/emscripten-core/emscripten/issues/22510, but I'm also fine with adding a new builtin macro if you don't want to block on that.