Skip to content

Commit

Permalink
JIT: Improve $this->property access in closures
Browse files Browse the repository at this point in the history
  • Loading branch information
dstogov committed Sep 24, 2021
1 parent eac6568 commit d1a0b93
Show file tree
Hide file tree
Showing 5 changed files with 282 additions and 94 deletions.
61 changes: 56 additions & 5 deletions ext/opcache/jit/zend_jit.c
Original file line number Diff line number Diff line change
Expand Up @@ -2693,6 +2693,7 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
zend_jit_addr op1_addr, op1_def_addr, op2_addr, op2_def_addr, res_addr;
zend_class_entry *ce;
bool ce_is_instanceof;
bool on_this;

if (JIT_G(bisect_limit)) {
jit_bisect_pos++;
Expand Down Expand Up @@ -3153,11 +3154,13 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
}
ce = NULL;
ce_is_instanceof = 0;
on_this = 0;
if (opline->op1_type == IS_UNUSED) {
op1_info = MAY_BE_OBJECT|MAY_BE_RC1|MAY_BE_RCN;
ce = op_array->scope;
ce_is_instanceof = (ce->ce_flags & ZEND_ACC_FINAL) != 0;
op1_addr = 0;
on_this = 1;
} else {
op1_info = OP1_INFO();
if (!(op1_info & MAY_BE_OBJECT)) {
Expand All @@ -3174,10 +3177,18 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
}
}
}
if (ssa->ops && ssa->vars) {
zend_ssa_op *ssa_op = &ssa->ops[opline - op_array->opcodes];
if (ssa_op->op1_use >= 0) {
if (ssa->vars[ssa_op->op1_use].definition >= 0) {
on_this = op_array->opcodes[ssa->vars[ssa_op->op1_use].definition].opcode == ZEND_FETCH_THIS;
}
}
}
}
if (!zend_jit_incdec_obj(&dasm_state, opline, op_array, ssa, ssa_op,
op1_info, op1_addr,
0, ce, ce_is_instanceof, 0, NULL, IS_UNKNOWN,
0, ce, ce_is_instanceof, on_this, 0, NULL, IS_UNKNOWN,
zend_may_throw(opline, ssa_op, op_array, ssa))) {
goto jit_failure;
}
Expand All @@ -3200,11 +3211,13 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
}
ce = NULL;
ce_is_instanceof = 0;
on_this = 0;
if (opline->op1_type == IS_UNUSED) {
op1_info = MAY_BE_OBJECT|MAY_BE_RC1|MAY_BE_RCN;
ce = op_array->scope;
ce_is_instanceof = (ce->ce_flags & ZEND_ACC_FINAL) != 0;
op1_addr = 0;
on_this = 1;
} else {
op1_info = OP1_INFO();
if (!(op1_info & MAY_BE_OBJECT)) {
Expand All @@ -3221,10 +3234,18 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
}
}
}
if (ssa->ops && ssa->vars) {
zend_ssa_op *ssa_op = &ssa->ops[opline - op_array->opcodes];
if (ssa_op->op1_use >= 0) {
if (ssa->vars[ssa_op->op1_use].definition >= 0) {
on_this = op_array->opcodes[ssa->vars[ssa_op->op1_use].definition].opcode == ZEND_FETCH_THIS;
}
}
}
}
if (!zend_jit_assign_obj_op(&dasm_state, opline, op_array, ssa, ssa_op,
op1_info, op1_addr, OP1_DATA_INFO(), OP1_DATA_RANGE(),
0, ce, ce_is_instanceof, 0, NULL, IS_UNKNOWN,
0, ce, ce_is_instanceof, on_this, 0, NULL, IS_UNKNOWN,
zend_may_throw(opline, ssa_op, op_array, ssa))) {
goto jit_failure;
}
Expand All @@ -3240,11 +3261,13 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
}
ce = NULL;
ce_is_instanceof = 0;
on_this = 0;
if (opline->op1_type == IS_UNUSED) {
op1_info = MAY_BE_OBJECT|MAY_BE_RC1|MAY_BE_RCN;
ce = op_array->scope;
ce_is_instanceof = (ce->ce_flags & ZEND_ACC_FINAL) != 0;
op1_addr = 0;
on_this = 1;
} else {
op1_info = OP1_INFO();
if (!(op1_info & MAY_BE_OBJECT)) {
Expand All @@ -3261,10 +3284,18 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
}
}
}
if (ssa->ops && ssa->vars) {
zend_ssa_op *ssa_op = &ssa->ops[opline - op_array->opcodes];
if (ssa_op->op1_use >= 0) {
if (ssa->vars[ssa_op->op1_use].definition >= 0) {
on_this = op_array->opcodes[ssa->vars[ssa_op->op1_use].definition].opcode == ZEND_FETCH_THIS;
}
}
}
}
if (!zend_jit_assign_obj(&dasm_state, opline, op_array, ssa, ssa_op,
op1_info, op1_addr, OP1_DATA_INFO(),
0, ce, ce_is_instanceof, 0, NULL, IS_UNKNOWN,
0, ce, ce_is_instanceof, on_this, 0, NULL, IS_UNKNOWN,
zend_may_throw(opline, ssa_op, op_array, ssa))) {
goto jit_failure;
}
Expand Down Expand Up @@ -3748,11 +3779,13 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
}
ce = NULL;
ce_is_instanceof = 0;
on_this = 0;
if (opline->op1_type == IS_UNUSED) {
op1_info = MAY_BE_OBJECT|MAY_BE_RC1|MAY_BE_RCN;
op1_addr = 0;
ce = op_array->scope;
ce_is_instanceof = (ce->ce_flags & ZEND_ACC_FINAL) != 0;
on_this = 1;
} else {
op1_info = OP1_INFO();
if (!(op1_info & MAY_BE_OBJECT)) {
Expand All @@ -3769,9 +3802,17 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
}
}
}
if (ssa->ops && ssa->vars) {
zend_ssa_op *ssa_op = &ssa->ops[opline - op_array->opcodes];
if (ssa_op->op1_use >= 0) {
if (ssa->vars[ssa_op->op1_use].definition >= 0) {
on_this = op_array->opcodes[ssa->vars[ssa_op->op1_use].definition].opcode == ZEND_FETCH_THIS;
}
}
}
}
if (!zend_jit_fetch_obj(&dasm_state, opline, op_array, ssa, ssa_op,
op1_info, op1_addr, 0, ce, ce_is_instanceof, 0, 0, NULL,
op1_info, op1_addr, 0, ce, ce_is_instanceof, on_this, 0, 0, NULL,
IS_UNKNOWN,
zend_may_throw(opline, ssa_op, op_array, ssa))) {
goto jit_failure;
Expand Down Expand Up @@ -3897,11 +3938,13 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
}
ce = NULL;
ce_is_instanceof = 0;
on_this = 0;
if (opline->op1_type == IS_UNUSED) {
op1_info = MAY_BE_OBJECT|MAY_BE_RC1|MAY_BE_RCN;
op1_addr = 0;
ce = op_array->scope;
ce_is_instanceof = (ce->ce_flags & ZEND_ACC_FINAL) != 0;
on_this = 1;
} else {
op1_info = OP1_INFO();
if (!(op1_info & MAY_BE_OBJECT)) {
Expand All @@ -3918,9 +3961,17 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
}
}
}
if (ssa->ops && ssa->vars) {
zend_ssa_op *ssa_op = &ssa->ops[opline - op_array->opcodes];
if (ssa_op->op1_use >= 0) {
if (ssa->vars[ssa_op->op1_use].definition >= 0) {
on_this = op_array->opcodes[ssa->vars[ssa_op->op1_use].definition].opcode == ZEND_FETCH_THIS;
}
}
}
}
if (!zend_jit_init_method_call(&dasm_state, opline, b, op_array, ssa, ssa_op, call_level,
op1_info, op1_addr, ce, ce_is_instanceof, 0, NULL,
op1_info, op1_addr, ce, ce_is_instanceof, on_this, 0, NULL,
NULL, 0, 0)) {
goto jit_failure;
}
Expand Down

2 comments on commit d1a0b93

@shqking
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi. I'm afraid the following two .phpt test cases failed after this commit under JIT=1213, 1214, 1215 or 1233, 1234,1235

Zend/tests/closure_038.phpt
Zend/tests/closure_039.phpt

Note that the failure occurred for both JIT/x86 and JIT/arm64 under all NTS/ZTS and HYRBID/CALL modes.
Could you help to take a look at it? Thanks.

@dstogov
Copy link
Member Author

Choose a reason for hiding this comment

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

This should be fixed now.

Please sign in to comment.