1. Patchew
  2. QEMU
  3. [PATCH v2 0/5] target/riscv: Enhanced ISA extension checks
  4. View patch

[PATCH v2 4/5] target/riscv: FP extension requirements

Tsukasa OI posted 5 patches 3 years, 1 month ago
[PATCH v2 4/5] target/riscv: FP extension requirements
Posted by Tsukasa OI 3 years, 1 month ago 
QEMU allowed inconsistent configurations that made floating point
arithmetic effectively unusable.

This commit adds certain checks for consistent FP arithmetic:

-   F requires Zicsr
-   Zfinx requires Zicsr
-   Zfh/Zfhmin require F
-   D requires F
-   V requires D

Because F/D/Zicsr are enabled by default (and an error will not occur unless
we manually disable one or more of prerequisites), this commit just enforces
the user to give consistent combinations.

Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
---
 target/riscv/cpu.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 0854ca9103..f910a41407 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -610,11 +610,36 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
             cpu->cfg.ext_ifencei = true;
         }
 
+        if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) {
+            error_setg(errp, "F extension requires Zicsr");
+            return;
+        }
+
+        if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) {
+            error_setg(errp, "Zfh/Zfhmin extensions require F extension");
+            return;
+        }
+
+        if (cpu->cfg.ext_d && !cpu->cfg.ext_f) {
+            error_setg(errp, "D extension requires F extension");
+            return;
+        }
+
+        if (cpu->cfg.ext_v && !cpu->cfg.ext_d) {
+            error_setg(errp, "V extension requires D extension");
+            return;
+        }
+
         if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
             cpu->cfg.ext_zhinxmin) {
             cpu->cfg.ext_zfinx = true;
         }
 
+        if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) {
+            error_setg(errp, "Zfinx extension requires Zicsr");
+            return;
+        }
+
         if (cpu->cfg.ext_zk) {
             cpu->cfg.ext_zkn = true;
             cpu->cfg.ext_zkr = true;
-- 
2.34.1
Re: [PATCH v2 4/5] target/riscv: FP extension requirements
Posted by Alistair Francis 3 years, 1 month ago 
On Sun, May 15, 2022 at 12:56 PM Tsukasa OI
<research_trasio@irq.a4lg.com> wrote:
>
> QEMU allowed inconsistent configurations that made floating point
> arithmetic effectively unusable.
>
> This commit adds certain checks for consistent FP arithmetic:
>
> -   F requires Zicsr
> -   Zfinx requires Zicsr
> -   Zfh/Zfhmin require F
> -   D requires F
> -   V requires D
>
> Because F/D/Zicsr are enabled by default (and an error will not occur unless
> we manually disable one or more of prerequisites), this commit just enforces
> the user to give consistent combinations.
>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 0854ca9103..f910a41407 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -610,11 +610,36 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>              cpu->cfg.ext_ifencei = true;
>          }
>
> +        if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) {
> +            error_setg(errp, "F extension requires Zicsr");
> +            return;
> +        }
> +
> +        if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) {
> +            error_setg(errp, "Zfh/Zfhmin extensions require F extension");
> +            return;
> +        }
> +
> +        if (cpu->cfg.ext_d && !cpu->cfg.ext_f) {
> +            error_setg(errp, "D extension requires F extension");
> +            return;
> +        }
> +
> +        if (cpu->cfg.ext_v && !cpu->cfg.ext_d) {
> +            error_setg(errp, "V extension requires D extension");
> +            return;
> +        }
> +
>          if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
>              cpu->cfg.ext_zhinxmin) {
>              cpu->cfg.ext_zfinx = true;
>          }
>
> +        if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) {
> +            error_setg(errp, "Zfinx extension requires Zicsr");
> +            return;
> +        }
> +
>          if (cpu->cfg.ext_zk) {
>              cpu->cfg.ext_zkn = true;
>              cpu->cfg.ext_zkr = true;
> --
> 2.34.1
>
Re: [PATCH v2 4/5] target/riscv: FP extension requirements
Posted by Weiwei Li 3 years, 1 month ago 
在 2022/5/15 上午10:56, Tsukasa OI 写道:
> QEMU allowed inconsistent configurations that made floating point
> arithmetic effectively unusable.
>
> This commit adds certain checks for consistent FP arithmetic:
>
> -   F requires Zicsr
> -   Zfinx requires Zicsr
> -   Zfh/Zfhmin require F
> -   D requires F
> -   V requires D

Why 'V requires D'? I know partial vector instructions require D, 
However,  I think they  just like c.fsd

instruction requires D, not 'C requires D' or 'D requires C'. Is there 
any rvv spec change I don't know?

Regards.

Weiwei Li

>
> Because F/D/Zicsr are enabled by default (and an error will not occur unless
> we manually disable one or more of prerequisites), this commit just enforces
> the user to give consistent combinations.
>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
> ---
>   target/riscv/cpu.c | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 0854ca9103..f910a41407 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -610,11 +610,36 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>               cpu->cfg.ext_ifencei = true;
>           }
>   
> +        if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) {
> +            error_setg(errp, "F extension requires Zicsr");
> +            return;
> +        }
> +
> +        if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) {
> +            error_setg(errp, "Zfh/Zfhmin extensions require F extension");
> +            return;
> +        }
> +
> +        if (cpu->cfg.ext_d && !cpu->cfg.ext_f) {
> +            error_setg(errp, "D extension requires F extension");
> +            return;
> +        }
> +
> +        if (cpu->cfg.ext_v && !cpu->cfg.ext_d) {
> +            error_setg(errp, "V extension requires D extension");
> +            return;
> +        }
> +
>           if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
>               cpu->cfg.ext_zhinxmin) {
>               cpu->cfg.ext_zfinx = true;
>           }
>   
> +        if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) {
> +            error_setg(errp, "Zfinx extension requires Zicsr");
> +            return;
> +        }
> +
>           if (cpu->cfg.ext_zk) {
>               cpu->cfg.ext_zkn = true;
>               cpu->cfg.ext_zkr = true;
Re: [PATCH v2 4/5] target/riscv: FP extension requirements
Posted by Tsukasa OI 3 years, 1 month ago 
On 2022/05/15 23:37, Weiwei Li wrote:
> 
> 在 2022/5/15 上午10:56, Tsukasa OI 写道:
>> QEMU allowed inconsistent configurations that made floating point
>> arithmetic effectively unusable.
>>
>> This commit adds certain checks for consistent FP arithmetic:
>>
>> -   F requires Zicsr
>> -   Zfinx requires Zicsr
>> -   Zfh/Zfhmin require F
>> -   D requires F
>> -   V requires D
> 
> Why 'V requires D'? I know partial vector instructions require D, However,  I think they  just like c.fsd
> 
> instruction requires D, not 'C requires D' or 'D requires C'. Is there any rvv spec change I don't know?

I thought it was crystal-clear.

RISC-V "V" Vector Extension Version 1.0 (riscv-v-spec-1.0.pdf)
18.3. V: Vector Extension for Application Processors
Page 94:

> The V extension requires the scalar processor implements the F and D
> extensions, and implements all vector floating-point instructions
> (Section Vector Floating-Point Instructions) for floating-point operands
> with EEW=32 or EEW=64 (including widening instructions and conversions
> between FP32 and FP64).

c.f. <https://github.com/riscv/riscv-v-spec/releases/tag/v1.0>

Thanks,

Tsukasa

> 
> Regards.
> 
> Weiwei Li
> 
>>
>> Because F/D/Zicsr are enabled by default (and an error will not occur unless
>> we manually disable one or more of prerequisites), this commit just enforces
>> the user to give consistent combinations.
>>
>> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
>> ---
>>   target/riscv/cpu.c | 25 +++++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 0854ca9103..f910a41407 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -610,11 +610,36 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>>               cpu->cfg.ext_ifencei = true;
>>           }
>>   +        if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) {
>> +            error_setg(errp, "F extension requires Zicsr");
>> +            return;
>> +        }
>> +
>> +        if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) {
>> +            error_setg(errp, "Zfh/Zfhmin extensions require F extension");
>> +            return;
>> +        }
>> +
>> +        if (cpu->cfg.ext_d && !cpu->cfg.ext_f) {
>> +            error_setg(errp, "D extension requires F extension");
>> +            return;
>> +        }
>> +
>> +        if (cpu->cfg.ext_v && !cpu->cfg.ext_d) {
>> +            error_setg(errp, "V extension requires D extension");
>> +            return;
>> +        }
>> +
>>           if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
>>               cpu->cfg.ext_zhinxmin) {
>>               cpu->cfg.ext_zfinx = true;
>>           }
>>   +        if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) {
>> +            error_setg(errp, "Zfinx extension requires Zicsr");
>> +            return;
>> +        }
>> +
>>           if (cpu->cfg.ext_zk) {
>>               cpu->cfg.ext_zkn = true;
>>               cpu->cfg.ext_zkr = true;
>
Re: [PATCH v2 4/5] target/riscv: FP extension requirements
Posted by Weiwei Li 3 years, 1 month ago 
在 2022/5/15 下午10:45, Tsukasa OI 写道:
> On 2022/05/15 23:37, Weiwei Li wrote:
>> 在 2022/5/15 上午10:56, Tsukasa OI 写道:
>>> QEMU allowed inconsistent configurations that made floating point
>>> arithmetic effectively unusable.
>>>
>>> This commit adds certain checks for consistent FP arithmetic:
>>>
>>> -   F requires Zicsr
>>> -   Zfinx requires Zicsr
>>> -   Zfh/Zfhmin require F
>>> -   D requires F
>>> -   V requires D
>> Why 'V requires D'? I know partial vector instructions require D, However,  I think they  just like c.fsd
>>
>> instruction requires D, not 'C requires D' or 'D requires C'. Is there any rvv spec change I don't know?
> I thought it was crystal-clear.
>
> RISC-V "V" Vector Extension Version 1.0 (riscv-v-spec-1.0.pdf)
> 18.3. V: Vector Extension for Application Processors
> Page 94:
>
>> The V extension requires the scalar processor implements the F and D
>> extensions, and implements all vector floating-point instructions
>> (Section Vector Floating-Point Instructions) for floating-point operands
>> with EEW=32 or EEW=64 (including widening instructions and conversions
>> between FP32 and FP64).
> c.f. <https://github.com/riscv/riscv-v-spec/releases/tag/v1.0>
>
> Thanks,
>
> Tsukasa

OK. Thanks.

Regards,

Weiwei Li

>
>> Regards.
>>
>> Weiwei Li
>>
>>> Because F/D/Zicsr are enabled by default (and an error will not occur unless
>>> we manually disable one or more of prerequisites), this commit just enforces
>>> the user to give consistent combinations.
>>>
>>> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
>>> ---
>>>    target/riscv/cpu.c | 25 +++++++++++++++++++++++++
>>>    1 file changed, 25 insertions(+)
>>>
>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>>> index 0854ca9103..f910a41407 100644
>>> --- a/target/riscv/cpu.c
>>> +++ b/target/riscv/cpu.c
>>> @@ -610,11 +610,36 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>>>                cpu->cfg.ext_ifencei = true;
>>>            }
>>>    +        if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) {
>>> +            error_setg(errp, "F extension requires Zicsr");
>>> +            return;
>>> +        }
>>> +
>>> +        if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) {
>>> +            error_setg(errp, "Zfh/Zfhmin extensions require F extension");
>>> +            return;
>>> +        }
>>> +
>>> +        if (cpu->cfg.ext_d && !cpu->cfg.ext_f) {
>>> +            error_setg(errp, "D extension requires F extension");
>>> +            return;
>>> +        }
>>> +
>>> +        if (cpu->cfg.ext_v && !cpu->cfg.ext_d) {
>>> +            error_setg(errp, "V extension requires D extension");
>>> +            return;
>>> +        }
>>> +
>>>            if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
>>>                cpu->cfg.ext_zhinxmin) {
>>>                cpu->cfg.ext_zfinx = true;
>>>            }
>>>    +        if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) {
>>> +            error_setg(errp, "Zfinx extension requires Zicsr");
>>> +            return;
>>> +        }
>>> +
>>>            if (cpu->cfg.ext_zk) {
>>>                cpu->cfg.ext_zkn = true;
>>>                cpu->cfg.ext_zkr = true;

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.