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

[Jeff Johnston <jjohnstn at redhat dot com>]

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?

-- Jeff J.