Skip to content

Commit a6ee6af

Browse files
shqkingdstogov
authored andcommitted
Support failed JIT test case: cmp_001.phpt
Comparison between LONG and DOUBLE is (partially) supported in a similar way to comparison between two LONG values. See the updates in function zend_jit_cmp(). Key difference lies in handling NaN. 1. Instruction 'fcmp' is used to substitue 'ucomisd' in x86 implementation. Both of them raise invalid operation exception only when either source operand is an SNaN.[1][2] 2. Parity flag is used in x86 to check whether either operand is NaN.[3] I think this is QNaN case. As for AArch64, we use instruction 'bvs'.[4] It's worthing noting that condition codes have different meanings for floating-point comparions(e.g. 'fcmp')[4] compared to the general-purpose comparisons(e.g. 'cmp').[5] For instance, 'b.hs' after 'fcmp' can check not only the cases "greater than, equal to" but also the case "unordered"(that is NaN). We may simply treat it as a combination of 'jae' and 'jp' in x86. 3. Instruction 'SETcc' is used in x86 for the case of ">=" or ">". Note that flag "swap" is set in implementation, and it falls into cases ZEND_IS_SMALLER or ZEND_IS_SMALLER_OR_EQUAL. We can use 'cset' in AArch64. However, it's weird that the NaN check is missing in x86. I suppose it might be a bug. Take the case ">=" as an example. The two operands can be either DOUBLE + LONG or DOUBLE + DOUBLE. See the relevant code where flag "swap" is set(i.e. function zend_jit_cmp_double_long() and function zend_jit_cmp_double_double()). For the case "NaN >= 1.0", the expected result should be FALSE, however, JIT/x86 would produce TRUE due to the following "setae al". Unfortunately I haven't constructed one test case to trigger that. In our implementation, we choose to follow the case of "<" or "<=", and I believe our implementation is safe anyway.. 4. Temporary FP register is also needed and we reserve v16. See the updates in file zend_jit_arm64.h. 5. Macro SET_ZVAL_TYPE_INFO_FROM_REG is misused in function zend_jit_zval_copy_deref(). The second argument should be 32-bit long and we fix it. Note that simple test cases involving NaN are tested locally. I believe it would get deeper testing by cmp_003.phpt(we will support it later). [1] https://developer.arm.com/documentation/dui0204/f/vector-floating-point-programming/vfp-instructions/fcmp?lang=en [2] https://www.felixcloutier.com/x86/ucomisd [3] https://en.wikipedia.org/wiki/Parity_flag [4] https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/condition-codes-4-floating-point-comparisons-using-vfp [5] https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/condition-codes-1-condition-flags-and-codes
1 parent b3e2178 commit a6ee6af

File tree

2 files changed

+123
-7
lines changed

2 files changed

+123
-7
lines changed

ext/opcache/jit/zend_jit_arm64.dasc

Lines changed: 121 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,13 @@
7979
|.define TMP3w, w13
8080
|.define TMP4, x14
8181
|.define TMP4w, w14
82+
|.define FPTMP, v16
8283

8384
|.define ZREG_TMP1, ZREG_X11
8485
|.define ZREG_TMP2, ZREG_X12
8586
|.define ZREG_TMP3, ZREG_X13
8687
|.define ZREG_TMP4, ZREG_X14
88+
|.define ZREG_FPTMP, ZREG_V16
8789

8890
|.define HYBRID_SPAD, #16 // padding for stack alignment
8991

@@ -533,6 +535,42 @@ static void* dasm_labels[zend_lb_MAX];
533535
| brk #0 // TODO
534536
|.endmacro
535537

