Accurately mark CallIndirect and CallRef as calls in Inlining - #9146
Conversation
In Binaryen's Inlining pass, multi-use functions (refs > 1) are eligible for inlining at -O3 only if they have no calls and no loops (!hasCalls && !hasLoops), as leaf inlining eliminates function call overhead without code explosion. However, FunctionInfoScanner previously omitted CallIndirect and CallRef from hasCalls tracking. In WasmGC / Java, polymorphic method dispatch is compiled as call_ref. Widely-used utilities containing virtual calls (such as String.valueOf(Object), which performs a null-check and an indirect call to x.toString()) were misclassified as leaf functions and aggressively inlined into thousands of call sites. Inlining non-leaf functions with call_ref does not eliminate the indirect call; it only duplicates the caller-side parameter preparation, null checks, and vtable indexing. This change marks hasCalls = true in visitCallIndirect and visitCallRef so that multi-caller non-leaf functions containing indirect/reference calls are not duplicated across the binary.
tlively
left a comment
tlively
left a comment
There was a problem hiding this comment.
This is also strictly beneficial on the Emscripten benchmark suite with a geomean of -0.129% uncompressed and a smaller reduction compressed. Some benchmarks shrink by as much as a few percent uncompressed.
cc @kripken for a second look.
|
@tlively did you see a speed difference, or just size? If the speed looks good, lgtm. I believe the old approach seemed to work well at the time (hence the long comment that is now removed), but many things changed in the optimizer since then 😄 so I can easily believe this is the better heuristic now. |
|
Performance looks ok. Some small improvements and some small regressions. Geomean across benchmarks of the mean time on each benchmark improved slightly, but geomean of the median time on each benchmark regressed slightly. Poppler had the biggest improvement (~3%), but embind and bullet both regressed by about 1%. |
|
Sounds fine to me. |
|
This change apparently had a big impact (13% improvement) on one of the emscripten codesize tests: emscripten-core/emscripten#27777 (comment) |
4 participants
In Binaryen's Inlining pass, multi-use functions (refs > 1) are eligible for inlining at -O3 only if they have no calls and no loops (!hasCalls && !hasLoops), as leaf inlining eliminates function call overhead without code explosion.
However, FunctionInfoScanner previously omitted CallIndirect and CallRef from hasCalls tracking. In WasmGC / Java, polymorphic method dispatch is compiled as call_ref. Widely-used utilities containing virtual calls (such as String.valueOf(Object), which performs a null-check and an indirect call to x.toString()) were misclassified as leaf functions and aggressively inlined into thousands of call sites. Inlining non-leaf functions with call_ref does not eliminate the indirect call; it only duplicates the caller-side parameter preparation, null checks, and vtable indexing.
This change marks hasCalls = true in visitCallIndirect and visitCallRef so that multi-caller non-leaf functions containing indirect/reference calls are not duplicated across the binary.