diff mbox series

[v3,2/2] target/riscv: change the api for single/double fmin/fmax

Message ID 20211015065500.3850513-3-frank.chang@sifive.com
State New
Headers show
Series add APIs to handle alternative sNaN propagation for fmax/fmin | expand

Commit Message

Frank Chang Oct. 15, 2021, 6:54 a.m. UTC
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>
---
 target/riscv/fpu_helper.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Richard Henderson Oct. 15, 2021, 5:05 p.m. UTC | #1
On 10/14/21 11:54 PM, frank.chang@sifive.com wrote:
> From: Chih-Min Chao<chihmin.chao@sifive.com>
> 
> The sNaN propagation behavior has been changed since
> cd20cee7 inhttps://github.com/riscv/riscv-isa-manual
> 
> Signed-off-by: Chih-Min Chao<chihmin.chao@sifive.com>
> ---
>   target/riscv/fpu_helper.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
> index 8700516a14c..1472ead2528 100644
> --- a/target/riscv/fpu_helper.c
> +++ b/target/riscv/fpu_helper.c
> @@ -174,14 +174,14 @@ 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(float32_minnum_noprop(frs1, frs2, &env->fp_status));
>   }

Don't you need to conditionalize behaviour on the isa revision?


r~
Frank Chang Oct. 16, 2021, 8:52 a.m. UTC | #2
On Sat, Oct 16, 2021 at 1:05 AM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 10/14/21 11:54 PM, frank.chang@sifive.com wrote:
> > From: Chih-Min Chao<chihmin.chao@sifive.com>
> >
> > The sNaN propagation behavior has been changed since
> > cd20cee7 inhttps://github.com/riscv/riscv-isa-manual
> >
> > Signed-off-by: Chih-Min Chao<chihmin.chao@sifive.com>
> > ---
> >   target/riscv/fpu_helper.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
> > index 8700516a14c..1472ead2528 100644
> > --- a/target/riscv/fpu_helper.c
> > +++ b/target/riscv/fpu_helper.c
> > @@ -174,14 +174,14 @@ 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(float32_minnum_noprop(frs1, frs2, &env->fp_status));
> >   }
>
> Don't you need to conditionalize behaviour on the isa revision?
>
>
I will pick the right API based on CPU privilege spec version.

Thanks,
Frank Chang


>
> r~
>
Richard Henderson Oct. 16, 2021, 5:56 p.m. UTC | #3
On 10/16/21 1:52 AM, Frank Chang wrote:
> On Sat, Oct 16, 2021 at 1:05 AM Richard Henderson <richard.henderson@linaro.org 
> <mailto:richard.henderson@linaro.org>> wrote:
> 
>     On 10/14/21 11:54 PM, frank.chang@sifive.com <mailto:frank.chang@sifive.com> wrote:
>      > From: Chih-Min Chao<chihmin.chao@sifive.com <mailto:chihmin.chao@sifive.com>>
>      >
>      > The sNaN propagation behavior has been changed since
>      > cd20cee7 inhttps://github.com/riscv/riscv-isa-manual
>     <http://github.com/riscv/riscv-isa-manual>
>      >
>      > Signed-off-by: Chih-Min Chao<chihmin.chao@sifive.com <mailto:chihmin.chao@sifive.com>>
>      > ---
>      >   target/riscv/fpu_helper.c | 8 ++++----
>      >   1 file changed, 4 insertions(+), 4 deletions(-)
>      >
>      > diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
>      > index 8700516a14c..1472ead2528 100644
>      > --- a/target/riscv/fpu_helper.c
>      > +++ b/target/riscv/fpu_helper.c
>      > @@ -174,14 +174,14 @@ 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(float32_minnum_noprop(frs1, frs2, &env->fp_status));
>      >   }
> 
>     Don't you need to conditionalize behaviour on the isa revision?
> 
> 
> I will pick the right API based on CPU privilege spec version.

There's a separate F-extension revision number: 2.2.

But I'll leave it up to those more knowledgeable about the revision combinations actually 
present in the field to decide.


r~
Frank Chang Oct. 17, 2021, 12:55 a.m. UTC | #4
On Sun, Oct 17, 2021 at 1:56 AM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 10/16/21 1:52 AM, Frank Chang wrote:
> > On Sat, Oct 16, 2021 at 1:05 AM Richard Henderson <
> richard.henderson@linaro.org
> > <mailto:richard.henderson@linaro.org>> wrote:
> >
> >     On 10/14/21 11:54 PM, frank.chang@sifive.com <mailto:
> frank.chang@sifive.com> wrote:
> >      > From: Chih-Min Chao<chihmin.chao@sifive.com <mailto:
> chihmin.chao@sifive.com>>
> >      >
> >      > The sNaN propagation behavior has been changed since
> >      > cd20cee7 inhttps://github.com/riscv/riscv-isa-manual
> >     <http://github.com/riscv/riscv-isa-manual>
> >      >
> >      > Signed-off-by: Chih-Min Chao<chihmin.chao@sifive.com <mailto:
> chihmin.chao@sifive.com>>
> >      > ---
> >      >   target/riscv/fpu_helper.c | 8 ++++----
> >      >   1 file changed, 4 insertions(+), 4 deletions(-)
> >      >
> >      > diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
> >      > index 8700516a14c..1472ead2528 100644
> >      > --- a/target/riscv/fpu_helper.c
> >      > +++ b/target/riscv/fpu_helper.c
> >      > @@ -174,14 +174,14 @@ 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(float32_minnum_noprop(frs1, frs2,
> &env->fp_status));
> >      >   }
> >
> >     Don't you need to conditionalize behaviour on the isa revision?
> >
> >
> > I will pick the right API based on CPU privilege spec version.
>
> There's a separate F-extension revision number: 2.2.
>
> But I'll leave it up to those more knowledgeable about the revision
> combinations actually
> present in the field to decide.
>
>
I did some history searches on RISC-V ISA spec Github repo.

F-extension was bumped to v2.2 at (2018/08/28):
https://github.com/riscv/riscv-isa-manual/releases/tag/draft-20180828-eb78171
The privilege spec is v1.10-draft at the time.

and later ratified at (2019/03/26):
https://github.com/riscv/riscv-isa-manual/releases/tag/IMFDQC-Ratification-20190305

The spec was updated to use IEEE 754-2019 min/max functions in commit:
#cd20cee7
<https://github.com/riscv/riscv-isa-manual/commit/cd20cee7efd9bac7c5aa127ec3b451749d2b3cce>
 (2019/06/05).

Privilege spec v1.11 is ratified at (2019/06/10):
https://github.com/riscv/riscv-isa-manual/releases/tag/Ratified-IMFDQC-and-Priv-v1.11

In fact, Unprivileged spec v2.2 was released at (2017/05/10):
https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-user-2.2

and Privilege spec v1.10 was released at (2017/05/10):
https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-priv-1.10

Privilege spec was then bumped to v1.11-draft in the next draft release
right after v1.10 (2018/05/24):
https://github.com/riscv/riscv-isa-manual/releases/tag/draft-20180524001518-9981ad7
(RVF was still v2.0 at the time.)

It seems that when Privilege spec v1.11 was ratified, RVF had been bumped
to v2.2,
and when Privilege spec v1.10 was ratified, RVF was still v2.0.

As in QEMU, there's only *priv_ver* variable existing for now.
So unless we introduce other variables like: *unpriv_ver* or *fext_ver*.
Otherwise, I think using *priv_ver* is still valid here.
Though it is not accurate, somehow confused,
and may not be true anymore in future standards.

Let me know which way is better for our maintenance.

Thanks,
Frank Chang

r~
>
Frank Chang Oct. 17, 2021, 6:57 a.m. UTC | #5
On Sun, Oct 17, 2021 at 8:55 AM Frank Chang <frank.chang@sifive.com> wrote:

> On Sun, Oct 17, 2021 at 1:56 AM Richard Henderson <
> richard.henderson@linaro.org> wrote:
>
>> On 10/16/21 1:52 AM, Frank Chang wrote:
>> > On Sat, Oct 16, 2021 at 1:05 AM Richard Henderson <
>> richard.henderson@linaro.org
>> > <mailto:richard.henderson@linaro.org>> wrote:
>> >
>> >     On 10/14/21 11:54 PM, frank.chang@sifive.com <mailto:
>> frank.chang@sifive.com> wrote:
>> >      > From: Chih-Min Chao<chihmin.chao@sifive.com <mailto:
>> chihmin.chao@sifive.com>>
>> >      >
>> >      > The sNaN propagation behavior has been changed since
>> >      > cd20cee7 inhttps://github.com/riscv/riscv-isa-manual
>> >     <http://github.com/riscv/riscv-isa-manual>
>> >      >
>> >      > Signed-off-by: Chih-Min Chao<chihmin.chao@sifive.com <mailto:
>> chihmin.chao@sifive.com>>
>> >      > ---
>> >      >   target/riscv/fpu_helper.c | 8 ++++----
>> >      >   1 file changed, 4 insertions(+), 4 deletions(-)
>> >      >
>> >      > diff --git a/target/riscv/fpu_helper.c
>> b/target/riscv/fpu_helper.c
>> >      > index 8700516a14c..1472ead2528 100644
>> >      > --- a/target/riscv/fpu_helper.c
>> >      > +++ b/target/riscv/fpu_helper.c
>> >      > @@ -174,14 +174,14 @@ 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(float32_minnum_noprop(frs1, frs2,
>> &env->fp_status));
>> >      >   }
>> >
>> >     Don't you need to conditionalize behaviour on the isa revision?
>> >
>> >
>> > I will pick the right API based on CPU privilege spec version.
>>
>> There's a separate F-extension revision number: 2.2.
>>
>> But I'll leave it up to those more knowledgeable about the revision
>> combinations actually
>> present in the field to decide.
>>
>>
> I did some history searches on RISC-V ISA spec Github repo.
>
> F-extension was bumped to v2.2 at (2018/08/28):
>
> https://github.com/riscv/riscv-isa-manual/releases/tag/draft-20180828-eb78171
> The privilege spec is v1.10-draft at the time.
>
> and later ratified at (2019/03/26):
>
> https://github.com/riscv/riscv-isa-manual/releases/tag/IMFDQC-Ratification-20190305
>
> The spec was updated to use IEEE 754-2019 min/max functions in commit:
> #cd20cee7
> <https://github.com/riscv/riscv-isa-manual/commit/cd20cee7efd9bac7c5aa127ec3b451749d2b3cce>
>  (2019/06/05).
>

Sorry, the commit date is 2017/06/05, not 2019/06/05.

But I think it's probably easier and clearer to just introduce an extra
*fext_ver* variable.
We can set CPUs which are Privilege spec v1.10 to RVF v2.0
(FEXT_VERSION_2_00_0),
and others with Privilege spec v1.11 to RVF v2.2 (FEXT_VERSION_2_02_0).

Any comments are welcome.

Regards,
Frank Chang


>
> Privilege spec v1.11 is ratified at (2019/06/10):
>
> https://github.com/riscv/riscv-isa-manual/releases/tag/Ratified-IMFDQC-and-Priv-v1.11
>
> In fact, Unprivileged spec v2.2 was released at (2017/05/10):
> https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-user-2.2
>
> and Privilege spec v1.10 was released at (2017/05/10):
> https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-priv-1.10
>
> Privilege spec was then bumped to v1.11-draft in the next draft release
> right after v1.10 (2018/05/24):
>
> https://github.com/riscv/riscv-isa-manual/releases/tag/draft-20180524001518-9981ad7
> (RVF was still v2.0 at the time.)
>
> It seems that when Privilege spec v1.11 was ratified, RVF had been bumped
> to v2.2,
> and when Privilege spec v1.10 was ratified, RVF was still v2.0.
>
> As in QEMU, there's only *priv_ver* variable existing for now.
> So unless we introduce other variables like: *unpriv_ver* or *fext_ver*.
> Otherwise, I think using *priv_ver* is still valid here.
> Though it is not accurate, somehow confused,
> and may not be true anymore in future standards.
>
> Let me know which way is better for our maintenance.
>
> Thanks,
> Frank Chang
>
> r~
>>
>
Alistair Francis Oct. 18, 2021, 12:18 a.m. UTC | #6
On Sun, Oct 17, 2021 at 4:59 PM Frank Chang <frank.chang@sifive.com> wrote:
>
> On Sun, Oct 17, 2021 at 8:55 AM Frank Chang <frank.chang@sifive.com> wrote:
>>
>> On Sun, Oct 17, 2021 at 1:56 AM Richard Henderson <richard.henderson@linaro.org> wrote:
>>>
>>> On 10/16/21 1:52 AM, Frank Chang wrote:
>>> > On Sat, Oct 16, 2021 at 1:05 AM Richard Henderson <richard.henderson@linaro.org
>>> > <mailto:richard.henderson@linaro.org>> wrote:
>>> >
>>> >     On 10/14/21 11:54 PM, frank.chang@sifive.com <mailto:frank.chang@sifive.com> wrote:
>>> >      > From: Chih-Min Chao<chihmin.chao@sifive.com <mailto:chihmin.chao@sifive.com>>
>>> >      >
>>> >      > The sNaN propagation behavior has been changed since
>>> >      > cd20cee7 inhttps://github.com/riscv/riscv-isa-manual
>>> >     <http://github.com/riscv/riscv-isa-manual>
>>> >      >
>>> >      > Signed-off-by: Chih-Min Chao<chihmin.chao@sifive.com <mailto:chihmin.chao@sifive.com>>
>>> >      > ---
>>> >      >   target/riscv/fpu_helper.c | 8 ++++----
>>> >      >   1 file changed, 4 insertions(+), 4 deletions(-)
>>> >      >
>>> >      > diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
>>> >      > index 8700516a14c..1472ead2528 100644
>>> >      > --- a/target/riscv/fpu_helper.c
>>> >      > +++ b/target/riscv/fpu_helper.c
>>> >      > @@ -174,14 +174,14 @@ 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(float32_minnum_noprop(frs1, frs2, &env->fp_status));
>>> >      >   }
>>> >
>>> >     Don't you need to conditionalize behaviour on the isa revision?
>>> >
>>> >
>>> > I will pick the right API based on CPU privilege spec version.
>>>
>>> There's a separate F-extension revision number: 2.2.
>>>
>>> But I'll leave it up to those more knowledgeable about the revision combinations actually
>>> present in the field to decide.
>>>
>>
>> I did some history searches on RISC-V ISA spec Github repo.
>>
>> F-extension was bumped to v2.2 at (2018/08/28):
>> https://github.com/riscv/riscv-isa-manual/releases/tag/draft-20180828-eb78171
>> The privilege spec is v1.10-draft at the time.
>>
>> and later ratified at (2019/03/26):
>> https://github.com/riscv/riscv-isa-manual/releases/tag/IMFDQC-Ratification-20190305
>>
>> The spec was updated to use IEEE 754-2019 min/max functions in commit: #cd20cee7 (2019/06/05).
>
>
> Sorry, the commit date is 2017/06/05, not 2019/06/05.
>
> But I think it's probably easier and clearer to just introduce an extra fext_ver variable.
> We can set CPUs which are Privilege spec v1.10 to RVF v2.0 (FEXT_VERSION_2_00_0),
> and others with Privilege spec v1.11 to RVF v2.2 (FEXT_VERSION_2_02_0).

I think it's probably simpler to just tie this to the priv_spec. It's
not completely accurate, but it should be close enough. Otherwise we
have the risk of having too many version variables and it becomes a
pain for users to deal with.

If the unpriv spec is better, we could also introduce that. We will
probably need that one day for something else anyway.

If you feel that we really need a fext_ver (to avoid large software
breakage for example) then it's also ok, we just need to justify why.

Alistair

>
> Any comments are welcome.
>
> Regards,
> Frank Chang
>
>>
>>
>> Privilege spec v1.11 is ratified at (2019/06/10):
>> https://github.com/riscv/riscv-isa-manual/releases/tag/Ratified-IMFDQC-and-Priv-v1.11
>>
>> In fact, Unprivileged spec v2.2 was released at (2017/05/10):
>> https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-user-2.2
>>
>> and Privilege spec v1.10 was released at (2017/05/10):
>> https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-priv-1.10
>>
>> Privilege spec was then bumped to v1.11-draft in the next draft release right after v1.10 (2018/05/24):
>> https://github.com/riscv/riscv-isa-manual/releases/tag/draft-20180524001518-9981ad7
>> (RVF was still v2.0 at the time.)
>>
>> It seems that when Privilege spec v1.11 was ratified, RVF had been bumped to v2.2,
>> and when Privilege spec v1.10 was ratified, RVF was still v2.0.
>>
>> As in QEMU, there's only priv_ver variable existing for now.
>> So unless we introduce other variables like: unpriv_ver or fext_ver.
>> Otherwise, I think using priv_ver is still valid here.
>> Though it is not accurate, somehow confused,
>> and may not be true anymore in future standards.
>>
>> Let me know which way is better for our maintenance.
>>
>> Thanks,
>> Frank Chang
>>
>>> r~
Frank Chang Oct. 18, 2021, 3:51 a.m. UTC | #7
On Mon, Oct 18, 2021 at 8:18 AM Alistair Francis <alistair23@gmail.com>
wrote:

> On Sun, Oct 17, 2021 at 4:59 PM Frank Chang <frank.chang@sifive.com>
> wrote:
> >
> > On Sun, Oct 17, 2021 at 8:55 AM Frank Chang <frank.chang@sifive.com>
> wrote:
> >>
> >> On Sun, Oct 17, 2021 at 1:56 AM Richard Henderson <
> richard.henderson@linaro.org> wrote:
> >>>
> >>> On 10/16/21 1:52 AM, Frank Chang wrote:
> >>> > On Sat, Oct 16, 2021 at 1:05 AM Richard Henderson <
> richard.henderson@linaro.org
> >>> > <mailto:richard.henderson@linaro.org>> wrote:
> >>> >
> >>> >     On 10/14/21 11:54 PM, frank.chang@sifive.com <mailto:
> frank.chang@sifive.com> wrote:
> >>> >      > From: Chih-Min Chao<chihmin.chao@sifive.com <mailto:
> chihmin.chao@sifive.com>>
> >>> >      >
> >>> >      > The sNaN propagation behavior has been changed since
> >>> >      > cd20cee7 inhttps://github.com/riscv/riscv-isa-manual
> >>> >     <http://github.com/riscv/riscv-isa-manual>
> >>> >      >
> >>> >      > Signed-off-by: Chih-Min Chao<chihmin.chao@sifive.com <mailto:
> chihmin.chao@sifive.com>>
> >>> >      > ---
> >>> >      >   target/riscv/fpu_helper.c | 8 ++++----
> >>> >      >   1 file changed, 4 insertions(+), 4 deletions(-)
> >>> >      >
> >>> >      > diff --git a/target/riscv/fpu_helper.c
> b/target/riscv/fpu_helper.c
> >>> >      > index 8700516a14c..1472ead2528 100644
> >>> >      > --- a/target/riscv/fpu_helper.c
> >>> >      > +++ b/target/riscv/fpu_helper.c
> >>> >      > @@ -174,14 +174,14 @@ 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(float32_minnum_noprop(frs1, frs2,
> &env->fp_status));
> >>> >      >   }
> >>> >
> >>> >     Don't you need to conditionalize behaviour on the isa revision?
> >>> >
> >>> >
> >>> > I will pick the right API based on CPU privilege spec version.
> >>>
> >>> There's a separate F-extension revision number: 2.2.
> >>>
> >>> But I'll leave it up to those more knowledgeable about the revision
> combinations actually
> >>> present in the field to decide.
> >>>
> >>
> >> I did some history searches on RISC-V ISA spec Github repo.
> >>
> >> F-extension was bumped to v2.2 at (2018/08/28):
> >>
> https://github.com/riscv/riscv-isa-manual/releases/tag/draft-20180828-eb78171
> >> The privilege spec is v1.10-draft at the time.
> >>
> >> and later ratified at (2019/03/26):
> >>
> https://github.com/riscv/riscv-isa-manual/releases/tag/IMFDQC-Ratification-20190305
> >>
> >> The spec was updated to use IEEE 754-2019 min/max functions in commit:
> #cd20cee7 (2019/06/05).
> >
> >
> > Sorry, the commit date is 2017/06/05, not 2019/06/05.
> >
> > But I think it's probably easier and clearer to just introduce an extra
> fext_ver variable.
> > We can set CPUs which are Privilege spec v1.10 to RVF v2.0
> (FEXT_VERSION_2_00_0),
> > and others with Privilege spec v1.11 to RVF v2.2 (FEXT_VERSION_2_02_0).
>
> I think it's probably simpler to just tie this to the priv_spec. It's
> not completely accurate, but it should be close enough. Otherwise we
> have the risk of having too many version variables and it becomes a
> pain for users to deal with.
>
> If the unpriv spec is better, we could also introduce that. We will
> probably need that one day for something else anyway.
>
> If you feel that we really need a fext_ver (to avoid large software
> breakage for example) then it's also ok, we just need to justify why.
>
> Alistair
>

