Re: [RFC][PATCH] New expf, exp2f, logf, log2f and powf implementations

[Corinna Vinschen <vinschen at redhat dot com>]

On Sep  1 11:30, Jeff Johnston wrote:
> On Fri, Sep 1, 2017 at 8:01 AM, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > On 30/08/17 19:12, Jeff Johnston wrote:
> >> I noticed that the ARM routines these are based on have an Apache
> >> license.  I also
> >> noted that you were the one who wrote the patches to that code that
> >> has become the newlib implementations.
> >> When modifying old code, you must keep the old license unless all the
> >> authors and license owner agree to
> >> relicense it.  In this case, ARM is the owner of the initial license
> >> and so you probably should stick with the same license or
> >> bounce it off your employer to make sure it is ok.
> >>
> >
> > arm is opensourcing this code under multiple licenses.
> >
> > arm contribution policy to newlib used to use the 3 clause bsd license,
> > if you think this is a problem i can resend with apache license only.
> > or may be i should make multi-licensing clear in the upstream repo?
> 
> I'm fine with the license as long as ARM is ok.  If you want to
> clarify the multiple licenses somewhere
> upstream, that might be helpful in the future.
> 
> >
> >> Can you comment on the correctness issues above?  Are there blatant
> >> errors in the current implementation that should be
> >> looked at (as opposed to rounding and "level of C Standard supported" issues)?
> >>
> >
> > i thought the log2f and exp2f would have incorrect exception
> > behaviour, but on further investigation they look ok other
> > than incorrect exc.name setting for matherr.
> >
> > powf is known to have some large ulp errors (>5 ulp), but i
> > haven't done extensive tests on newlib.
> >
> >> IMO, opting-in is reasonable considering that some platforms will not
> >> meet the criteria and will break unless someone does the
> >> work to go through and configure them properly.  For example, do all
> >> the embedded compilers support hex floating-point constants by
> >> this point?
> >>
> >
> > yes i didn't want to enable it everywhere because there can
> > be lot of compatibility issues in embedded toolchains, but i
> > do want to enable it for aarch64 at least and on arm when the
> > new code is known to work.
> >
> 
> I'm fine with the patch.  Corinna, do you still want the opt-in removed or can
> it be checked in?

It's a bit worrysome that we have a macro OBSOLETE_MATH hidden
in an easy forgettable header.  It's nice and all that we get
better math functions for arm, but given that the code is generic,
I think every effort should be made to make sure all targets
get the new code.

Also, it would be nice to discuss Joel's questions.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Attachment: signature.asc
Description: PGP signature