[v2,10/18] RISC-V: Hard float support for 32-bit
Commit Message
From: Zong Li <zongbox@gmail.com>
This patch contains hardware floating-point support for the RV32IF and
RV32IFD
---
sysdeps/riscv/rv32/rvd/s_lrint.c | 31 ++++++++++++++++++++++++++++++
sysdeps/riscv/rv32/rvd/s_lround.c | 31 ++++++++++++++++++++++++++++++
sysdeps/riscv/rv32/rvf/s_lrintf.c | 31 ++++++++++++++++++++++++++++++
sysdeps/riscv/rv32/rvf/s_lroundf.c | 31 ++++++++++++++++++++++++++++++
4 files changed, 124 insertions(+)
create mode 100644 sysdeps/riscv/rv32/rvd/s_lrint.c
create mode 100644 sysdeps/riscv/rv32/rvd/s_lround.c
create mode 100644 sysdeps/riscv/rv32/rvf/s_lrintf.c
create mode 100644 sysdeps/riscv/rv32/rvf/s_lroundf.c
Comments
On Wed, 3 Jun 2020, Alistair Francis via Libc-alpha wrote:
> This patch contains hardware floating-point support for the RV32IF and
> RV32IFD
Full stop please.
> diff --git a/sysdeps/riscv/rv32/rvd/s_lrint.c b/sysdeps/riscv/rv32/rvd/s_lrint.c
> new file mode 100644
> index 0000000000..df406aacb6
> --- /dev/null
> +++ b/sysdeps/riscv/rv32/rvd/s_lrint.c
> @@ -0,0 +1,31 @@
> +/* lrint(). RISC-V version.
I think this has to mention this is the 32-bit version somehow, like
"32-bit RISC-V" or suchlike ("RV32" might be too cryptic/slang). Feel
free to find a better wording (I'm not particularly happy to start a
sentence with a number).
> + Copyright (C) 2017-2020 Free Software Foundation, Inc.
Again, 2020 only, and likewise throughout. Also I missed one case in
01/18 and may have elsewhere, please double-check the remaining patches.
Otherwise OK as far as code already proposed for this change is
concerned.
What about the other math functions though? We have a lot of optimised
versions in `sysdeps/riscv/rv64/rv{f,d}/', which seem suitable for RV32 as
it stands, such as `s_llrintf.c' or `s_nearbyint.c'. Instead we build
generic `sysdeps/ieee754/{flt-32,dbl-64}/' variants.
Shouldn't we also move the XLEN-agnostic optimised functions to
`sysdeps/riscv/rv{f,d}/' with this change? I think we only need to keep
those that use the `long int' type at the interface in the XLEN-specific
directory.
Maciej
On Fri, Jul 10, 2020 at 5:49 PM Maciej W. Rozycki via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> On Wed, 3 Jun 2020, Alistair Francis via Libc-alpha wrote:
>
> > This patch contains hardware floating-point support for the RV32IF and
> > RV32IFD
>
> Full stop please.
Fixed.
>
> > diff --git a/sysdeps/riscv/rv32/rvd/s_lrint.c b/sysdeps/riscv/rv32/rvd/s_lrint.c
> > new file mode 100644
> > index 0000000000..df406aacb6
> > --- /dev/null
> > +++ b/sysdeps/riscv/rv32/rvd/s_lrint.c
> > @@ -0,0 +1,31 @@
> > +/* lrint(). RISC-V version.
>
> I think this has to mention this is the 32-bit version somehow, like
> "32-bit RISC-V" or suchlike ("RV32" might be too cryptic/slang). Feel
> free to find a better wording (I'm not particularly happy to start a
> sentence with a number).
Fixed.
>
> > + Copyright (C) 2017-2020 Free Software Foundation, Inc.
>
> Again, 2020 only, and likewise throughout. Also I missed one case in
> 01/18 and may have elsewhere, please double-check the remaining patches.
Fixed in the whole series.
>
> Otherwise OK as far as code already proposed for this change is
> concerned.
Thanks
>
> What about the other math functions though? We have a lot of optimised
> versions in `sysdeps/riscv/rv64/rv{f,d}/', which seem suitable for RV32 as
> it stands, such as `s_llrintf.c' or `s_nearbyint.c'. Instead we build
> generic `sysdeps/ieee754/{flt-32,dbl-64}/' variants.
I haven't looked into the others, but I think you are right and this
can be improved.
>
> Shouldn't we also move the XLEN-agnostic optimised functions to
> `sysdeps/riscv/rv{f,d}/' with this change? I think we only need to keep
> those that use the `long int' type at the interface in the XLEN-specific
> directory.
They are all xlen dependent though. All of the 64-bit ones use
fcvt.l.d (or something similar) which doesn't exist on RV32.
Although the functions end up being the same, the actual assembly
instruction called is different. We can look at consolidating them
into a single file and do a xlen/__WORDSIZE macro check.
At this stage in glibc development I don't really want to change the
floating point helpers. Can we leave this as is and then for the next
release I can consolidate all of these into single files that do
xlen/__WORDSIZE #ifdefs? That way we will just have a single file (for
each operation) for RISC-V that will call a different assembly
instruction based on xlen or __WORDSIZE.
Alistair
>
> Maciej
On Sat, 11 Jul 2020, Alistair Francis wrote:
> > Shouldn't we also move the XLEN-agnostic optimised functions to
> > `sysdeps/riscv/rv{f,d}/' with this change? I think we only need to keep
> > those that use the `long int' type at the interface in the XLEN-specific
> > directory.
>
> They are all xlen dependent though. All of the 64-bit ones use
> fcvt.l.d (or something similar) which doesn't exist on RV32.
Umm, right, I missed this detail, sorry for the confusion.
> Although the functions end up being the same, the actual assembly
> instruction called is different. We can look at consolidating them
> into a single file and do a xlen/__WORDSIZE macro check.
I don't think there is a simple alternative available for RV32 that would
be worth keeping together with the RV64 variant. Did I miss anything?
What instruction(s) do you have in mind?
> At this stage in glibc development I don't really want to change the
> floating point helpers. Can we leave this as is and then for the next
> release I can consolidate all of these into single files that do
> xlen/__WORDSIZE #ifdefs? That way we will just have a single file (for
> each operation) for RISC-V that will call a different assembly
> instruction based on xlen or __WORDSIZE.
Sure, in these circumstances this change is fine with me then.
Maciej
On Sat, Jul 11, 2020 at 3:14 PM Maciej W. Rozycki <macro@wdc.com> wrote:
>
> On Sat, 11 Jul 2020, Alistair Francis wrote:
>
> > > Shouldn't we also move the XLEN-agnostic optimised functions to
> > > `sysdeps/riscv/rv{f,d}/' with this change? I think we only need to keep
> > > those that use the `long int' type at the interface in the XLEN-specific
> > > directory.
> >
> > They are all xlen dependent though. All of the 64-bit ones use
> > fcvt.l.d (or something similar) which doesn't exist on RV32.
>
> Umm, right, I missed this detail, sorry for the confusion.
>
> > Although the functions end up being the same, the actual assembly
> > instruction called is different. We can look at consolidating them
> > into a single file and do a xlen/__WORDSIZE macro check.
>
> I don't think there is a simple alternative available for RV32 that would
> be worth keeping together with the RV64 variant. Did I miss anything?
> What instruction(s) do you have in mind?
Something like this for the generic RISC-V lround:
long int
__lroundf (float x)
{
#if __WORDSIZE == 64
int64_t res;
asm ("fcvt.l.s %0, %1, rmm" : "=r" (res) : "f" (x));
#else
int32_t res;
asm ("fcvt.w.s %0, %1, rmm" : "=r" (res) : "f" (x));
#endif
return res;
}
I'm not sure if it's clearer, but for some of the more complex
functions (roundeven for example) it might be easier.
It also means if there is a bug fixed in one it'll end up fixed for both.
>
> > At this stage in glibc development I don't really want to change the
> > floating point helpers. Can we leave this as is and then for the next
> > release I can consolidate all of these into single files that do
> > xlen/__WORDSIZE #ifdefs? That way we will just have a single file (for
> > each operation) for RISC-V that will call a different assembly
> > instruction based on xlen or __WORDSIZE.
>
> Sure, in these circumstances this change is fine with me then.
Great! Thanks
Alistair
>
> Maciej
On Sun, 12 Jul 2020, Alistair Francis wrote:
> > I don't think there is a simple alternative available for RV32 that would
> > be worth keeping together with the RV64 variant. Did I miss anything?
> > What instruction(s) do you have in mind?
>
> Something like this for the generic RISC-V lround:
>
> long int
> __lroundf (float x)
> {
> #if __WORDSIZE == 64
> int64_t res;
> asm ("fcvt.l.s %0, %1, rmm" : "=r" (res) : "f" (x));
> #else
> int32_t res;
> asm ("fcvt.w.s %0, %1, rmm" : "=r" (res) : "f" (x));
> #endif
> return res;
> }
>
> I'm not sure if it's clearer, but for some of the more complex
> functions (roundeven for example) it might be easier.
>
> It also means if there is a bug fixed in one it'll end up fixed for both.
Ah, you mean the `float' to `long int' conversion functions, necessarily
ABI-specific due to the changing width of the latter data type. Well, I
meant the operations involving FCVT.L.D/FCVT.D.L. I can see no RISC-V
solution for them that would surpass the generic implementation.
As far as your example above is concerned if we decided to merge the
files at all, I would reduce it to:
#if __WORDSIZE == 64
# define OP "fcvt.l.s"
#elif __WORDSIZE == 32
# define OP "fcvt.w.s"
#else
# error Unsupported
#endif
long int
__lroundf (float x)
{
long int res;
asm (OP "\t%0, %1, rmm" : "=r" (res) : "f" (x));
return res;
}
or suchlike (I'm not sure if there's any gain here from `res' having an
explicit-width data type). Likewise with the rest.
Maciej
On Sun, Jul 12, 2020 at 3:10 PM Maciej W. Rozycki <macro@wdc.com> wrote:
>
> On Sun, 12 Jul 2020, Alistair Francis wrote:
>
> > > I don't think there is a simple alternative available for RV32 that would
> > > be worth keeping together with the RV64 variant. Did I miss anything?
> > > What instruction(s) do you have in mind?
> >
> > Something like this for the generic RISC-V lround:
> >
> > long int
> > __lroundf (float x)
> > {
> > #if __WORDSIZE == 64
> > int64_t res;
> > asm ("fcvt.l.s %0, %1, rmm" : "=r" (res) : "f" (x));
> > #else
> > int32_t res;
> > asm ("fcvt.w.s %0, %1, rmm" : "=r" (res) : "f" (x));
> > #endif
> > return res;
> > }
> >
> > I'm not sure if it's clearer, but for some of the more complex
> > functions (roundeven for example) it might be easier.
> >
> > It also means if there is a bug fixed in one it'll end up fixed for both.
>
> Ah, you mean the `float' to `long int' conversion functions, necessarily
> ABI-specific due to the changing width of the latter data type. Well, I
> meant the operations involving FCVT.L.D/FCVT.D.L. I can see no RISC-V
> solution for them that would surpass the generic implementation.
>
> As far as your example above is concerned if we decided to merge the
> files at all, I would reduce it to:
>
> #if __WORDSIZE == 64
> # define OP "fcvt.l.s"
> #elif __WORDSIZE == 32
> # define OP "fcvt.w.s"
> #else
> # error Unsupported
> #endif
>
> long int
> __lroundf (float x)
> {
> long int res;
> asm (OP "\t%0, %1, rmm" : "=r" (res) : "f" (x));
> return res;
> }
>
> or suchlike (I'm not sure if there's any gain here from `res' having an
> explicit-width data type). Likewise with the rest.
Sounds good. Now that RV32 support is in I have some patches for this,
I'll send them out soon.
Alistair
>
> Maciej
new file mode 100644
@@ -0,0 +1,31 @@
+/* lrint(). RISC-V version.
+ Copyright (C) 2017-2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library. If not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <math.h>
+#include <libm-alias-double.h>
+#include <stdint.h>
+
+long int
+__lrint (double x)
+{
+ int32_t res;
+ asm ("fcvt.w.d %0, %1" : "=r" (res) : "f" (x));
+ return res;
+}
+
+libm_alias_double (__lrint, lrint)
new file mode 100644
@@ -0,0 +1,31 @@
+/* lround(). RISC-V version.
+ Copyright (C) 2017-2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library. If not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <math.h>
+#include <libm-alias-double.h>
+#include <stdint.h>
+
+long int
+__lround (double x)
+{
+ int32_t res;
+ asm ("fcvt.w.d %0, %1, rmm" : "=r" (res) : "f" (x));
+ return res;
+}
+
+libm_alias_double (__lround, lround)
new file mode 100644
@@ -0,0 +1,31 @@
+/* lrintf(). RISC-V version.
+ Copyright (C) 2017-2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library. If not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <math.h>
+#include <libm-alias-float.h>
+#include <stdint.h>
+
+long int
+__lrintf (float x)
+{
+ int32_t res;
+ asm ("fcvt.w.s %0, %1" : "=r" (res) : "f" (x));
+ return res;
+}
+
+libm_alias_float (__lrint, lrint)
new file mode 100644
@@ -0,0 +1,31 @@
+/* lroundf(). RISC-V version.
+ Copyright (C) 2017-2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library. If not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <math.h>
+#include <libm-alias-float.h>
+#include <stdint.h>
+
+long int
+__lroundf (float x)
+{
+ int32_t res;
+ asm ("fcvt.w.s %0, %1, rmm" : "=r" (res) : "f" (x));
+ return res;
+}
+
+libm_alias_float (__lround, lround)