Fix RLSRA for intervals of length 1 #53
Merged
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
This commit changes the behavior of `TRILBuilder` w.r.t building calls.
Since calls (`icall`, `acall` and so on) are not treetops, they were
not added to the block. Therefore if the result of the call (return
value) was not used, the call was elided from built IL. This is wrong,
call may have side-effects so they must not be elided.
This commit changes the behavior of `TRILBuilder` so that each call is
automatically anchored using `treetop`. For example, following:
builder istore: {
builder icall: { 'get_some_int' } .
'temp' }
used to produce:
N002 istore 'temp'
N001 icall 'get_some_int'
With this commit, it produces:
N002 treetop
N001 icall 'get_some_int'
N003 istore 'temp'
N001 ==> icall 'get_some_int'
If ever we really want to elide unnecessary calls (whateber that means)
it should be implemented as an optimization pass.
This strengthen invariant checking before proceeding to actual allocation, namely: * checks that each virtual register is at least once defined (assigned) and * checks that each virtual register is used only after it is defined or not used at all.
Consider following - somewhat pathological - TRIL fragment:
N...
N004 treetop
N003 iadd
N001 iload 'x'
N002 iload 'y'
N...
such that the value of the `iadd` node (`N003`) is never used. The iadd
node is compiled to (say) something like:
addi {vr003}, {vr001}, {vr002}
This instruction defines (assigns) a virtual register `{vr003}` but
since its value is never used, the interval for `{vr003}` of length 1
and such interval was *not* created by interval splitting.
Now consider even trickier example:
N...
N004 treetop
N003 icall 'some_func'
N...
such that value of the icall node (`N003`) is never used just like in
previous example (in other words: the code calls a function `some_func`
but does not use its return value - an entirely valid, if not common,
situation).
Here, the call instruction does define (say) virtual register `{vr003}`
by having a dependency of linkage-defined return register on `{vr003}`.
But since value of `{vr003}` is never used, there's no need to generate
register move from linkage-defined return register to `{vr003}`.
This commit fixes RLSRA for cases like these.
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 commit fixes edge but entirely valid case of register live intervals of lenght 1 - for deeper explanation see a1b97f0.
Luckily this bug was caught by failing assertions. Lessons learned are: (i) always assert that all invariants you think hold actually hold and (ii) there's never enough assertions :-)