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

[Huang Tao <eric.huang@linux.alibaba.com>]

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