link: run fpcast-emu before optimization passes#26478
Open
thiblahute wants to merge 1 commit intoemscripten-core:mainfrom
Open
link: run fpcast-emu before optimization passes#26478thiblahute wants to merge 1 commit intoemscripten-core:mainfrom
thiblahute wants to merge 1 commit intoemscripten-core:mainfrom
Conversation
sbc100
reviewed
Mar 18, 2026
Collaborator
sbc100
left a comment
There was a problem hiding this comment.
I love the simplicity of the fix, but I wonder if we can come up with smaller/simpler test case?
1fa9355 to
b6d5733
Compare
Author
|
Indeed, I wrote the test case that was as close as possible with the actual trigger I had, simplified now. |
sbc100
reviewed
Mar 18, 2026
kripken
approved these changes
Mar 18, 2026
Member
kripken
left a comment
There was a problem hiding this comment.
Nice! I was hoping this would be faster, great to see that it is. It makes sense I guess - now all the casts use the same type, and any indirect call of a constant is optimizable.
Move the `--fpcast-emu` binaryen pass earlier in the pass pipeline so it runs before -O2. Previously, directize (part of -O2) would see type-mismatched call_indirect entries and replace them with unreachable before fpcast-emu had a chance to rewrite the table entries with matching types. This caused crashes for common C idioms like calling a 1-arg function through a 2-arg function pointer (e.g. GLib's `g_list_free_full` pattern). This reordering also improves performance: on a real-world GStreamer test binary, the reordered pipeline produces a 1.8% smaller wasm output (5.30MB vs 5.40MB) and seems to run almost ~2x faster (not sure why). Also move the fpcast-emu block outside `if optimizing:` so it runs even without -O2 in the link flags as this is not an optimization pass in practice. Also add a regression test. See WebAssembly/binaryen#8475
b6d5733 to
4978e58
Compare
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.
Move the
--fpcast-emubinaryen pass earlier in the pass pipeline so it runs before -O2. Previously, directize (part of -O2) would see type-mismatched call_indirect entries and replace them with unreachable before fpcast-emu had a chance to rewrite the table entries with matching types. This caused crashes for common C idioms like calling a 1-arg function through a 2-arg function pointer (e.g. GLib'sg_list_free_fullpattern).This reordering also improves performance, on a real-world GStreamer test binary, the reordered pipeline produces a 1.8% smaller wasm output (5.30MB vs 5.40MB) and seems to run almost ~2x faster (not sure why).
Also move the fpcast-emu block outside
if optimizing:so it runs even without -O2 in the link flags as this is not an optimization pass in practice.Also add a regression test.
See WebAssembly/binaryen#8475