Re: [Qemu-devel] [PATCH v3b 13/18] target/arm: Implement SVE Partition Break Group

[Peter Maydell <peter.maydell@linaro.org>]

On 30 May 2018 at 19:01, Richard Henderson <richard.henderson@linaro.org> wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/helper-sve.h    |  18 +++
>  target/arm/sve_helper.c    | 247 +++++++++++++++++++++++++++++++++++++
>  target/arm/translate-sve.c | 106 ++++++++++++++++
>  target/arm/sve.decode      |  19 +++
>  4 files changed, 390 insertions(+)

> +static bool compute_brk(uint64_t *retb, uint64_t n, uint64_t g,
> +                        bool brk, bool after)
> +{
> +    uint64_t b;
> +
> +    if (brk) {
> +        b = 0;
> +    } else if ((g & n) == 0) {
> +        /* For all G, no N are set; break not found.  */
> +        b = g;
> +    } else {
> +        /* Break somewhere in N.  Locate it.  */
> +        b = g & n;            /* guard true, pred true*/

Missing space before '*/'.

> +        b = b & -b;           /* first such */
> +        if (after) {
> +            b = b | (b - 1);  /* break after same */
> +        } else {
> +            b = b - 1;        /* break before same */
> +        }
> +        brk = true;
> +    }
> +
> +    *retb = b;
> +    return brk;
> +}

> +
> +/* Given a computation function, compute a merging BRK.  */
> +static void compute_brk_m(uint64_t *d, uint64_t *n, uint64_t *g,
> +                          intptr_t oprsz, bool after)

Comment suggests function should take a function pointer argument,
but it doesn't ?

> +{
> +    bool brk = false;
> +    intptr_t i;
> +
> +    for (i = 0; i < DIV_ROUND_UP(oprsz, 8); ++i) {
> +        uint64_t this_b, this_g = g[i];
> +
> +        brk = compute_brk(&this_b, n[i], this_g, brk, after);
> +        d[i] = (this_b & this_g) | (d[i] & ~this_g);
> +    }
> +}

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM