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
 #define TCG_TARGET_HAS_andc_vec         0
 #define TCG_TARGET_HAS_orc_vec          0
 #define TCG_TARGET_HAS_nand_vec         0
-- 
2.43.0

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 thought ELEN had to be at least XLEN.
Anyway, if ELEN >= 64, then VLEN must also be >= 64.

In any case, I think we should not set CPUINFO_ZVE64X if the vlen is too small.  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.


r~

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~

On 8/27/24 18:31, LIU Zhiwei wrote:
> 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.

Pardon?

While you might check __riscv_zve64x at compile-time, you would still fall through to


---
     }

+   if (info & CPUINFO_ZVE64X) {
+       unsigned long vl;
+       asm("vsetvli %0, r0, e64" : "=r"(vl));
+       if (vl) {
+           riscv_vlen = vl * 8;
+       } else {
+           info &= ~CPUINFO_ZVE64X;
+       }
+   }
+
     info |= CPUINFO_ALWAYS;
     cpuinfo = info;
---

Do not attempt to merge the vsetvli from the SIGILL probe; I expect that path to become 
unused and eventually vanish.


r~