IMPORTED_TABLE setting to import function pointer table?

[hoodmane]

Could we add an -sIMPORTED_TABLE setting?

Since -sRELOCATABLE is no longer the default, I've been getting problems with this code:
https://github.com/python/cpython/blob/main/Python/emscripten_trampoline.c#L35
because wasmTable and wasmMemory are undefined. Here I want to define an import via a separate wasm module so that we can use a fall back if wasm-gc is not supported. That module needs the table and the memory. I want to use it as a direct import, so I need to compile the other wasm module before the main wasm module is compiled. Thus, it needs both the table and the memory to be imported. I can force the memory to be imported with -sIMPORTED_MEMORY, but I need a separate setting for the table.

[sbc100]

I assume you are talking about an dynamically linked application since you mention "-sRELOCATABLE is no longer the default"?

I agree it could likely make sense to add IMPORTED_TABLE to match IMPORTED_MEMORY.

However, as always we try to resist adding new settings until we really need to since each new setting adds a significant testing and maintenance overhead. Maybe if we could understand you use case a little better in order to justify the new setting? i.e. maybe we can document the type of users like yourself who might need this setting.

It sounds like the use case is that the memory and/or table need to be used in other modules before the main module is instantiated, is that right? Is that the main driver here? And the reason you can't instantiate the main module first in order to get the memory and the table is that you want to provide direct imports to the main module. Would a level indirection therefore also fix your issue?

[hoodmane]

Yeah we are passing -sMAIN_MODULE.

A level of indirection could fix the issue. I was concerned that it would add some additional complexity and perform worse -- this call is very hot. But perhaps a call_indirect to always the same function will be as efficient as a direct call after JIT. If this is the case then we probably don't need this setting.

[hoodmane]

I opened #26228 so we can see what adding the setting looks like. But we can just close it if you think I should go the other way. It feels to me that if there is going to be an -sIMPORTED_MEMORY there should also be an -sIMPORTED_TABLE but that's just my opinon.

[sbc100]

That does sounds reasonable yes.

[sbc100]

One more question about your use case: It sounds like you are using this a way to limit the variant builds that you ship, but isolating the part of the program that uses wasm-gc into a separate module. In, instead you did the feature detection up front and then download a completely separate wasm file when wasm gc is enabled, would that also solve your problem? I'm tempted to suggest that you take that approach rather then adding more settings to emscripten, unless there is some reason why that approach doesn't work? It also seems like it would be simpler since then you wouldn't need to roll your own special dynamic linking thing for this wasm-gc using module: You could just have wasm-ld produce two separate executables?

[hoodmane]

instead you did the feature detection up front and then download a completely separate wasm file when wasm gc is enabled, would that also solve your problem?

This is certainly possible, but it would require more complex changes to the Python makefile and distribution system. Or perhaps we could add a configure flag and build the entire thing twice then make it a downstream problem to select the appropriate binary if desired. I think I would rather just dispatch via a call_indirect than do this, but I'd have to check what the performance cost of that is.

[hoodmane]

Here's another idea: what if I adjusted my function to take a funcref instead of a table index. Then I don't need the table import at all.

[hoodmane]

__builtin_wasm_test_function_pointer_signature takes a table index so I guess I have to do:
__builtin_wasm_table_set(table, 0, funcref) and then __builtin_wasm_test_function_pointer_signature((ty)0)

[hoodmane]

There's another problem which is that compiling a file with -g with

static funcref_t __indirect_function_table[0];

and at least one indirect call causes an error:

error: symbol is not a wasm funcref table

The following does work though:

static funcref_t __indirect_function_table[0] __attribute__((nodebug));

[hoodmane]

Fix:
llvm/llvm-project#182026

[hoodmane]

Oh but then the next problem is that this makes __indirect_function_table a defined symbol in this object file, which generates a separate funcref table distinct from the __indirect_function_table used from other object files...

[sbc100]

build the entire thing twice then make it a downstream problem to select the appropriate binary if desired.

This seems like a pretty reasonable approach to me. Is it very difficult to ship some kind of feature detection on the JS side. Something like:

if (browserSupportsGC()) {
   downloadPythonGC():
} else {
   downloadPython();
}

Correct me if I'm wrong, but are you not already having to essentially exactly this for your "other wasm module"?