Skip to content

Commit 4765331

Browse files
dstogovdstogov
committed
Don't optimize MUL into SHIFT if we have to check for overflow
1 parent cd0dadf commit 4765331

File tree

3 files changed

+64
-116
lines changed

3 files changed

+64
-116
lines changed

‎ext/opcache/jit/zend_jit_arm64.dasc

Lines changed: 26 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -3848,53 +3848,37 @@ static int zend_jit_math_long_long(dasm_State **Dst,
38483848
}
38493849

38503850
if (opcode == ZEND_MUL &&
3851-
Z_MODE(op2_addr) == IS_CONST_ZVAL &&
3852-
zend_long_is_power_of_two(Z_LVAL_P(Z_ZV(op2_addr)))) {
3853-
3854-
if (Z_LVAL_P(Z_ZV(op2_addr)) == 2) {
3855-
if (Z_MODE(op1_addr) == IS_REG) {
3856-
| adds Rx(result_reg), Rx(Z_REG(op1_addr)), Rx(Z_REG(op1_addr))
3857-
} else {
3858-
| GET_ZVAL_LVAL result_reg, op1_addr, TMP1
3859-
| adds Rx(result_reg), Rx(result_reg), Rx(result_reg)
3860-
}
3851+
Z_MODE(op2_addr) == IS_CONST_ZVAL &&
3852+
Z_LVAL_P(Z_ZV(op2_addr)) == 2) {
3853+
if (Z_MODE(op1_addr) == IS_REG) {
3854+
| adds Rx(result_reg), Rx(Z_REG(op1_addr)), Rx(Z_REG(op1_addr))
38613855
} else {
3862-
| GET_ZVAL_LVAL ZREG_TMP1, op1_addr, TMP2
3863-
| mov TMP2, #zend_long_floor_log2(Z_LVAL_P(Z_ZV(op2_addr)))
3864-
| lsl Rx(result_reg), TMP1, TMP2
3865-
if (may_overflow) {
3866-
/* Compare 'op' and '((op << n) >> n)' for overflow.
3867-
* Flag: bne -> overflow. beq -> no overflow.
3868-
*/
3869-
use_ovf_flag = 0;
3870-
| asr TMP3, Rx(result_reg), TMP2
3871-
| cmp TMP1, TMP3
3872-
}
3856+
| GET_ZVAL_LVAL result_reg, op1_addr, TMP1
3857+
| adds Rx(result_reg), Rx(result_reg), Rx(result_reg)
38733858
}
38743859
} else if (opcode == ZEND_MUL &&
3875-
Z_MODE(op1_addr) == IS_CONST_ZVAL &&
3876-
zend_long_is_power_of_two(Z_LVAL_P(Z_ZV(op1_addr)))) {
3877-
3878-
if (Z_LVAL_P(Z_ZV(op1_addr)) == 2) {
3879-
if (Z_MODE(op2_addr) == IS_REG) {
3880-
| adds Rx(result_reg), Rx(Z_REG(op2_addr)), Rx(Z_REG(op2_addr))
3881-
} else {
3882-
| GET_ZVAL_LVAL result_reg, op2_addr, TMP1
3883-
| adds Rx(result_reg), Rx(result_reg), Rx(result_reg)
3884-
}
3860+
Z_MODE(op2_addr) == IS_CONST_ZVAL &&
3861+
!may_overflow &&
3862+
zend_long_is_power_of_two(Z_LVAL_P(Z_ZV(op2_addr)))) {
3863+
| GET_ZVAL_LVAL result_reg, op1_addr, TMP1
3864+
| mov TMP1, #zend_long_floor_log2(Z_LVAL_P(Z_ZV(op2_addr)))
3865+
| lsl Rx(result_reg), Rx(result_reg), TMP1
3866+
} else if (opcode == ZEND_MUL &&
3867+
Z_MODE(op1_addr) == IS_CONST_ZVAL &&
3868+
Z_LVAL_P(Z_ZV(op1_addr)) == 2) {
3869+
if (Z_MODE(op2_addr) == IS_REG) {
3870+
| adds Rx(result_reg), Rx(Z_REG(op2_addr)), Rx(Z_REG(op2_addr))
38853871
} else {
3886-
| GET_ZVAL_LVAL ZREG_TMP1, op2_addr, TMP2
3887-
| mov TMP2, #zend_long_floor_log2(Z_LVAL_P(Z_ZV(op1_addr)))
3888-
| lsl Rx(result_reg), TMP1, TMP2
3889-
if (may_overflow) {
3890-
/* Compare 'op' and '((op << n) >> n)' for overflow.
3891-
* Flag: bne -> overflow. beq -> no overflow.
3892-
*/
3893-
use_ovf_flag = 0;
3894-
| asr TMP3, Rx(result_reg), TMP2
3895-
| cmp TMP1, TMP3
3896-
}
3872+
| GET_ZVAL_LVAL result_reg, op2_addr, TMP1
3873+
| adds Rx(result_reg), Rx(result_reg), Rx(result_reg)
38973874
}
3875+
} else if (opcode == ZEND_MUL &&
3876+
Z_MODE(op1_addr) == IS_CONST_ZVAL &&
3877+
!may_overflow &&
3878+
zend_long_is_power_of_two(Z_LVAL_P(Z_ZV(op1_addr)))) {
3879+
| GET_ZVAL_LVAL result_reg, op2_addr, TMP1
3880+
| mov TMP1, #zend_long_floor_log2(Z_LVAL_P(Z_ZV(op1_addr)))
3881+
| lsl Rx(result_reg), Rx(result_reg), TMP1
38983882
} else if (opcode == ZEND_DIV &&
38993883
(Z_MODE(op2_addr) == IS_CONST_ZVAL &&
39003884
is_power_of_two(Z_LVAL_P(Z_ZV(op2_addr))))) {

‎ext/opcache/jit/zend_jit_x86.dasc

Lines changed: 28 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -4180,7 +4180,6 @@ static int zend_jit_math_long_long(dasm_State **Dst,
41804180
bool same_ops = zend_jit_same_addr(op1_addr, op2_addr);
41814181
zend_reg result_reg;
41824182
zend_reg tmp_reg = ZREG_R0;
4183-
bool use_ovf_flag = 1;
41844183

41854184
if (Z_MODE(res_addr) == IS_REG && (res_info & MAY_BE_LONG)) {
41864185
if (may_overflow && (res_info & MAY_BE_GUARD)
@@ -4201,67 +4200,39 @@ static int zend_jit_math_long_long(dasm_State **Dst,
42014200
}
42024201

42034202
if (opcode == ZEND_MUL &&
4204-
Z_MODE(op2_addr) == IS_CONST_ZVAL &&
4205-
Z_LVAL_P(Z_ZV(op2_addr)) > 0 &&
4206-
IS_SIGNED_32BIT(Z_LVAL_P(Z_ZV(op2_addr))) &&
4207-
is_power_of_two(Z_LVAL_P(Z_ZV(op2_addr)))) {
4208-
4209-
if (Z_MODE(op1_addr) == IS_REG && Z_LVAL_P(Z_ZV(op2_addr)) == 2) {
4203+
Z_MODE(op2_addr) == IS_CONST_ZVAL &&
4204+
Z_LVAL_P(Z_ZV(op2_addr)) == 2) {
4205+
if (Z_MODE(op1_addr) == IS_REG && !may_overflow) {
42104206
| lea Ra(result_reg), [Ra(Z_REG(op1_addr))+Ra(Z_REG(op1_addr))]
42114207
} else {
42124208
| GET_ZVAL_LVAL result_reg, op1_addr
4213-
| shl Ra(result_reg), floor_log2(Z_LVAL_P(Z_ZV(op2_addr)))
4214-
if (may_overflow) {
4215-
/* Compare 'op' and '((op << n) >> n)' for overflow.
4216-
* Flag: jne -> overflow. je -> no overflow.
4217-
*/
4218-
use_ovf_flag = 0;
4219-
| sar Ra(result_reg), floor_log2(Z_LVAL_P(Z_ZV(op2_addr)))
4220-
if (Z_MODE(op1_addr) == IS_CONST_ZVAL) {
4221-
| cmp Ra(result_reg), Z_LVAL_P(Z_ZV(op1_addr))
4222-
} else if (Z_MODE(op1_addr) == IS_MEM_ZVAL) {
4223-
| cmp Ra(result_reg), [Ra(Z_REG(op1_addr))+Z_OFFSET(op1_addr)]
4224-
} else if (Z_MODE(op1_addr) == IS_REG) {
4225-
| cmp Ra(result_reg), Ra(Z_REG(op1_addr))
4226-
} else {
4227-
ZEND_UNREACHABLE();
4228-
}
4229-
| pushf
4230-
| shl Ra(result_reg), floor_log2(Z_LVAL_P(Z_ZV(op2_addr)))
4231-
| popf
4232-
}
4209+
| add Ra(result_reg), Ra(result_reg)
42334210
}
42344211
} else if (opcode == ZEND_MUL &&
4235-
Z_MODE(op1_addr) == IS_CONST_ZVAL &&
4236-
Z_LVAL_P(Z_ZV(op1_addr)) > 0 &&
4237-
IS_SIGNED_32BIT(Z_LVAL_P(Z_ZV(op1_addr))) &&
4238-
is_power_of_two(Z_LVAL_P(Z_ZV(op1_addr)))) {
4239-
4240-
if (Z_MODE(op2_addr) == IS_REG && Z_LVAL_P(Z_ZV(op1_addr)) == 2) {
4212+
Z_MODE(op2_addr) == IS_CONST_ZVAL &&
4213+
!may_overflow &&
4214+
Z_LVAL_P(Z_ZV(op2_addr)) > 0 &&
4215+
IS_SIGNED_32BIT(Z_LVAL_P(Z_ZV(op2_addr))) &&
4216+
is_power_of_two(Z_LVAL_P(Z_ZV(op2_addr)))) {
4217+
| GET_ZVAL_LVAL result_reg, op1_addr
4218+
| shl Ra(result_reg), floor_log2(Z_LVAL_P(Z_ZV(op2_addr)))
4219+
} else if (opcode == ZEND_MUL &&
4220+
Z_MODE(op1_addr) == IS_CONST_ZVAL &&
4221+
Z_LVAL_P(Z_ZV(op1_addr)) == 2) {
4222+
if (Z_MODE(op2_addr) == IS_REG && !may_overflow) {
42414223
| lea Ra(result_reg), [Ra(Z_REG(op2_addr))+Ra(Z_REG(op2_addr))]
42424224
} else {
42434225
| GET_ZVAL_LVAL result_reg, op2_addr
4244-
| shl Ra(result_reg), floor_log2(Z_LVAL_P(Z_ZV(op1_addr)))
4245-
if (may_overflow) {
4246-
/* Compare 'op' and '((op << n) >> n)' for overflow.
4247-
* Flag: jne -> overflow. je -> no overflow.
4248-
*/
4249-
use_ovf_flag = 0;
4250-
| sar Ra(result_reg), floor_log2(Z_LVAL_P(Z_ZV(op1_addr)))
4251-
if (Z_MODE(op2_addr) == IS_CONST_ZVAL) {
4252-
| cmp Ra(result_reg), Z_LVAL_P(Z_ZV(op2_addr))
4253-
} else if (Z_MODE(op2_addr) == IS_MEM_ZVAL) {
4254-
| cmp Ra(result_reg), [Ra(Z_REG(op2_addr))+Z_OFFSET(op2_addr)]
4255-
} else if (Z_MODE(op2_addr) == IS_REG) {
4256-
| cmp Ra(result_reg), Ra(Z_REG(op2_addr))
4257-
} else {
4258-
ZEND_UNREACHABLE();
4259-
}
4260-
| pushf
4261-
| shl Ra(result_reg), floor_log2(Z_LVAL_P(Z_ZV(op1_addr)))
4262-
| popf
4263-
}
4226+
| lea Ra(result_reg), [Ra(result_reg)+Ra(result_reg)]
42644227
}
4228+
} else if (opcode == ZEND_MUL &&
4229+
Z_MODE(op1_addr) == IS_CONST_ZVAL &&
4230+
!may_overflow &&
4231+
Z_LVAL_P(Z_ZV(op1_addr)) > 0 &&
4232+
IS_SIGNED_32BIT(Z_LVAL_P(Z_ZV(op1_addr))) &&
4233+
is_power_of_two(Z_LVAL_P(Z_ZV(op1_addr)))) {
4234+
| GET_ZVAL_LVAL result_reg, op2_addr
4235+
| shl Ra(result_reg), floor_log2(Z_LVAL_P(Z_ZV(op1_addr)))
42654236
} else if (opcode == ZEND_DIV &&
42664237
(Z_MODE(op2_addr) == IS_CONST_ZVAL &&
42674238
is_power_of_two(Z_LVAL_P(Z_ZV(op2_addr))))) {
@@ -4300,36 +4271,20 @@ static int zend_jit_math_long_long(dasm_State **Dst,
43004271
int32_t exit_point = zend_jit_trace_get_exit_point(opline, 0);
43014272
const void *exit_addr = zend_jit_trace_get_exit_addr(exit_point);
43024273
if ((res_info & MAY_BE_ANY) == MAY_BE_LONG) {
4303-
if (use_ovf_flag) {
4304-
| jo &exit_addr
4305-
} else {
4306-
| jne &exit_addr
4307-
}
4274+
| jo &exit_addr
43084275
if (Z_MODE(res_addr) == IS_REG && result_reg != Z_REG(res_addr)) {
43094276
| mov Ra(Z_REG(res_addr)), Ra(result_reg)
43104277
}
43114278
} else if ((res_info & MAY_BE_ANY) == MAY_BE_DOUBLE) {
4312-
if (use_ovf_flag) {
4313-
| jno &exit_addr
4314-
} else {
4315-
| je &exit_addr
4316-
}
4279+
| jno &exit_addr
43174280
} else {
43184281
ZEND_UNREACHABLE();
43194282
}
43204283
} else {
43214284
if (res_info & MAY_BE_LONG) {
4322-
if (use_ovf_flag) {
4323-
| jo >1
4324-
} else {
4325-
| jne >1
4326-
}
4285+
| jo >1
43274286
} else {
4328-
if (use_ovf_flag) {
4329-
| jno >1
4330-
} else {
4331-
| je >1
4332-
}
4287+
| jno >1
43334288
}
43344289
}
43354290
}

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ function mul2_big_int64(int $a) {
3535
var_dump($res);
3636
}
3737

38+
function mul2(int $a) {
39+
$res = $a * 2; // $a + $a
40+
var_dump($res);
41+
}
42+
3843
mul2_8(3);
3944
mul2_8(-11);
4045
mul2_8(0x7fffffffffffffff);
@@ -47,6 +52,8 @@ mul2_big_int32(0x10000000000);
4752
mul2_big_int64(3);
4853
mul2_big_int64(-3);
4954
mul2_big_int64(0x100000000);
55+
mul2(10);
56+
mul2(0x7fffffffffffffff);
5057
?>
5158
--EXPECT--
5259
int(24)
@@ -60,4 +67,6 @@ int(-805306368)
6067
float(2.9514790517935283E+20)
6168
int(12884901888)
6269
int(-12884901888)
63-
float(1.8446744073709552E+19)
70+
float(1.8446744073709552E+19)
71+
int(20)
72+
float(1.8446744073709552E+19)

0 commit comments

Comments
 (0)