-
Notifications
You must be signed in to change notification settings - Fork 825
GUFA: Represent imported functions as globals #8025
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
Conversation
|
OTOH, maybe it is better to not land this yet, and risk the possible misoptimization, which as discussed here is quite theoretical, at least for now: We could leave this PR up as the solution (with 3-4% overhead) in case this ever becomes an issue? |
|
(Though if we do that, we do need to add some hacks in GUFA, as mentioned in that discussion: Without this PR, imported functions appear as Literals, which we optimize too aggressively, even if they have inexact type - we always assume a Literal is a concrete value with fixed type.) |
tlively
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
|
I do like that this is more principled than doing a surgical fix in GUFA to avoid being overly aggressive with imported function literals. |
|
Yeah, I really can't think of another principled solution here, so maybe we do need to live with this slight overhead. I'll fuzz a little more and then land. |
The meaning of Global in PossibleContents changes from "an actual wasm
Global" to "an immutable global wasm thing, either a Global or a Function."
Imported wasm Functions are effectively immutable values in the global
scope - not concrete, specifically-known functions - so this is more correct.
In particular, two imported Globals (of compatible types) might or might not
be the same, and likewise with imported Functions. And that is not the case
for defined Functions - they are definitely different.
This does not fix the issues related to imported function type handling -
#7993 does that - but this refactoring makes it possible to properly
optimize imported functions afterward.
This does make the PossibleContents object grow from 32 to 40 bytes,
which might be why this adds 3-4% overhead to GUFA, but I don't see a
good way to avoid that, unfortunately. The loss of the ability to optimize
imported functions might be worth that 3-4% (we recently saw a few digits
improvement due to properly optimizing imported externs, for example,
from #8005)