Better aim spills #1547
Merged
Better aim spills #1547
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I'd completely forgotten that we use it in our (rather terrible) heuristic for "what to spill" so introduce the (hopefully temporary) `used_later_than` function to capture this, at which point the field can be made private to the reverse analyser.
Previously the reverse analyser only recorded whether an instruction's output value was used. This commit records which (later!) instructions use an instruction's output value. For the time being, we only use this to make a slightly better heuristic about "what to spill". Previously we used the (fairly terrible) proxy of "spill the value which lives the longest". Now we use the heuristic "spill the value which is not used for the longest time". On small examples, these two often coincide, but on bigger examples they definitely won't.
When we've run out of registers, we have to chose a register to "spill". This commit recognises that we often don't have to spill anything, either because the register contains a constant or because the register is duplicating an already-spilled value. We thus prefer not to actually spill (i.e. store!) anything if one of those cases happens, only spilling if we really have to.
`Param`s are not normal instructions: they're effectively declarations of
the state of the system at the point that a trace is about to start.
Previously we accidentally treated these in two different ways:
1. In "full, normal" compilation mode, they were implicitly never
removed because they appeared in `TraceHeaderStart` and
`TraceBodyStart`.
2. In tests, they could be removed if they weren't "used".
This commit makes `Param`s never removable by treating them as "internal
instructions", so tests in (2) now observe the same behaviour as (1).
This is one of those little edge cases where having "funny" instructions
causes weirdness. Previously, we said that there was a def/use edge
between a `Param` and `Trace{Header|Body}Start`. That's not really true:
`Trace{Header|Body}Start` are declarations more than uses. By
considering them to be users of `Param`s, we make some other analyses a
bit weirder. This commit thus explicitly teaches `def_use` to ignore
these instructions.
When a trace starts, we typically have lots of `Param`s that appear in guards and `TraceHeaderEnd`/`TraceBodyEnd` but nowhere else. These registers then gum up the register allocator, making our spill choices poor -- even though we don't typically use them much. There are various ways we might be able to tackle this: this commit does the simplest thing, which is to spill such registers at the very start of the trace.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
This is |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
|
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR gradually nudges spilling to a better place: we avoid spillings unnecessarily (38419d3) and we spill things we don't need on the fast (d879093). Both of these are intended to help with medium to long traces, with the latter potentially significantly reducing pressure on the register allocator. The various other commits are various helpers/cleanups needed to make the main commits possible.