From 042ba779e628f88d7521c58aa45d882df370cab1 Mon Sep 17 00:00:00 2001 From: Dmitry Stogov Date: Wed, 2 Oct 2024 14:00:21 +0300 Subject: [PATCH] Improve JIT TRACE coverage 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 00f66676f39f..a303197d1c5c 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 a36beab6fef1..ee505c2f4670 100644 --- a/ext/opcache/jit/zend_jit_trace.c +++ b/ext/opcache/jit/zend_jit_trace.c @@ -8066,7 +8066,7 @@ int ZEND_FASTCALL zend_jit_trace_hot_root(zend_execute_data *execute_data, const 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 2eeb43a4f754..d42c3c6366d4 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;