Skip to content

Tests for #5698 #5700

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Tests for #5698 #5700

wants to merge 11 commits into from

Conversation

kripken
Copy link
Member

@kripken kripken commented May 3, 2023

Oddly I can't create a PR directly to your fork @gkdn - maybe something needs to be set up for that? I can see other forks in the UI to open the PR against, but not yours.

Anyhow, you can just copy the commits from here.

gkdn and others added 11 commits May 1, 2023 19:50
This workarounds the extra work around the edge case where;
 - Function is too big to full-inline
 - It is a candidate for partial inline
 - Outlined version becomes eligible for full-inline.

In such a case, binaryen would introduce a temporary state with
partial inlined functions and later on inline them. J2CL hit this
scenario for String literal which resulted in significant
regressions in compilation time.

This patch updates partial inlining analysis to identify the
edge case and direct to full-inling when that happens.

It probably worths adding new tests but unfortunately I'm not
familiar enough with Binaryen to add such tests.
Co-authored-by: Alon Zakai <alonzakai@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants