Inlining: Inline trivial calls #6143
Merged
Conversation
This file contains 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
tlively
approved these changes
Dec 1, 2023
Comment on lines
+230
to
+231
| // This function body is a call with some trivial (size 1) operands like | ||
| // LocalGet or Const, so it is a trivial call. |
There was a problem hiding this comment.
Counting by number of Expressions rather than by number of bytes, right?
There was a problem hiding this comment.
Yes, Measurer just counts Expressions. That happens to be useful here, but in general we might want to add something more accurate some day for bytes.
|
LGTM |
kripken
added a commit
to emscripten-core/emscripten
that referenced
this pull request
Dec 5, 2023
This improvement on the Binaryen side is blocking the roll atm: WebAssembly/binaryen#6143 Breakage: https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8762517450873465489/+/u/Emscripten_testsuite__other_/stdout
radekdoulik
pushed a commit
to dotnet/binaryen
that referenced
this pull request
Jul 12, 2024
A trivial call is something like a function that just calls another immediately,
function foo(x, y) {
return bar(y, 15);
}
We can inline those and expect to benefit in most cases, though we might
increase code size slightly. Hence it makes sense to inline such cases, even
though in general we are careful and do not inline functions with calls in
them; a "trampoline" like that likely has most of the work in the call itself,
which we can avoid by inlining.
Suggested based on findings in Java.
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.
A trivial call is something like a function that just calls another immediately,
We can inline those and expect to benefit in most cases, though we might
increase code size slightly. Hence it makes sense to inline such cases, even
though in general we are careful and do not inline functions with calls in
them; a "trampoline" like that likely has most of the work in the call itself,
which we can avoid by inlining.
Suggested by @gkdn based on findings in Java.
I ran this on the emscripten code size tests and saw nothing that looked
negative or concerning. In fact, this ends up reducing code size slightly
more often than it increases (and all the changes I inspected look like
they should improve speed).