[PATCH v2 10/18] RISC-V: Hard float support for 32-bit

[Alistair Francis]

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