-
Notifications
You must be signed in to change notification settings - Fork 746
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
[wasm-split] Run RemoveUnusedElements on secondary modules #6945
Conversation
Wasm-split generally assumes that calls to secondary functions made before the secondary module has been loaded and instantiated should go to imported placeholder functions that can be responsible for loading the secondary module and forwarding the call to the loaded function. That scheme makes the loading entirely transparent from the application's point of view, which is not always a good thing. Other schemes would make it impossible for a secondary function to be called before the secondary module has been explicitly loaded, in which case the placeholder functions would never be called. To improve code size and simplify instantiation under these schemes, add a new `--no-placeholders` option that skips adding imported placeholder functions.
Add a mode that splits a module into arbitrarily many parts based on a simple manifest file. This is currently implemented by splitting out one module at a time in a loop, but this could change in the future if splitting out all the modules at once would improve the quality of the output.
Rather than analyze what module elements from the primary module a secondary module will need, the splitting logic conservatively imports all module elements from the primary module into the secondary module. Run RemoveUnusedElements on the secondary module to remove any of these imports that happen to be unnecessary. Leave a TODO mentioning the possibility of being more selective about which module elements get exported to reduce code size in the primary module, too.
src/ir/module-splitting.cpp
Outdated
@@ -334,6 +335,7 @@ struct ModuleSplitter { | |||
exportImportCalledPrimaryFunctions(); | |||
setupTablePatching(); | |||
shareImportableItems(); | |||
removeUnusedElements(); |
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.
If this ran on the primary then it would run after each of the N splits in MultiSplit mode, which would be slow - but it looks like you only run it on the secondary? If so maybe rename this to reflect that?
I guess it's not possible the primary can also benefit from this? (assuming the module is already optimized)
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.
Right, everything besides functions is exported from the primary module, so there would be no removable module elements.
I'll clarify the method name.
@@ -1,23 +1,34 @@ | |||
;; RUN: wasm-split %s --keep-funcs=foo,bar --export-prefix='%' -o1 %t.1.wasm -o2 %t.2.wasm | |||
;; RUN: wasm-split %s --keep-funcs=foo,bar --export-prefix='%' -o1 %t.1.wasm -o2 %t.2.wasm --no-placeholders |
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.
Why change this?
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.
Just to simplify the output a little bit. It's one less new import in the primary module. We newly export $baz
to make sure it is kept alive in the secondary module via a use in the primary module.
@@ -16,7 +16,7 @@ | |||
|
|||
(module | |||
(func $foo | |||
(nop) | |||
(call $bar) |
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.
Why change this?
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.
To prevent $bar
from being removed for being unused in the secondary module.
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.
lgtm % questions
Rather than analyze what module elements from the primary module a
secondary module will need, the splitting logic conservatively imports
all module elements from the primary module into the secondary module.
Run RemoveUnusedElements on the secondary module to remove any of these
imports that happen to be unnecessary. Leave a TODO mentioning the
possibility of being more selective about which module elements get
exported to reduce code size in the primary module, too.