Skip to content

JIT/AArch64: Support shifted immediate #7165

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.

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

Merged
merged 2 commits into from
Jun 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
41 changes: 24 additions & 17 deletions ext/opcache/jit/zend_jit_arm64.dasc
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,10 @@ typedef struct TLVDescriptor TLVDescriptor;

#define IS_SIGNED_32BIT(val) ((((intptr_t)(val)) <= 0x7fffffff) && (((intptr_t)(val)) >= (-2147483647 - 1)))

/* Encoding of immediate. TODO: shift mode may be supported in the near future. */
/* Encoding of immediate. */
#define MAX_IMM12 0xfff // maximum value for imm12
#define MAX_IMM16 0xffff // maximum value for imm16
#define CMP_IMM MAX_IMM12 // cmp insn
#define MOVZ_IMM MAX_IMM16 // movz insn
#define ADD_SUB_IMM MAX_IMM12 // add/sub/adds/subs insn
#define LDR_STR_PIMM64 (MAX_IMM12*8) // ldr/str insn for 64-bit register: pimm is imm12 * 8
#define LDR_STR_PIMM32 (MAX_IMM12*4) // ldr/str insn for 32-bit register: pimm is imm12 * 4
#define LDRB_STRB_PIMM MAX_IMM12 // ldrb/strb insn
Expand Down Expand Up @@ -172,6 +170,15 @@ static bool arm64_may_use_adrp(const void *addr)
return 0;
}

/* Determine whether "val" falls into two allowed ranges:
* Range 1: [0, 0xfff]
* Range 2: LSL #12 to Range 1
* Used to guard the immediate encoding for add/adds/sub/subs/cmp/cmn instructions. */
static bool arm64_may_encode_imm12(const int64_t val)
{
return (val >= 0 && (val <= MAX_IMM12 || !(val & 0xffffffffff000fff)));
}

