-
Notifications
You must be signed in to change notification settings - Fork 5.4k
YJIT: Optimize putobject+opt_ltlt for integers #10401
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
Come to think of it, we might even want to remove the other implementation that don't know that rhs is constant for sure and only keep this one 🤔 |
This PR is pretty cool. I'm amazed that it makes that much of a performance difference on optcarrot. Waaaaaaaaat!? Might get us over the 4x faster threshold on speed.yjit.org 😆
There are some benchmarks that use a variable rhs in the shift. I think that ideally, we should implement variable-shift support in our backend/cross-platform-assembler. Now that you have support for generating code for multiple YARV instructions at once, it could make sense to use this to tackle comparison + branch insns, especially for the most common cases. Could be a significant win on most benchmarks. |
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.
Clever 👌 :)
In `jit_rb_int_lshift()`, we guard against the right hand side changing since we want to avoid generating variable length shifts. When control reaches a `putobject` and `opt_ltlt` pair, though, we know that the right hand side never changes. This commit detects this situation and substitutes an implementation that does not guard against the right hand side changing, saving that work. Deleted some `putobject` Rust tests since they aren't that valuable and cause linking issues. Nice boost to `optcarrot` and `protoboeuf`: ``` ---------- ------------------ bench yjit-pre/yjit-post optcarrot 1.09 protoboeuf 1.12 ---------- ------------------ ```
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.
In the future, it'd be nice to propagate (small) integer constants in Context or JITState and somehow generate the same code without introducing a fused codegen, but it seems hard to optimize away mov
instruction in putobject in the backend for now.
In
jit_rb_int_lshift()
, we guard against the right hand side changingsince we want to avoid generating variable length shifts. When control
reaches a putobject and
opt_ltlt
pair, though, we know that the righthand side never changes.
This commit detects this situation and substitutes an implementation
that does not guard against the right hand side changing, saving that
work.
Deleted some putobject Rust tests since they aren't that valuable and
cause linking issues.
Nice boost to
optcarrot
andprotoboeuf
: