MINIMAL_RUNTIME: Fix declaration of wasmExports when using embind

[sbc100]

The conditions under which wasmExports was declared global vs locally
were out of sync.

Also, move the declaration of the global wasmExports alongside the
other wasm globals.

See: #20145

[walkingeyerobot]

Thanks so much!

[juj]

To me this reads like a two byte loss everywhere. A new global variable ,g needs to appear in global scope in each build that was able to be a reused local before.

The function call to initRuntime(wasmExports) is always inlined by Closure, so passing an argument to that function did not take up code bytes.

The code size wins seem to happen in hello_webgl2_wasm.js and hello_webgl_wasm.js, but for some reason this PR does not contain the new .js files for those builds? Do you know why those were left out? (that should be autogenerated, right?)

I would prefer to keep the old code, unless there is really a code size win in some cases. But to me the code size win seems like it might be comparing against an old stale build in tree? (was there some slack allowed, so the old exemplars might be out of date?)

[juj]

Maybe the accurate fix to #20145 would be to change

#if LibraryManager.has('library_exports.js')

to

#if LibraryManager.has('library_exports.js') || EMBIND

in https://github.com/emscripten-core/emscripten/blob/3a55fe189e2d12298a0350b16fa551c5df5348a1/src/postamble_minimal.js#L103C5-L103C5

[sbc100]

Maybe the accurate fix to #20145 would be to change

#if LibraryManager.has('library_exports.js')

to

#if LibraryManager.has('library_exports.js') || EMBIND

in https://github.com/emscripten-core/emscripten/blob/3a55fe189e2d12298a0350b16fa551c5df5348a1/src/postamble_minimal.js#L103C5-L103C5

Ah yes, that looks like a good solution.

We might want to extend that so I might add a new internal settings such as USES_WASM_EXPORTS.

BTW, I wonder if we should do the same for wasmTable?

[juj]

We might want to extend that so I might add a new internal settings such as USES_WASM_EXPORTS.

Yeah, if there are more settings that need it, then that might make sense.

BTW, I wonder if we should do the same for wasmTable?

My understanding is that no reasonable application can run without wasmTable being a global, since calling a function pointer from wasm always requires accessing wasmTable. I.e. already if you want to set up a requestAnimationFrame() callback, you'll need to call wasmTable.get().

But for very small pages, that might be a possibility, so I would definitely not object against having the same mechanism for that.

[sbc100]

We might want to extend that so I might add a new internal settings such as USES_WASM_EXPORTS.

Yeah, if there are more settings that need it, then that might make sense.

BTW, I wonder if we should do the same for wasmTable?

My understanding is that no reasonable application can run without wasmTable being a global, since calling a function pointer from wasm always requires accessing wasmTable. I.e. already if you want to set up a requestAnimationFrame() callback, you'll need to call wasmTable.get().

But for very small pages, that might be a possibility, so I would definitely not object against having the same mechanism for that.

Yeah, I don't think hello world needs wasmTable.. its only for apps that use some things like dynCall or addFunction. I'll give it go.

[sbc100]

Interestingly I can't see why embind depends on wasmExports.. do you know why that might be?

[sbc100]

Closing this PR. I will open a new one to fix the EMBIND discrepency.