Skip to content

Commit 368c729

Browse files
dstogovdstogov
committed
Merge branch 'master' into jit-dynasm
* master: Fixed incorect constant conditional jump elimination
2 parents be2efb9 + 7650d90 commit 368c729

File tree

6 files changed

+199
-73
lines changed

6 files changed

+199
-73
lines changed

‎ext/opcache/Optimizer/block_pass.c‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,52 @@ static void zend_optimize_block(zend_basic_block *block, zend_op_array *op_array
195195
VAR_SOURCE(op1) = NULL;
196196
literal_dtor(&ZEND_OP1_LITERAL(src));
197197
MAKE_NOP(src);
198+
switch (opline->opcode) {
199+
case ZEND_JMPZ:
200+
if (zend_is_true(&ZEND_OP1_LITERAL(opline))) {
201+
MAKE_NOP(opline);
202+
DEL_SOURCE(block, block->successors[0]);
203+
block->successors_count = 1;
204+
block->successors[0] = block->successors[1];
205+
} else {
206+
opline->opcode = ZEND_JMP;
207+
COPY_NODE(opline->op1, opline->op2);
208+
DEL_SOURCE(block, block->successors[1]);
209+
block->successors_count = 1;
210+
}
211+
break;
212+
case ZEND_JMPNZ:
213+
if (zend_is_true(&ZEND_OP1_LITERAL(opline))) {
214+
opline->opcode = ZEND_JMP;
215+
COPY_NODE(opline->op1, opline->op2);
216+
DEL_SOURCE(block, block->successors[1]);
217+
block->successors_count = 1;
218+
} else {
219+
MAKE_NOP(opline);
220+
DEL_SOURCE(block, block->successors[0]);
221+
block->successors_count = 1;
222+
block->successors[0] = block->successors[1];
223+
}
224+
break;
225+
case ZEND_JMPZNZ:
226+
if (zend_is_true(&ZEND_OP1_LITERAL(opline))) {
227+
zend_op *target_opline = ZEND_OFFSET_TO_OPLINE(opline, opline->extended_value);
228+
ZEND_SET_OP_JMP_ADDR(opline, opline->op1, target_opline);
229+
DEL_SOURCE(block, block->successors[0]);
230+
block->successors[0] = block->successors[1];
231+
} else {
232+
zend_op *target_opline = ZEND_OP2_JMP_ADDR(opline);
233+
ZEND_SET_OP_JMP_ADDR(opline, opline->op1, target_opline);
234+
DEL_SOURCE(block, block->successors[0]);
235+
}
236+
block->successors_count = 1;
237+
opline->op1_type = IS_UNUSED;
238+
opline->extended_value = 0;
239+
opline->opcode = ZEND_JMP;
240+
break;
241+
default:
242+
break;
243+
}
198244
} else {
199245
zval_ptr_dtor_nogc(&c);
200246
}

