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

More register allocator improvements #1620

Merged
merged 4 commits into from
Feb 21, 2025
Merged

Conversation

ltratt
Copy link
Contributor

@ltratt ltratt commented Feb 21, 2025

This PR optimises the register allocator, more often moving values than spilling them.

In the recent-ish rewrite of the register allocator, I didn't make use
of `can_be_same_as_input: true` to keep things simple. This commit makes
use of this to allow the register allocator to sometimes avoid
allocating an extra register. This helps reduce overall register
pressure.
This additional code is pointless in most cases and -- particularly if
we hit the `swap` case -- can be quite slow. Right now, this entire loop
is somewhat suboptimal, in the sense that it doesn't guarantee to find
the minimal set of register moves. We can definitely do better, but it's
more work than I want to think about right now.
Sometimes we have values in registers that we're going to need in the
future, but they're in registers that we need to use *right now*.
Previously, we spilled them immediately. This commit tries, when
possible, to move those values to another register. There is more we
could do to optimise this, but this still tends to reduce the number of
spills and unspills, so it's already a win.
@vext01 vext01 added this pull request to the merge queue Feb 21, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 21, 2025
We could previously accidentally spill the "dummy" value for `None`,
leading to crashes.

While I'm here, tidy up and clarify the specification of `None` to make
it clearer when it can be used.
@ltratt
Copy link
Contributor Author

ltratt commented Feb 21, 2025

@vext01 With the fix in 85e17b6 this PR should hopefully merge.

@vext01 vext01 added this pull request to the merge queue Feb 21, 2025
Merged via the queue into ykjit:master with commit 10e8426 Feb 21, 2025
2 checks passed
@ltratt ltratt deleted the more_reg_alloc branch February 22, 2025 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants