GH-112354: _GUARD_IS_TRUE_POP side-exits to target the next instruction, not themselves. - #114078
Conversation
Sorry, something went wrong.
| ADD_TO_TRACE(uopcode, max_length, 0, INSTR_IP(next_instr, code)); | ||
| goto top; | ||
| } | ||
| ADD_TO_TRACE(uopcode, max_length, 0, INSTR_IP(target_instr, code)); |
There was a problem hiding this comment.
I don't follow - why is the target different depending on whether jump_likely or not?
Sorry, something went wrong.
There was a problem hiding this comment.
Because if we expect to jump then the next instruction in the trace is the target, so if the guard fails we go to the following instruction.
Conversely, if we expect not to jump then the next instruction in the trace is the next instruction, so if the guard fails we go to the target.
POP_JUMP_IF_TRUE L2 L1 ...
Becomes:
If we expect TOS to be True:
_GUARD_IS_TRUE_POP L1 trace continues from L2
If we expect TOS to be False:
_GUARD_IS_FALSE_POP L2 trace continues from L1
Sorry, something went wrong.
iritkatriel
left a comment
There was a problem hiding this comment.
Makes sense.
Sorry, something went wrong.
gvanrossum
left a comment
There was a problem hiding this comment.
PS. There's still an unused STORE_SP() definition in ceval_macros.h.
Sorry, something went wrong.
…nstruction, not themselves. (pythonGH-114078)
…nstruction, not themselves. (pythonGH-114078)
…nstruction, not themselves. (pythonGH-114078)
In tier 2, JUMP_IF_TRUE and friends are converted to guards. Currently if they fail, they resume execution in tier 1 at the same instruction. Apart from being inefficient, this doesn't guarantee forward progress if the jump is the first instruction, which will happen when we optimize side exits.
This PR changes the target of _GUARD_IS_TRUE_POP, etc, to be the successor instruction, not the original JUMP_IF_TRUE/FALSE.