diff --git a/ext/opcache/jit/ir/ir_ra.c b/ext/opcache/jit/ir/ir_ra.c index f22e06083786..2109ba3d422d 100644 --- a/ext/opcache/jit/ir/ir_ra.c +++ b/ext/opcache/jit/ir/ir_ra.c @@ -1910,6 +1910,73 @@ static int ir_try_swap_operands(ir_ctx *ctx, ir_ref i, ir_insn *insn) return 0; } +/* Reuse a bound COPY's storage when it covers the source's optimized lifetime. */ +static void ir_share_copy_spill_slots(ir_ctx *ctx) +{ + ir_ref ref; + + if (!ctx->binding) { + return; + } + for (ref = 1; ref < ctx->insns_count; ref += ir_insn_len(&ctx->ir_base[ref])) { + ir_insn *insn = &ctx->ir_base[ref]; + ir_ref source, binding; + ir_live_interval *source_interval, *copy_interval; + ir_use_list *uses; + ir_ref i; + ir_live_range range, *source_range; + + if (insn->op != IR_COPY || insn->op2 != IR_COPY_HARD) { + continue; + } + source = insn->op1; + if (source <= 0 || source != ref - 1 || ctx->ir_base[source].op >= IR_PHI + || ctx->ir_base[source].type != insn->type + || ctx->cfg_map[source] != ctx->cfg_map[ref] + || !ctx->vregs[source] || !ctx->vregs[ref] + || ir_binding_find(ctx, source)) { + continue; + } + binding = ir_binding_find(ctx, ref); + if (!binding) { + continue; + } + source_interval = ctx->live_intervals[ctx->vregs[source]]; + copy_interval = ctx->live_intervals[ctx->vregs[ref]]; + /* Do not borrow storage from a coalesced value or across a loop entry. */ + if (((source_interval->flags | copy_interval->flags) & IR_LIVE_INTERVAL_COALESCED) + || source_interval->range.start < IR_START_LIVE_POS_FROM_REF(source) + || source_interval->range.start >= IR_END_LIVE_POS_FROM_REF(source) + || copy_interval->range.start < IR_START_LIVE_POS_FROM_REF(ref) + || copy_interval->range.start >= IR_END_LIVE_POS_FROM_REF(ref)) { + continue; + } + /* Ignore only the adjacent source definition before the COPY. Every + * remaining source range, including holes and backedges, must be covered. */ + range = source_interval->range; + range.start = copy_interval->range.start; + source_range = range.end > range.start ? &range : range.next; + if (source_range && !ir_ivals_inside(©_interval->range, source_range)) { + continue; + } + /* A source snapshot may require a different location from the bound COPY. */ + uses = &ctx->use_lists[source]; + for (i = 0; i < uses->count; i++) { + if (ctx->ir_base[ctx->use_edges[uses->refs + i]].op == IR_SNAPSHOT) { + break; + } + } + if (i != uses->count) { + continue; + } + /* Adjacent definitions have no intervening memory access or exit. The COPY's + * live ranges cover every later use of the identically typed source. + * Keep both identities and the allocator's binding compatibility checks. */ + IR_ASSERT(binding < 0); + ir_bind(ctx, binding, source); + } +} + int ir_coalesce(ir_ctx *ctx) { uint32_t b, n, succ, pred_b, count = 0; @@ -2110,6 +2177,8 @@ int ir_coalesce(ir_ctx *ctx) ir_mem_free(xlat); } + ir_share_copy_spill_slots(ctx); + return 1; } diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c index fc38dd5647fd..6be780600826 100644 --- a/ext/opcache/jit/zend_jit_ir.c +++ b/ext/opcache/jit/zend_jit_ir.c @@ -1378,7 +1378,13 @@ static void zend_jit_def_reg(zend_jit_ctx *jit, zend_jit_addr addr, ir_ref val) val = ir_emit2(&jit->ctx, IR_OPT(IR_COPY, jit->ctx.ir_base[val].type), val, 1); } } - if (!zend_jit_spilling_may_cause_conflict(jit, var, val)) { + bool may_cause_conflict = zend_jit_spilling_may_cause_conflict(jit, var, val); + /* Keep CV identities separate even when the assignment cannot be bound. */ + if (jit->ssa->vars[var].var < jit->current_op_array->last_var + && jit->ctx.ir_base[val].op < IR_PHI) { + val = ir_emit2(&jit->ctx, IR_OPT(IR_COPY, jit->ctx.ir_base[val].type), val, IR_COPY_HARD); + } + if (!may_cause_conflict) { val = ir_bind(&jit->ctx, -EX_NUM_TO_VAR(jit->ssa->vars[var].var), val); } } @@ -6828,14 +6834,24 @@ static int zend_jit_assign_to_variable(zend_jit_ctx *jit, ZEND_ASSERT(end_inputs->count == res_inputs->count || (end_inputs->count == 0 && res_inputs->count == 1)); jit->delay_var = -1; jit->delay_refs = NULL; - if (res_inputs->count == 1) { - phi = res_inputs->refs[0]; - } else { - phi = ir_PHI_N((var_def_info & MAY_BE_LONG & MAY_BE_LONG) ? IR_LONG : IR_DOUBLE, - res_inputs->count, res_inputs->refs); + /* Keep identical constants unbound: a COPY may be hoisted and spill into a live CV. */ + phi = res_inputs->refs[0]; + if (res_inputs->count > 1) { + uint32_t i = 1; + + if (IR_IS_CONST_REF(phi)) { + while (i < res_inputs->count && res_inputs->refs[i] == phi) { + i++; + } + } + if (i < res_inputs->count) { + phi = ir_PHI_N((var_def_info & MAY_BE_LONG) ? IR_LONG : IR_DOUBLE, + res_inputs->count, res_inputs->refs); + } } if (Z_MODE(var_addr) == IS_REG) { - if ((var_info & (MAY_BE_REF|MAY_BE_STRING|MAY_BE_ARRAY|MAY_BE_OBJECT|MAY_BE_RESOURCE)) || ref_addr) { + if (!IR_IS_CONST_REF(phi) + && ((var_info & (MAY_BE_REF|MAY_BE_STRING|MAY_BE_ARRAY|MAY_BE_OBJECT|MAY_BE_RESOURCE)) || ref_addr)) { phi = ir_emit2(&jit->ctx, IR_OPT(IR_COPY, jit->ctx.ir_base[phi].type), phi, 1); } zend_jit_def_reg(jit, var_addr, phi); diff --git a/ext/opcache/tests/jit/gh24683.phpt b/ext/opcache/tests/jit/gh24683.phpt new file mode 100644 index 000000000000..366c045aefb8 --- /dev/null +++ b/ext/opcache/tests/jit/gh24683.phpt @@ -0,0 +1,96 @@ +--TEST-- +GH-24683 (Tracing JIT spills a hoisted constant into a live CV) +--EXTENSIONS-- +opcache +--SKIPIF-- + +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.jit=tracing +opcache.jit_buffer_size=64M +opcache.jit_hot_loop=61 +opcache.jit_hot_func=127 +opcache.jit_hot_return=8 +opcache.jit_hot_side_exit=8 +opcache.file_update_protection=0 +--FILE-- +value; } +} + +function step(Bus $bus, array &$state, array &$ram, int $pc): int +{ + $ptr = $state['ptr']; $i = $state['i']; $n = $state['n']; $ret = $state['ret']; + $c = 0; + while (true) { + switch ($pc) { + case 1: + $ptr = 0x80080000; + $a = $ptr - 0x6260; + if (($a & 3) !== 0) { goto trap; } + $p = $a & 0x1FFFFFFF; + $inRam = $p < 0x00800000; + $x = $inRam ? $ram[$p >> 2] : $bus->read($a); + $ptr = $x; // 0x1F8010F4 + $t = $i !== $n; // unused, but needed to reproduce + $c += 6; // unused, but needed to reproduce + // fall through + case 2: + $a = $ptr; + $p = $a & 0x1FFFFFFF; + $inRam = $p < 0x00800000; + $y = $inRam ? $ram[$p >> 2] : $bus->read($a); + $pc = ($y & 0x7F000000) !== 0 ? 3 : 4; + continue 2; + case 3: + if ($pc === 99) { goto trap; } + $ret = 1; + $pc = $ram[0]; // 5 + // fall through + default: + goto leave; + } + } + leave: + $state['ptr'] = $ptr; $state['ret'] = $ret; + return $pc; + trap: + $state['ptr'] = $ptr; +} + +$bus = new Bus(); +$ram = array_fill(0, 0x80000, 0); +$ram[0] = 5; +$ram[0x79DA0 >> 2] = 0x1F8010F4; + +$wrong = 0; +for ($k = 0; $k < 10; $k++) { + $bus->value = ($k % 128) << 24; + $state = ['ptr' => 0x1F8010F4, 'i' => 0, 'n' => 0, 'ret' => 0]; + $pc = 2; + for ($m = 0; $m < 100; $m++) { + $pc = step($bus, $state, $ram, $pc); + if ($state['ptr'] !== 0x1F8010F4) { + printf("k=%d m=%d: \$ptr is 0x%X, expected 0x1F8010F4\n", $k, $m, $state['ptr']); + $wrong++; + } + if ($pc !== 5) { + break; + } + $pc = $state['ret']; + } +} +echo "wrong: $wrong\n"; +--EXPECT-- +wrong: 0 diff --git a/ext/opcache/tests/jit/gh24683_cse.inc b/ext/opcache/tests/jit/gh24683_cse.inc new file mode 100644 index 000000000000..2da60a7f5a91 --- /dev/null +++ b/ext/opcache/tests/jit/gh24683_cse.inc @@ -0,0 +1,28 @@ +> 2]; + $r29 = ($r29 - 24) & 0xFFFFFFFF; + $a = $r29 + 20; + $ram[$a >> 2] = $r31; // a different slot + $a = $r14; + if (($a & 0x7F800003) === 0) { + } + $r4 = $r14; + if ($budget === 0) { + return 1; + } + if (($a & 0x7F800003) === 0) { + } else { + if (!$flag) { + } + } + $a = $r29 + 20; + if (($a & 0x7F800003) === 0) { + $r31 = $ram[$p >> 2]; // should read slot 0x108 >> 2 again + } + $r[4] = $r4; $r[5] = $r5; $r[14] = $r14; $r[15] = $r15; $r[24] = $r24; $r[25] = $r25; $r[29] = $r29; $r[31] = $r31; + return $r31; +}; diff --git a/ext/opcache/tests/jit/gh24683_cse.phpt b/ext/opcache/tests/jit/gh24683_cse.phpt new file mode 100644 index 000000000000..b2594ad02f5d --- /dev/null +++ b/ext/opcache/tests/jit/gh24683_cse.phpt @@ -0,0 +1,31 @@ +--TEST-- +GH-24683 (CSE result outlives its CV spill slot) +--EXTENSIONS-- +opcache +--SKIPIF-- + +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.jit=tracing +opcache.jit_buffer_size=64M +opcache.jit_hot_loop=61 +opcache.jit_hot_func=127 +opcache.jit_hot_return=8 +opcache.jit_hot_side_exit=8 +opcache.file_update_protection=0 +--FILE-- +> 2] = 0xA000E1EC; + $regs = array_fill(0, 32, 0); + $regs[29] = 0x85D8; + $regs[31] = 0xE10; + if ($fn($regs, $ram, 58) !== 0xA000E1EC) { $wrong++; } +} +echo "$wrong of 1000 calls wrong\n"; +--EXPECT-- +0 of 1000 calls wrong diff --git a/ext/opcache/tests/jit/gh24683_cse_float.phpt b/ext/opcache/tests/jit/gh24683_cse_float.phpt new file mode 100644 index 000000000000..63f25457958e --- /dev/null +++ b/ext/opcache/tests/jit/gh24683_cse_float.phpt @@ -0,0 +1,47 @@ +--TEST-- +GH-24683 (Floating-point CSE survives a CV reassignment) +--EXTENSIONS-- +opcache +--SKIPIF-- + +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.jit=tracing +opcache.jit_buffer_size=64M +opcache.jit_hot_loop=61 +opcache.jit_hot_func=127 +opcache.jit_hot_return=8 +opcache.jit_hot_side_exit=8 +opcache.file_update_protection=0 +--FILE-- + 700 && $k % 17 === 0) ? 91 : 0; + $result = pressure($input, $out, $tag); + $value = $input[0] + 11.25; + $expected = $tag === 91 ? [-1, [$value]] : [$value, [$value, 2, $value]]; + if ([$result, $out] !== $expected) { + $wrong++; + } +} +echo "wrong: $wrong\n"; +--EXPECT-- +wrong: 0 diff --git a/ext/opcache/tests/jit/gh24683_cse_pressure.phpt b/ext/opcache/tests/jit/gh24683_cse_pressure.phpt new file mode 100644 index 000000000000..2f3586cd6175 --- /dev/null +++ b/ext/opcache/tests/jit/gh24683_cse_pressure.phpt @@ -0,0 +1,53 @@ +--TEST-- +GH-24683 (Bound copies do not exhaust the tracing JIT spill frame) +--EXTENSIONS-- +opcache +--SKIPIF-- + +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.jit=tracing +opcache.jit_buffer_size=64M +; Isolate function-entry compilation from a caller loop that could fall back. +opcache.jit_hot_loop=0 +opcache.jit_hot_func=127 +opcache.jit_hot_return=8 +opcache.jit_hot_side_exit=8 +opcache.file_update_protection=0 +opcache.jit_debug=0x14000 +--FILE-- + 700 && $k % 17 === 0) ? 91 : 0); + hash_update($hash, serialize([$result, $out])); +} +echo hash_final($hash), "\n"; +--EXPECTREGEX-- +(?:---- TRACE [0-9]+ compiled\n\n)+01fee1fb9cedccbd27d2b9c70613b77b7095e42bdbcc80eb820337d461d21ec8