-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Add IMPORTED_TABLE setting
#26228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add IMPORTED_TABLE setting
#26228
Changes from all commits
3d9563a
aa72e94
1257fc7
46798c1
092a91f
2c38718
131350b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -429,11 +429,19 @@ def emscript(in_wasm, out_wasm, outfile_js, js_syms, finalize=True, base_metadat | |
| set_memory(static_bump) | ||
| logger.debug('stack_low: %d, stack_high: %d, heap_base: %d', settings.STACK_LOW, settings.STACK_HIGH, settings.HEAP_BASE) | ||
|
|
||
| # When building relocatable output (e.g. MAIN_MODULE) the reported table | ||
| # size does not include the reserved slot at zero for the null pointer. | ||
| # So we need to offset the elements by 1. | ||
| if settings.INITIAL_TABLE == -1: | ||
| settings.INITIAL_TABLE = dylink_sec.table_size + 1 | ||
| if settings.IMPORTED_TABLE and settings.INITIAL_TABLE == -1: | ||
| # For builds with IMPORTED_TABLE, get the table size from the wasm module's | ||
| # table import. | ||
| with webassembly.Module(in_wasm) as module: | ||
| table_import = module.get_function_table_import() | ||
| if not table_import: | ||
| exit_with_error('IMPORTED_TABLE requires a table import in the wasm module') | ||
| settings.INITIAL_TABLE = table_import.limits.initial | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's used when we set
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about if Or, in your use case would you prefer if emcc itself defined the table in JS?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer it if emcc defines the table itself. For instance, otherwise I'd need to implement similar logic that looks at the import section and checks how big the initial size is. I just want wasmTable to be defined during |
||
| if settings.RELOCATABLE: | ||
| # When building relocatable output (e.g. MAIN_MODULE) the reported table | ||
| # size does not include the reserved slot at zero for the null pointer. | ||
| # So we need to offset the elements by 1. | ||
| settings.INITIAL_TABLE += 1 | ||
|
|
||
| if metadata.invoke_funcs: | ||
| settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$getWasmTableEntry'] | ||
|
|
@@ -823,10 +831,11 @@ def add_standard_wasm_imports(send_items_map): | |
| if settings.IMPORTED_MEMORY: | ||
| send_items_map['memory'] = 'wasmMemory' | ||
|
|
||
| if settings.RELOCATABLE: | ||
| if settings.IMPORTED_TABLE: | ||
| send_items_map['__indirect_function_table'] = 'wasmTable' | ||
| if settings.MEMORY64: | ||
| send_items_map['__table_base32'] = '___table_base32' | ||
|
|
||
| if settings.RELOCATABLE and settings.MEMORY64: | ||
| send_items_map['__table_base32'] = '___table_base32' | ||
|
|
||
| if settings.AUTODEBUG: | ||
| extra_sent_items += [ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm less sure about this part. Don't you want to define the table externally in the cases of
IMPORTED_TABLE?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little sad to make this change because I was hoping to completely remove this block of code. Since we no longer use
-sRELOCTABLEfor dynamic linking I was hoping to remove all support for-sRELOCATABLE.