1. Patchew
  2. QEMU
  3. [PULL 00/18] riscv-to-apply queue
  4. View patch

[PULL 16/18] target/riscv: change the api for RVF/RVD fmin/fmax

Alistair Francis posted 18 patches 4 years, 1 month ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, Bin Meng <bin.meng@windriver.com>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <Alistair.Francis@wdc.com>, Alistair Francis <alistair.francis@wdc.com>, Peter Maydell <peter.maydell@linaro.org>, Aurelien Jarno <aurelien@aurel32.net>
There is a newer version of this series
[PULL 16/18] target/riscv: change the api for RVF/RVD fmin/fmax
Posted by Alistair Francis 4 years, 1 month ago 
From: Chih-Min Chao <chihmin.chao@sifive.com>

The sNaN propagation behavior has been changed since
cd20cee7 in https://github.com/riscv/riscv-isa-manual.

Signed-off-by: Chih-Min Chao <chihmin.chao@sifive.com>
Signed-off-by: Frank Chang <frank.chang@sifive.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20211016085428.3001501-3-frank.chang@sifive.com
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/fpu_helper.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
index 8700516a14..d62f470900 100644
--- a/target/riscv/fpu_helper.c
+++ b/target/riscv/fpu_helper.c
@@ -174,14 +174,18 @@ uint64_t helper_fmin_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2)
 {
     float32 frs1 = check_nanbox_s(rs1);
     float32 frs2 = check_nanbox_s(rs2);
-    return nanbox_s(float32_minnum(frs1, frs2, &env->fp_status));
+    return nanbox_s(env->priv_ver < PRIV_VERSION_1_11_0 ?
+                    float32_minnum(frs1, frs2, &env->fp_status) :
+                    float32_minimum_number(frs1, frs2, &env->fp_status));
 }
 
 uint64_t helper_fmax_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2)
 {
     float32 frs1 = check_nanbox_s(rs1);
     float32 frs2 = check_nanbox_s(rs2);
-    return nanbox_s(float32_maxnum(frs1, frs2, &env->fp_status));
+    return nanbox_s(env->priv_ver < PRIV_VERSION_1_11_0 ?
+                    float32_maxnum(frs1, frs2, &env->fp_status) :
+                    float32_maximum_number(frs1, frs2, &env->fp_status));
 }
 
 uint64_t helper_fsqrt_s(CPURISCVState *env, uint64_t rs1)
@@ -283,12 +287,16 @@ uint64_t helper_fdiv_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2)
 
 uint64_t helper_fmin_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2)
 {
-    return float64_minnum(frs1, frs2, &env->fp_status);
+    return env->priv_ver < PRIV_VERSION_1_11_0 ?
+            float64_minnum(frs1, frs2, &env->fp_status) :
+            float64_minimum_number(frs1, frs2, &env->fp_status);
 }
 
 uint64_t helper_fmax_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2)
 {
-    return float64_maxnum(frs1, frs2, &env->fp_status);
+    return env->priv_ver < PRIV_VERSION_1_11_0 ?
+            float64_maxnum(frs1, frs2, &env->fp_status) :
+            float64_maximum_number(frs1, frs2, &env->fp_status);
 }
 
 uint64_t helper_fcvt_s_d(CPURISCVState *env, uint64_t rs1)
-- 
2.31.1
Re: [PULL 16/18] target/riscv: change the api for RVF/RVD fmin/fmax
Posted by Frank Chang 4 years, 1 month ago 
On Thu, Oct 28, 2021 at 12:45 PM Alistair Francis <
alistair.francis@opensource.wdc.com> wrote:

> From: Chih-Min Chao <chihmin.chao@sifive.com>
>
> The sNaN propagation behavior has been changed since
> cd20cee7 in https://github.com/riscv/riscv-isa-manual.
>
> Signed-off-by: Chih-Min Chao <chihmin.chao@sifive.com>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> Message-id: 20211016085428.3001501-3-frank.chang@sifive.com
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/fpu_helper.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
> index 8700516a14..d62f470900 100644
> --- a/target/riscv/fpu_helper.c
> +++ b/target/riscv/fpu_helper.c
> @@ -174,14 +174,18 @@ uint64_t helper_fmin_s(CPURISCVState *env, uint64_t
> rs1, uint64_t rs2)
>  {
>      float32 frs1 = check_nanbox_s(rs1);
>      float32 frs2 = check_nanbox_s(rs2);
> -    return nanbox_s(float32_minnum(frs1, frs2, &env->fp_status));
> +    return nanbox_s(env->priv_ver < PRIV_VERSION_1_11_0 ?
> +                    float32_minnum(frs1, frs2, &env->fp_status) :
> +                    float32_minimum_number(frs1, frs2, &env->fp_status));
>  }
>
>  uint64_t helper_fmax_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2)
>  {
>      float32 frs1 = check_nanbox_s(rs1);
>      float32 frs2 = check_nanbox_s(rs2);
> -    return nanbox_s(float32_maxnum(frs1, frs2, &env->fp_status));
> +    return nanbox_s(env->priv_ver < PRIV_VERSION_1_11_0 ?
> +                    float32_maxnum(frs1, frs2, &env->fp_status) :
> +                    float32_maximum_number(frs1, frs2, &env->fp_status));
>  }
>
>  uint64_t helper_fsqrt_s(CPURISCVState *env, uint64_t rs1)
> @@ -283,12 +287,16 @@ uint64_t helper_fdiv_d(CPURISCVState *env, uint64_t
> frs1, uint64_t frs2)
>
>  uint64_t helper_fmin_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2)
>  {
> -    return float64_minnum(frs1, frs2, &env->fp_status);
> +    return env->priv_ver < PRIV_VERSION_1_11_0 ?
> +            float64_minnum(frs1, frs2, &env->fp_status) :
> +            float64_minimum_number(frs1, frs2, &env->fp_status);
>  }
>
>  uint64_t helper_fmax_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2)
>  {
> -    return float64_maxnum(frs1, frs2, &env->fp_status);
> +    return env->priv_ver < PRIV_VERSION_1_11_0 ?
> +            float64_maxnum(frs1, frs2, &env->fp_status) :
> +            float64_maximum_number(frs1, frs2, &env->fp_status);
>  }
>
>  uint64_t helper_fcvt_s_d(CPURISCVState *env, uint64_t rs1)
> --
> 2.31.1
>
>
Hi Alistair,

Did you pull the latest v5 patchset?
https://lists.nongnu.org/archive/html/qemu-riscv/2021-10/msg00557.html

I added more texts in the commit message to describe why we tie RVF version
with Priv version.
I think it's still okay to pull this one as I don't think there's any
functional changes, IIRC.

Regards,
Frank Chang
Re: [PULL 16/18] target/riscv: change the api for RVF/RVD fmin/fmax
Posted by Alistair Francis 4 years, 1 month ago 
On Thu, Oct 28, 2021 at 6:22 PM Frank Chang <frank.chang@sifive.com> wrote:
>
> On Thu, Oct 28, 2021 at 12:45 PM Alistair Francis <alistair.francis@opensource.wdc.com> wrote:
>>
>> From: Chih-Min Chao <chihmin.chao@sifive.com>
>>
>> The sNaN propagation behavior has been changed since
>> cd20cee7 in https://github.com/riscv/riscv-isa-manual.
>>
>> Signed-off-by: Chih-Min Chao <chihmin.chao@sifive.com>
>> Signed-off-by: Frank Chang <frank.chang@sifive.com>
>> Acked-by: Alistair Francis <alistair.francis@wdc.com>
>> Message-id: 20211016085428.3001501-3-frank.chang@sifive.com
>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>> ---
>>  target/riscv/fpu_helper.c | 16 ++++++++++++----
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
>> index 8700516a14..d62f470900 100644
>> --- a/target/riscv/fpu_helper.c
>> +++ b/target/riscv/fpu_helper.c
>> @@ -174,14 +174,18 @@ uint64_t helper_fmin_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2)
>>  {
>>      float32 frs1 = check_nanbox_s(rs1);
>>      float32 frs2 = check_nanbox_s(rs2);
>> -    return nanbox_s(float32_minnum(frs1, frs2, &env->fp_status));
>> +    return nanbox_s(env->priv_ver < PRIV_VERSION_1_11_0 ?
>> +                    float32_minnum(frs1, frs2, &env->fp_status) :
>> +                    float32_minimum_number(frs1, frs2, &env->fp_status));
>>  }
>>
>>  uint64_t helper_fmax_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2)
>>  {
>>      float32 frs1 = check_nanbox_s(rs1);
>>      float32 frs2 = check_nanbox_s(rs2);
>> -    return nanbox_s(float32_maxnum(frs1, frs2, &env->fp_status));
>> +    return nanbox_s(env->priv_ver < PRIV_VERSION_1_11_0 ?
>> +                    float32_maxnum(frs1, frs2, &env->fp_status) :
>> +                    float32_maximum_number(frs1, frs2, &env->fp_status));
>>  }
>>
>>  uint64_t helper_fsqrt_s(CPURISCVState *env, uint64_t rs1)
>> @@ -283,12 +287,16 @@ uint64_t helper_fdiv_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2)
>>
>>  uint64_t helper_fmin_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2)
>>  {
>> -    return float64_minnum(frs1, frs2, &env->fp_status);
>> +    return env->priv_ver < PRIV_VERSION_1_11_0 ?
>> +            float64_minnum(frs1, frs2, &env->fp_status) :
>> +            float64_minimum_number(frs1, frs2, &env->fp_status);
>>  }
>>
>>  uint64_t helper_fmax_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2)
>>  {
>> -    return float64_maxnum(frs1, frs2, &env->fp_status);
>> +    return env->priv_ver < PRIV_VERSION_1_11_0 ?
>> +            float64_maxnum(frs1, frs2, &env->fp_status) :
>> +            float64_maximum_number(frs1, frs2, &env->fp_status);
>>  }
>>
>>  uint64_t helper_fcvt_s_d(CPURISCVState *env, uint64_t rs1)
>> --
>> 2.31.1
>>
>
> Hi Alistair,
>
> Did you pull the latest v5 patchset?
> https://lists.nongnu.org/archive/html/qemu-riscv/2021-10/msg00557.html

Strange, I don't see it on the patches list. All I see is v4

Alistair

>
> I added more texts in the commit message to describe why we tie RVF version with Priv version.
> I think it's still okay to pull this one as I don't think there's any functional changes, IIRC.
>
> Regards,
> Frank Chang
Re: [PULL 16/18] target/riscv: change the api for RVF/RVD fmin/fmax
Posted by Richard Henderson 4 years, 1 month ago 
On 10/28/21 4:30 AM, Alistair Francis wrote:
> On Thu, Oct 28, 2021 at 6:22 PM Frank Chang <frank.chang@sifive.com> wrote:
>>
>> On Thu, Oct 28, 2021 at 12:45 PM Alistair Francis <alistair.francis@opensource.wdc.com> wrote:
>>>
>>> From: Chih-Min Chao <chihmin.chao@sifive.com>
>>>
>>> The sNaN propagation behavior has been changed since
>>> cd20cee7 in https://github.com/riscv/riscv-isa-manual.
>>>
>>> Signed-off-by: Chih-Min Chao <chihmin.chao@sifive.com>
>>> Signed-off-by: Frank Chang <frank.chang@sifive.com>
>>> Acked-by: Alistair Francis <alistair.francis@wdc.com>
>>> Message-id: 20211016085428.3001501-3-frank.chang@sifive.com
>>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>>> ---
>>>   target/riscv/fpu_helper.c | 16 ++++++++++++----
>>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
>>> index 8700516a14..d62f470900 100644
>>> --- a/target/riscv/fpu_helper.c
>>> +++ b/target/riscv/fpu_helper.c
>>> @@ -174,14 +174,18 @@ uint64_t helper_fmin_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2)
>>>   {
>>>       float32 frs1 = check_nanbox_s(rs1);
>>>       float32 frs2 = check_nanbox_s(rs2);
>>> -    return nanbox_s(float32_minnum(frs1, frs2, &env->fp_status));
>>> +    return nanbox_s(env->priv_ver < PRIV_VERSION_1_11_0 ?
>>> +                    float32_minnum(frs1, frs2, &env->fp_status) :
>>> +                    float32_minimum_number(frs1, frs2, &env->fp_status));
>>>   }
>>>
>>>   uint64_t helper_fmax_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2)
>>>   {
>>>       float32 frs1 = check_nanbox_s(rs1);
>>>       float32 frs2 = check_nanbox_s(rs2);
>>> -    return nanbox_s(float32_maxnum(frs1, frs2, &env->fp_status));
>>> +    return nanbox_s(env->priv_ver < PRIV_VERSION_1_11_0 ?
>>> +                    float32_maxnum(frs1, frs2, &env->fp_status) :
>>> +                    float32_maximum_number(frs1, frs2, &env->fp_status));
>>>   }
>>>
>>>   uint64_t helper_fsqrt_s(CPURISCVState *env, uint64_t rs1)
>>> @@ -283,12 +287,16 @@ uint64_t helper_fdiv_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2)
>>>
>>>   uint64_t helper_fmin_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2)
>>>   {
>>> -    return float64_minnum(frs1, frs2, &env->fp_status);
>>> +    return env->priv_ver < PRIV_VERSION_1_11_0 ?
>>> +            float64_minnum(frs1, frs2, &env->fp_status) :
>>> +            float64_minimum_number(frs1, frs2, &env->fp_status);
>>>   }
>>>
>>>   uint64_t helper_fmax_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2)
>>>   {
>>> -    return float64_maxnum(frs1, frs2, &env->fp_status);
>>> +    return env->priv_ver < PRIV_VERSION_1_11_0 ?
>>> +            float64_maxnum(frs1, frs2, &env->fp_status) :
>>> +            float64_maximum_number(frs1, frs2, &env->fp_status);
>>>   }
>>>
>>>   uint64_t helper_fcvt_s_d(CPURISCVState *env, uint64_t rs1)
>>> --
>>> 2.31.1
>>>
>>
>> Hi Alistair,
>>
>> Did you pull the latest v5 patchset?
>> https://lists.nongnu.org/archive/html/qemu-riscv/2021-10/msg00557.html
> 
> Strange, I don't see it on the patches list. All I see is v4

It was one of the cases in which the cover didn't make it to qemu-devel.
The two patches are

https://lore.kernel.org/qemu-devel/20211021160847.2748577-2-frank.chang@sifive.com/
https://lore.kernel.org/qemu-devel/20211021160847.2748577-3-frank.chang@sifive.com/


r~
Re: [PULL 16/18] target/riscv: change the api for RVF/RVD fmin/fmax
Posted by Alistair Francis 4 years, 1 month ago 
On Fri, Oct 29, 2021 at 12:22 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 10/28/21 4:30 AM, Alistair Francis wrote:
> > On Thu, Oct 28, 2021 at 6:22 PM Frank Chang <frank.chang@sifive.com> wrote:
> >>
> >> On Thu, Oct 28, 2021 at 12:45 PM Alistair Francis <alistair.francis@opensource.wdc.com> wrote:
> >>>
> >>> From: Chih-Min Chao <chihmin.chao@sifive.com>
> >>>
> >>> The sNaN propagation behavior has been changed since
> >>> cd20cee7 in https://github.com/riscv/riscv-isa-manual.
> >>>
> >>> Signed-off-by: Chih-Min Chao <chihmin.chao@sifive.com>
> >>> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> >>> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> >>> Message-id: 20211016085428.3001501-3-frank.chang@sifive.com
> >>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> >>> ---
> >>>   target/riscv/fpu_helper.c | 16 ++++++++++++----
> >>>   1 file changed, 12 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
> >>> index 8700516a14..d62f470900 100644
> >>> --- a/target/riscv/fpu_helper.c
> >>> +++ b/target/riscv/fpu_helper.c
> >>> @@ -174,14 +174,18 @@ uint64_t helper_fmin_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2)
> >>>   {
> >>>       float32 frs1 = check_nanbox_s(rs1);
> >>>       float32 frs2 = check_nanbox_s(rs2);
> >>> -    return nanbox_s(float32_minnum(frs1, frs2, &env->fp_status));
> >>> +    return nanbox_s(env->priv_ver < PRIV_VERSION_1_11_0 ?
> >>> +                    float32_minnum(frs1, frs2, &env->fp_status) :
> >>> +                    float32_minimum_number(frs1, frs2, &env->fp_status));
> >>>   }
> >>>
> >>>   uint64_t helper_fmax_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2)
> >>>   {
> >>>       float32 frs1 = check_nanbox_s(rs1);
> >>>       float32 frs2 = check_nanbox_s(rs2);
> >>> -    return nanbox_s(float32_maxnum(frs1, frs2, &env->fp_status));
> >>> +    return nanbox_s(env->priv_ver < PRIV_VERSION_1_11_0 ?
> >>> +                    float32_maxnum(frs1, frs2, &env->fp_status) :
> >>> +                    float32_maximum_number(frs1, frs2, &env->fp_status));
> >>>   }
> >>>
> >>>   uint64_t helper_fsqrt_s(CPURISCVState *env, uint64_t rs1)
> >>> @@ -283,12 +287,16 @@ uint64_t helper_fdiv_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2)
> >>>
> >>>   uint64_t helper_fmin_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2)
> >>>   {
> >>> -    return float64_minnum(frs1, frs2, &env->fp_status);
> >>> +    return env->priv_ver < PRIV_VERSION_1_11_0 ?
> >>> +            float64_minnum(frs1, frs2, &env->fp_status) :
> >>> +            float64_minimum_number(frs1, frs2, &env->fp_status);
> >>>   }
> >>>
> >>>   uint64_t helper_fmax_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2)
> >>>   {
> >>> -    return float64_maxnum(frs1, frs2, &env->fp_status);
> >>> +    return env->priv_ver < PRIV_VERSION_1_11_0 ?
> >>> +            float64_maxnum(frs1, frs2, &env->fp_status) :
> >>> +            float64_maximum_number(frs1, frs2, &env->fp_status);
> >>>   }
> >>>
> >>>   uint64_t helper_fcvt_s_d(CPURISCVState *env, uint64_t rs1)
> >>> --
> >>> 2.31.1
> >>>
> >>
> >> Hi Alistair,
> >>
> >> Did you pull the latest v5 patchset?
> >> https://lists.nongnu.org/archive/html/qemu-riscv/2021-10/msg00557.html
> >
> > Strange, I don't see it on the patches list. All I see is v4
>
> It was one of the cases in which the cover didn't make it to qemu-devel.
> The two patches are
>
> https://lore.kernel.org/qemu-devel/20211021160847.2748577-2-frank.chang@sifive.com/
> https://lore.kernel.org/qemu-devel/20211021160847.2748577-3-frank.chang@sifive.com/

Argh! I guess the missing cover letter means it didn't appear in
patches (at least not that I saw) and I didn't double check the
version.

Sorry about the trouble. I'll prep a v2 PULL with the v5 commit.

Alistair

>
>
> r~

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.