Re: [PATCH 0/3]: Add math support for non LDBL_EQ_DBL architecture

On Wed, Apr 5, 2023 at 3:12 PM Jeff Johnston <[email protected]> wrote:

> See below.
>
> On Wed, Apr 5, 2023 at 5:24 AM Corinna Vinschen <[email protected]>
> wrote:
>
> > Hi Jennifer,
> >
> > On Apr  3 15:58, Jennifer Averett wrote:
> > > The attached set of patches add long double support for i386, aarch64
> > and
> > > x86_64.  The riscv and powerpc are supported by FreeBSD but will need
> > more
> > > work to be supported by newlib.  FreeBSD has separate 64 and 32 bit
> > powerpc
> > > support which would have to be integrated for newlib. FreeBSD riscv
> > support
> > > is 64 and there are issues with fenv.h that would have to be addressed.
> >
> > Thanks for your patchset, it looks pretty well to me, though I like
> > to have input on this from my co-maintainer Jeff, too.
> >
>
> Looks good.  I do have a comment about how the flag checks should not be
> removed in math.h.  The _REENT_ONLY checking should still be honored as
> well as
> the m68k flag: __math_68881.


OK. Jennifer can revert that part of the patch.


> There could be platforms where long double is
> supported (possibly doubles are 32 bits and long double is 64 bits), but
> there are only the 3 platforms that have the machine support headers at
> present.  As well, from ieeefp.h there are other
> LDBL_MANT_DIG possibilities mentioned (e.g. 53, 65, 112) which the new code
> cannot handle.
>

Agreed. We did not go on a quest to support everything at first. It may be
possible to  provide a generic _fpmath.h which wraps ieeefp.h which could
enable a few more architectures.

And there is nothing stopping someone from adding more ldNN directories.

I think the riscv and ppc should be possible to add from the FreeBSD code
but it will take more architecture specific tinkering than we were willing
to
do in this initial patch.

I'm not disagreeing with you at all. Just saying we took a first step and
did all the architectures which were straightforward. Plenty of opportunity
for follow up.


>
> > I noticed that you exclude Cygwin from the new code, which makes sense
> > as long as we provide our own long double math taken from Mingw-w64.
> >
> > However, there's something not quite right.  When trying to build I get
> > symbol conflicts for the fdim{f,l} and scalbln{f,l} symbols in the link
> > stage (paths shortend for readability):
> >
> > ld: libm.a(libm_a-s_fdim.o): in function `fdimf':
> > newlib/libm/ld/s_fdim.c:47: multiple definition of `fdimf';
> > libm.a(libm_a-sf_fdim.o):newlib/libm/common/sf_fdim.c:16: first defined
> here
> >
> > ld: libm.a(libm_a-s_fdim.o): in function `fdiml':
> > newlib/libm/ld/s_fdim.c:48: multiple definition of `fdiml';
> > libdll.a(fdiml.o):winsup/cygwin/math/fdiml.c:11: first defined here
> >
> > ld: libm.a(libm_a-s_scalbln.o): in function `scalblnf':
> > newlib/libm/ld/s_scalbln.c:46: multiple definition of `scalblnf';
> > libm.a(libm_a-sf_scalbln.o):newlib/libm/common/sf_scalbln.c:34: first
> > defined here
> >
> > ld: libm.a(libm_a-s_scalbln.o): in function `scalblnl':
> > newlib/libm/ld/s_scalbln.c:53: multiple definition of `scalblnl';
> > libdll.a(scalbnl.o):winsup/cygwin/scalbnl.S:19: first defined here
> >
> > The conflicts really ony occur for these four functions.  Any chance
> > to fix these?
> >
> >
> > Thanks,
> > Corinna
> >
> >
>