Skip to content
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

YJIT: Optimize putobject+opt_ltlt for integers #10401

Merged
merged 1 commit into from
Mar 28, 2024
Merged

Conversation

XrXr
Copy link
Member

@XrXr XrXr commented Mar 28, 2024

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:

yjit-pre: ruby 3.4.0dev (2024-03-28T19:30:38Z master 8780059c38) +YJIT [arm64-darwin23]
yjit-post: ruby 3.4.0dev (2024-03-28T19:46:08Z yjit-fused-ltlt 811aa3eb6b) +YJIT [arm64-darwin23]

----------  -------------  ----------  --------------  ----------  -----------------  ------------------
bench       yjit-pre (ms)  stddev (%)  yjit-post (ms)  stddev (%)  yjit-post 1st itr  yjit-pre/yjit-post
optcarrot   915.8          0.7         838.2           0.8         1.09               1.09              
protoboeuf  29.6           12.6        26.5            2.5         1.05               1.12              
----------  -------------  ----------  --------------  ----------  -----------------  ------------------

@XrXr XrXr marked this pull request as ready for review March 28, 2024 20:06
@XrXr XrXr requested a review from a team as a code owner March 28, 2024 20:06
@XrXr
Copy link
Member Author

XrXr commented Mar 28, 2024

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 🤔

yjit/src/codegen.rs Outdated Show resolved Hide resolved
@maximecb
Copy link
Contributor

maximecb commented Mar 28, 2024

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 😆

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 🤔

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.

Copy link
Contributor

@maximecb maximecb left a 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
----------  ------------------
```
@XrXr XrXr enabled auto-merge (rebase) March 28, 2024 21:12
Copy link
Member

@k0kubun k0kubun left a 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.

@XrXr XrXr merged commit f3c3574 into ruby:master Mar 28, 2024
98 checks passed
@XrXr XrXr deleted the yjit-fused-ltlt branch March 28, 2024 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants