Skip to content

Conversation

@shqking
Copy link
Contributor

@shqking shqking commented Mar 30, 2021

DynASM/x86 permits register names of the form Ra(expr) where "expr" is
an arbitrary C expression and "Ra" is register class. This allows
registers to be selected dynamically at runtime. However the Arm64 port
only accepts static register names such as "x1", "w5", etc.

This patch extends the DynASM parser for arm64 to accept expressions of
the form Rx(expr) where "x" is a register class such as "x", "w", "q",
etc. A new action DASM_VREG patches the instruction with the dynamic
register index passed as an argument (similar to how dynamic register
names work on x86).

To correctly patch the instruction we need to know the bit position of
the register in the instruction word. This is now passed into
parse_reg() and encoded in the low bits of the DASM_VREG opcode.

To avoid duplication of the bit position in code like

 shl(parse_reg(..., 5), 5)

parse_reg() now returns the shifted register index.

Besides, with the introduction of dynmiac register names, the original
method, i.e 'p[-2]', to accessing 'scale' field for action DASM_IMML,
might be polluted. As a result, load/store with an immediate or type
maps, would be affected.
This patch passes 'scale' as the parameter to DASM_IMML.

Example [1]:

  | cmp Rx(15), #42
  | mov x3, #1
  | add Rw(5+1), Rw(7), Rw(22)
  | ldr Rx(4), [Rx(2), #8]!
  | str x0, [Rx(2), #offsetof(struct stu, age)]
  | ldr x8, STATE->get_ch

Disassembly:

  0xffffb5498000: cmp             x15, #0x2a
  0xffffb5498004: movz            w3, #0x1
  0xffffb5498008: add             w6, w7, w22
  0xffffb549800c: ldr             x4, [x2, #8]!
  0xffffb5498010: str             x0, [x2, #8]
  0xffffb5498014: ldr             x8, [x2, #8]

Test environment:
We're using an ARM-based server, with Ubuntu-20 and GCC-10. Disambler
library capstone[2] should be installed in advance.
After building the PHP, the 'minilua' can be found in
'PHP-SRC/ext/opcache/' directory. Our test case can be run with the
following commands:

  $ PHP-SRC/ext/opcache/minilua \
    PHP-SRC/ext/opcache/jit/dynasm/dynasm.lua -o test.c \
    -D ARM64=1 test-dyn-regs.c
  $ gcc test.c -o test -lcapstone
  $ ./test

[1]
https://github.com/shqking/misc/blob/main/php-dynasm-test/test-dyn-regs.c
[2] https://www.capstone-engine.org/

Co-Developed-by: Nick Gasson Nick.Gasson@arm.com

DynASM/x86 permits register names of the form Ra(expr) where "expr" is
an arbitrary C expression and "Ra" is register class. This allows
registers to be selected dynamically at runtime. However the Arm64 port
only accepts static register names such as "x1", "w5", etc.

This patch extends the DynASM parser for arm64 to accept expressions of
the form Rx(expr) where "x" is a register class such as "x", "w", "q",
etc. A new action DASM_VREG patches the instruction with the dynamic
register index passed as an argument (similar to how dynamic register
names work on x86).

To correctly patch the instruction we need to know the bit position of
the register in the instruction word. This is now passed into
parse_reg() and encoded in the low bits of the DASM_VREG opcode.

To avoid duplication of the bit position in code like
  shl(parse_reg(..., 5), 5)
parse_reg() now returns the shifted register index.

Besides, with the introduction of dynmiac register names, the original
method, i.e 'p[-2]', to accessing 'scale' field for action DASM_IMML,
might be polluted. As a result, load/store with an immediate or type
maps, would be affected.
This patch passes 'scale' as the parameter to DASM_IMML.

Example [1]:

  | cmp Rx(15), php#42
  | mov x3, php#1
  | add Rw(5+1), Rw(7), Rw(22)
  | ldr Rx(4), [Rx(2), php#8]!
  | str x0, [Rx(2), #offsetof(struct stu, age)]
  | ldr x8, STATE->get_ch

Disassembly:

  0xffffb5498000: cmp             x15, #0x2a
  0xffffb5498004: movz            w3, #0x1
  0xffffb5498008: add             w6, w7, w22
  0xffffb549800c: ldr             x4, [x2, php#8]!
  0xffffb5498010: str             x0, [x2, php#8]
  0xffffb5498014: ldr             x8, [x2, php#8]

Test environment:
We're using an ARM-based server, with Ubuntu-20 and GCC-10. Disambler
library capstone[2] should be installed in advance.
After building the PHP, the 'minilua' can be found in
'PHP-SRC/ext/opcache/' directory. Our test case can be run with the
following commands:

  $ PHP-SRC/ext/opcache/minilua \
    PHP-SRC/ext/opcache/jit/dynasm/dynasm.lua -o test.c \
    -D ARM64=1 test-dyn-regs.c
  $ gcc test.c -o test -lcapstone
  $ ./test

[1]
https://github.com/shqking/misc/blob/main/php-dynasm-test/test-dyn-regs.c
[2] https://www.capstone-engine.org/

Co-Developed-by: Nick Gasson <Nick.Gasson@arm.com>
@shqking
Copy link
Contributor Author

shqking commented Mar 30, 2021

Hi @dstogov

I put our DynASM/arm64 changes into two separate patches.
This PR is for 'dynamic register names' feature, and the other one(See #6820) is for 'global label reference' feature.

I suppose these two missing/buggy features for Arm64 backend would still affect other projects using DynASM library and these two patches should be sent back to LuaJIT as well.

Could you help to take a look at them?
And yes, Mike Pall's review might be needed here if available.

Thanks.

@dstogov
Copy link
Member

dstogov commented Mar 30, 2021

hi @MikePall, We are using DynAsm (and a lot of your ideas) in our JIT compiler for PHP. See https://github.com/php/php-src/tree/master/ext/opcache/jit/dynasm We almost didn't have to patch DynAsm. The only significant change was ".aword" handing on X86_64 (see ad66053 and ddba2a7). Now developers from ARM started to work on AArch64 back-end and proposed a couple of DynAsm patches. Could you please make a quick review. May be it make sense to include them into the main LuaJit distribution.

@MikePall
Copy link

Applied with some changes upstream. Thanks!

@shqking
Copy link
Contributor Author

shqking commented Apr 1, 2021

Thanks for your help @dstogov @MikePall

I will close this PR since the related changes have been committed in LuaJIT and PHP master.

@shqking shqking closed this Apr 1, 2021
@shqking shqking deleted the dynasm-dyn-regs branch April 2, 2021 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants