From e0fcfacedac5396888da05ef8ceea9d632878b53 Mon Sep 17 00:00:00 2001 From: aasul Date: Mon, 26 Oct 2026 22:36:53 +0100 Subject: [PATCH 1/2] OPcache: preserve identical constants across assignment cleanup Keep an identical typed constant as the result of assignment cleanup branches instead of creating a PHI and a CV-bound COPY. A movable COPY can otherwise spill the constant into the CV before the assignment's guard, overwriting a value that the trace still needs. Add the original constant-assignment regression from GH-24683. Refs GH-24683. --- ext/opcache/jit/zend_jit_ir.c | 22 +++++-- ext/opcache/tests/jit/gh24683.phpt | 96 ++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 6 deletions(-) create mode 100644 ext/opcache/tests/jit/gh24683.phpt diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c index fc38dd5647fd..b3eb5cc934b4 100644 --- a/ext/opcache/jit/zend_jit_ir.c +++ b/ext/opcache/jit/zend_jit_ir.c @@ -6828,14 +6828,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 From b3a500a540ada2d37bc60e3b8cdaceb34b3a0e8f Mon Sep 17 00:00:00 2001 From: aasul Date: Mon, 26 Oct 2026 22:40:24 +0100 Subject: [PATCH 2/2] OPcache: separate reusable values from CV assignment storage Give pure-expression CV assignments their own hard COPY, including when the existing conflict check rejects a binding. A later assignment must not overwrite storage still needed by a reusable expression. After optimized liveness and coalescing, share a checked bound COPY's spill slot only when adjacent definitions have compatible types, the COPY's ranges cover every later source range, neither interval was coalesced or live before its definition, and no snapshot names the source. Keep separate value identities and private source spills otherwise. This avoids exhausting the fixed trace spill frame in safe reuse cases. Add the original two-file C reproduction, a natural floating-point CSE regression, and a test requiring successful compilation under pressure. Refs GH-24683. --- ext/opcache/jit/ir/ir_ra.c | 69 +++++++++++++++++++ ext/opcache/jit/zend_jit_ir.c | 8 ++- ext/opcache/tests/jit/gh24683_cse.inc | 28 ++++++++ ext/opcache/tests/jit/gh24683_cse.phpt | 31 +++++++++ ext/opcache/tests/jit/gh24683_cse_float.phpt | 47 +++++++++++++ .../tests/jit/gh24683_cse_pressure.phpt | 53 ++++++++++++++ 6 files changed, 235 insertions(+), 1 deletion(-) create mode 100644 ext/opcache/tests/jit/gh24683_cse.inc create mode 100644 ext/opcache/tests/jit/gh24683_cse.phpt create mode 100644 ext/opcache/tests/jit/gh24683_cse_float.phpt create mode 100644 ext/opcache/tests/jit/gh24683_cse_pressure.phpt 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 b3eb5cc934b4..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); } } 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