Change and extend the reverse analyser #1531
Conversation
| @@ -262,24 +256,36 @@ impl<'a> LSRegAlloc<'a> { | |||
| self.stack.size() | |||
| } | |||
|
|
|||
| /// Is the instruction at [iidx] dead? | |||
| pub(crate) fn is_inst_dead(&self, iidx: InstIdx) -> bool { | |||
There was a problem hiding this comment.
Does "dead" mean "was alive and now is not", or could it also mean "has not been live yet, but will be"?
I think the former, since iirc this replaces something with a name like "is_still_alive_after()"?
There was a problem hiding this comment.
Good question. It means "has DCE identified it as equivalent to a tombstone?" "Dead" was my quick attempt to capture that, but I can see that it might not be entirely clear. I'm open to suggestions!
There was a problem hiding this comment.
Does has_inst_died() capture it perhaps? The past tense is an attempt to anchor the event in time.
There was a problem hiding this comment.
It's not exactly time based: it really means "has DCE identified this as dead". That could even happen conceptually during trace creation. So "is" is probably right. I think "died" might be the contentious bit: it could be that we should call is is_inst_tombstone or similar?
There was a problem hiding this comment.
Changed to is_inst_tombstone in the fixup! commits below. [Note: I accidentally pushed another change which I didn't intend, so force pushed that away.]
ltratt
force-pushed
the
rev_analyse_change
branch
from
b7c6f9c to
f592b47
Compare
|
please squash. |
We will need to propagate spill hints with another mechanism, so renaming this to be more specific is a start. There is no functional change with this commit.
This is a stepping stone towards a different representation.
Soon we will want to do something a bit more fiddly than just assigning to a vector, so duplicating ever more code in many places will become intolerable.
In an ideal world, we'd probably share `RevAnalyse` amongst the code generator and register allocator. That isn't quite doable yet.
Other code generators will also need to do the same. However, we do want to run DCE before showing the user jit-post-opt output, so we still run DCE in that situation.
This is the precursor to us passing register information to the body phase: there is no meaningful functional change here.
This allows it to add register hints to the body that reflect the state of the allocator at the *end* of the header rather than, as before, the *beginning*. In many cases, these two sets are the same, but this allows the header to pass additional things in registers to the body.
The challenge here for the reverse analyser is that some instructions -- currently `PtrAdd`s and `Load`s -- can keep a variable alive without having the register allocator explicitly move it. Fortunately the logic for this is relatively simple.
ltratt
force-pushed
the
rev_analyse_change
branch
from
f592b47 to
af3277b
Compare
|
Squashed. |
This PR gradually gets the reverse analyser in better shape, ultimately speeding big_loop up by another 5%. There are really 3 things going on here:
VarLocations (277a52b and c3680da).OutputCanBeSameAsInputinstructions (6ceac88).[FWIW, I experimented with a couple of designs for (2), but came to the conclusion that there's not much point in wholesale change here. It might be marginally better to treat header and body as separate
Modules which wejmpbetween (I think that would be closer in spirit to RPython), but we are now fairly well set-up to deal with the two combined. I don't think changing now would gain us much, and it would cause a lot of churn.]