From ddc49153f1ceace4ce9c16b14b398b15aef540f3 Mon Sep 17 00:00:00 2001 From: Dmitry Stogov Date: Wed, 2 Oct 2024 19:27:31 +0300 Subject: [PATCH] Improve JIT TRACE coverage (#16171) Now it's possible that PHP tracing JIT loses some parts of the "hot" code. In case we have a root LOOP trace with an inlined call of some function, and we get a SIDE exit inside that function - we recorded a side trace, but finished it a the RETURN of the inlined function. As result the opcodes betwee RETURN from SIDE trace and LOOP exit were not covered by tracer and were executed in interpreter. This patch introduces a "ret_depth" argument that prevents stopping tracing on RETURN of such SIDE trace. --- ext/opcache/jit/zend_jit_internal.h | 7 ++++++- ext/opcache/jit/zend_jit_trace.c | 30 ++++++++++++++++++++++++--- ext/opcache/jit/zend_jit_vm_helpers.c | 24 +++++++++++++++++++-- 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/ext/opcache/jit/zend_jit_internal.h b/ext/opcache/jit/zend_jit_internal.h index 00f66676f39..a303197d1c5 100644 --- a/ext/opcache/jit/zend_jit_internal.h +++ b/ext/opcache/jit/zend_jit_internal.h @@ -647,7 +647,12 @@ ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_jit_ret_trace_helper(ZEND_OPCODE_HAND ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_jit_loop_trace_helper(ZEND_OPCODE_HANDLER_ARGS); int ZEND_FASTCALL zend_jit_trace_hot_root(zend_execute_data *execute_data, const zend_op *opline); -zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data *execute_data, const zend_op *opline, zend_jit_trace_rec *trace_buffer, uint8_t start, uint32_t is_megamorphc); +zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data *execute_data, + const zend_op *opline, + zend_jit_trace_rec *trace_buffer, + uint8_t start, + uint32_t is_megamorphc, + int ret_depth); static zend_always_inline const zend_op* zend_jit_trace_get_exit_opline(zend_jit_trace_rec *trace, const zend_op *opline, bool *exit_if_true) { diff --git a/ext/opcache/jit/zend_jit_trace.c b/ext/opcache/jit/zend_jit_trace.c index a36beab6fef..ee505c2f467 100644 --- a/ext/opcache/jit/zend_jit_trace.c +++ b/ext/opcache/jit/zend_jit_trace.c @@ -8066,7 +8066,7 @@ repeat: JIT_G(tracing) = 1; stop = zend_jit_trace_execute(execute_data, opline, trace_buffer, - ZEND_OP_TRACE_INFO(opline, offset)->trace_flags & ZEND_JIT_TRACE_START_MASK, 0); + ZEND_OP_TRACE_INFO(opline, offset)->trace_flags & ZEND_JIT_TRACE_START_MASK, 0, 0); JIT_G(tracing) = 0; if (stop & ZEND_JIT_TRACE_HALT) { @@ -8390,6 +8390,8 @@ int ZEND_FASTCALL zend_jit_trace_hot_side(zend_execute_data *execute_data, uint3 zend_jit_trace_rec trace_buffer[ZEND_JIT_TRACE_MAX_LENGTH]; uint32_t is_megamorphic = 0; uint32_t polymorphism = 0; + uint32_t root; + int ret_depth = 0; trace_num = ZEND_JIT_TRACE_NUM; @@ -8414,7 +8416,8 @@ int ZEND_FASTCALL zend_jit_trace_hot_side(zend_execute_data *execute_data, uint3 goto abort; } - if (zend_jit_traces[zend_jit_traces[parent_num].root].child_count >= JIT_G(max_side_traces)) { + root = zend_jit_traces[parent_num].root; + if (zend_jit_traces[root].child_count >= JIT_G(max_side_traces)) { stop = ZEND_JIT_TRACE_STOP_TOO_MANY_CHILDREN; goto abort; } @@ -8434,8 +8437,29 @@ int ZEND_FASTCALL zend_jit_trace_hot_side(zend_execute_data *execute_data, uint3 } } + /* Check if this is a side trace of a root LOOP trace */ + if ((zend_jit_traces[root].flags & ZEND_JIT_TRACE_LOOP) + && zend_jit_traces[root].op_array != &EX(func)->op_array) { + const zend_op_array *op_array = zend_jit_traces[root].op_array; + const zend_op *opline = zend_jit_traces[root].opline; + zend_jit_op_array_trace_extension *jit_extension = + (zend_jit_op_array_trace_extension*)ZEND_FUNC_INFO(op_array); + + if (jit_extension->trace_info[opline - op_array->opcodes].trace_flags & ZEND_JIT_TRACE_START_LOOP) { + zend_execute_data *ex = execute_data; + int n = 0; + do { + ex = ex->prev_execute_data; + n++; + } while (ex && zend_jit_traces[root].op_array != &ex->func->op_array); + if (ex && n <= ZEND_JIT_TRACE_MAX_RET_DEPTH) { + ret_depth = n; + } + } + } + JIT_G(tracing) = 1; - stop = zend_jit_trace_execute(execute_data, EX(opline), trace_buffer, ZEND_JIT_TRACE_START_SIDE, is_megamorphic); + stop = zend_jit_trace_execute(execute_data, EX(opline), trace_buffer, ZEND_JIT_TRACE_START_SIDE, is_megamorphic, ret_depth); JIT_G(tracing) = 0; if (stop & ZEND_JIT_TRACE_HALT) { diff --git a/ext/opcache/jit/zend_jit_vm_helpers.c b/ext/opcache/jit/zend_jit_vm_helpers.c index 2eeb43a4f75..d42c3c6366d 100644 --- a/ext/opcache/jit/zend_jit_vm_helpers.c +++ b/ext/opcache/jit/zend_jit_vm_helpers.c @@ -575,7 +575,7 @@ static int zend_jit_trace_subtrace(zend_jit_trace_rec *trace_buffer, int start, * +--------+----------+----------+----------++----------+----------+----------+ * | RETURN |INNER_LOOP| | rec-ret || LINK | | LINK | * +--------+----------+----------+----------++----------+----------+----------+ - * | SIDE | unroll | | return || LINK | LINK | LINK | + * | SIDE | unroll | | side-ret || LINK | LINK | LINK | * +--------+----------+----------+----------++----------+----------+----------+ * * loop: LOOP if "cycle" and level == 0, otherwise INNER_LOOP @@ -586,10 +586,16 @@ static int zend_jit_trace_subtrace(zend_jit_trace_rec *trace_buffer, int start, * loop-ret: LOOP_EXIT if level == 0, otherwise continue (wait for loop) * return: RETURN if level == 0 * rec_ret: RECURSIVE_RET if "cycle" and ret_level > N, otherwise continue + * side_ret: RETURN if level == 0 && ret_level == ret_depth, otherwise continue * */ -zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data *ex, const zend_op *op, zend_jit_trace_rec *trace_buffer, uint8_t start, uint32_t is_megamorphic) +zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data *ex, + const zend_op *op, + zend_jit_trace_rec *trace_buffer, + uint8_t start, + uint32_t is_megamorphic, + int ret_depth) { #ifdef HAVE_GCC_GLOBAL_REGS @@ -1060,6 +1066,20 @@ zend_jit_trace_stop ZEND_FASTCALL zend_jit_trace_execute(zend_execute_data *ex, ZEND_JIT_TRACE_STOP_RECURSION_EXIT) { stop = ZEND_JIT_TRACE_STOP_RECURSION_EXIT; break; + } else if ((start & ZEND_JIT_TRACE_START_SIDE) + && ret_level < ret_depth) { + TRACE_RECORD(ZEND_JIT_TRACE_BACK, 0, op_array); + ret_level++; + last_loop_opline = NULL; + + if (prev_call) { + int ret = zend_jit_trace_record_fake_init_call(prev_call, trace_buffer, idx, 0); + if (ret < 0) { + stop = ZEND_JIT_TRACE_STOP_BAD_FUNC; + break; + } + idx = ret; + } } else { stop = ZEND_JIT_TRACE_STOP_RETURN; break;