RemoveUnusedModuleElements: Trim never-called functions from element segment edges#8883
RemoveUnusedModuleElements: Trim never-called functions from element segment edges#8883JPL11 wants to merge 1 commit into
Conversation
…segment edges An entry in a table only matters if some call_indirect can actually call it with a matching type, or if the table can be inspected in another way (table.get, exports, etc., any of which marks the entire table as used). Previously, a function that could never be called but sat in a live element segment was kept as an empty stub. Now, when such an entry is at the edge of its segment we trim it from the segment (bumping the offset for leading entries) and remove the function entirely. A slot we do not write is null, so a call_indirect reaching it traps on the null rather than on the signature mismatch, a change we allow. Only edges are trimmed, keeping segments contiguous, so the compact function-index binary encoding is preserved. We are careful to skip tables that are used in any way besides indirect calls, tables with initial values, segments that may trap at instantiation, and segments that overlap other segments. Helps WebAssembly#3029
There was a problem hiding this comment.
Pull request overview
This PR enhances RemoveUnusedModuleElements to more aggressively shrink tables by trimming never-callable entries from the edges of active element segments. Doing so allows fully removing those functions (and potentially their types) instead of retaining them as unreachable stubs, while conservatively skipping trimming when it could be externally observable or affect instantiation traps.
Changes:
- Add element-segment edge trimming logic to remove never-callable
ref.func(andref.null) entries and delete now-unreferenced functions. - Adjust analysis to defer referencing bare
ref.funcentries until after trimming so trimmed functions can be removed in the same pass run. - Add/adjust lit tests to cover trimming behavior and the conservative skip conditions (export/import/table.get/init value/instantiation trap/overlaps/non-const offsets), plus subtype handling expectations.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/passes/RemoveUnusedModuleElements.cpp |
Implements trimming of dead edge entries in element segments and defers ref.func referencing until after trimming. |
test/lit/passes/remove-unused-module-elements-tables.wast |
Updates expectations to reflect trimming removing dead edge entries (and fully removing a function). |
test/lit/passes/remove-unused-module-elements_ci-types.wast |
Updates subtype-related expectations for trimming and removal behavior in typed indirect-call scenarios. |
test/lit/passes/remove-unused-module-elements-table-trim.wast |
Adds a comprehensive new lit test file covering trimming and all conservative skip conditions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| bool overlaps = false; | ||
| for (auto& [otherStart, otherEnd] : tableRanges[segment->table]) { | ||
| if (otherStart == start && otherEnd == end) { | ||
| // Skip ourselves. (An identical other segment is an overlap we | ||
| // must detect, but then it also sees us as an overlap, either way | ||
| // |overlaps| is set.) | ||
| continue; | ||
| } | ||
| if (otherStart < end && start < otherEnd) { | ||
| overlaps = true; | ||
| break; | ||
| } | ||
| } |
| (module | ||
| ;; Two segments overlap, so a trimmed slot in one might really be written by | ||
| ;; the other, in a particular order: we cannot trim either. | ||
| ;; CHECK: (type $0 (func (param i32))) |
|
Hmm, this adds a lot of complexity, and I doubt it is worth it: these never-called functions have bodies that are just an I am open to improving things here if we can find a simple solution. I might also change my mind given data that this is a serious issue that I am not fully seeing. |
|
Thanks for taking a look, and that's a fair point. After dedup the residual cost per never-called entry is really just the index bytes in the element segment plus one shared Two thoughts on where to go:
Happy to go either way. I'll default to running the measurement unless you'd rather just close. |
|
I think we can do a little more for #3029. I'll comment there to keep the discussion in one place. |
|
I think my idea there is simpler than this PR, and achieves much of the same goal, at least if the second part is feasible. We can close this then, though if that idea doesn't pan out, and if we find a simple way to reimplement this PR, we might want to reopen it then. |
Helps #3029.
An entry in a table only matters if some
call_indirectcan actually call it with a matching type, or if the table can be inspected in some other way (table.get, exports, imports - any of which marks the entire table as used). Previously, a function that could never be called but sat in a live element segment was kept as a stub with anunreachablebody. Now, when such an entry is at the edge of its segment we trim it from the segment (bumping the offset for leading entries) and remove the function entirely - including its type, if nothing else uses it. As @kripken noted on the issue, replacing the signature-mismatch trap with a null trap is fine, and that is all that trimming can do: an unwritten slot is null.Only the edges are trimmed, which keeps segments contiguous and preserves the compact function-index binary encoding; nulling interior entries would force the expression encoding for the whole segment, which costs more than it saves in the common case. Interior never-called entries still become empty stubs as before. (Possible follow-up: null out interior entries when a segment already uses the expression encoding, or split segments with a cost model like MemoryPacking's.)
Trimming is skipped conservatively when:
table.get, etc.),Since subtyping is handled through the existing per-type table info, an entry whose type is a subtype of a called type is correctly kept (covered in the updated
ci-typestest).To make the trimmed functions removable in the same run, references for bare
ref.funcentries in referenced-only segments are now added after trimming rather than during the analysis.Tested with a new lit file covering the trimming and each of the skip conditions, plus updated expectations in the two existing table tests. Full lit suite, gtest, and
check.pypass;--fuzz-execon a trimmed module shows identical results, with the expected null-instead-of-signature trap on a trimmed slot.