‎ext/opcache/Optimizer/sccp.c‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,93 @@ static zend_bool try_replace_op1(
276276
zval zv;
277277
ZVAL_COPY(&zv, value);
278278
if (zend_optimizer_update_op1_const(ctx->scdf.op_array, opline, &zv)) {
279+
/* Reconstruct SSA */
280+
int num;
281+
zend_basic_block *block;
282+
283+
switch (opline->opcode) {
284+
case ZEND_JMPZ:
285+
if (zend_is_true(&zv)) {
286+
MAKE_NOP(opline);
287+
num = ctx->scdf.ssa->cfg.map[opline - ctx->scdf.op_array->opcodes];
288+
block = &ctx->scdf.ssa->cfg.blocks[num];
289+
if (block->successors_count == 2) {
290+
if (block->successors[1] != block->successors[0]) {
291+
zend_ssa_remove_predecessor(ctx->scdf.ssa, num, block->successors[0]);
292+
}
293+
block->successors_count = 1;
294+
block->successors[0] = block->successors[1];
295+
}
296+
} else {
297+
opline->opcode = ZEND_JMP;
298+
COPY_NODE(opline->op1, opline->op2);
299+
num = ctx->scdf.ssa->cfg.map[opline - ctx->scdf.op_array->opcodes];
300+
block = &ctx->scdf.ssa->cfg.blocks[num];
301+
if (block->successors_count == 2) {
302+
if (block->successors[1] != block->successors[0]) {
303+
zend_ssa_remove_predecessor(ctx->scdf.ssa, num, block->successors[1]);
304+
}
305+
block->successors_count = 1;
306+
}
307+
}
308+
break;
309+
case ZEND_JMPNZ:
310+
if (zend_is_true(&zv)) {
311+
opline->opcode = ZEND_JMP;
312+
COPY_NODE(opline->op1, opline->op2);
313+
num = ctx->scdf.ssa->cfg.map[opline - ctx->scdf.op_array->opcodes];
314+
block = &ctx->scdf.ssa->cfg.blocks[num];
315+
if (block->successors_count == 2) {
316+
if (block->successors[1] != block->successors[0]) {
317+
zend_ssa_remove_predecessor(ctx->scdf.ssa, num, block->successors[1]);
318+
}
319+
block->successors_count = 1;
320+
}
321+
} else {
322+
MAKE_NOP(opline);
323+
num = ctx->scdf.ssa->cfg.map[opline - ctx->scdf.op_array->opcodes];
324+
block = &ctx->scdf.ssa->cfg.blocks[num];
325+
if (block->successors_count == 2) {
326+
if (block->successors[1] != block->successors[0]) {
327+
zend_ssa_remove_predecessor(ctx->scdf.ssa, num, block->successors[0]);
328+
}
329+
block->successors_count = 1;
330+
block->successors[0] = block->successors[1];
331+
}
332+
}
333+
break;
334+
case ZEND_JMPZNZ:
335+
if (zend_is_true(&zv)) {
336+
zend_op *target_opline = ZEND_OFFSET_TO_OPLINE(opline, opline->extended_value);
337+
ZEND_SET_OP_JMP_ADDR(opline, opline->op1, target_opline);
338+
num = ctx->scdf.ssa->cfg.map[opline - ctx->scdf.op_array->opcodes];
339+
block = &ctx->scdf.ssa->cfg.blocks[num];
340+
if (block->successors_count == 2) {
341+
if (block->successors[1] != block->successors[0]) {
342+
zend_ssa_remove_predecessor(ctx->scdf.ssa, num, block->successors[0]);
343+
}
344+
block->successors_count = 1;
345+
block->successors[0] = block->successors[1];
346+
}
347+
} else {
348+
zend_op *target_opline = ZEND_OP2_JMP_ADDR(opline);
349+
ZEND_SET_OP_JMP_ADDR(opline, opline->op1, target_opline);
350+
num = ctx->scdf.ssa->cfg.map[opline - ctx->scdf.op_array->opcodes];
351+
block = &ctx->scdf.ssa->cfg.blocks[num];
352+
if (block->successors_count == 2) {
353+
if (block->successors[1] != block->successors[0]) {
354+
zend_ssa_remove_predecessor(ctx->scdf.ssa, num, block->successors[1]);
355+
}
356+
block->successors_count = 1;
357+
}
358+
}
359+
opline->op1_type = IS_UNUSED;
360+
opline->extended_value = 0;
361+
opline->opcode = ZEND_JMP;
362+
break;
363+
default:
364+
break;
365+
}
279366
return 1;
280367
} else {
281368
// TODO: check the following special cases ???

‎ext/opcache/Optimizer/zend_optimizer.c‎

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -258,41 +258,7 @@ int zend_optimizer_update_op1_const(zend_op_array *op_array,
258258
zend_op *opline,
259259
zval *val)
260260
{
261-
zend_op *target_opline;
262-
263261
switch (opline->opcode) {
264-
case ZEND_JMPZ:
265-
if (zend_is_true(val)) {
266-
MAKE_NOP(opline);
267-
} else {
268-
opline->opcode = ZEND_JMP;
269-
COPY_NODE(opline->op1, opline->op2);
270-
opline->op2_type = IS_UNUSED;
271-
}
272-
zval_ptr_dtor_nogc(val);
273-
return 1;
274-
case ZEND_JMPNZ:
275-
if (zend_is_true(val)) {
276-
opline->opcode = ZEND_JMP;
277-
COPY_NODE(opline->op1, opline->op2);
278-
opline->op2_type = IS_UNUSED;
279-
} else {
280-
MAKE_NOP(opline);
281-
}
282-
zval_ptr_dtor_nogc(val);
283-
return 1;
284-
case ZEND_JMPZNZ:
285-
if (zend_is_true(val)) {
286-
target_opline = ZEND_OFFSET_TO_OPLINE(opline, opline->extended_value);
287-
} else {
288-
target_opline = ZEND_OP2_JMP_ADDR(opline);
289-
}
290-
ZEND_SET_OP_JMP_ADDR(opline, opline->op1, target_opline);
291-
opline->op1_type = IS_UNUSED;
292-
opline->extended_value = 0;
293-
opline->opcode = ZEND_JMP;
294-
zval_ptr_dtor_nogc(val);
295-
return 1;
296262
case ZEND_FREE:
297263
MAKE_NOP(opline);
298264
zval_ptr_dtor_nogc(val);

‎ext/opcache/Optimizer/zend_ssa.c‎

Lines changed: 47 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1350,6 +1350,52 @@ void zend_ssa_remove_uses_of_var(zend_ssa *ssa, int var_num) /* {{{ */
13501350
}
13511351
/* }}} */
13521352

1353+
void zend_ssa_remove_predecessor(zend_ssa *ssa, int from, int to) /* {{{ */
1354+
{
1355+
zend_basic_block *next_block = &ssa->cfg.blocks[to];
1356+
zend_ssa_block *next_ssa_block = &ssa->blocks[to];
1357+
zend_ssa_phi *phi;
1358+
int j;
1359+
1360+
/* Find at which predecessor offset this block is referenced */
1361+
int pred_offset = -1;
1362+
int *predecessors = &ssa->cfg.predecessors[next_block->predecessor_offset];
1363+
1364+
for (j = 0; j < next_block->predecessors_count; j++) {
1365+
if (predecessors[j] == from) {
1366+
pred_offset = j;
1367+
break;
1368+
}
1369+
}
1370+
1371+
/* If there are duplicate successors, the predecessors may have been removed in
1372+
* a previous iteration already. */
1373+
if (pred_offset == -1) {
1374+
return;
1375+
}
1376+
1377+
/* For phis in successor blocks, remove the operands associated with this block */
1378+
for (phi = next_ssa_block->phis; phi; phi = phi->next) {
1379+
if (phi->pi >= 0) {
1380+
if (phi->pi == from) {
1381+
zend_ssa_remove_uses_of_var(ssa, phi->ssa_var);
1382+
zend_ssa_remove_phi(ssa, phi);
1383+
}
1384+
} else {
1385+
ZEND_ASSERT(phi->sources[pred_offset] >= 0);
1386+
zend_ssa_remove_phi_source(ssa, phi, pred_offset, next_block->predecessors_count);
1387+
}
1388+
}
1389+
1390+
/* Remove this predecessor */
1391+
next_block->predecessors_count--;
1392+
if (pred_offset < next_block->predecessors_count) {
1393+
predecessors = &ssa->cfg.predecessors[next_block->predecessor_offset + pred_offset];
1394+
memmove(predecessors, predecessors + 1, (next_block->predecessors_count - pred_offset) * sizeof(uint32_t));
1395+
}
1396+
}
1397+
/* }}} */
1398+
13531399
void zend_ssa_remove_block(zend_op_array *op_array, zend_ssa *ssa, int i) /* {{{ */
13541400
{
13551401
zend_basic_block *block = &ssa->cfg.blocks[i];
@@ -1380,45 +1426,7 @@ void zend_ssa_remove_block(zend_op_array *op_array, zend_ssa *ssa, int i) /* {{{
13801426
}
13811427

13821428
for (s = 0; s < block->successors_count; s++) {
1383-
zend_basic_block *next_block = &ssa->cfg.blocks[block->successors[s]];
1384-
zend_ssa_block *next_ssa_block = &ssa->blocks[block->successors[s]];
1385-
zend_ssa_phi *phi;
1386-
1387-
/* Find at which predecessor offset this block is referenced */
1388-
int pred_offset = -1;
1389-
predecessors = &ssa->cfg.predecessors[next_block->predecessor_offset];
1390-
for (j = 0; j < next_block->predecessors_count; j++) {
1391-
if (predecessors[j] == i) {
1392-
pred_offset = j;
1393-
break;
1394-
}
1395-
}
1396-
1397-
/* If there are duplicate successors, the predecessors may have been removed in
1398-
* a previous iteration already. */
1399-
if (pred_offset == -1) {
1400-
continue;
1401-
}
1402-
1403-
/* For phis in successor blocks, remove the operands associated with this block */
1404-
for (phi = next_ssa_block->phis; phi; phi = phi->next) {
1405-
if (phi->pi >= 0) {
1406-
if (phi->pi == i) {
1407-
zend_ssa_remove_uses_of_var(ssa, phi->ssa_var);
1408-
zend_ssa_remove_phi(ssa, phi);
1409-
}
1410-
} else {
1411-
ZEND_ASSERT(phi->sources[pred_offset] >= 0);
1412-
zend_ssa_remove_phi_source(ssa, phi, pred_offset, next_block->predecessors_count);
1413-
}
1414-
}
1415-
1416-
/* Remove this predecessor */
1417-
next_block->predecessors_count--;
1418-
if (pred_offset < next_block->predecessors_count) {
1419-
predecessors = &ssa->cfg.predecessors[next_block->predecessor_offset + pred_offset];
1420-
memmove(predecessors, predecessors + 1, (next_block->predecessors_count - pred_offset) * sizeof(uint32_t));
1421-
}
1429+
zend_ssa_remove_predecessor(ssa, i, block->successors[s]);
14221430
}
14231431

14241432
/* Remove successors of predecessors */

‎ext/opcache/Optimizer/zend_ssa.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ int zend_build_ssa(zend_arena **arena, const zend_script *script, const zend_op_
147147
int zend_ssa_compute_use_def_chains(zend_arena **arena, const zend_op_array *op_array, zend_ssa *ssa);
148148
int zend_ssa_unlink_use_chain(zend_ssa *ssa, int op, int var);
149149

150+
void zend_ssa_remove_predecessor(zend_ssa *ssa, int from, int to);
150151
void zend_ssa_remove_instr(zend_ssa *ssa, zend_op *opline, zend_ssa_op *ssa_op);
151152
void zend_ssa_remove_phi(zend_ssa *ssa, zend_ssa_phi *phi);
152153
void zend_ssa_remove_uses_of_var(zend_ssa *ssa, int var_num);

‎ext/opcache/tests/jmp_elim_001.phpt‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
--TEST--
2+
Edge-cases in constant conditional jump elimination
3+
--SKIPIF--
4+
<?php if (PHP_INT_SIZE != 8) die("skip for machines with 64-bit longs"); ?>
5+
<?php require_once('skipif.inc'); ?>
6+
--FILE--
7+
<?php
8+
$webserver = "Apache";
9+
$cpuArc = "x86_64";
10+
$archName = (strstr($cpuArc, "64") || PHP_INT_SIZE === 8) ? "64" : "32";
11+
$info = array('os' => PHP_OS,
12+
'phpversion' => phpversion(),
13+
'arch' => $archName,
14+
'webserver' =>$webserver);
15+
header('Content-Type: application/json');
16+
echo json_encode($info) . "\n";
17+
--EXPECT--
18+
{"os":"Linux","phpversion":"7.2.0-dev","arch":"64","webserver":"Apache"}

0 commit comments

Comments
 (0)