-
Notifications
You must be signed in to change notification settings - Fork 824
GlobalStructInference: Optimize gets of immutable global struct data #8005
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
| (struct.new $vtable | ||
| (global.get $imported) | ||
| ) |
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.
It would be neat (and not too hard, I think) to expand this to support arbitrarily deep paths through struct.new, array.new, and global.get expressions in global initializers.
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.
Yes, that is what the TODO about unnesting is about. (I think we need to unnest, like the other code here? Unless there is a simpler way I am missing.)
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.
It would be fairly straightforward to have a utility that is given the root of some sequence of accessors (kind of like a gep in LLVM, except not all rolled into one instruction) and then follows the access path as long as it is known to remain constant, returning (a pointer to) the result.
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.
Would that be useful anywhere else, do you think?
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.
Hmm, I'd have to think about that. I saw that meanwhile you uploaded a solution that unnests the intermediate values into globals. The only benefit a path-walking utility would have over that would be fewer new globals to be optimized back out later. A downside would be potentially quadratic behavior, though.
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. Keeping our 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).
A
struct.getof a global, where the global creates a struct, can beoptimized by other passes, but not in all cases: Precompute can only
handle Literals (as it uses the interpreter), and GUFA works on a type-
based manner. The general case is important too, which looks like this:
This PR adds it to GlobalStructInference. That pass has a bunch of
useful infrastructure for it (parsing of struct.news, unnesting of values,
etc.), making it simple there.
Fixes #8002