Skip to content

Commit 8c9c679

Browse files
aasulaasul
committed
opcache: preserve loop-carried cv values across trace guards
1 parent 84da3f5 commit 8c9c679

3 files changed

Lines changed: 141 additions & 0 deletions

File tree

‎ext/opcache/jit/zend_jit_ir.c‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17476,6 +17476,20 @@ static bool zend_jit_opline_supports_reg(const zend_op_array *op_array, zend_ssa
1747617476

1747717477
static bool zend_jit_var_supports_reg(zend_ssa *ssa, int var)
1747817478
{
17479+
/* A transient assignment to a live loop-carried CV needs an explicit,
17480+
* control-dependent store. Its spill must not be hoisted before the
17481+
* loop guards, where the incoming value is still observable. */
17482+
if (ssa->vars[var].definition >= 0
17483+
&& !ssa->vars[var].phi_use_chain
17484+
&& ssa->ops[ssa->vars[var].definition].op1_def == var) {
17485+
int use = ssa->ops[ssa->vars[var].definition].op1_use;
17486+
17487+
if (use >= 0 && !ssa->vars[use].no_val
17488+
&& ssa->vars[use].definition_phi
17489+
&& (ssa->cfg.blocks[ssa->vars[use].definition_phi->block].flags & ZEND_BB_LOOP_HEADER)) {
17490+
return 0;
17491+
}
17492+
}
1747917493
if (ssa->vars[var].no_val) {
1748017494
/* we don't need the value */
1748117495
return 0;

‎ext/opcache/tests/jit/gh24683_b.phpt‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
--TEST--
2+
GH-24683 (B): tracing JIT must preserve overwritten runtime CV values
3+
--EXTENSIONS--
4+
opcache
5+
--SKIPIF--
6+
<?php if (PHP_INT_SIZE != 8) die("skip 64-bit only"); ?>
7+
--INI--
8+
opcache.enable=1
9+
opcache.enable_cli=1
10+
opcache.jit=tracing
11+
opcache.jit_buffer_size=64M
12+
opcache.jit_hot_loop=61
13+
opcache.jit_hot_func=127
14+
opcache.jit_hot_return=8
15+
opcache.jit_hot_side_exit=8
16+
opcache.file_update_protection=0
17+
--FILE--
18+
<?php
19+
// php -d opcache.enable_cli=1 -d opcache.jit=tracing -d opcache.jit_buffer_size=64M repro.php
20+
//
21+
// Case 1 stores to $ram and moves on to case 2. Case 2 sets $x to a value it
22+
// only knows at run time (0x800E0000 | $zero, where $zero is 0), uses it as an
23+
// address, and overwrites $x with the byte loaded from there (4). Once the loop
24+
// trace is compiled, $x sometimes comes back as 0x800E0000 instead.
25+
26+
final class Bus
27+
{
28+
public function read8(int $address): int { return 0; }
29+
}
30+
31+
$fn = static function (Bus $bus, array &$r, array &$ram, array &$sp, int $pc): int {
32+
$value = $r[4];
33+
$base = $r[15];
34+
$zero = $r[0];
35+
while (true) {
36+
switch ($pc) {
37+
case 1:
38+
$a = ($base + 2) & 0xFFFFFFFF;
39+
{
40+
$p = $a & 0x1FFFFFFF;
41+
$inRam = $a < 0xC0000000 && $p < 0x00800000;
42+
if ($inRam) { $k = ($p & 0x1FFFFF) >> 2; $ram[$k] = ($ram[$k] & ~(65535 << (($p & 3) << 3))) | (($value & 65535) << (($p & 3) << 3)); }
43+
}
44+
$pc = 2;
45+
continue 2;
46+
case 2:
47+
$x = 0x800E0000 | $zero;
48+
$a = ($x + 0xFFFFEDED) & 0xFFFFFFFF;
49+
$p = $a & 0x1FFFFFFF;
50+
$byte = ((($inRam ? ($ram[($p & 0x1FFFFF) >> 2] >> (($p & 3) << 3)) & 0xFF : ($inSp ? ($sp[($p - 0x1F800000) >> 2] >> (($p & 3) << 3)) & 0xFF : $bus->read8($a))) ^ 0x80) - 0x80) & 0xFFFFFFFF;
51+
$x = $byte;
52+
$pc = 3;
53+
continue 2;
54+
default:
55+
goto leave;
56+
}
57+
}
58+
leave:
59+
$r[12] = $x;
60+
return $pc;
61+
};
62+
63+
$bus = new Bus();
64+
$ram = array_fill(0, 0x80000, 0);
65+
$ram[0xDEDED >> 2] = 0x0003047F; // the byte at 0xDEDED is 4
66+
$sp = [];
67+
$wrong = 0;
68+
for ($k = 0; $k < 300; $k++) {
69+
$r = array_fill(0, 32, 0);
70+
$r[4] = 2;
71+
$fn($bus, $r, $ram, $sp, 1);
72+
if ($r[12] !== 4) {
73+
if ($wrong++ === 0) {
74+
printf("call %d: \$x is 0x%X, expected 4\n", $k, $r[12]);
75+
}
76+
}
77+
}
78+
echo "wrong: $wrong\n";
79+
--EXPECT--
80+
wrong: 0

‎ext/opcache/tests/jit/gh24683_b_loop.phpt‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
--TEST--
2+
GH-24683 (B): loop guards must observe the incoming CV value
3+
--EXTENSIONS--
4+
opcache
5+
--SKIPIF--
6+
<?php if (PHP_INT_SIZE != 8) die("skip 64-bit only"); ?>
7+
--INI--
8+
opcache.enable=1
9+
opcache.enable_cli=1
10+
opcache.jit=tracing
11+
opcache.jit_buffer_size=64M
12+
opcache.jit_hot_loop=61
13+
opcache.jit_hot_func=127
14+
opcache.jit_hot_return=8
15+
opcache.jit_hot_side_exit=8
16+
opcache.file_update_protection=0
17+
--FILE--
18+
<?php
19+
function test(array $r, int $pc) {
20+
$zero = $r[0];
21+
$x = $r[2];
22+
while (true) {
23+
switch ($pc) {
24+
case 2:
25+
$x = 0x800E0000 | $zero;
26+
$a = ($x + 0xFFFFEDED) & 0xFFFFFFFF;
27+
$p = $a & 0x1FFFFFFF;
28+
$byte = ((($r[1] >> (($p & 3) << 3)) & 0xFF) ^ 0x80) - 0x80;
29+
$x = $byte;
30+
$pc = 3;
31+
continue 2;
32+
default: goto leave;
33+
}
34+
}
35+
leave: return $x;
36+
}
37+
$wrong = 0;
38+
for ($i = 0; $i < 1000; $i++) {
39+
$r = [0, 0x0003047F, 4];
40+
$pc = $i < 500 ? 2 : 3;
41+
if (test($r, $pc) !== 4) {
42+
$wrong++;
43+
}
44+
}
45+
echo "wrong: $wrong\n";
46+
--EXPECT--
47+
wrong: 0

0 commit comments

Comments
 (0)