538+
// Define DOUBLE_CMP and DOUBLE_CMP to replace SSE_AVX_OP and SSE_OP in x86 implementation.
539+
// Conduct floating point operation 'ins'. Operand1 is from 'reg', and operands2 is from 'addr'.
540+
// Use DOUBLE_CMP for comparisons and use DOUBLE_OP for arithmetic operations.
541+
|.macro DOUBLE_CMP, ins, reg, addr, tmp_reg, fp_tmp_reg
542+
|| if (Z_MODE(addr) == IS_CONST_ZVAL) {
543+
| brk #0 // TODO
544+
| LOAD_ADDR Rx(tmp_reg), Z_ZV(addr)
545+
| ldr Rd(fp_tmp_reg-ZREG_V0), [Rx(tmp_reg)]
546+
| ins Rd(reg-ZREG_V0), Rd(fp_tmp_reg-ZREG_V0)
547+
|| } else if (Z_MODE(addr) == IS_MEM_ZVAL) {
548+
| SAFE_MEM_ACC_WITH_UOFFSET ldr, Rd(fp_tmp_reg-ZREG_V0), Rx(Z_REG(addr)), Z_OFFSET(addr), Rx(tmp_reg)
549+
| ins Rd(reg-ZREG_V0), Rd(fp_tmp_reg-ZREG_V0)
550+
|| } else if (Z_MODE(addr) == IS_REG) {
551+
| brk #0 // TODO
552+
| ins Rd(reg-ZREG_V0), Rd(Z_REG(addr)-ZREG_V0)
553+
|| } else {
554+
|| ZEND_UNREACHABLE();
555+
|| }
556+
|.endmacro
557+
558+
|.macro DOUBLE_OP, ins, reg, addr, tmp_reg, fp_tmp_reg
559+
| brk #0 // TODO
560+
|| if (Z_MODE(addr) == IS_CONST_ZVAL) {
561+
| LOAD_ADDR Rx(tmp_reg), Z_ZV(addr)
562+
| ldr Rd(fp_tmp_reg-ZREG_V0), [Rx(tmp_reg)]
563+
| ins Rd(reg-ZREG_V0), Rd(reg-ZREG_V0), Rd(fp_tmp_reg-ZREG_V0)
564+
|| } else if (Z_MODE(addr) == IS_MEM_ZVAL) {
565+
| SAFE_MEM_ACC_WITH_UOFFSET ldr, Rd(fp_tmp_reg-ZREG_V0), Rx(Z_REG(addr)), Z_OFFSET(addr), Rx(tmp_reg)
566+
| ins Rd(reg-ZREG_V0), Rd(reg-ZREG_V0), Rd(fp_tmp_reg-ZREG_V0)
567+
|| } else if (Z_MODE(addr) == IS_REG) {
568+
| ins Rd(reg-ZREG_V0), Rd(reg-ZREG_V0), Rd(Z_REG(addr)-ZREG_V0)
569+
|| } else {
570+
|| ZEND_UNREACHABLE();
571+
|| }
572+
|.endmacro
573+
536574
// Define DOUBLE_GET_LONG to replace SSE_GET_LONG in x86 implementation.
537575
// Convert the LONG value 'lval' into DOUBLE type, and move it into 'reg'
538576
|.macro DOUBLE_GET_LONG, reg, lval, tmp_reg
@@ -4281,7 +4319,49 @@ static int zend_jit_cmp_long_long(dasm_State **Dst,
42814319

42824320
static int zend_jit_cmp_double_common(dasm_State **Dst, const zend_op *opline, zend_jit_addr res_addr, bool swap, zend_uchar smart_branch_opcode, uint32_t target_label, uint32_t target_label2, const void *exit_addr)
42834321
{
4284-
| brk #0 // TODO
4322+
if (smart_branch_opcode) {
4323+
| brk #0 // TODO
4324+
} else {
4325+
switch (opline->opcode) {
4326+
case ZEND_IS_EQUAL:
4327+
case ZEND_IS_IDENTICAL:
4328+
case ZEND_CASE:
4329+
case ZEND_CASE_STRICT:
4330+
| brk #0 // TODO
4331+
break;
4332+
case ZEND_IS_NOT_EQUAL:
4333+
case ZEND_IS_NOT_IDENTICAL:
4334+
| brk #0 // TODO
4335+
break;
4336+
case ZEND_IS_SMALLER:
4337+
| bvs >1
4338+
| mov REG0, #IS_TRUE
4339+
|| if (swap) {
4340+
| bhi >2 // TODO: why the NaN check is missing in x86?
4341+
|| } else {
4342+
| blo >2
4343+
|| }
4344+
|1:
4345+
| mov REG0, #IS_FALSE
4346+
|2:
4347+
break;
4348+
case ZEND_IS_SMALLER_OR_EQUAL:
4349+
| bvs >1
4350+
| mov REG0, #IS_TRUE
4351+
|| if (swap) {
4352+
| bhs >2 // TODO: why the NaN check is missing in x86?
4353+
|| } else {
4354+
| bls >2
4355+
|| }
4356+
|1:
4357+
| mov REG0, #IS_FALSE
4358+
|2:
4359+
break;
4360+
default:
4361+
ZEND_UNREACHABLE();
4362+
}
4363+
| SET_ZVAL_TYPE_INFO_FROM_REG res_addr, REG0w, TMP1
4364+
}
42854365

42864366
return 1;
42874367
}
@@ -4290,7 +4370,8 @@ static int zend_jit_cmp_long_double(dasm_State **Dst, const zend_op *opline, zen
42904370
{
42914371
zend_reg tmp_reg = ZREG_FPR0;
42924372

4293-
| brk #0 // TODO
4373+
| DOUBLE_GET_ZVAL_LVAL tmp_reg, op1_addr, ZREG_REG0, ZREG_TMP1
4374+
| DOUBLE_CMP fcmp, tmp_reg, op2_addr, ZREG_TMP1, ZREG_FPTMP
42944375

42954376
return zend_jit_cmp_double_common(Dst, opline, res_addr, 0, smart_branch_opcode, target_label, target_label2, exit_addr);
42964377
}
@@ -4299,7 +4380,8 @@ static int zend_jit_cmp_double_long(dasm_State **Dst, const zend_op *opline, zen
42994380
{
43004381
zend_reg tmp_reg = ZREG_FPR0;
43014382

4302-
| brk #0 // TODO
4383+
| DOUBLE_GET_ZVAL_LVAL tmp_reg, op2_addr, ZREG_REG0, ZREG_TMP1
4384+
| DOUBLE_CMP fcmp, tmp_reg, op1_addr, ZREG_TMP1, ZREG_FPTMP
43034385

43044386
return zend_jit_cmp_double_common(Dst, opline, res_addr, /* swap */ 1, smart_branch_opcode, target_label, target_label2, exit_addr);
43054387
}
@@ -4358,7 +4440,13 @@ static int zend_jit_cmp(dasm_State **Dst,
43584440
| IF_NOT_ZVAL_TYPE op2_addr, IS_LONG, >3, TMP1w, TMP2
43594441
|.cold_code
43604442
|3:
4361-
| brk #0 // TODO
4443+
if (op2_info & (MAY_BE_ANY-(MAY_BE_LONG|MAY_BE_DOUBLE))) {
4444+
| IF_NOT_ZVAL_TYPE op2_addr, IS_DOUBLE, >9, TMP1w, TMP2
4445+
}
4446+
if (!zend_jit_cmp_long_double(Dst, opline, op1_addr, op2_addr, res_addr, smart_branch_opcode, target_label, target_label2, exit_addr)) {
4447+
return 0;
4448+
}
4449+
| b >6
43624450
|.code
43634451
} else {
43644452
| brk #0 // TODO
@@ -4370,7 +4458,34 @@ static int zend_jit_cmp(dasm_State **Dst,
43704458
if (op1_info & MAY_BE_DOUBLE) {
43714459
|.cold_code
43724460
|4:
4373-
| brk #0 // TODO
4461+
if (op1_info & (MAY_BE_ANY-(MAY_BE_LONG|MAY_BE_DOUBLE))) {
4462+
| IF_NOT_ZVAL_TYPE op1_addr, IS_DOUBLE, >9, TMP1w, TMP2
4463+
}
4464+
if (op2_info & MAY_BE_DOUBLE) {
4465+
if (!same_ops && (op2_info & (MAY_BE_ANY-MAY_BE_DOUBLE))) {
4466+
if (!same_ops) {
4467+
| IF_NOT_ZVAL_TYPE op2_addr, IS_DOUBLE, >5, TMP1w, TMP2
4468+
} else {
4469+
| brk #0 // TODO
4470+
| IF_NOT_ZVAL_TYPE op2_addr, IS_DOUBLE, >9, TMP1w, TMP2
4471+
}
4472+
}
4473+
| brk #0 // TODO
4474+
if (!zend_jit_cmp_double_double(Dst, opline, op1_addr, op2_addr, res_addr, smart_branch_opcode, target_label, target_label2, exit_addr)) {
4475+
return 0;
4476+
}
4477+
| b >6
4478+
}
4479+
if (!same_ops) {
4480+
|5:
4481+
if (op2_info & (MAY_BE_ANY-(MAY_BE_LONG|MAY_BE_DOUBLE))) {
4482+
| IF_NOT_ZVAL_TYPE op2_addr, IS_LONG, >9, TMP1w, TMP2
4483+
}
4484+
if (!zend_jit_cmp_double_long(Dst, opline, op1_addr, op2_addr, res_addr, smart_branch_opcode, target_label, target_label2, exit_addr)) {
4485+
return 0;
4486+
}
4487+
| b >6
4488+
}
43744489
|.code
43754490
}
43764491
} else if ((op1_info & MAY_BE_DOUBLE) &&
@@ -6469,7 +6584,7 @@ static int zend_jit_zval_copy_deref(dasm_State **Dst, zend_jit_addr res_addr, ze
64696584
| GC_ADDREF REG1, TMP1w
64706585
|2:
64716586
| SET_ZVAL_PTR res_addr, REG1, TMP1
6472-
| SET_ZVAL_TYPE_INFO_FROM_REG res_addr, REG2, TMP1
6587+
| SET_ZVAL_TYPE_INFO_FROM_REG res_addr, REG2w, TMP1
64736588

64746589
return 1;
64756590
}

ext/opcache/jit/zend_jit_arm64.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ typedef struct _zend_jit_registers_buf {
129129
#define ZREG_TMP2 ZREG_X12
130130
#define ZREG_TMP3 ZREG_X13
131131
#define ZREG_TMP4 ZREG_X14
132+
#define ZREG_FPTMP ZREG_V16
132133

133134
#define ZREG_COPY ZREG_REG0
134135
#define ZREG_FIRST_FPR ZREG_V0
@@ -138,7 +139,7 @@ typedef uint64_t zend_regset;
138139
# define ZEND_REGSET_FIXED \
139140
(ZEND_REGSET(ZREG_RSP) | ZEND_REGSET(ZREG_RLR) | ZEND_REGSET(ZREG_RFP) | \
140141
ZEND_REGSET(ZREG_RPR) | ZEND_REGSET(ZREG_FP) | ZEND_REGSET(ZREG_IP) | \
141-
ZEND_REGSET_INTERVAL(ZREG_TMP1, ZREG_TMP4))
142+
ZEND_REGSET_INTERVAL(ZREG_TMP1, ZREG_TMP4) | ZEND_REGSET(ZREG_FPTMP))
142143
# define ZEND_REGSET_GP \
143144
ZEND_REGSET_DIFFERENCE(ZEND_REGSET_INTERVAL(ZREG_X0, ZREG_X30), ZEND_REGSET_FIXED)
144145
# define ZEND_REGSET_FP \

0 commit comments

Comments
 (0)