Re: Fw: [PATCH 3/6] Avoid implicit floating point conversions
C Howland via Newlib <[email protected]>
| Newsgroups | gmane.comp.lib.newlib |
|---|---|
| Message-ID | <CANk6obSP0s10HmJK6xqkuSAHj5ERk9bPYj4dhXtwv6mM2T_0hg@mail.gmail.com> |
> ------------------------------ > *From:* Newlib <[email protected]> on behalf of Keith Packard > via Newlib <[email protected]> > *Sent:* Thursday, August 20, 2020 6:14 PM > *To:* [email protected] <[email protected]> > *Subject:* [PATCH 3/6] Avoid implicit floating point conversions > > > > These were found with clang -Wdouble-promotion and show places where > floating point values were being implicitly converted between > representations. These conversions can result in unexpected use of > double precision arithmetic. Those which are intentional all have an > explicit cast added. > There are many troubles here along with many good things. Some specific ones are noted below, but a general one is that clang appears to be generating spurious complaints--or else we need to fix something else related to using clang. The general problem is with replacing isnan() and isinf() with isnanf() and isinff(). This is spurious because as of C99 isnan() and isinf() are defined to be macros that take a real-floating type; they should be fine as they are. In addition, the f variants of those are not standard and ought to be called as __isinff() and __isnan() if the subs really still need to be done. Doesn't clang have something like GCC's __builtin_isnan() to be used, for example? > > diff --git a/newlib/libc/include/complex.h b/newlib/libc/include/complex.h > index 0a3ea97ed..13b300021 100644 > --- a/newlib/libc/include/complex.h > +++ b/newlib/libc/include/complex.h > @@ -9,8 +9,10 @@ > #define _COMPLEX_H > > #define complex _Complex > -#define _Complex_I 1.0fi > +#define _Complex_I 1.0i > +#define _Complex_If 1.0if > #define I _Complex_I > +#define I_f _Complex_If > The standard defines _Complex_I as being type const float _Complex. > > #include <sys/cdefs.h> > > > diff --git a/newlib/libc/include/math.h b/newlib/libc/include/math.h > index 5e6155cc4..d8e25ab5c 100644 > --- a/newlib/libc/include/math.h > +++ b/newlib/libc/include/math.h > @@ -28,11 +28,27 @@ _BEGIN_STD_C > # endif > > # ifndef INFINITY > -# define INFINITY (__builtin_inff()) > +# define INFINITY (__builtin_inf()) > +# endif > The standard says INFINITY is type float. Which makes this change wrong and the following addition superfluous. (You need INFINIITYD to complete the set.) > + > +# ifndef INFINITYF > +# define INFINITYF (__builtin_inff()) > +# endif > + > +# ifndef INFINITYL > +# define INFINITYL (__builtin_infl()) > # endif > > # ifndef NAN > -# define NAN (__builtin_nanf("")) > +# define NAN (__builtin_nan("")) > +# endif > The standard says NAN is type float. Same general problem as for INFINITY. > + > +# ifndef NANF > +# define NANF (__builtin_nanf("")) > +# endif > + > +# ifndef NANL > +# define NANL (__builtin_nanl("")) > # endif > > #else /* !gcc >= 3.3 */ > > > diff --git a/newlib/libm/common/sqrtl.c b/newlib/libm/common/sqrtl.c > index 9976f35e7..234f1b7a3 100644 > --- a/newlib/libm/common/sqrtl.c > +++ b/newlib/libm/common/sqrtl.c > @@ -98,25 +98,6 @@ inc (long double x) > return ux.extu_ld; > } > > -/* Return (x - ulp) for normal positive x. Assumes no underflow. */ > - > -static inline long double > -dec (long double x) > -{ > - union ieee_ext_u ux = { .extu_ld = x, }; > - > - if (ux.extu_ext.ext_fracl-- == 0) > - { > - if (ux.extu_ext.ext_frach-- == LDBL_NBIT) > - { > - ux.extu_ext.ext_exp--; > - ux.extu_ext.ext_frach |= LDBL_NBIT; > - } > - } > - > - return ux.extu_ld; > -} > - > /* This is slow, but simple and portable. */ > > long double > @@ -143,7 +124,7 @@ sqrtl (long double x) > if (ux.extu_ext.ext_exp == 0) > { > /* Adjust subnormal numbers. */ > - ux.extu_ld *= 0x1.0p514; > + ux.extu_ld *= 0x1.0p514l; > k = -514; > } > else > @@ -167,10 +148,10 @@ sqrtl (long double x) > /* Newton's iteration. > Split ux.extu_ld into a high and low part to achieve additional > precision. */ > > - xn = sqrt ((double) ux.extu_ld); /* 53-bit estimate of sqrtl(x). */ > + xn = (long double) sqrt ((double) ux.extu_ld); /* 53-bit estimate > of sqrtl(x). */ > > #if LDBL_MANT_DIG > 100 > - xn = (xn + (ux.extu_ld / xn)) * 0.5; /* 106-bit estimate. */ > + xn = (xn + (ux.extu_ld / xn)) * 0.5l; /* 106-bit estimate. */ > #endif > > lo = ux.extu_ld; > diff --git a/newlib/libm/complex/cacosf.c b/newlib/libm/complex/cacosf.c > index 3874dd5f6..ba698e584 100644 > --- a/newlib/libm/complex/cacosf.c > +++ b/newlib/libm/complex/cacosf.c > @@ -5,7 +5,7 @@ > * All rights reserved. > * > * This code is derived from software written by Stephen L. Moshier. > - * It is redistributed by the NetBSD Foundation by permission of the > author. > + * I_ft is redistributed by the NetBSD Foundation by permission of the > author. > * > * Redistribution and use in source and binary forms, with or without > * modification, are permitted provided that the following conditions > @@ -23,7 +23,7 @@ > * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > BUSINESS > - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN > + * I_fNTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER > IN > * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) > * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF > THE > * POSSIBILITY OF SUCH DAMAGE. > @@ -41,6 +41,6 @@ cacosf(float complex z) > float complex w; > > w = casinf(z); > - w = ((float)M_PI_2 - crealf(w)) - cimagf(w) * I; > + w = ((float)M_PI_2 - crealf(w)) - cimagf(w) * I_f; > return w; > } > diff --git a/newlib/libm/complex/cacoshf.c b/newlib/libm/complex/cacoshf.c > index 41a557ad7..2ce04258e 100644 > --- a/newlib/libm/complex/cacoshf.c > +++ b/newlib/libm/complex/cacoshf.c > @@ -5,7 +5,7 @@ > * All rights reserved. > * > * This code is derived from software written by Stephen L. Moshier. > - * It is redistributed by the NetBSD Foundation by permission of the > author. > + * I_ft is redistributed by the NetBSD Foundation by permission of the > author. > * > * Redistribution and use in source and binary forms, with or without > * modification, are permitted provided that the following conditions > @@ -23,7 +23,7 @@ > * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > BUSINESS > - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN > + * I_fNTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER > IN > * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) > * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF > THE > * POSSIBILITY OF SUCH DAMAGE. > diff --git a/newlib/libm/complex/casinf.c b/newlib/libm/complex/casinf.c > index 9a9f759ef..c4c120de4 100644 > --- a/newlib/libm/complex/casinf.c > +++ b/newlib/libm/complex/casinf.c > @@ -5,7 +5,7 @@ > * All rights reserved. > * > * This code is derived from software written by Stephen L. Moshier. > - * It is redistributed by the NetBSD Foundation by permission of the > author. > + * I_ft is redistributed by the NetBSD Foundation by permission of the > author. > * > * Redistribution and use in source and binary forms, with or without > * modification, are permitted provided that the following conditions > @@ -23,7 +23,7 @@ > * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > BUSINESS > - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN > + * I_fNTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER > IN > * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) > * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF > THE > * POSSIBILITY OF SUCH DAMAGE. > @@ -52,12 +52,12 @@ casinf(float complex z) > #if 0 /* MD: test is incorrect, casin(>1) is defined */ > if (y == 0.0f) { > if (fabsf(x) > 1.0) { > - w = M_PI_2 + 0.0f * I; > + w = M_PI_2 + 0.0f * I_f; > #if 0 > mtherr ("casin", DOMAIN); > #endif > } else { > - w = asinf(x) + 0.0f * I; > + w = asinf(x) + 0.0f * I_f; > } > return w; > } > @@ -104,19 +104,19 @@ return; > */ > > > - ca = x + y * I; > - ct = ca * I; > + ca = x + y * I_f; > + ct = ca * I_f; > /* sqrt( 1 - z*z) */ > /* cmul( &ca, &ca, &zz ) */ > /*x * x - y * y */ > - zz = (x - y) * (x + y) + (2.0f * x * y) * I; > + zz = (x - y) * (x + y) + (2.0f * x * y) * I_f; > > - zz = 1.0f - crealf(zz) - cimagf(zz) * I; > + zz = 1.0f - crealf(zz) - cimagf(zz) * I_f; > z2 = csqrtf(zz); > > zz = ct + z2; > zz = clogf(zz); > /* multiply by 1/i = -i */ > - w = zz * (-1.0f * I); > + w = zz * (-1.0f * I_f); > return w; > } > diff --git a/newlib/libm/complex/casinhf.c b/newlib/libm/complex/casinhf.c > index 0db55a0ad..b9b92d2c7 100644 > --- a/newlib/libm/complex/casinhf.c > +++ b/newlib/libm/complex/casinhf.c > @@ -5,7 +5,7 @@ > * All rights reserved. > * > * This code is derived from software written by Stephen L. Moshier. > - * It is redistributed by the NetBSD Foundation by permission of the > author. > + * I_ft is redistributed by the NetBSD Foundation by permission of the > author. > * > * Redistribution and use in source and binary forms, with or without > * modification, are permitted provided that the following conditions > @@ -23,7 +23,7 @@ > * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > BUSINESS > - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN > + * I_fNTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER > IN > * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) > * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF > THE > * POSSIBILITY OF SUCH DAMAGE. > @@ -39,6 +39,6 @@ casinhf(float complex z) > { > float complex w; > > - w = -1.0f * I * casinf(z * I); > + w = -1.0f * I_f * casinf(z * I_f); > return w; > } > diff --git a/newlib/libm/complex/catanf.c b/newlib/libm/complex/catanf.c > index ac1a65c08..1043e193b 100644 > --- a/newlib/libm/complex/catanf.c > +++ b/newlib/libm/complex/catanf.c > @@ -5,7 +5,7 @@ > * All rights reserved. > * > * This code is derived from software written by Stephen L. Moshier. > - * It is redistributed by the NetBSD Foundation by permission of the > author. > + * I_ft is redistributed by the NetBSD Foundation by permission of the > author. > * > * Redistribution and use in source and binary forms, with or without > * modification, are permitted provided that the following conditions > @@ -23,7 +23,7 @@ > * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > BUSINESS > - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN > + * I_fNTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER > IN > * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) > * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF > THE > * POSSIBILITY OF SUCH DAMAGE. > @@ -67,13 +67,13 @@ catanf(float complex z) > > t = y + 1.0f; > a = (x2 + (t * t))/a; > - w = w + (0.25f * logf(a)) * I; > + w = w + (0.25f * logf(a)) * I_f; > return w; > > ovrf: > #if 0 > mtherr ("catan", OVERFLOW); > #endif > - w = HUGE_VALF + HUGE_VALF * I; > + w = HUGE_VALF + HUGE_VALF * I_f; > return w; > } > diff --git a/newlib/libm/complex/catanhf.c b/newlib/libm/complex/catanhf.c > index fe6127a9d..96dfafb26 100644 > --- a/newlib/libm/complex/catanhf.c > +++ b/newlib/libm/complex/catanhf.c > @@ -5,7 +5,7 @@ > * All rights reserved. > * > * This code is derived from software written by Stephen L. Moshier. > - * It is redistributed by the NetBSD Foundation by permission of the > author. > + * I_ft is redistributed by the NetBSD Foundation by permission of the > author. > * > * Redistribution and use in source and binary forms, with or without > * modification, are permitted provided that the following conditions > @@ -23,7 +23,7 @@ > * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > BUSINESS > - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN > + * I_fNTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER > IN > * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) > * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF > THE > * POSSIBILITY OF SUCH DAMAGE. > @@ -39,6 +39,6 @@ catanhf(float complex z) > { > float complex w; > > - w = -1.0f * I * catanf(z * I); > + w = -1.0f * I_f * catanf(z * I_f); > return w; > } > diff --git a/newlib/libm/complex/ccosf.c b/newlib/libm/complex/ccosf.c > index 805e24feb..1520aec36 100644 > --- a/newlib/libm/complex/ccosf.c > +++ b/newlib/libm/complex/ccosf.c > @@ -5,7 +5,7 @@ > * All rights reserved. > * > * This code is derived from software written by Stephen L. Moshier. > - * It is redistributed by the NetBSD Foundation by permission of the > author. > + * I_ft is redistributed by the NetBSD Foundation by permission of the > author. > * > * Redistribution and use in source and binary forms, with or without > * modification, are permitted provided that the following conditions > @@ -23,7 +23,7 @@ > * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > BUSINESS > - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN > + * I_fNTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER > IN > * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) > * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF > THE > * POSSIBILITY OF SUCH DAMAGE. > @@ -43,6 +43,6 @@ ccosf(float complex z) > float ch, sh; > > _cchshf(cimagf(z), &ch, &sh); > - w = cosf(crealf(z)) * ch - (sinf(crealf(z)) * sh) * I; > + w = cosf(crealf(z)) * ch - (sinf(crealf(z)) * sh) * I_f; > return w; > } > diff --git a/newlib/libm/complex/ccoshf.c b/newlib/libm/complex/ccoshf.c > index af11353e4..118e85542 100644 > --- a/newlib/libm/complex/ccoshf.c > +++ b/newlib/libm/complex/ccoshf.c > @@ -5,7 +5,7 @@ > * All rights reserved. > * > * This code is derived from software written by Stephen L. Moshier. > - * It is redistributed by the NetBSD Foundation by permission of the > author. > + * I_ft is redistributed by the NetBSD Foundation by permission of the > author. > * > * Redistribution and use in source and binary forms, with or without > * modification, are permitted provided that the following conditions > @@ -23,7 +23,7 @@ > * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > BUSINESS > - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN > + * I_fNTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER > IN > * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) > * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF > THE > * POSSIBILITY OF SUCH DAMAGE. > @@ -43,6 +43,6 @@ ccoshf(float complex z) > > x = crealf(z); > y = cimagf(z); > - w = coshf(x) * cosf(y) + (sinhf(x) * sinf(y)) * I; > + w = coshf(x) * cosf(y) + (sinhf(x) * sinf(y)) * I_f; > return w; > } > diff --git a/newlib/libm/complex/cephes_subrf.c > b/newlib/libm/complex/cephes_subrf.c > index 4a325811f..98b5a74f3 100644 > --- a/newlib/libm/complex/cephes_subrf.c > +++ b/newlib/libm/complex/cephes_subrf.c > @@ -5,7 +5,7 @@ > * All rights reserved. > * > * This code is derived from software written by Stephen L. Moshier. > - * It is redistributed by the NetBSD Foundation by permission of the > author. > + * I_ft is redistributed by the NetBSD Foundation by permission of the > author. > * > * Redistribution and use in source and binary forms, with or without > * modification, are permitted provided that the following conditions > @@ -23,7 +23,7 @@ > * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > BUSINESS > - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN > + * I_fNTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER > IN > * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) > * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF > THE > * POSSIBILITY OF SUCH DAMAGE. > @@ -61,7 +61,7 @@ _cchshf(float x, float *c, float *s) > static const double DP1 = 3.140625; > static const double DP2 = 9.67502593994140625E-4; > static const double DP3 = 1.509957990978376432E-7; > -#define MACHEPF 3.0e-8 > +#define MACHEPF 3.0e-8f > > float > _redupif(float x) > @@ -77,7 +77,7 @@ _redupif(float x) > > i = t; /* the multiple */ > t = i; > - t = ((x - t * DP1) - t * DP2) - t * DP3; > + t = (((double) x - (double) t * DP1) - (double) t * DP2) - > (double) t * DP3; > This is simply nuts: the language clearly defines promotion rules. All get turned into double if there's a single double in it, so one cast should be enough. If the compiler complains otherwise we should be complaining about the compiler, not kowtowing to its warnings. We should not allow compiler warnings to make code unreadable. > return t; > > I did not exhaustively look at every change, only scanning a bit towards the end. Except as noted, seemed generally sane. (I finally got a non-work email address to use for Newlib, since my company started putting auto-tags on outgoing email that causes the Newlib list server to bounce them.) I see that Joseph Myers types faster than me and so some of these comments have already been made, but since I've already typed it in I'm not going back to re-edit. Craig