[PATCH v4 01/12] util: Add RISC-V vector extension probe in cpuinfo

LIU Zhiwei posted 12 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH v4 01/12] util: Add RISC-V vector extension probe in cpuinfo
Posted by LIU Zhiwei 2 months, 1 week ago
From: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com>

Add support for probing RISC-V vector extension availability in
the backend. This information will be used when deciding whether
to use vector instructions in code generation.

Cache lg2(vlenb) for the backend. The storing of lg2(vlenb) means
we can convert all of the division into subtraction.

While the compiler doesn't support RISCV_HWPROBE_EXT_ZVE64X,
we use RISCV_HWPROBE_IMA_V instead.

Signed-off-by: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com>
Reviewed-by: Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
---
 host/include/riscv/host/cpuinfo.h |  2 ++
 util/cpuinfo-riscv.c              | 24 ++++++++++++++++++++++--
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/host/include/riscv/host/cpuinfo.h b/host/include/riscv/host/cpuinfo.h
index 2b00660e36..cdc784e7b6 100644
--- a/host/include/riscv/host/cpuinfo.h
+++ b/host/include/riscv/host/cpuinfo.h
@@ -10,9 +10,11 @@
 #define CPUINFO_ZBA             (1u << 1)
 #define CPUINFO_ZBB             (1u << 2)
 #define CPUINFO_ZICOND          (1u << 3)
+#define CPUINFO_ZVE64X          (1u << 4)
 
 /* Initialized with a constructor. */
 extern unsigned cpuinfo;
+extern unsigned riscv_lg2_vlenb;
 
 /*
  * We cannot rely on constructor ordering, so other constructors must
diff --git a/util/cpuinfo-riscv.c b/util/cpuinfo-riscv.c
index 497ce12680..bab782745b 100644
--- a/util/cpuinfo-riscv.c
+++ b/util/cpuinfo-riscv.c
@@ -4,6 +4,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/host-utils.h"
 #include "host/cpuinfo.h"
 
 #ifdef CONFIG_ASM_HWPROBE_H
@@ -12,6 +13,7 @@
 #endif
 
 unsigned cpuinfo;
+unsigned riscv_lg2_vlenb;
 static volatile sig_atomic_t got_sigill;
 
 static void sigill_handler(int signo, siginfo_t *si, void *data)
@@ -33,7 +35,7 @@ static void sigill_handler(int signo, siginfo_t *si, void *data)
 /* Called both as constructor and (possibly) via other constructors. */
 unsigned __attribute__((constructor)) cpuinfo_init(void)
 {
-    unsigned left = CPUINFO_ZBA | CPUINFO_ZBB | CPUINFO_ZICOND;
+    unsigned left = CPUINFO_ZBA | CPUINFO_ZBB | CPUINFO_ZICOND | CPUINFO_ZVE64X;
     unsigned info = cpuinfo;
 
     if (info) {
@@ -49,6 +51,9 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
 #endif
 #if defined(__riscv_arch_test) && defined(__riscv_zicond)
     info |= CPUINFO_ZICOND;
+#endif
+#if defined(__riscv_arch_test) && defined(__riscv_zve64x)
+    info |= CPUINFO_ZVE64X;
 #endif
     left &= ~info;
 
@@ -64,7 +69,8 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
             && pair.key >= 0) {
             info |= pair.value & RISCV_HWPROBE_EXT_ZBA ? CPUINFO_ZBA : 0;
             info |= pair.value & RISCV_HWPROBE_EXT_ZBB ? CPUINFO_ZBB : 0;
-            left &= ~(CPUINFO_ZBA | CPUINFO_ZBB);
+            info |= pair.value & RISCV_HWPROBE_IMA_V ? CPUINFO_ZVE64X : 0;
+            left &= ~(CPUINFO_ZBA | CPUINFO_ZBB | CPUINFO_ZVE64X);
 #ifdef RISCV_HWPROBE_EXT_ZICOND
             info |= pair.value & RISCV_HWPROBE_EXT_ZICOND ? CPUINFO_ZICOND : 0;
             left &= ~CPUINFO_ZICOND;
@@ -112,6 +118,20 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
         assert(left == 0);
     }
 
+    if (info & CPUINFO_ZVE64X) {
+        /*
+         * We are guaranteed by RVV-1.0 that VLEN is a power of 2.
+         * We are guaranteed by Zve64x that VLEN >= 64, and that
+         * EEW of {8,16,32,64} are supported.
+         *
+         * Cache VLEN in a convenient form.
+         */
+        unsigned long vlenb;
+        /* Read csr "vlenb" with "csrr %0, vlenb" : "=r"(vlenb) */
+        asm volatile(".insn i 0x73, 0x2, %0, zero, -990" : "=r"(vlenb));
+        riscv_lg2_vlenb = ctz32(vlenb);
+    }
+
     info |= CPUINFO_ALWAYS;
     cpuinfo = info;
     return info;
-- 
2.43.0
Re: [PATCH v4 01/12] util: Add RISC-V vector extension probe in cpuinfo
Posted by Richard Henderson 2 months, 1 week ago
On 9/11/24 06:26, LIU Zhiwei wrote:
> While the compiler doesn't support RISCV_HWPROBE_EXT_ZVE64X,
> we use RISCV_HWPROBE_IMA_V instead.

Language is incorrect here.  The compiler has nothing to do with it.
Perhaps "If the installed kernel header files do not support...".

However, if you use only RISCV_HWPROBE_IMA_V, then you do not have any of the additional 
guarantees of Zve64x.

The kernel api for RISCV_HWPROBE_EXT_ZVE64X was introduced in 6.10.
If that is acceptable as a minimum, the simplest solution is

#ifndef RISCV_HWPROBE_EXT_ZVE64X
#define RISCV_HWPROBE_EXT_ZVE64X        (1ULL << 39)
#endif

If the running kernel is old, then the bit will not be set and we will not attempt to use RVV.

If we need to support older kernels, then we'll have to go back to probing with vsetvl to 
determine if all of the additional guarantees of Zve64x are met.


r~


> 
> Signed-off-by: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com>
> Reviewed-by: Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
> ---
>   host/include/riscv/host/cpuinfo.h |  2 ++
>   util/cpuinfo-riscv.c              | 24 ++++++++++++++++++++++--
>   2 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/host/include/riscv/host/cpuinfo.h b/host/include/riscv/host/cpuinfo.h
> index 2b00660e36..cdc784e7b6 100644
> --- a/host/include/riscv/host/cpuinfo.h
> +++ b/host/include/riscv/host/cpuinfo.h
> @@ -10,9 +10,11 @@
>   #define CPUINFO_ZBA             (1u << 1)
>   #define CPUINFO_ZBB             (1u << 2)
>   #define CPUINFO_ZICOND          (1u << 3)
> +#define CPUINFO_ZVE64X          (1u << 4)
>   
>   /* Initialized with a constructor. */
>   extern unsigned cpuinfo;
> +extern unsigned riscv_lg2_vlenb;
>   
>   /*
>    * We cannot rely on constructor ordering, so other constructors must
> diff --git a/util/cpuinfo-riscv.c b/util/cpuinfo-riscv.c
> index 497ce12680..bab782745b 100644
> --- a/util/cpuinfo-riscv.c
> +++ b/util/cpuinfo-riscv.c
> @@ -4,6 +4,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/host-utils.h"
>   #include "host/cpuinfo.h"
>   
>   #ifdef CONFIG_ASM_HWPROBE_H
> @@ -12,6 +13,7 @@
>   #endif
>   
>   unsigned cpuinfo;
> +unsigned riscv_lg2_vlenb;
>   static volatile sig_atomic_t got_sigill;
>   
>   static void sigill_handler(int signo, siginfo_t *si, void *data)
> @@ -33,7 +35,7 @@ static void sigill_handler(int signo, siginfo_t *si, void *data)
>   /* Called both as constructor and (possibly) via other constructors. */
>   unsigned __attribute__((constructor)) cpuinfo_init(void)
>   {
> -    unsigned left = CPUINFO_ZBA | CPUINFO_ZBB | CPUINFO_ZICOND;
> +    unsigned left = CPUINFO_ZBA | CPUINFO_ZBB | CPUINFO_ZICOND | CPUINFO_ZVE64X;
>       unsigned info = cpuinfo;
>   
>       if (info) {
> @@ -49,6 +51,9 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
>   #endif
>   #if defined(__riscv_arch_test) && defined(__riscv_zicond)
>       info |= CPUINFO_ZICOND;
> +#endif
> +#if defined(__riscv_arch_test) && defined(__riscv_zve64x)
> +    info |= CPUINFO_ZVE64X;
>   #endif
>       left &= ~info;
>   
> @@ -64,7 +69,8 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
>               && pair.key >= 0) {
>               info |= pair.value & RISCV_HWPROBE_EXT_ZBA ? CPUINFO_ZBA : 0;
>               info |= pair.value & RISCV_HWPROBE_EXT_ZBB ? CPUINFO_ZBB : 0;
> -            left &= ~(CPUINFO_ZBA | CPUINFO_ZBB);
> +            info |= pair.value & RISCV_HWPROBE_IMA_V ? CPUINFO_ZVE64X : 0;
> +            left &= ~(CPUINFO_ZBA | CPUINFO_ZBB | CPUINFO_ZVE64X);
>   #ifdef RISCV_HWPROBE_EXT_ZICOND
>               info |= pair.value & RISCV_HWPROBE_EXT_ZICOND ? CPUINFO_ZICOND : 0;
>               left &= ~CPUINFO_ZICOND;
> @@ -112,6 +118,20 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
>           assert(left == 0);
>       }
>   
> +    if (info & CPUINFO_ZVE64X) {
> +        /*
> +         * We are guaranteed by RVV-1.0 that VLEN is a power of 2.
> +         * We are guaranteed by Zve64x that VLEN >= 64, and that
> +         * EEW of {8,16,32,64} are supported.
> +         *
> +         * Cache VLEN in a convenient form.
> +         */
> +        unsigned long vlenb;
> +        /* Read csr "vlenb" with "csrr %0, vlenb" : "=r"(vlenb) */
> +        asm volatile(".insn i 0x73, 0x2, %0, zero, -990" : "=r"(vlenb));
> +        riscv_lg2_vlenb = ctz32(vlenb);
> +    }
> +
>       info |= CPUINFO_ALWAYS;
>       cpuinfo = info;
>       return info;
Re: [PATCH v4 01/12] util: Add RISC-V vector extension probe in cpuinfo
Posted by LIU Zhiwei 2 months, 1 week ago
On 2024/9/12 2:34, Richard Henderson wrote:
> On 9/11/24 06:26, LIU Zhiwei wrote:
>> While the compiler doesn't support RISCV_HWPROBE_EXT_ZVE64X,
>> we use RISCV_HWPROBE_IMA_V instead.
>
> Language is incorrect here.  The compiler has nothing to do with it.
> Perhaps "If the installed kernel header files do not support...".
OK. Thanks.
>
> However, if you use only RISCV_HWPROBE_IMA_V, then you do not have any 
> of the additional guarantees of Zve64x.

IMHO, RISCV_HWPROBE_IMA_V is more strictly constrainted than 
RISCV_HWPROBE_EXT_ZVE64X.
At least in current QEMU implemenation, the V vector extension depends 
on the Zve64d extension.

Thanks,
Zhiwei

> The kernel api for RISCV_HWPROBE_EXT_ZVE64X was introduced in 6.10.
> If that is acceptable as a minimum, the simplest solution is
>
> #ifndef RISCV_HWPROBE_EXT_ZVE64X
> #define RISCV_HWPROBE_EXT_ZVE64X        (1ULL << 39)
> #endif
>
> If the running kernel is old, then the bit will not be set and we will 
> not attempt to use RVV.
>
> If we need to support older kernels, then we'll have to go back to 
> probing with vsetvl to determine if all of the additional guarantees 
> of Zve64x are met.
>
>
> r~
>
>
>>
>> Signed-off-by: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com>
>> Reviewed-by: Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
>> ---
>>   host/include/riscv/host/cpuinfo.h |  2 ++
>>   util/cpuinfo-riscv.c              | 24 ++++++++++++++++++++++--
>>   2 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/host/include/riscv/host/cpuinfo.h 
>> b/host/include/riscv/host/cpuinfo.h
>> index 2b00660e36..cdc784e7b6 100644
>> --- a/host/include/riscv/host/cpuinfo.h
>> +++ b/host/include/riscv/host/cpuinfo.h
>> @@ -10,9 +10,11 @@
>>   #define CPUINFO_ZBA             (1u << 1)
>>   #define CPUINFO_ZBB             (1u << 2)
>>   #define CPUINFO_ZICOND          (1u << 3)
>> +#define CPUINFO_ZVE64X          (1u << 4)
>>     /* Initialized with a constructor. */
>>   extern unsigned cpuinfo;
>> +extern unsigned riscv_lg2_vlenb;
>>     /*
>>    * We cannot rely on constructor ordering, so other constructors must
>> diff --git a/util/cpuinfo-riscv.c b/util/cpuinfo-riscv.c
>> index 497ce12680..bab782745b 100644
>> --- a/util/cpuinfo-riscv.c
>> +++ b/util/cpuinfo-riscv.c
>> @@ -4,6 +4,7 @@
>>    */
>>     #include "qemu/osdep.h"
>> +#include "qemu/host-utils.h"
>>   #include "host/cpuinfo.h"
>>     #ifdef CONFIG_ASM_HWPROBE_H
>> @@ -12,6 +13,7 @@
>>   #endif
>>     unsigned cpuinfo;
>> +unsigned riscv_lg2_vlenb;
>>   static volatile sig_atomic_t got_sigill;
>>     static void sigill_handler(int signo, siginfo_t *si, void *data)
>> @@ -33,7 +35,7 @@ static void sigill_handler(int signo, siginfo_t 
>> *si, void *data)
>>   /* Called both as constructor and (possibly) via other 
>> constructors. */
>>   unsigned __attribute__((constructor)) cpuinfo_init(void)
>>   {
>> -    unsigned left = CPUINFO_ZBA | CPUINFO_ZBB | CPUINFO_ZICOND;
>> +    unsigned left = CPUINFO_ZBA | CPUINFO_ZBB | CPUINFO_ZICOND | 
>> CPUINFO_ZVE64X;
>>       unsigned info = cpuinfo;
>>         if (info) {
>> @@ -49,6 +51,9 @@ unsigned __attribute__((constructor)) 
>> cpuinfo_init(void)
>>   #endif
>>   #if defined(__riscv_arch_test) && defined(__riscv_zicond)
>>       info |= CPUINFO_ZICOND;
>> +#endif
>> +#if defined(__riscv_arch_test) && defined(__riscv_zve64x)
>> +    info |= CPUINFO_ZVE64X;
>>   #endif
>>       left &= ~info;
>>   @@ -64,7 +69,8 @@ unsigned __attribute__((constructor)) 
>> cpuinfo_init(void)
>>               && pair.key >= 0) {
>>               info |= pair.value & RISCV_HWPROBE_EXT_ZBA ? 
>> CPUINFO_ZBA : 0;
>>               info |= pair.value & RISCV_HWPROBE_EXT_ZBB ? 
>> CPUINFO_ZBB : 0;
>> -            left &= ~(CPUINFO_ZBA | CPUINFO_ZBB);
>> +            info |= pair.value & RISCV_HWPROBE_IMA_V ? 
>> CPUINFO_ZVE64X : 0;
>> +            left &= ~(CPUINFO_ZBA | CPUINFO_ZBB | CPUINFO_ZVE64X);
>>   #ifdef RISCV_HWPROBE_EXT_ZICOND
>>               info |= pair.value & RISCV_HWPROBE_EXT_ZICOND ? 
>> CPUINFO_ZICOND : 0;
>>               left &= ~CPUINFO_ZICOND;
>> @@ -112,6 +118,20 @@ unsigned __attribute__((constructor)) 
>> cpuinfo_init(void)
>>           assert(left == 0);
>>       }
>>   +    if (info & CPUINFO_ZVE64X) {
>> +        /*
>> +         * We are guaranteed by RVV-1.0 that VLEN is a power of 2.
>> +         * We are guaranteed by Zve64x that VLEN >= 64, and that
>> +         * EEW of {8,16,32,64} are supported.
>> +         *
>> +         * Cache VLEN in a convenient form.
>> +         */
>> +        unsigned long vlenb;
>> +        /* Read csr "vlenb" with "csrr %0, vlenb" : "=r"(vlenb) */
>> +        asm volatile(".insn i 0x73, 0x2, %0, zero, -990" : 
>> "=r"(vlenb));
>> +        riscv_lg2_vlenb = ctz32(vlenb);
>> +    }
>> +
>>       info |= CPUINFO_ALWAYS;
>>       cpuinfo = info;
>>       return info;
>

Re: [PATCH v4 01/12] util: Add RISC-V vector extension probe in cpuinfo
Posted by Richard Henderson 2 months ago
On 9/18/24 07:14, LIU Zhiwei wrote:
> 
> On 2024/9/12 2:34, Richard Henderson wrote:
>> On 9/11/24 06:26, LIU Zhiwei wrote:
>>> While the compiler doesn't support RISCV_HWPROBE_EXT_ZVE64X,
>>> we use RISCV_HWPROBE_IMA_V instead.
>>
>> Language is incorrect here.  The compiler has nothing to do with it.
>> Perhaps "If the installed kernel header files do not support...".
> OK. Thanks.
>>
>> However, if you use only RISCV_HWPROBE_IMA_V, then you do not have any of the additional 
>> guarantees of Zve64x.
> 
> IMHO, RISCV_HWPROBE_IMA_V is more strictly constrainted than RISCV_HWPROBE_EXT_ZVE64X.
> At least in current QEMU implemenation, the V vector extension depends on the Zve64d 
> extension.

Ok.  That is a better explanation for the patch description.


r~