Skip to content
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

[NFC-ish] Avoid repeated ReFinalize etc. when inlining #6967

Merged
merged 9 commits into from
Sep 24, 2024

Conversation

kripken
Copy link
Member

@kripken kripken commented Sep 24, 2024

We may inline multiple times into a single function. Previously, if we did so, we
did the "fixups" such as ReFinalize and non-nullable local fixes once per such
inlining. But that is wasteful as each ReFinalize etc. scans the whole function,
and could be done after we copy all the code from all the inlinings, which is
what this PR does: it splits doInlining() into one function that inlines code and
one that does the updates after, and the update is done after all inlinings.

This turns out to be very important, a 5x speedup on a large real-world wasm
file I am looking at. The reason is that we actually inline more than once in
half the cases, and sometimes far more - in one case we inline over 1,000
times into a function! It is helpful to replaces 1,000 ReFinalizes with a single one...

This is practically NFC, but it turns out that there are some tiny noticeable
differences between running ReFinalize once at the end vs. once after each
inlining. These differences are not really functional or observable in the
behavior of the code, and optimizations would remove them anyhow, but
they are noticeable in two tests here. The changes to tests are, in order:

  • Different block names, just because the counter we use sees more things.
  • In a testcase with unreachable code, we inline twice into a function, and
    the first inlining brings in an unreachable, and ReFinalizing early will lead
    to it propagating differently than if we wait to ReFinalize. (It actually leads
    to another cycle of inlining in that case, as a fluke.)

Copy link
Member

@aheejin aheejin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice that this simpli-sh change can bring that huge improvements!

@kripken kripken merged commit ccef354 into WebAssembly:main Sep 24, 2024
13 checks passed
@kripken kripken deleted the parallel.inline.2 branch September 24, 2024 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants