Skip to content

VAR|TMP overhaul #20628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
iluuu1994 wants to merge 19 commits into
base: master
Choose a base branch
from
+16,595 −23,241
Open

VAR|TMP overhaul #20628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
c771409
VAR|TMP overhaul
iluuu1994 Nov 27, 2025
f8b7219
Revert now unchanged test
iluuu1994 Dec 1, 2025
5710a4a
Avoid FREE for INCLUDE_OR_EVAL
iluuu1994 Dec 2, 2025
9bdeafe
Fix eliding of call result in optimizer
iluuu1994 Dec 2, 2025
e38a22d
Compile calls with R in known by-value arg position
iluuu1994 Dec 2, 2025
dba2294
ZEND_DEREF can't throw, and doesn't have side-effects
iluuu1994 Dec 2, 2025
143dfe8
Support FREE dce with T = QM_ASSIGN src
iluuu1994 Dec 2, 2025
6320984
Elide ZEND_DEREF for by-ref list assign in stmt context
iluuu1994 Dec 2, 2025
9ad830a
Fix QM_ASSIGN optimization
iluuu1994 Dec 2, 2025
ce61886
Re-enable optimization
iluuu1994 Dec 2, 2025
9ddff66
Fix YIELD_FROM dce without result
iluuu1994 Dec 2, 2025
a4fa551
Fix QM_ASSIGN optimization, again
iluuu1994 Dec 2, 2025
6babcab
Fix test
iluuu1994 Dec 2, 2025
545d56f
Fix cufa array_slice optimization
iluuu1994 Dec 2, 2025
d63a4ff
Replace ZEND_DEREF with ZEND_QM_ASSIGN
iluuu1994 Dec 4, 2025
4ad5861
Revert unneded change
iluuu1994 Dec 4, 2025
448ce15
Use zend_unwrap_reference() in zend_return_unwrap_ref()
iluuu1994 Dec 4, 2025
8491949
Simplify condition
iluuu1994 Dec 4, 2025
30e866d
Various tweaks
iluuu1994 Dec 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 19 additions & 14 deletions Zend/Optimizer/block_pass.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,29 +289,34 @@ static void zend_optimize_block(zend_basic_block *block, zend_op_array *op_array
MAKE_NOP(opline);
++(*opt_count);
break;
case ZEND_ASSIGN:
case ZEND_ASSIGN_DIM:
case ZEND_ASSIGN_OBJ:
case ZEND_ASSIGN_STATIC_PROP:
case ZEND_ASSIGN_OP:
case ZEND_ASSIGN_DIM_OP:
case ZEND_ASSIGN_OBJ_OP:
case ZEND_ASSIGN_STATIC_PROP_OP:
case ZEND_PRE_INC:
case ZEND_PRE_DEC:
case ZEND_PRE_INC_OBJ:
case ZEND_PRE_DEC_OBJ:
case ZEND_PRE_INC_STATIC_PROP:
case ZEND_PRE_DEC_STATIC_PROP:
case ZEND_QM_ASSIGN:
if (src < op_array->opcodes + block->start) {
break;
}
src->result_type = IS_UNUSED;
VAR_SOURCE(opline->op1) = NULL;
MAKE_NOP(opline);
++(*opt_count);
if (src->op1_type & (IS_VAR|IS_TMP_VAR)) {
src->opcode = ZEND_FREE;
Copy link
Member

@dstogov dstogov Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replacing QM_ASSIGN with FREE opens possibility to deeper application of the same optimization.

} else if (src->op1_type == IS_CONST) {
MAKE_NOP(src);
} else if (src->op1_type == IS_CV) {
src->opcode = ZEND_CHECK_VAR;
SET_UNUSED(src->result);
}
break;
default:
if (!zend_op_may_elide_result(src->opcode)) {
break;
}
if (src < op_array->opcodes + block->start) {
break;
}
src->result_type = IS_UNUSED;
VAR_SOURCE(opline->op1) = NULL;
MAKE_NOP(opline);
++(*opt_count);
break;
}
}
Expand Down
8 changes: 7 additions & 1 deletion Zend/Optimizer/dce.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,16 @@ static inline bool may_have_side_effects(
case ZEND_EXT_FCALL_END:
case ZEND_TICKS:
case ZEND_YIELD:
case ZEND_YIELD_FROM:
case ZEND_VERIFY_NEVER_TYPE:
/* Intrinsic side effects */
return true;
case ZEND_YIELD_FROM: {
uint32_t t1 = OP1_INFO();
if ((t1 & (MAY_BE_ANY|MAY_BE_UNDEF)) == MAY_BE_ARRAY && MAY_BE_EMPTY_ONLY(t1)) {
return false;
}
return true;
}
Comment on lines +168 to +174
Copy link
Member

@dstogov dstogov Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows DCE pass to drop YIELD_FROM instruction at some cases. I think, it may be never dropped.

Copy link
Member Author

@iluuu1994 iluuu1994 Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this is not a very useful optimization, though see ext/opcache/tests/opt/sccp_032.phpt where YIELD_FROM is already elided through SCCP. I tried to not break any of the existing optimizations.

Copy link
Member

@dstogov dstogov Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems I was wrong and elimination is possible. This is not clear from the YIELD_FROM code, because the control comes to ZEND_VM_RETURN() and returns back to next opline through zend_generator_resume().

case ZEND_DO_FCALL:
case ZEND_DO_FCALL_BY_NAME:
case ZEND_DO_ICALL:
Expand Down
7 changes: 7 additions & 0 deletions Zend/Optimizer/zend_inference.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5336,6 +5336,13 @@ ZEND_API bool zend_may_throw_ex(const zend_op *opline, const zend_ssa_op *ssa_op
return 1;
}
return 0;
case ZEND_YIELD_FROM: {
uint32_t t1 = OP1_INFO();
if ((t1 & (MAY_BE_ANY|MAY_BE_UNDEF)) == MAY_BE_ARRAY && MAY_BE_EMPTY_ONLY(t1)) {
return false;
}
return true;
Copy link
Member Author

@iluuu1994 iluuu1994 Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YIELD_FROM from generators can throw if the generator is closed, even for empty arrays. So this optimization may be unsound.

Copy link
Member

@bwoebi bwoebi Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not mind doing the inverse - changing YIELD_FROM to not throw on empty array, though...

Copy link
Member

@dstogov dstogov Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking into account the "closed" generator, this is wrong.

}
default:
return 1;
}
Expand Down
12 changes: 12 additions & 0 deletions Zend/tests/gh20628_001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--TEST--
Nullsafe operator does not support BP_VAR_W
--FILE--
<?php

function &test($foo) {
return $foo?->bar();
}

?>
--EXPECTF--
Fatal error: Cannot take reference of a nullsafe chain in %s on line %d
22 changes: 22 additions & 0 deletions Zend/tests/gh20628_002.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
--TEST--
Pipes support return-by-reference
--FILE--
<?php

function &foo($val) {
global $x;
$x = $val;
return $x;
}

function &bar() {
return 42 |> foo(...);
}

$xRef = &bar();
$xRef++;
var_dump($x);

?>
--EXPECT--
int(43)
2 changes: 2 additions & 0 deletions Zend/tests/list/bug73663.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
--TEST--
Bug #73663 ("Invalid opcode 65/16/8" occurs with a variable created with list())
--XFAIL--
list() assign resulting in a ref only when there's a single by-ref fetch is kinda weird.
--FILE--
<?php
function change(&$ref) {
Expand Down
8 changes: 4 additions & 4 deletions Zend/tests/operator_unsupported_types.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ Unsupported operand types: stdClass * stdClass
Unsupported operand types: stdClass * resource
Unsupported operand types: stdClass * string
Unsupported operand types: resource * array
Unsupported operand types: stdClass * resource
Unsupported operand types: resource * stdClass
Unsupported operand types: resource * resource
Unsupported operand types: resource * string
Unsupported operand types: string * array
Expand Down Expand Up @@ -753,7 +753,7 @@ Unsupported operand types: stdClass & stdClass
Unsupported operand types: stdClass & resource
Unsupported operand types: stdClass & string
Unsupported operand types: resource & array
Unsupported operand types: stdClass & resource
Unsupported operand types: resource & stdClass
Unsupported operand types: resource & resource
Unsupported operand types: resource & string
Unsupported operand types: string & array
Expand Down Expand Up @@ -828,7 +828,7 @@ Unsupported operand types: stdClass | stdClass
Unsupported operand types: stdClass | resource
Unsupported operand types: stdClass | string
Unsupported operand types: resource | array
Unsupported operand types: stdClass | resource
Unsupported operand types: resource | stdClass
Unsupported operand types: resource | resource
Unsupported operand types: resource | string
Unsupported operand types: string | array
Expand Down Expand Up @@ -903,7 +903,7 @@ Unsupported operand types: stdClass ^ stdClass
Unsupported operand types: stdClass ^ resource
Unsupported operand types: stdClass ^ string
Unsupported operand types: resource ^ array
Unsupported operand types: stdClass ^ resource
Unsupported operand types: resource ^ stdClass
Unsupported operand types: resource ^ resource
Unsupported operand types: resource ^ string
Unsupported operand types: string ^ array
Expand Down
2 changes: 1 addition & 1 deletion Zend/zend_closures.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ static ZEND_NAMED_FUNCTION(zend_closure_call_magic) /* {{{ */ {
fcc.called_scope = zend_get_called_scope(EG(current_execute_data));

zend_call_function(&fci, &fcc);

zend_return_unwrap_ref(EG(current_execute_data), return_value);
zval_ptr_dtor(&fci.params[1]);
}
/* }}} */
Expand Down
Loading
Loading