JIT: Don't reuse IP register for EX(call) #18392
Conversation
| @@ -1608,6 +1613,18 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op | |||
| call_level++; | |||
| } | |||
|
|
|||
| #if ZEND_DEBUG && 0 | |||
There was a problem hiding this comment.
Debugging helper, will remove
| int32_t call_ref; | ||
| int8_t call_reg; |
There was a problem hiding this comment.
Deoptimization relied on EX(call) being in IP when ZEND_JIT_EXIT_RESTORE_CALL was set. I now use the SNAPSHOT mechanism so that deopt knows in which register the call actually is.
| // JIT: if (UNEXPECTED(used_stack > (size_t)(((char*)EG(vm_stack_end)) - (char*)call))) { | ||
| jit_STORE_IP(jit, ir_LOAD_A(jit_EG(vm_stack_top))); | ||
| rx = ir_LOAD_A(jit_EG(vm_stack_top)); |
There was a problem hiding this comment.
The variable rx is named after the RX register of the old JIT, which was an alias for r15 (IP):
php-src/ext/opcache/jit/zend_jit_x86.dasc
Line 30 in 81d9a27
php-src/ext/opcache/jit/zend_jit_x86.dasc
Line 9113 in 81d9a27
It could be renamed to call, but this would increase the diff size.
ext/opcache/jit/zend_jit_trace.c
Outdated
| @@ -4388,6 +4394,14 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par | |||
| bool gen_handler = false; | |||
|
|
|||
| opline = p->opline; | |||
|
|
|||
| #if ZEND_DEBUG | |||
There was a problem hiding this comment.
Debugging helper, will remove
| static void jit_STORE_IP(zend_jit_ctx *jit, ir_ref ref) | ||
| { | ||
| jit_STORE_IP_no_reset(jit, ref); | ||
| zend_jit_reset_last_valid_opline(jit); |
There was a problem hiding this comment.
We must forget the last valid opline here by default, otherwise zend_jit_set_ip() will use a wrong offset. This used to be done in zend_jit_reuse_ip().
| bool ZEND_FASTCALL zend_jit_deprecated_helper(OPLINE_D) | ||
| bool ZEND_FASTCALL zend_jit_deprecated_helper(zend_execute_data *call) | ||
| { | ||
| zend_execute_data *call = (zend_execute_data *) opline; |
There was a problem hiding this comment.
I totally don't understand the details of this PR, but this change alone LGTM. It was super confusing to me when I implemented #[\NoDiscard].
There was a problem hiding this comment.
A helper is called with the argument %r15 for the VM IP (normally). However, register %r15 is reused in the JIT in some cases to point to the current execute data, such as here. So the opline pointer was never an opline pointer to begin with.
JIT reuses the IP/opline fixed register to store an
EX(call)cache during function calls. This allows a sequence ofINIT_FCALL,SEND,SEND, ...,DO_FCALLto access the current call directly via this register instead of repeatedly fetching it fromEX(call), and sometimes to avoid savingEX(call).My understanding is that we reuse IP out of convenience, as registers were assigned manually in the old JIT. Doing otherwise would have required to reserve a 3rd register at least in code generated between
INIT_FCALLandDO_FCALL. This also reduces register pressure, especially on x32.This has a few drawbacks:
zend_jit_trace_exit_stubsaves IP toEX(opline), when it may be being reused forEX(call)(e.g. here)EX(call)cache is lost every time we need to save IPHere I let IR allocate a register for the EX(call) cache, instead of storing it in the IP register.
bench.phpshows no regression in executed instructions under valgrind, and a 1% improvement in wall time.First commit passes
zend_jit_ctxtozend_jit_trace_get_exit_point(), which creates a lot of noise. Second commit is smaller.