i#2626 AArch64 encoder: Add isz operand and vector ADD to encoder. #3016
Conversation
This patch adds an isz operand to encode the vector element width for non-FP vector instructions. It also adds support for vector ADD to the encoder/decoder. Additional tests and macros should be added once the script in the project-aarch64-generate-patterns branch gets updated. Issue #2626 Change-Id: I2bca21610205c3b2ba7bb67f990fe108d210001c
…ctor-add Change-Id: Ie8adf4f164d83f7bdcf85cb9e9f44b487c1de5d7
Change-Id: I7414163022cb784fdd4d7af29cb6c184e7394c45
|
Does it make sense to refer to "the project-aarch64-generate-patterns branch" in the commit message? Shall I assume that the last sentence of the commit message will be something like "Additional tests and macros will be added by a later commit"? |
core/arch/aarch64/codec.c
Outdated
| static inline bool | ||
| encode_opnd_isz(uint enc, int opcode, byte *pc, opnd_t opnd, OUT uint *enc_out) | ||
| { | ||
| if (opnd_get_immed_int(opnd) < ISZ_BYTE || opnd_get_immed_int(opnd) > ISZ_DOUBLE) |
There was a problem hiding this comment.
Since opnd_get_immed_int might be a function, I would avoid using it three times here and instead put uint bits = opnd_get_immed_int(opnd).
Also, since line 1984 wants bits to be 0, 1, 2 or 3, I'm not sure the use of ISZ_BYTE and ISZ_DOUBLE is helpful here, so I would have put if (bits > 3), but that's just my opinion and I wouldn't be surprised if some people would disagree.
There was a problem hiding this comment.
Since opnd_get_immed_int might be a function, I would avoid using it three times here and instead put uint bits = opnd_get_immed_int(opnd).
Sometimes I tend to have too much faith in compilers. I've changed it.
Also, since line 1984 wants bits to be 0, 1, 2 or 3, I'm not sure the use of ISZ_BYTE and ISZ_DOUBLE is helpful here, so I would have put if (bits > 3), but that's just my opinion and I wouldn't be surprised if some people would disagree.
I wasn't sure, but yes, they don't add too much value I think. Ideally, I think there would be an enum, shared with the FP ops. I could do that as a follow up.
core/arch/aarch64/codec.txt
Outdated
| @@ -131,6 +131,8 @@ | |||
| ---------?x---------x----------- vindex_SD # Index for vector with single or double | |||
| # elements, depending on bit 22 (sz) | |||
| ?--------xx--------------------- imm16sh # shift for MOVK/... (immediate); checks 31 | |||
| --------xx---------------------- isz # element size of a vector register ( | |||
There was a problem hiding this comment.
If you wanted to get the comment into one line you could put something like: # element size of vector reg (8<<x bits)
| @@ -957,6 +959,10 @@ x101101011000000000101xxxxxxxxxx cls wx0 : wx5 | |||
| 1101101011000000000011xxxxxxxxxx rev x0 : x5 | |||
|
|
|||
| # Data Processing - Scalar Floating-Point and Advanced SIMD | |||
|
|
|||
| # ADD | |||
There was a problem hiding this comment.
Is there a rule for how the instructions are ordered in this file? (I think I was following the "Index by Encoding" in our internal web pages at some point...) If it's feasible, it might be good to follow some canonical ordering and mark omissions with a comment. (But perhaps it isn't feasible.)
There was a problem hiding this comment.
The neon patterns should currently follow alphabetic order (as on the A64 -- SIMD and Floating-point Instructions (alphabetic order) index page of the public XML ISA spec). That's how the generator script happens to process them, but IMO that makes it easier to read. On second thought, it might be easier to extend to generator script to work by the index page in the future.
| @@ -1561,6 +1561,16 @@ fd3fffff : str d31, [sp,#32760] : str %d31 -> +0x7ff8(%sp)[8byte] | |||
| fd481041 : ldr d1, [x2,#4128] : ldr +0x1020(%x2)[8byte] -> %d1 | |||
| fd7fffff : ldr d31, [sp,#32760] : ldr +0x7ff8(%sp)[8byte] -> %d31 | |||
|
|
|||
|
|
|||
| # ADD (vector) | |||
| 4e2c856a : add v10.16b, v11.16b, v12.16b : add %q11 %q12 $0x00 -> %q10 | |||
There was a problem hiding this comment.
The script dis-a64.pl contains a format specification that would attempt to make these colons line up. In fact, I think at some point dis-a64.txt could survive reformatting by dis-a64.pl. Perhaps worth thinking about getting that to work again, but not as part of this commit.
There was a problem hiding this comment.
Right, I've aligned the ADD lines. It should be easy to update the generator script to align on a per-opcode basis.
Change-Id: I4c017e16910f18a3b56bb1fc59df743aea908e40
Probably not. I'll make sure to update that in the final message. |
|
Thanks Edmund! |
This patch adds an isz operand to encode the vector element width for non-FP vector instructions. It also adds support for vector ADD to the encoder/decoder. Additional tests and macros will be added by a later commit. Issue #2626
This patch adds an isz operand to encode the vector element width for
non-FP vector instructions. It also adds support for vector ADD to the
encoder/decoder. Additional tests and macros should be added once the
script in the project-aarch64-generate-patterns branch gets updated.
Issue #2626