Re: [PATCH v1 15/15] tcg/riscv: Enable vector TCG host-native

[LIU Zhiwei]

On 2024/8/14 18:15, Richard Henderson wrote:

> On
>       8/13/24 21:34, LIU Zhiwei wrote:
>       
> > From: TANG Tiancheng
> >         <tangtiancheng.ttc@alibaba-inc.com>
> >         
> >         
> >         Signed-off-by: TANG Tiancheng
> >         <tangtiancheng.ttc@alibaba-inc.com>
> >         
> >         Reviewed-by: Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
> >         
> >         ---
> >         
> >           tcg/riscv/tcg-target.h | 10 +++++++---
> >         
> >           1 file changed, 7 insertions(+), 3 deletions(-)
> >         
> >         
> >         diff --git a/tcg/riscv/tcg-target.h b/tcg/riscv/tcg-target.h
> >         
> >         index eb5129a976..fe6c50e49e 100644
> >         
> >         --- a/tcg/riscv/tcg-target.h
> >         
> >         +++ b/tcg/riscv/tcg-target.h
> >         
> >         @@ -143,9 +143,13 @@ typedef enum {
> >         
> >           #define TCG_TARGET_HAS_tst              0
> >         
> >             /* vector instructions */
> >         
> >         -#define TCG_TARGET_HAS_v64              0
> >         
> >         -#define TCG_TARGET_HAS_v128             0
> >         
> >         -#define TCG_TARGET_HAS_v256             0
> >         
> >         +extern int riscv_vlen;
> >         
> >         +#define have_rvv    ((cpuinfo & CPUINFO_ZVE64X) &&
> >         \
> >         
> >         +                     (riscv_vlen >= 64))
> >         
> >         +
> >         
> >         +#define TCG_TARGET_HAS_v64              have_rvv
> >         
> >         +#define TCG_TARGET_HAS_v128             have_rvv
> >         
> >         +#define TCG_TARGET_HAS_v256             have_rvv
> >         
> >       
>       
>       Can ELEN ever be less than 64 for riscv64?
>       
>

I think so.  At least the specification allow this case. According to the specification,

"Any of these extensions can be added to base ISAs with XLEN=32 or XLEN=64."

includes zve32x, where ELEN is 32 and XLEN is 64.

> I
>       thought ELEN had to be at least XLEN.
>       
>       Anyway, if ELEN >= 64, then VLEN must also be >= 64.
>       
>     
    YES.
>       
>       In any case, I think we should not set CPUINFO_ZVE64X if the vlen
>       is too small.  
    Agree.
> We can
>       initialize both values in util/cpuinfo-riscv.c, rather than
>       initializing vlen in tcg.
>       
>       
> > +static void riscv_get_vlenb(void){
> >         
> >         +    /* Get vlenb for Vector: csrrs %0, vlenb, zero. */
> >         
> >         +    asm volatile("csrrs %0, 0xc22, x0" : "=r"(riscv_vlen));
> >         
> >         +    riscv_vlen *= 8;
> >         
> >         +}
> >         
> >       
>       
>       While this is an interesting and required datum, if ELEN < XLEN
>       is possible, then perhaps
>       
>       
>           asm("vsetvli %0, r0, e64" : "=r"(vl));
>       
>       
>       is a better probe, verifying that vl != 0, i.e. e64 is supported,
>       and recording vlen as vl * 64, i.e. VLMAX.
>       
>

We will use this one. But probe the vlen in util/cpuinfo-riscv.c has no meaning as we sometimes use the compiler settings or hw_probe API. In these cases, the vlen detected in util/cpuinfo-riscv.c is zero.

Thanks,
Zhiwei

>       
>       
>       r~
>       
>