[PATCH] target/arm: Fix sve2 ldnt1 and stnt1

[PATCH] target/arm: Fix sve2 ldnt1 and stnt1

[Richard Henderson]

The order of arguments between ldnt1 and ld1 are swapped in the
architecture, and similarly for stnt1 and st1.  Swap them in the
decode so that we have "m" be the vector operand and "n" be the
general operand.

Fixes: https://gitlab.com/qemu-project/qemu/-/issues/826
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/sve.decode | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/target/arm/sve.decode b/target/arm/sve.decode
index fd96baeb68..91a0873b7c 100644
--- a/target/arm/sve.decode
+++ b/target/arm/sve.decode
@@ -1577,22 +1577,25 @@ USDOT_zzzz      01000100 .. 0 ..... 011 110 ..... .....  @rda_rn_rm
 
 # SVE2 64-bit gather non-temporal load
 #   (scalar plus 64-bit unscaled offsets)
-LDNT1_zprz      1100010 msz:2 00 rm:5 1 u:1 0 pg:3 rn:5 rd:5 \
+# Note that Rm and Rn are swapped, so that the vector and general
+# register arguments match LD1_zprz.
+LDNT1_zprz      1100010 msz:2 00 rn:5 1 u:1 0 pg:3 rm:5 rd:5 \
                 &rprr_gather_load xs=2 esz=3 scale=0 ff=0
 
 # SVE2 32-bit gather non-temporal load (scalar plus 32-bit unscaled offsets)
-LDNT1_zprz      1000010 msz:2 00 rm:5 10 u:1 pg:3 rn:5 rd:5 \
+LDNT1_zprz      1000010 msz:2 00 rn:5 10 u:1 pg:3 rm:5 rd:5 \
                 &rprr_gather_load xs=0 esz=2 scale=0 ff=0
 
 ### SVE2 Memory Store Group
 
 # SVE2 64-bit scatter non-temporal store (vector plus scalar)
-STNT1_zprz      1110010 .. 00 ..... 001 ... ..... ..... \
-                @rprr_scatter_store xs=2 esz=3 scale=0
+# Note the Rm and Rn swap, similar to LDNT1_zprz.
+STNT1_zprz      1110010 msz:2 00 rn:5 001 pg:3 rm:5 rd:5 \
+                &rprr_scatter_store xs=2 esz=3 scale=0
 
 # SVE2 32-bit scatter non-temporal store (vector plus scalar)
-STNT1_zprz      1110010 .. 10 ..... 001 ... ..... ..... \
-                @rprr_scatter_store xs=0 esz=2 scale=0
+STNT1_zprz      1110010 msz:2 10 rn:5 001 pg:3 rm:5 rd:5 \
+                &rprr_scatter_store xs=0 esz=2 scale=0
 
 ### SVE2 Crypto Extensions
 
-- 
2.25.1

Re: [PATCH] target/arm: Fix sve2 ldnt1 and stnt1

[Richard Henderson]

On 3/6/22 09:40, Richard Henderson wrote:
> The order of arguments between ldnt1 and ld1 are swapped in the
> architecture, and similarly for stnt1 and st1.  Swap them in the
> decode so that we have "m" be the vector operand and "n" be the
> general operand.
> 
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/826
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/arm/sve.decode | 15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)

Oh, Peter, perhaps squash this into the other fix for LDNT1?

r~

Re: [PATCH] target/arm: Fix sve2 ldnt1 and stnt1

[Peter Maydell]

On Sun, 6 Mar 2022 at 19:40, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The order of arguments between ldnt1 and ld1 are swapped in the
> architecture, and similarly for stnt1 and st1.  Swap them in the
> decode so that we have "m" be the vector operand and "n" be the
> general operand.
>
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/826
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/sve.decode | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)


Applied to target-arm.next, thanks. It seems to me sufficiently
distinct that I won't bother squashing it with the other.

PS: the keyword gitlab looks for to auto-close issues is
"Resolves:", not "Fixes:".

-- PMM

Re: [PATCH] target/arm: Fix sve2 ldnt1 and stnt1

[Peter Maydell]

On Sun, 6 Mar 2022 at 19:40, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The order of arguments between ldnt1 and ld1 are swapped in the
> architecture, and similarly for stnt1 and st1.  Swap them in the
> decode so that we have "m" be the vector operand and "n" be the
> general operand.
>
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/826
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---


Looking at this more closely, I don't think these two fixes
are sufficient. In particular, "the operand fields are swapped"
is not the only difference here. For LD1 the scalar register
can be SP, but for LDNT1 it can be XZR. Our trans_LDNT1_zprz
calls trans_LD1_zprz, so it gets this wrong.

I'm going to drop both patches for the moment.

thanks
-- PMM