Re: [PATCH] Don't do double divide in powf.

[Craig Howland <howland at LGSInnovations dot com>]

On 12/12/2017 11:50 AM, Jon Beniston wrote:
> > > > --- a/newlib/libm/math/wf_pow.c
> > > > +++ b/newlib/libm/math/wf_pow.c
> > > > @@ -108,7 +108,7 @@
> > > >    		    if (_LIB_VERSION == _SVID_)
> > > >    		        exc.retval = 0.0;
> > > >    		    else
> > > > -		        exc.retval = 0.0/0.0;	/* X/Open allow NaN */
> > > > +		        exc.retval = nan("");	/* X/Open allow NaN */
> > > As far as I can see, that still pulls in an unnecessary function call,
> > > nan().
> > >
> > > What about using NAN?  That's basically __builtin_nanf("") and usually
> > > resolves without function call on any platform.
> > No to either of the above.  The 0/0 construct is to cause a floating point "invalid"
> > exception, which the functions do not do.  (The divide has to be done
> > at runtime to cause the exception.)  The double divide
> > can be made float by changing it to 0.0F/0.0F.
> That doesn't always work though, particularly with s/w floating point libs.
>
> Could it be abstracted to a target specific function/macro to raise an invalid exception / return a NaN?
>
> There's quite a few other functions that use the technique.
     Well, if it does not work then either 1) the particular compiler and SW 
lib in question are not complying with the C standard, or 2) the specific 
exception is not supported.  Case 2 is fine because it is permitted.
     While it theoretically can be abstracted, in practice for Newlib it cannot 
be because fenv.h support is lacking.  The beauty of things like 0.0/0.0 is that 
they give the desired results in a target-independent manner.  Coding of fenv 
functions (e.g. feraiseexcept()) either end up using the same tricks to remain 
target independent or require target-specific coding (whether related to the HW 
or a SW implementation).
     I think, however, that in general if an implementation fails to work as 
you say sometimes happens that it won't actually matter. (Assuming you mean that 
it does not generate the exception, as opposed to choosing to use double instead 
of float for the divide in the case under consideration.)  That is, if the 
exception is not generated due to a bug, it can be considered to be the same as 
if it were not supported by design.
     If, however, you are saying that 0.0F/0.0F might actually result in a 
double soft divide routine being called, then we're up against a bug that should 
be addressed in some manner that does not break properly-working 
implementations.  (This same general technique is used in lots of places, even 
though the specific 0.0/0.0 looks to only be in maybe only 6 float functions.)  
One means of doing this would be to add partial fenv.h support, but this is far 
beyond the scope of the patch under consideration.
     To put it in a different way:  even though Newlib does not presently have 
direct fenv.h support, the math routines in general were written to properly 
support floating-point exceptions.  We should not break that so that we will 
more easily be able to add the fenv.h support in the future, nor break any 
applications which have added it for themselves.
     In this case, the 0.0/0.0 is a real mistake in the source and the correct 
fix is to make it 0.0F/0.0F.  If there are some buggy implementations that don't 
deal with this properly, then they can be the subject of a future patch for them.
            Craig