[wasm-split] Remove unnecessary global exports#8832
Open
aheejin wants to merge 2 commits into
Open
Conversation
Globals and tables can have initializers that can contain other globals. Currently we just scan them as uses. For example, if global $g is used both in the primary and the secondary and its initializer is `(global.get $h)`, $h is also marked as "used" in both modules. But currently we only move a module item to a secondary module only when that item is exclusively used by that module. So if a global is used in the primary and the secondary, it will stay in the primary and then be exported to the secondary. But in the current code, becaus $g is marked as used in both modules and its initializer will be walked in both modules, $h is also marked as used in both modules. Becuase $g doesn't move to the secondary and only is imported there, the secondary doesn't need $h. But because it is marked as "used", the secondary module imports $h unnecessarily. The multi-split case is similar. The case is the same for table initiaializers. The difference between the two is global initializers can contain another global, so we need a worklist to compute the transitive closure. This fixes it by figuring out who the "owner" is for each global and table, and mark it "used" in a secondary module only when that is the sole user. Otherwise it will be marked as "used" in the primary. This does not meaningfully change computation time and reduces the primary module size around 0.3% for new acx_gallery and essentials and 1% for old acx_gallery.
tlively
reviewed
Jun 11, 2026
Comment on lines
+829
to
+841
| for (auto& sec : secondaryUsed) { | ||
| if ((sec.*field).contains(name)) { | ||
| owner = &sec; | ||
| users++; | ||
| } | ||
| } | ||
| if (users == 0) { | ||
| return nullptr; | ||
| } | ||
| if (users > 1) { | ||
| return &primaryUsed; | ||
| } | ||
| return owner; |
Member
There was a problem hiding this comment.
If we detect the case of a second owner by looking for and existing owner, then we can break early avoid tracking users entirely.
Suggested change
| for (auto& sec : secondaryUsed) { | |
| if ((sec.*field).contains(name)) { | |
| owner = &sec; | |
| users++; | |
| } | |
| } | |
| if (users == 0) { | |
| return nullptr; | |
| } | |
| if (users > 1) { | |
| return &primaryUsed; | |
| } | |
| return owner; | |
| for (auto& sec : secondaryUsed) { | |
| if ((sec.*field).contains(name)) { | |
| if (owner) { | |
| owner = &primaryUsed; | |
| break; | |
| } | |
| owner = &sec; | |
| } | |
| } | |
| return owner; |
Comment on lines
+844
to
+846
| // Scan table initializers into their owning modules. If a table is used by a | ||
| // single secondary module, its initializer dependencies belong to that | ||
| // secondary module. Otherwise, they belong to the primary module. |
Member
There was a problem hiding this comment.
Does this handle the case where the same global is used both in a moved table initializer and in some other location that prevents it from being moved? It looks like the code might handle this, but the comment suggests it does not.
It would be good to add tests for this kind of case if we don't have them already.
Comment on lines
+852
to
+856
| UsedNames* owner = getOwner(table->name, &UsedNames::tables); | ||
| if (!owner) { | ||
| continue; | ||
| } | ||
| NameCollector(*owner).walk(table->init); |
Member
There was a problem hiding this comment.
Suggested change
| UsedNames* owner = getOwner(table->name, &UsedNames::tables); | |
| if (!owner) { | |
| continue; | |
| } | |
| NameCollector(*owner).walk(table->init); | |
| if (UsedNames* owner = getOwner(table->name, &UsedNames::tables)) { | |
| NameCollector(*owner).walk(table->init); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Globals and tables can have initializers that can contain other globals. Currently we just scan them as uses. For example, if global $g is used both in the primary and the secondary and its initializer is
(global.get $h), $h is also marked as "used" in both modules.But currently we only move a module item to a secondary module only when that item is exclusively used by that module. So if a global is used in the primary and the secondary, it will stay in the primary and then be exported to the secondary.
But in the current code, becaus $g is marked as used in both modules and its initializer will be walked in both modules, $h is also marked as used in both modules. Becuase $g doesn't move to the secondary and only is imported there, the secondary doesn't need $h. But because it is marked as "used", the secondary module imports $h unnecessarily. The multi-split case is similar.
The case is the same for table initiaializers. The difference between the two is global initializers can contain another global, so we need a worklist to compute the transitive closure.
This fixes it by figuring out who the "owner" is for each global and table, and mark it "used" in a secondary module only when that is the sole user. Otherwise it will be marked as "used" in the primary.
This does not meaningfully change computation time and reduces the primary module size around 0.3% for new acx_gallery and essentials and 1% for old acx_gallery.