/* Determine whether an immediate value can be encoded as the immediate operand of logical instructions. */
static bool logical_immediate_p(uint64_t value, uint32_t reg_size)
{
Expand Down Expand Up @@ -364,9 +371,9 @@ static bool logical_immediate_p(uint64_t value, uint32_t reg_size)
|.macro CMP_32_WITH_CONST, reg, val, tmp_reg
|| if (val == 0) {
| cmp reg, wzr
|| } else if (((int32_t)(val)) > 0 && ((int32_t)(val)) <= CMP_IMM) {
|| } else if (arm64_may_encode_imm12((int64_t)(val))) {
| cmp reg, #val
|| } else if (((int32_t)(val)) < 0 && ((int32_t)(val)) >= -CMP_IMM) {
|| } else if (arm64_may_encode_imm12((int64_t)(-val))) {
| cmn reg, #-val
|| } else {
| LOAD_32BIT_VAL tmp_reg, val
Expand All @@ -377,9 +384,9 @@ static bool logical_immediate_p(uint64_t value, uint32_t reg_size)
|.macro CMP_64_WITH_CONST_32, reg, val, tmp_reg
|| if (val == 0) {
| cmp reg, xzr
|| } else if (((int32_t)(val)) > 0 && ((int32_t)(val)) <= CMP_IMM) {
|| } else if (arm64_may_encode_imm12((int64_t)(val))) {
| cmp reg, #val
|| } else if (((int32_t)(val)) < 0 && ((int32_t)(val)) >= -CMP_IMM) {
|| } else if (arm64_may_encode_imm12((int64_t)(-val))) {
| cmn reg, #-val
|| } else {
| LOAD_32BIT_VAL tmp_reg, val
Expand All @@ -390,9 +397,9 @@ static bool logical_immediate_p(uint64_t value, uint32_t reg_size)
|.macro CMP_64_WITH_CONST, reg, val, tmp_reg
|| if (val == 0) {
| cmp reg, xzr
|| } else if (((int64_t)(val)) > 0 && ((int64_t)(val)) <= CMP_IMM) {
|| } else if (arm64_may_encode_imm12((int64_t)(val))) {
| cmp reg, #val
|| } else if (((int64_t)(val)) < 0 && ((int64_t)(val)) >= -CMP_IMM) {
|| } else if (arm64_may_encode_imm12((int64_t)(-val))) {
Copy link
Member

Choose a reason for hiding this comment

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

Did you test this?
In any case, it would be great to add test(s) for edge cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have run ~4k .phpt test cases under "Zend/tests/, tests/ ext/opcache/tests/jit/", and didn't find any failure.

Good suggestion! I will add several new test cases soon. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Two test cases are added, and in my local testing, immediate values in legitimate ranges can be encoded as the imm field as expected.

  1. for cmp and cmn instructions, I only tested CMP_64_WITH_CONST. Note: I tested that negative values can be encoded into the imm field fo cmn instruction.
  2. for ADD and SUB, I only tested adds and subs with macro ADD_SUB_64_WITH_CONST.

I currently failed to construct test cases to use CMP_32_WITH_CONST, CMP_64_WITH_CONST_32, ADD_SUB_32_WITH_CONST, ADD_SUB_64_WITH_CONST_32, but I think arm64_may_encode_imm12 would work for them as well since these 4 macros share the same encoding logic of imm12 with CMP_64_WITH_CONST and ADD_SUB_64_WITH_CONST.

| cmn reg, #-val
|| } else {
| LOAD_64BIT_VAL tmp_reg, val
Expand All @@ -406,7 +413,7 @@ static bool logical_immediate_p(uint64_t value, uint32_t reg_size)
|.macro ADD_SUB_32_WITH_CONST, add_sub_ins, dst_reg, src_reg1, val, tmp_reg
|| if (val == 0) {
| add_sub_ins dst_reg, src_reg1, wzr
|| } else if (((int32_t)(val)) > 0 && ((int32_t)(val)) <= ADD_SUB_IMM) {
|| } else if (arm64_may_encode_imm12((int64_t)(val))) {
| add_sub_ins dst_reg, src_reg1, #val
|| } else {
| LOAD_32BIT_VAL tmp_reg, val
Expand All @@ -417,7 +424,7 @@ static bool logical_immediate_p(uint64_t value, uint32_t reg_size)
|.macro ADD_SUB_64_WITH_CONST_32, add_sub_ins, dst_reg, src_reg1, val, tmp_reg
|| if (val == 0) {
| add_sub_ins dst_reg, src_reg1, xzr
|| } else if (((int32_t)(val)) > 0 && ((int32_t)(val)) <= ADD_SUB_IMM) {
|| } else if (arm64_may_encode_imm12((int64_t)(val))) {
| add_sub_ins dst_reg, src_reg1, #val
|| } else {
| LOAD_32BIT_VAL tmp_reg, val
Expand All @@ -428,7 +435,7 @@ static bool logical_immediate_p(uint64_t value, uint32_t reg_size)
|.macro ADD_SUB_64_WITH_CONST, add_sub_ins, dst_reg, src_reg1, val, tmp_reg
|| if (val == 0) {
| add_sub_ins dst_reg, src_reg1, xzr
|| } else if (((int64_t)(val)) > 0 && ((int64_t)(val)) <= ADD_SUB_IMM) {
|| } else if (arm64_may_encode_imm12((int64_t)(val))) {
| add_sub_ins dst_reg, src_reg1, #val
|| } else {
| LOAD_64BIT_VAL tmp_reg, val
Expand Down Expand Up @@ -723,7 +730,7 @@ static bool logical_immediate_p(uint64_t value, uint32_t reg_size)

/* Update IP with 32-bit immediate 'val'. */
|.macro ADD_IP_WITH_CONST, val, tmp_reg
|| ZEND_ASSERT(val >= 0 && val <= ADD_SUB_IMM);
|| ZEND_ASSERT(arm64_may_encode_imm12((int64_t)(val)));
|| if (GCC_GLOBAL_REGS) {
| add IP, IP, #val
|| } else {
Expand Down Expand Up @@ -931,7 +938,7 @@ static bool logical_immediate_p(uint64_t value, uint32_t reg_size)
* the computation result is stored back into 'addr'.
* Note: it should be guaranteed that 'val' can be encoded into add/sub instruction. */
|.macro LONG_ADD_SUB_WITH_IMM, add_sub_ins, addr, val, tmp_reg1, tmp_reg2
|| ZEND_ASSERT(val >= 0 && val <= ADD_SUB_IMM);
|| ZEND_ASSERT(arm64_may_encode_imm12((int64_t)(val)));
|| if (Z_MODE(addr) == IS_MEM_ZVAL) {
|| if (((uint32_t)(Z_OFFSET(addr))) > LDR_STR_PIMM64) {
| LOAD_32BIT_VAL tmp_reg2, Z_OFFSET(addr)
Expand Down Expand Up @@ -8260,7 +8267,7 @@ static int zend_jit_push_call_frame(dasm_State **Dst, const zend_op *opline, con
}

if (func) {
|| if (used_stack <= ADD_SUB_IMM) {
|| if (arm64_may_encode_imm12((int64_t)used_stack)) {
| MEM_UPDATE_ZTS add, ldr, str, #used_stack, executor_globals, vm_stack_top, REG2, TMP1
|| } else {
| LOAD_32BIT_VAL TMP1w, used_stack
Expand Down Expand Up @@ -9104,7 +9111,7 @@ static int zend_jit_do_fcall(dasm_State **Dst, const zend_op *opline, const zend
| LOAD_IP_ADDR (func->op_array.opcodes + num_args)
} else {
| ldr REG0, EX->func
|| ZEND_ASSERT((num_args * sizeof(zend_op)) <= ADD_SUB_IMM);
|| ZEND_ASSERT(arm64_may_encode_imm12((int64_t)(num_args * sizeof(zend_op))));
if (GCC_GLOBAL_REGS) {
| ldr IP, [REG0, #offsetof(zend_op_array, opcodes)]
if (num_args) {
Expand Down Expand Up @@ -9210,7 +9217,7 @@ static int zend_jit_do_fcall(dasm_State **Dst, const zend_op *opline, const zend
| ble >3
| // zval *var = EX_VAR_NUM(num_args);
| add REG1, FP, REG1, lsl #4
|| ZEND_ASSERT(ZEND_CALL_FRAME_SLOT * sizeof(zval) <= ADD_SUB_IMM);
|| ZEND_ASSERT(arm64_may_encode_imm12((int64_t)(ZEND_CALL_FRAME_SLOT * sizeof(zval))));
| add REG1, REG1, #(ZEND_CALL_FRAME_SLOT * sizeof(zval))
|2:
| SET_Z_TYPE_INFO REG1, IS_UNDEF, TMP1w
Expand Down
67 changes: 67 additions & 0 deletions ext/opcache/tests/jit/add_007.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
--TEST--
JIT ADD: 007 Addition with immediate values
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.file_update_protection=0
opcache.jit_buffer_size=1M
opcache.protect_memory=1
--EXTENSIONS--
opcache
--SKIPIF--
<?php
if (PHP_INT_SIZE != 8) die("skip: 64-bit only"); ?>
--FILE--
<?php
function foo($a) {
$b = 0;
$c = 31;
$d = 0xfff;
$e = 0x1000;
$f = 0xfff000;
$g = 0xff001; // Cannot be encoded into imm12 field
$h = 0x1000000; // Cannot be encoded into imm12 field
$i = 0xf12345678; // Cannot be encoded into imm12 field
$j = -31; // Cannot be encoded into imm12 field

$a = $a + $b;
$a = $a + $c;
$a = $a + $d;
$a = $a + $e;
$a = $a + $f;
$a = $a + $g;
$a = $a + $h;
$a = $a + $i;
$a = $a + $j;
var_dump($a);
}

function bar($a) {
$b = 0;
$c = 31;
$d = 0xfff;
$e = 0x1000;
$f = 0xfff000;
$g = 0xff001; // Cannot be encoded into imm12 field
$h = 0x1000000; // Cannot be encoded into imm12 field
$i = 0xf12345678; // Cannot be encoded into imm12 field
$j = -31; // Cannot be encoded into imm12 field

$a = $a - $b;
$a = $a - $c;
$a = $a - $d;
$a = $a - $e;
$a = $a - $f;
$a = $a - $g;
$a = $a - $h;
$a = $a - $i;
$a = $a - $j;
var_dump($a);
}

foo(42);
bar(0x1f12345678);
?>
--EXPECT--
int(64764532386)
int(68684873728)
75 changes: 75 additions & 0 deletions ext/opcache/tests/jit/cmp_005.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
--TEST--
JIT CMP: 005 Comparisons with immediate values
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.file_update_protection=0
opcache.jit_buffer_size=1M
opcache.protect_memory=1
--EXTENSIONS--
opcache
--SKIPIF--
<?php
if (PHP_INT_SIZE != 8) die("skip: 64-bit only"); ?>
--FILE--
<?php
function foo($a) {
$b = 0;
$c = 31;
$d = 0xfff;
$e = 0x1000;
$f = 0xfff000;
$g = 0xff001; // Cannot be encoded into imm12 field
$h = 0x1000000; // Cannot be encoded into imm12 field
$i = 0xf12345678; // Cannot be encoded into imm12 field

var_dump($a > $b ? 1 : 0);
var_dump($a > $c ? 1 : 0);
var_dump($a > $d ? 1 : 0);
var_dump($a > $e ? 1 : 0);
var_dump($a > $f ? 1 : 0);
var_dump($a > $g ? 1 : 0);
var_dump($a > $h ? 1 : 0);
var_dump($a > $i ? 1 : 0);
}

function bar($a) {
$b = 0;
$c = -31;
$d = -4095; // negation of 0xfff
$e = -4096; // negation of 0x1000
$f = -16773120; // negation of 0xfff000
$g = -1044481; // negation of 0xff001
$h = -16777216; // negation of 0x1000000
$i = -64729929336; // negation of 0xf12345678

var_dump($a > $b ? 1 : 0);
var_dump($a > $c ? 1 : 0);
var_dump($a > $d ? 1 : 0);
var_dump($a > $e ? 1 : 0);
var_dump($a > $f ? 1 : 0);
var_dump($a > $g ? 1 : 0);
var_dump($a > $h ? 1 : 0);
var_dump($a > $i ? 1 : 0);
}

foo(42);
bar(42);
?>
--EXPECT--
int(1)
int(1)
int(0)
int(0)
int(0)
int(0)
int(0)
int(0)
int(1)
int(1)
int(1)
int(1)
int(1)
int(1)
int(1)
int(1)