Re: [PATCH 5/5] target/riscv: Use ELEN for Fractional LMUL check

[LIU Zhiwei <zhiwei_liu@linux.alibaba.com>]

[-- Attachment #1: Type: text/plain, Size: 2317 bytes --]

Hi Alistair,

On 4/7/26 12:36 PM, alistair23@gmail.com wrote:
> From: Alistair Francis<alistair.francis@wdc.com>
>
> The RISC-V spec states that
>
> """
> For a given supported fractional LMUL setting, implementations
> must support SEW settings between SEWMIN and LMUL * ELEN, inclusive.
> """

I think the meaning  of "must support"  in specification is "must at 
least support" instead of "must only support".

Sail or Spike have the same check as this patch to prevent user program 
can run on them, but
can't run on RTLs, as RTLs implementation may support SEW besides the 
SEWMIN and LMUL * ELEN range.

We can refer to discussion here: 
https://github.com/riscv-software-src/riscv-isa-sim/pull/620

In my opinion, we had better add a option such as "frac_sew_check" to 
make user set right value for their RTL implementation.

Otherwise,
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>

Thanks,
Zhiwei

> We were previously checking VLEN, instead of ELEN, so let's update to
> check ELEN instead of VLEN for fractional scaling.
>
> Resolves:https://gitlab.com/qemu-project/qemu/-/work_items/3196
> Signed-off-by: Alistair Francis<alistair.francis@wdc.com>
> ---
>   target/riscv/vector_helper.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index 83dd26314d..b4fc791eb7 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -47,18 +47,17 @@ target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
>       target_ulong reserved = s2 &
>                               MAKE_64BIT_MASK(R_VTYPE_RESERVED_SHIFT,
>                                               xlen - 1 - R_VTYPE_RESERVED_SHIFT);
> -    uint16_t vlen = cpu->cfg.vlenb << 3;
>       int8_t lmul;
>   
>       if (vlmul & 4) {
>           /*
>            * Fractional LMUL, check:
>            *
> -         * VLEN * LMUL >= SEW
> -         * VLEN >> (8 - lmul) >= sew
> -         * (vlenb << 3) >> (8 - lmul) >= sew
> +         * ELEN * LMUL >= SEW
> +         * ELEN >> (8 - vlmul) >= sew
>            */
> -        if (vlmul == 4 || (vlen >> (8 - vlmul)) < sew) {
> +        if (vlmul == 4 ||
> +            (cpu->cfg.elen >> (8 - vlmul)) < sew) {
>               vill = true;
>           }
>       }

[-- Attachment #2: Type: text/html, Size: 3568 bytes --]