A little problem with Unpriv spec is that it uses v2.2 and later with the
date as of version tag.
It shouldn't be a real problem because we can still use the date v2.2. was
released.

But if it's okay to tie RVF version with Priv spec version,
then let's just use Priv spec version for now.
We can introduce other version variables when we really need them in the
future.

Thanks,
Frank Chang


>
> >
> > Any comments are welcome.
> >
> > Regards,
> > Frank Chang
> >
> >>
> >>
> >> Privilege spec v1.11 is ratified at (2019/06/10):
> >>
> https://github.com/riscv/riscv-isa-manual/releases/tag/Ratified-IMFDQC-and-Priv-v1.11
> >>
> >> In fact, Unprivileged spec v2.2 was released at (2017/05/10):
> >> https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-user-2.2
> >>
> >> and Privilege spec v1.10 was released at (2017/05/10):
> >> https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-priv-1.10
> >>
> >> Privilege spec was then bumped to v1.11-draft in the next draft release
> right after v1.10 (2018/05/24):
> >>
> https://github.com/riscv/riscv-isa-manual/releases/tag/draft-20180524001518-9981ad7
> >> (RVF was still v2.0 at the time.)
> >>
> >> It seems that when Privilege spec v1.11 was ratified, RVF had been
> bumped to v2.2,
> >> and when Privilege spec v1.10 was ratified, RVF was still v2.0.
> >>
> >> As in QEMU, there's only priv_ver variable existing for now.
> >> So unless we introduce other variables like: unpriv_ver or fext_ver.
> >> Otherwise, I think using priv_ver is still valid here.
> >> Though it is not accurate, somehow confused,
> >> and may not be true anymore in future standards.
> >>
> >> Let me know which way is better for our maintenance.
> >>
> >> Thanks,
> >> Frank Chang
> >>
> >>> r~
>
diff mbox series

Patch

diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
index 8700516a14c..1472ead2528 100644
--- a/target/riscv/fpu_helper.c
+++ b/target/riscv/fpu_helper.c
@@ -174,14 +174,14 @@  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(float32_minnum_noprop(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(float32_maxnum_noprop(frs1, frs2, &env->fp_status));
 }
 
 uint64_t helper_fsqrt_s(CPURISCVState *env, uint64_t rs1)
@@ -283,12 +283,12 @@  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 float64_minnum_noprop(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 float64_maxnum_noprop(frs1, frs2, &env->fp_status);
 }
 
 uint64_t helper_fcvt_s_d(CPURISCVState *env, uint64_t rs1)