target/arm: Some SVE2p1 fixes

[PATCH for-10.1 08/10] target/arm: Don't nest H() macro calls in SVE DO_REDUCE

Posted by Peter Maydell 3 months, 4 weeks ago

In the part of the SVE DO_REDUCE macro used by the SVE2p1 FMAXQV,
FMINQV, etc insns, we incorrectly applied the H() macro twice when
calculating an offset to add to the vn pointer.  This has no effect
on little-endian hosts but on big-endian hosts the two invocations
will cancel each other out and we will access the wrong part of the
array.

The "s * 16" part of the expression is already aligned, so we only
need to use the H macro on the "e". Correct the macro usage.

Fixes: 1de7ecfc12d05 ("target/arm: Implement FADDQV, F{MIN, MAX}{NM}QV for SVE2p1")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/tcg/sve_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/tcg/sve_helper.c b/target/arm/tcg/sve_helper.c
index 105cc5dd122..bf894f0bf13 100644
--- a/target/arm/tcg/sve_helper.c
+++ b/target/arm/tcg/sve_helper.c
@@ -4509,7 +4509,7 @@ void helper_sve2p1_##NAME##qv_##SUF(void *vd, void *vn, void *vg,     \
         TYPE data[ARM_MAX_VQ];                                        \
         for (unsigned s = 0; s < segments; s++) {                     \
             uint16_t pg = *(uint16_t *)(vg + H1_2(s * 2));            \
-            TYPE nn = *(TYPE *)(vn + H(s * 16 + H(e)));               \
+            TYPE nn = *(TYPE *)(vn + (s * 16 + H(e)));                \
             data[s] = (pg >> e) & 1 ? nn : IDENT;                     \
         }                                                             \
         *(TYPE *)(vd + H(e)) = FUNC##_reduce(data, status, segments); \
-- 
2.43.0

Re: [PATCH for-10.1 08/10] target/arm: Don't nest H() macro calls in SVE DO_REDUCE

Posted by Richard Henderson 3 months, 4 weeks ago

On 7/18/25 10:30, Peter Maydell wrote:
> In the part of the SVE DO_REDUCE macro used by the SVE2p1 FMAXQV,
> FMINQV, etc insns, we incorrectly applied the H() macro twice when
> calculating an offset to add to the vn pointer.  This has no effect
> on little-endian hosts but on big-endian hosts the two invocations
> will cancel each other out and we will access the wrong part of the
> array.
> 
> The "s * 16" part of the expression is already aligned, so we only
> need to use the H macro on the "e". Correct the macro usage.
> 
> Fixes: 1de7ecfc12d05 ("target/arm: Implement FADDQV, F{MIN, MAX}{NM}QV for SVE2p1")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   target/arm/tcg/sve_helper.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/arm/tcg/sve_helper.c b/target/arm/tcg/sve_helper.c
> index 105cc5dd122..bf894f0bf13 100644
> --- a/target/arm/tcg/sve_helper.c
> +++ b/target/arm/tcg/sve_helper.c
> @@ -4509,7 +4509,7 @@ void helper_sve2p1_##NAME##qv_##SUF(void *vd, void *vn, void *vg,     \
>           TYPE data[ARM_MAX_VQ];                                        \
>           for (unsigned s = 0; s < segments; s++) {                     \
>               uint16_t pg = *(uint16_t *)(vg + H1_2(s * 2));            \
> -            TYPE nn = *(TYPE *)(vn + H(s * 16 + H(e)));               \
> +            TYPE nn = *(TYPE *)(vn + (s * 16 + H(e)));                \
>               data[s] = (pg >> e) & 1 ? nn : IDENT;                     \
>           }                                                             \
>           *(TYPE *)(vd + H(e)) = FUNC##_reduce(data, status, segments); \

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~