[PATCH v2 06/11] [Binutils] aarch64: Fix sve2p1 ld[1-4]/st[1-4]q instruction operands.

[Richard Earnshaw (lists)]

On 12/06/2024 16:59, srinath wrote:
> 
> Hi,
> 
> This patch fixes encoding and syntax for sve2p1 instructions ld[1-4]q/st[1-4]q
> as mentioned below, for the issues reported here.
> https://sourceware.org/pipermail/binutils/2024-February/132408.html
> 
> 1) Previously all the ld[1-4]q/st[1-4]q instructions are wrongly added as
> predicated instructions and this issue is fixed in this patch by replacing
> "SVE2p1_INSNC" with "SVE2p1_INSN" macro.
> 2) Wrong first operand in all the ld[1-4]q/st[1-4]q instructions is fixed
> by replacing "SVE_Zt" with "SVE_ZtxN".
> 3) Wrong operand qualifiers in ld1q and st1q instructions are also fixed in
> this patch.
> 
> Fixing above mentioned issues helps with following:
> 1) ld1q and st1q first register operand accepts enclosed figure braces.
> 2) ld2q, ld3q, ld4q, st2q, st3q, and st4q instructions accepts wrapping
>    sequence of vector registers.
> 
> For the instructions ld[2-4]q/st[2-4]q, tests for wrapping sequence of vector
> registers are added along with short-form of operands for non-wrapping sequence.
> 
> I have added test using following logic:
> ld2q {Z0.Q, Z1.Q}, p0/Z, [x0,  #0, MUL VL]  //raw insn encoding (all zeroes)
> ld2q {Z31.Q, Z0.Q}, p0/Z, [x0,  #0, MUL VL] // encoding of <Zt1>
> ld2q {Z0.Q, Z1.Q}, p7/Z, [x0,  #0, MUL VL] // encoding of <Pg>
> ld2q {Z0.Q, Z1.Q}, p0/Z, [x30,  #0, MUL VL] // encoding of <Xm>
> ld2q {Z0.Q, Z1.Q}, p0/Z, [x0,  #-16, MUL VL] // encoding of <imm> (low value)
> ld2q {Z0.Q, Z1.Q}, p0/Z, [x0,  #14, MUL VL] // encoding of <imm> (high value)
> ld2q {Z31.Q, Z0.Q}, p7/Z, [x30,  #-16, MUL VL] // encoding of all fields (all ones)
> ld2q {Z30.Q, Z31.Q}, p1/Z, [x3,  #-2, MUL VL] // random encoding.
> 
> For all the above form of instructions the hyphenated form is preferred for
> disassembly if there are more than one register in the list, and the register
> numbers are monotonically increasing in increments of one.
> 
> Regression testing for aarch64-none-elf target and found no regressions.
> 
> Ok for binutils-master?
> 
> Regards,
> Srinath.
> ---
>  gas/config/tc-aarch64.c                      |   3 -
>  gas/testsuite/gas/aarch64/sme-5-illegal.l    |   8 +-
>  gas/testsuite/gas/aarch64/sme-6-illegal.l    |   8 +-
>  gas/testsuite/gas/aarch64/sve2p1-1-bad.l     |  14 --
>  gas/testsuite/gas/aarch64/sve2p1-1.d         |  14 --
>  gas/testsuite/gas/aarch64/sve2p1-1.s         |  15 --
>  gas/testsuite/gas/aarch64/sve2p1-4-invalid.d |   3 +
>  gas/testsuite/gas/aarch64/sve2p1-4-invalid.l | 116 +++++++++++++++
>  gas/testsuite/gas/aarch64/sve2p1-4-invalid.s | 119 +++++++++++++++
>  gas/testsuite/gas/aarch64/sve2p1-4.d         | 144 ++++++++++++++++++
>  gas/testsuite/gas/aarch64/sve2p1-4.s         | 147 +++++++++++++++++++
>  include/opcode/aarch64.h                     |   3 -
>  opcodes/aarch64-opc.c                        |  11 +-
>  opcodes/aarch64-tbl.h                        |  43 +++---
>  14 files changed, 556 insertions(+), 92 deletions(-)
>  create mode 100644 gas/testsuite/gas/aarch64/sve2p1-4-invalid.d
>  create mode 100644 gas/testsuite/gas/aarch64/sve2p1-4-invalid.l
>  create mode 100644 gas/testsuite/gas/aarch64/sve2p1-4-invalid.s
>  create mode 100644 gas/testsuite/gas/aarch64/sve2p1-4.d
>  create mode 100644 gas/testsuite/gas/aarch64/sve2p1-4.s
> 

+.*:	c41fa000 	ld1q	{z0.q}, p0/z, \[z0.d, xzr\]

The specification for this says

LD1Q { <Zt>.Q }, <Pg>/Z, [<Zn>.D{, <Xm>}]

and further says that Xm defaults to Xzr when omitted.  So I would have thought the preferred disassembly for this case would be to omit the zero register, giving

ld1q	{z0.q}, p0/z, [z0.d]

as the output.

----

On a related note, I think we need a parsing test for the omitted argument as well, so:

+ld1q { Z0.Q }, P0/Z, [Z0.D, xzr]
+ld1q { Z0.Q }, P0/Z, [Z0.D]


----

A similar issue for disassembly with this case:

+ld2q {Z0.Q, Z1.Q}, p0/Z, [x0,  #0, MUL VL]

and the other (ld3q/ld4q) cases.  When the immediate is 0, we should also test

+ld2q {Z0.Q, Z1.Q}, p0/Z, [x0]

(we do disassemble to this form, I see)

----


+ld2q {Z0.Q, Z1.Q}, p0/Z, [x0,  x30, LSL  #4]

I think it would be better to test for 

+ld2q {Z0.Q, Z1.Q}, p0/Z, [x0,  xzr, LSL  #4]

Here as that tests all the bits of the Rm field.

----

And similarly for the store, of course.

R.