RISC-V: Increase max vlen to 4096

[PATCH] RISC-V: Increase max vlen to 4096

Posted by Patrick O'Neill 1 year ago

QEMU currently limits the max vlenb to 1024. GCC sets the upper bound
to 4096 [1]. There doesn't seem to be an upper bound set by the spec [2]
so this patch just changes QEMU to match GCC's upper bound.

[1] https://github.com/gcc-mirror/gcc/blob/5d2a360f0a541646abb11efdbabc33c6a04de7ee/gcc/testsuite/gcc.target/riscv/rvv/base/zvl-unimplemented-2.c#L4
[2] https://github.com/riscv/riscv-v-spec/issues/204

Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
---
Tested by applying to QEMU v8.1.2 and running the GCC testsuite in QEMU
user mode with rv64gcv_zvl4096b. Failures are somewhat reasonable and on
first inspection appear to be in the same ballpark as failures for
rv64gcv_zvl1024b. Since I used tip-of-tree GCC I'm expecting those
failures to be GCC-caused & from skimming the debug log they appear to
be.
---
 target/riscv/cpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 6ea22e0eea..2ff3a72fc0 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -97,7 +97,7 @@ typedef enum {
 #include "debug.h"
 #endif

-#define RV_VLEN_MAX 1024
+#define RV_VLEN_MAX 4096
 #define RV_MAX_MHPMEVENTS 32
 #define RV_MAX_MHPMCOUNTERS 32

--
2.34.1

Re: [PATCH] RISC-V: Increase max vlen to 4096

Posted by Philippe Mathieu-Daudé 1 year ago

Hi Patrick,

On 23/11/23 01:17, Patrick O'Neill wrote:
> QEMU currently limits the max vlenb to 1024. GCC sets the upper bound
> to 4096 [1]. There doesn't seem to be an upper bound set by the spec [2]
> so this patch just changes QEMU to match GCC's upper bound.
> 
> [1] https://github.com/gcc-mirror/gcc/blob/5d2a360f0a541646abb11efdbabc33c6a04de7ee/gcc/testsuite/gcc.target/riscv/rvv/base/zvl-unimplemented-2.c#L4
> [2] https://github.com/riscv/riscv-v-spec/issues/204
> 
> Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
> ---
> Tested by applying to QEMU v8.1.2 and running the GCC testsuite in QEMU
> user mode with rv64gcv_zvl4096b. Failures are somewhat reasonable and on
> first inspection appear to be in the same ballpark as failures for
> rv64gcv_zvl1024b. Since I used tip-of-tree GCC I'm expecting those
> failures to be GCC-caused & from skimming the debug log they appear to
> be.
> ---
>   target/riscv/cpu.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 6ea22e0eea..2ff3a72fc0 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -97,7 +97,7 @@ typedef enum {
>   #include "debug.h"
>   #endif
> 
> -#define RV_VLEN_MAX 1024
> +#define RV_VLEN_MAX 4096

This seems to break the "cpu/vector" field migration. Maybe we don't
care, but this should be clarified in the commit description.

Regards,

Phil.

Re: [PATCH] RISC-V: Increase max vlen to 4096

Posted by Patrick O'Neill 1 year ago

Hi Phil,

On 11/23/23 02:21, Philippe Mathieu-Daudé wrote:
> Hi Patrick,
>
> On 23/11/23 01:17, Patrick O'Neill wrote:
>> QEMU currently limits the max vlenb to 1024. GCC sets the upper bound
>> to 4096 [1]. There doesn't seem to be an upper bound set by the spec [2]
>> so this patch just changes QEMU to match GCC's upper bound.
>>
>> [1] 
>> https://github.com/gcc-mirror/gcc/blob/5d2a360f0a541646abb11efdbabc33c6a04de7ee/gcc/testsuite/gcc.target/riscv/rvv/base/zvl-unimplemented-2.c#L4
>> [2] https://github.com/riscv/riscv-v-spec/issues/204
>>
>> Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
>> ---
>> Tested by applying to QEMU v8.1.2 and running the GCC testsuite in QEMU
>> user mode with rv64gcv_zvl4096b. Failures are somewhat reasonable and on
>> first inspection appear to be in the same ballpark as failures for
>> rv64gcv_zvl1024b. Since I used tip-of-tree GCC I'm expecting those
>> failures to be GCC-caused & from skimming the debug log they appear to
>> be.
>> ---
>>   target/riscv/cpu.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index 6ea22e0eea..2ff3a72fc0 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -97,7 +97,7 @@ typedef enum {
>>   #include "debug.h"
>>   #endif
>>
>> -#define RV_VLEN_MAX 1024
>> +#define RV_VLEN_MAX 4096
>
> This seems to break the "cpu/vector" field migration. Maybe we don't
> care, but this should be clarified in the commit description.
I wasn't aware of that (this is actually my first patch to qemu!). Do
you have a pointer to more information about the migration so I can
write an appropriate blurb/understand what the migration was/did?

Thanks,
Patrick
>
> Regards,
>
> Phil.
>

Re: [PATCH] RISC-V: Increase max vlen to 4096

Posted by Alistair Francis 11 months, 3 weeks ago

On Tue, Nov 28, 2023 at 3:09 AM Patrick O'Neill <patrick@rivosinc.com> wrote:
>
> Hi Phil,
>
> On 11/23/23 02:21, Philippe Mathieu-Daudé wrote:
> > Hi Patrick,
> >
> > On 23/11/23 01:17, Patrick O'Neill wrote:
> >> QEMU currently limits the max vlenb to 1024. GCC sets the upper bound
> >> to 4096 [1]. There doesn't seem to be an upper bound set by the spec [2]
> >> so this patch just changes QEMU to match GCC's upper bound.
> >>
> >> [1]
> >> https://github.com/gcc-mirror/gcc/blob/5d2a360f0a541646abb11efdbabc33c6a04de7ee/gcc/testsuite/gcc.target/riscv/rvv/base/zvl-unimplemented-2.c#L4
> >> [2] https://github.com/riscv/riscv-v-spec/issues/204
> >>
> >> Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
> >> ---
> >> Tested by applying to QEMU v8.1.2 and running the GCC testsuite in QEMU
> >> user mode with rv64gcv_zvl4096b. Failures are somewhat reasonable and on
> >> first inspection appear to be in the same ballpark as failures for
> >> rv64gcv_zvl1024b. Since I used tip-of-tree GCC I'm expecting those
> >> failures to be GCC-caused & from skimming the debug log they appear to
> >> be.
> >> ---
> >>   target/riscv/cpu.h | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> >> index 6ea22e0eea..2ff3a72fc0 100644
> >> --- a/target/riscv/cpu.h
> >> +++ b/target/riscv/cpu.h
> >> @@ -97,7 +97,7 @@ typedef enum {
> >>   #include "debug.h"
> >>   #endif
> >>
> >> -#define RV_VLEN_MAX 1024
> >> +#define RV_VLEN_MAX 4096
> >
> > This seems to break the "cpu/vector" field migration. Maybe we don't
> > care, but this should be clarified in the commit description.
> I wasn't aware of that (this is actually my first patch to qemu!). Do
> you have a pointer to more information about the migration so I can
> write an appropriate blurb/understand what the migration was/did?

If you increase the version_id and minimum_version_id in this struct
that should handle it:

static const VMStateDescription vmstate_vector = {
    .name = "cpu/vector",
    .version_id = 2,
    .minimum_version_id = 2,
    .needed = vector_needed,
    .fields = (VMStateField[]) {
        VMSTATE_UINT64_ARRAY(env.vreg, RISCVCPU, 32 * RV_VLEN_MAX / 64),
        VMSTATE_UINTTL(env.vxrm, RISCVCPU),
        VMSTATE_UINTTL(env.vxsat, RISCVCPU),
        VMSTATE_UINTTL(env.vl, RISCVCPU),
        VMSTATE_UINTTL(env.vstart, RISCVCPU),
        VMSTATE_UINTTL(env.vtype, RISCVCPU),
        VMSTATE_BOOL(env.vill, RISCVCPU),
        VMSTATE_END_OF_LIST()
    }
};

Alistair

>
> Thanks,
> Patrick
> >
> > Regards,
> >
> > Phil.
> >
>