Re: [PATCH v3 24/51] target/arm: Implement SME ZERO

[Peter Maydell]

On Mon, 20 Jun 2022 at 19:23, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/helper-sme.h    |  2 ++
>  target/arm/translate-a64.h |  1 +
>  target/arm/sme.decode      |  4 ++++
>  target/arm/sme_helper.c    | 25 +++++++++++++++++++++++++
>  target/arm/translate-a64.c | 14 ++++++++++++++
>  target/arm/translate-sme.c | 13 +++++++++++++
>  6 files changed, 59 insertions(+)
>

> +void helper_sme_zero(CPUARMState *env, uint32_t imm, uint32_t svl)
> +{
> +    uint32_t i;
> +
> +    /*
> +     * Special case clearing the entire ZA space.
> +     * This falls into the CONSTRAINED UNPREDICTABLE zeroing of any
> +     * parts of the ZA storage outside of SVL.
> +     */
> +    if (imm == 0xff) {
> +        memset(env->zarray, 0, sizeof(env->zarray));
> +        return;
> +    }
> +
> +    /*
> +     * Recall that ZAnH.D[m] is spread across ZA[n+8*m].
> +     * Unless SVL == ARM_MAX_VQ, each row is discontiguous.

This comment led me down a garden path for a while. Each
row in a tile *is* contiguous, whatever the value of SVL.
What isn't contiguous is the entire tile, because the rows
of the tile are striped across the ZA[] array so rows that
are adjacent in the tile aren't adjacent in the ZA[] array.
(And this is true even if SVL is ARM_MAX_VQ.)

> +     */
> +    for (i = 0; i < svl; i++) {
> +        if (imm & (1 << (i % 8))) {
> +            memset(&env->zarray[i], 0, svl);
> +        }
> +    }
> +}

With the comment fixed,
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

I'll also go back to the patch that adds env->zarray to
suggest a beefed-up comment there, because I just had to
spend half an hour with the spec to make sure I understood
what the zarray is compared to the architecture (some of
which is the spec being complicated, of course ;-))

thanks
-- PMM