Skip to content

Commit d9c45d8

Browse files
committed
Improve type inference for COALESCE
Place a pi node on the non-null edge to remove a spurious undef/null type. Additionally, adjust the profitability heuristic to be more accurate if the "other predecessor" writes to the variable. Ideally this should not just consider the direct predecessors, but it's sufficient for this case. This partially addresses bug #79353 by removing the discrepancy between ?? and ??=.
1 parent b2f7be7 commit d9c45d8

File tree

2 files changed

+70
-8
lines changed

2 files changed

+70
-8
lines changed

ext/opcache/Optimizer/zend_ssa.c

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,30 @@ static zend_bool dominates(const zend_basic_block *blocks, int a, int b) {
3232
return a == b;
3333
}
3434

35-
static zend_bool dominates_other_predecessors(
36-
const zend_cfg *cfg, const zend_basic_block *block, int check, int exclude) {
35+
static zend_bool will_rejoin(
36+
const zend_cfg *cfg, const zend_dfg *dfg, const zend_basic_block *block,
37+
int other_successor, int exclude, int var) {
3738
int i;
3839
for (i = 0; i < block->predecessors_count; i++) {
3940
int predecessor = cfg->predecessors[block->predecessor_offset + i];
40-
if (predecessor != exclude && !dominates(cfg->blocks, check, predecessor)) {
41-
return 0;
41+
if (predecessor == exclude) {
42+
continue;
43+
}
44+
45+
/* The variable is changed in this predecessor,
46+
* so we will not rejoin with the original value. */
47+
// TODO: This should not be limited to the direct predecessor block.
48+
if (DFG_ISSET(dfg->def, dfg->size, predecessor, var)) {
49+
continue;
50+
}
51+
52+
/* The other successor dominates this predecessor,
53+
* so we will get the original value from it. */
54+
if (dominates(cfg->blocks, other_successor, predecessor)) {
55+
return 1;
4256
}
4357
}
44-
return 1;
58+
return 0;
4559
}
4660

4761
static zend_bool needs_pi(const zend_op_array *op_array, zend_dfg *dfg, zend_ssa *ssa, int from, int to, int var) /* {{{ */
@@ -68,11 +82,11 @@ static zend_bool needs_pi(const zend_op_array *op_array, zend_dfg *dfg, zend_ssa
6882
return 1;
6983
}
7084

71-
/* Check that the other successor of the from block does not dominate all other predecessors.
72-
* If it does, we'd probably end up annihilating a positive+negative pi assertion. */
85+
/* Check whether we will rejoin with the original value coming from the other successor,
86+
* in which case the pi node will not have an effect. */
7387
other_successor = from_block->successors[0] == to
7488
? from_block->successors[1] : from_block->successors[0];
75-
return !dominates_other_predecessors(&ssa->cfg, to_block, other_successor, from);
89+
return !will_rejoin(&ssa->cfg, dfg, to_block, other_successor, from, var);
7690
}
7791
/* }}} */
7892

@@ -252,6 +266,14 @@ static void place_essa_pis(
252266
bt = blocks[j].successors[0];
253267
bf = blocks[j].successors[1];
254268
break;
269+
case ZEND_COALESCE:
270+
if (opline->op1_type == IS_CV) {
271+
int var = EX_VAR_TO_NUM(opline->op1.var);
272+
if ((pi = add_pi(arena, op_array, dfg, ssa, j, blocks[j].successors[0], var))) {
273+
pi_not_type_mask(pi, MAY_BE_NULL);
274+
}
275+
}
276+
continue;
255277
default:
256278
continue;
257279
}

ext/opcache/tests/opt/coalesce.phpt

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
--TEST--
2+
COALESCE optimization
3+
--INI--
4+
opcache.enable=1
5+
opcache.enable_cli=1
6+
opcache.optimization_level=-1
7+
opcache.opt_debug_level=0x20000
8+
--SKIPIF--
9+
<?php require_once('skipif.inc'); ?>
10+
--FILE--
11+
<?php
12+
13+
function a() {
14+
$test = $test ?? true;
15+
return $test;
16+
}
17+
18+
function b() {
19+
$test ??= true;
20+
return $test;
21+
}
22+
23+
?>
24+
--EXPECTF--
25+
$_main: ; (lines=1, args=0, vars=0, tmps=0)
26+
; (after optimizer)
27+
; %s
28+
L0 (14): RETURN int(1)
29+
30+
a: ; (lines=2, args=0, vars=1, tmps=1)
31+
; (after optimizer)
32+
; %s
33+
L0 (4): T1 = COALESCE CV0($test) L1
34+
L1 (5): RETURN bool(true)
35+
36+
b: ; (lines=2, args=0, vars=1, tmps=1)
37+
; (after optimizer)
38+
; %s
39+
L0 (9): T1 = COALESCE CV0($test) L1
40+
L1 (10): RETURN bool(true)

0 commit comments

Comments
 (0)