Re: [PATCH v2] target/riscv: Fix the element agnostic function problem

[Huang Tao]

On 2024/3/21 16:18, Richard Henderson wrote:
> On 3/20/24 17:58, Huang Tao wrote:
> > In RVV and vcrypto instructions, the masked and tail elements are set 
> > to 1s
> > using vext_set_elems_1s function if the vma/vta bit is set. It is the 
> > element
> > agnostic policy.
> >
> > However, this function can't deal the big endian situation. This 
> > patch fixes
> > the problem by adding handling of such case.
> >
> > Signed-off-by: Huang Tao <eric.huang@linux.alibaba.com>
> > Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> > Changes in v2:
> > - Keep the api of vext_set_elems_1s
> > - Reduce the number of patches.
> > ---
> >   target/riscv/vector_internals.c | 22 ++++++++++++++++++++++
> >   1 file changed, 22 insertions(+)
> >
> > diff --git a/target/riscv/vector_internals.c 
> > b/target/riscv/vector_internals.c
> > index 12f5964fbb..3e45b9b4a7 100644
> > --- a/target/riscv/vector_internals.c
> > +++ b/target/riscv/vector_internals.c
> > @@ -30,6 +30,28 @@ void vext_set_elems_1s(void *base, uint32_t 
> > is_agnostic, uint32_t cnt,
> >       if (tot - cnt == 0) {
> >           return ;
> >       }
> > +
> > +#if HOST_BIG_ENDIAN
> > +    /*
> > +     * Deal the situation when the elements are insdie
> > +     * only one uint64 block including setting the
> > +     * masked-off element.
> > +     */
> > +    if ((tot - 1) ^ cnt < 8) {
> > +        memset(base + H1(tot - 1), -1, tot - cnt);
> > +        return;
> > +    }
>
> (1) tot will always be a multiple of 8, afaik, so there's no need for 
> this first block.
> (2) Using if not #if means that the code is always compile-tested, 
> even if it is eliminated.
>
>
> r~

tot is not always be a multiple of 8. In the vector instructions, the 
helper fuinctions will use vext_set_elems_1s to set one masked-off 
element. In that case, tot = cnt + esz, and tot is not the end of a 
vector register.

There is an example in GEN_VEXT_SHIFT_VV:

    for (i = env->vstart; i < vl; i++) {
        if (!vm && !vext_elem_mask(v0, i)) {
            /* set masked-off elements to 1s */
            vext_set_elems_1s(vd, vma, i * esz, (i + 1) * esz);
            continue;
        }

As for the second point, I will use if instead of #if in the next version.

Thanks,

Huang Tao