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.
Instead of relying on the optimizer to retain FREE/FE_FREE opcodes, extend live-ranges to the end of the try block.
Just some experimentation to solve issues that arise with
match
blocks. This attempts to solve the issue described in #5448, which arises with the following scenario:Because
THROW
will jump out of the current context, it needs to free all the temporaryVAR
s (andTMPVAR
s) that would normally be consumed by the skipped opcodes. We calculate live-ranges for this purpose. A live-range defines the "time" (sequence of opcodes) in which theVAR
holds a valid value which has to be freed if exited prematurely. Live-ranges are calculated by scanning the opcodes backwards to find the first (i.e. lowest) usage, and the first (i.e. lowest) definition of theVAR
. In this case,V0
(the result of thefoo()
function call) lives from opcode0002
to0005
(exclusive). TheTHROW
opcode at position0004
will look at which values are live at its position, and won't be used after (i.e. don't outlive) thetry
block, if available. This will freeV0
in this case.Normally, DCE would eliminate everything after
THROW
since it is unreachable, removingADD V0 bool(true)
which is the only use ofV0
. This would break calculation of the live-range of thisV0
with the current implementation. As such, theVAR
would no longer be freed. This issue only arose withthrow
expressions because in statement context noVAR
s are live (with some exceptions liveswitch
conditions, loop iterators, etc). The current solution (#5450) is to treatthrow
expressions as non-terminators when building the CFG. This will persist the unreachable opcodes that follow.With
match
blocks, we will once again experience this issue, even forthrow
statements.With DCE, this is essentially equivalent to
foo() + throw new Exception()
. Our current solution does not work for this case however, because thethrow
statement is treated as a terminator. We might also disable splitting CFG nodes for allthrow
statements inmatch
blocks, that might actually be the easiest solution.I've tried a different approach in this PR. Instead of relying on the retention of the dead consuming opcodes, the live-range of unused
VAR
s may be extended until the end of the currenttry
block, if available. The idea is that the consuming opcode will only be eliminated if the current context is guaranteed to exit. A exception will always at least skip over the opcodes in the currenttry
block. We also rely on the fact that aVAR
can't begin outside of atry
block and end inside of it, and vice versa. The upside is that we may get rid of some other hack that retain dead opcodes exclusively for the sake of live-range calculation.There are a few problems with this approach too.
zend_optimize_temporary_variables()
.zend_optimize_temporary_variables()
will reuse theVAR
of a definition with no use. This will eliminate the live-range for the first definition. This algorithm would need to be adjusted.VAR
in different branches.THROW
must not eliminate the use ofV1
in the lower branch, because that will shorten its live-range to the first usage. I'm unaware of such a sequence of opcodes being emitted in PHP, but it's easy to imagine. To my understanding, all other cases can eliminate the used opcodes.COALESCE
opcode only conditionally initializes the result, in case the null branch is skipped. If the null branch contains athrow
, we will optimize away the second definition of theVAR
, elongating the live-range. However, in the null branch theVAR
isn't actually live, leading to a use-of-uninitialized-value. We could initialize result inCOALESCE
toIS_UNDEF
, but that will lead to a slight performance regression. I'm not yet sure what the best way is to solve this.A similar problem exists for
return
,continue
,break
,goto
, etc. inmatch
blocks. These will need to emitFREE
opcodes forVAR
s that are live. This is similar to #5448. That's a different issue to tackle.