This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/4] Allow direct use of math_ldbl.h in testsuite.


On 02/20/2017 08:03 AM, Zack Weinberg wrote:
> A few 'long double'-related tests include math_private.h just for
> their variety of math_ldbl.h, which contains macros for assembling and
> disassembling the binary representation of 'long double'.  math_ldbl.h
> insists on being included from math_private.h, but if we relax this
> restriction (and fix some portability sloppiness) we can use it
> directly and not have to expose all of math_private.h to the
> testsuite.
> 
> The only bit of this that I'm not 100% happy with is,
> ldbl-128ibm/math_ldbl.h was using EXTRACT_WORDS64 from math_private.h.
> I replicated the logic of EXTRACT_WORDS64 into the function that
> needed it, which is the expeditious local fix.  It seems abstractly
> better to add similar functionality to all varieties of ieee754.h, but
> that would change a public API so I wasn't comfortable doing it in
> this patch series.

Looks good to me with the expansion of the comment for 
the ldbl-128ibm/math_ldbl.h changes.

> zw
> 
> 	* sysdeps/generic/math_private.h: Use __BIG_ENDIAN and
> 	__LITTLE_ENDIAN, not BIG_ENDIAN and LITTLE_ENDIAN.
> 
> 	* sysdeps/generic/math_ldbl.h
> 	* sysdeps/ia64/fpu/math_ldbl.h
> 	* sysdeps/ieee754/ldbl-128/math_ldbl.h
> 	* sysdeps/ieee754/ldbl-128ibm/math_ldbl.h
> 	* sysdeps/ieee754/ldbl-96/math_ldbl.h
> 	* sysdeps/powerpc/fpu/math_ldbl.h
> 	* sysdeps/x86_64/fpu/math_ldbl.h:
> 	Allow direct inclusion.  Use uintNN_t instead of u_intNN_t.
> 	Use __BIG_ENDIAN and __LITTLE_ENDIAN, not BIG_ENDIAN and
> 	LITTLE_ENDIAN.  Include endian.h and/or stdint.h if necessary.
> 
> 	* sysdeps/ieee754/ldbl-128ibm/math_ldbl.h: Don't use EXTRACT_WORDS64.
> 
> 	* sysdeps/ieee754/ldbl-96/test-canonical-ldbl-96.c
> 	* sysdeps/ieee754/ldbl-96/test-totalorderl-ldbl-96.c
> 	* sysdeps/ieee754/ldbl-128ibm/test-canonical-ldbl-128ibm.c
> 	* sysdeps/ieee754/ldbl-128ibm/test-totalorderl-ldbl-128ibm.c:
> 	Include math_ldbl.h, not math_private.h.
> ---
>  sysdeps/generic/math_ldbl.h                        | 10 +++++----
>  sysdeps/generic/math_private.h                     |  4 ++--
>  sysdeps/ia64/fpu/math_ldbl.h                       | 22 ++++++++++--------
>  sysdeps/ieee754/ldbl-128/math_ldbl.h               | 26 +++++++++++++---------
>  sysdeps/ieee754/ldbl-128ibm/math_ldbl.h            | 18 ++++++++++-----
>  .../ldbl-128ibm/test-canonical-ldbl-128ibm.c       |  2 +-
>  .../ldbl-128ibm/test-totalorderl-ldbl-128ibm.c     |  2 +-
>  sysdeps/ieee754/ldbl-96/math_ldbl.h                | 22 ++++++++++--------
>  sysdeps/ieee754/ldbl-96/test-canonical-ldbl-96.c   |  2 +-
>  sysdeps/ieee754/ldbl-96/test-totalorderl-ldbl-96.c |  2 +-
>  sysdeps/powerpc/fpu/math_ldbl.h                    |  7 +++---
>  sysdeps/x86_64/fpu/math_ldbl.h                     | 13 ++++++-----
>  12 files changed, 77 insertions(+), 53 deletions(-)
> 
> diff --git a/sysdeps/generic/math_ldbl.h b/sysdeps/generic/math_ldbl.h
> index f0b97ef8f9..4e7ee7912e 100644
> --- a/sysdeps/generic/math_ldbl.h
> +++ b/sysdeps/generic/math_ldbl.h
> @@ -1,5 +1,7 @@
> -#ifndef _MATH_PRIVATE_H_
> -#error "Never use <math_ldbl.h> directly; include <math_private.h> instead."
> -#endif
> +#ifndef _MATH_LDBL_H_
> +#define _MATH_LDBL_H_ 1
>  
> -/* This is empty.  Any machine using long double type will override this header.  */
> +/* Any machine with a 'long double' distinct from 'double' must
> +   override this header.  */
> +
> +#endif
> diff --git a/sysdeps/generic/math_private.h b/sysdeps/generic/math_private.h
> index c0d4e3dbcd..be65b9487b 100644
> --- a/sysdeps/generic/math_private.h
> +++ b/sysdeps/generic/math_private.h
> @@ -37,7 +37,7 @@
>  /* A union which permits us to convert between a double and two 32 bit
>     ints.  */
>  
> -#if __FLOAT_WORD_ORDER == BIG_ENDIAN
> +#if __FLOAT_WORD_ORDER == __BIG_ENDIAN
>  
>  typedef union
>  {
> @@ -52,7 +52,7 @@ typedef union
>  
>  #endif
>  
> -#if __FLOAT_WORD_ORDER == LITTLE_ENDIAN
> +#if __FLOAT_WORD_ORDER == __LITTLE_ENDIAN
>  
>  typedef union
>  {
> diff --git a/sysdeps/ia64/fpu/math_ldbl.h b/sysdeps/ia64/fpu/math_ldbl.h
> index 475ca795fa..748e9aff5a 100644
> --- a/sysdeps/ia64/fpu/math_ldbl.h
> +++ b/sysdeps/ia64/fpu/math_ldbl.h
> @@ -1,11 +1,13 @@
> -#ifndef _MATH_PRIVATE_H_
> -#error "Never use <math_ldbl.h> directly; include <math_private.h> instead."
> -#endif
> +#ifndef _MATH_LDBL_H_
> +#define _MATH_LDBL_H_ 1
> +
> +#include <stdint.h>
> +#include <endian.h>
>  
>  /* A union which permits us to convert between a long double and
>     three 32 bit ints.  */
>  
> -#if __FLOAT_WORD_ORDER == BIG_ENDIAN
> +#if __FLOAT_WORD_ORDER == __BIG_ENDIAN
>  
>  typedef union
>  {
> @@ -15,22 +17,22 @@ typedef union
>      unsigned int empty0:32;
>      int sign_exponent:16;
>      unsigned int empty1:16;
> -    u_int32_t msw;
> -    u_int32_t lsw;
> +    uint32_t msw;
> +    uint32_t lsw;
>    } parts;
>  } ieee_long_double_shape_type;
>  
>  #endif
>  
> -#if __FLOAT_WORD_ORDER == LITTLE_ENDIAN
> +#if __FLOAT_WORD_ORDER == __LITTLE_ENDIAN
>  
>  typedef union
>  {
>    long double value;
>    struct
>    {
> -    u_int32_t lsw;
> -    u_int32_t msw;
> +    uint32_t lsw;
> +    uint32_t msw;
>      int sign_exponent:16;
>      unsigned int empty1:16;
>      unsigned int empty0:32;
> @@ -98,3 +100,5 @@ do {								\
>    se_u.parts.sign_exponent = (exp);				\
>    (d) = se_u.value;						\
>  } while (0)
> +
> +#endif /* math_ldbl.h */
> diff --git a/sysdeps/ieee754/ldbl-128/math_ldbl.h b/sysdeps/ieee754/ldbl-128/math_ldbl.h
> index c1980c9401..6ea2c385df 100644
> --- a/sysdeps/ieee754/ldbl-128/math_ldbl.h
> +++ b/sysdeps/ieee754/ldbl-128/math_ldbl.h
> @@ -1,41 +1,43 @@
> -#ifndef _MATH_PRIVATE_H_
> -#error "Never use <math_ldbl.h> directly; include <math_private.h> instead."
> -#endif
> +#ifndef _MATH_LDBL_H_
> +#define _MATH_LDBL_H_ 1
> +
> +#include <stdint.h>
> +#include <endian.h>
>  
>  /* A union which permits us to convert between a long double and
>     four 32 bit ints or two 64 bit ints.  */
>  
> -#if __FLOAT_WORD_ORDER == BIG_ENDIAN
> +#if __FLOAT_WORD_ORDER == __BIG_ENDIAN
>  
>  typedef union
>  {
>    long double value;
>    struct
>    {
> -    u_int64_t msw;
> -    u_int64_t lsw;
> +    uint64_t msw;
> +    uint64_t lsw;
>    } parts64;
>    struct
>    {
> -    u_int32_t w0, w1, w2, w3;
> +    uint32_t w0, w1, w2, w3;
>    } parts32;
>  } ieee854_long_double_shape_type;
>  
>  #endif
>  
> -#if __FLOAT_WORD_ORDER == LITTLE_ENDIAN
> +#if __FLOAT_WORD_ORDER == __LITTLE_ENDIAN
>  
>  typedef union
>  {
>    long double value;
>    struct
>    {
> -    u_int64_t lsw;
> -    u_int64_t msw;
> +    uint64_t lsw;
> +    uint64_t msw;
>    } parts64;
>    struct
>    {
> -    u_int32_t w3, w2, w1, w0;
> +    uint32_t w3, w2, w1, w0;
>    } parts32;
>  } ieee854_long_double_shape_type;
>  
> @@ -96,3 +98,5 @@ do {								\
>  */
>  #define _Float128 long double
>  #define L(x) x##L
> +
> +#endif /* math_ldbl.h */
> diff --git a/sysdeps/ieee754/ldbl-128ibm/math_ldbl.h b/sysdeps/ieee754/ldbl-128ibm/math_ldbl.h
> index 625ce00e13..5420fb27af 100644
> --- a/sysdeps/ieee754/ldbl-128ibm/math_ldbl.h
> +++ b/sysdeps/ieee754/ldbl-128ibm/math_ldbl.h
> @@ -1,6 +1,5 @@
> -#ifndef _MATH_PRIVATE_H_
> -#error "Never use <math_ldbl.h> directly; include <math_private.h> instead."
> -#endif
> +#ifndef _MATH_LDBL_H_
> +#define _MATH_LDBL_H_ 1
>  
>  #include <ieee754.h>
>  #include <stdint.h>
> @@ -239,9 +238,14 @@ ldbl_nearbyint (double a)
>  static inline void
>  ldbl_canonicalize_int (double *a, double *aa)
>  {
> -  int64_t ax, aax;
> -  EXTRACT_WORDS64 (ax, *a);
> -  EXTRACT_WORDS64 (aax, *aa);
> +  /* We can't use math_private.h macros in this file.  */

Suggest:

/* Previously we used EXTRACT_WORDS64 from math_private.h, but in order
   to avoid including internal headers we duplicate that code here.  */

> +  uint64_t ax, aax;
> +  union { double value; uint64_t word; } extractor;
> +  extractor.value = *a;
> +  ax = extractor.word;
> +  extractor.value = *aa;
> +  aax = extractor.word;
> +
>    int expdiff = ((ax >> 52) & 0x7ff) - ((aax >> 52) & 0x7ff);
>    if (expdiff <= 53)
>      {
> @@ -263,3 +267,5 @@ ldbl_canonicalize_int (double *a, double *aa)
>  	}
>      }
>  }
> +
> +#endif /* math_ldbl.h */
> diff --git a/sysdeps/ieee754/ldbl-128ibm/test-canonical-ldbl-128ibm.c b/sysdeps/ieee754/ldbl-128ibm/test-canonical-ldbl-128ibm.c
> index 8be4499bd5..75735db18e 100644
> --- a/sysdeps/ieee754/ldbl-128ibm/test-canonical-ldbl-128ibm.c
> +++ b/sysdeps/ieee754/ldbl-128ibm/test-canonical-ldbl-128ibm.c
> @@ -18,7 +18,7 @@
>  
>  #include <float.h>
>  #include <math.h>
> -#include <math_private.h>
> +#include <math_ldbl.h>
>  #include <stdbool.h>
>  #include <stdio.h>
>  
> diff --git a/sysdeps/ieee754/ldbl-128ibm/test-totalorderl-ldbl-128ibm.c b/sysdeps/ieee754/ldbl-128ibm/test-totalorderl-ldbl-128ibm.c
> index 86869aceda..eaada2f848 100644
> --- a/sysdeps/ieee754/ldbl-128ibm/test-totalorderl-ldbl-128ibm.c
> +++ b/sysdeps/ieee754/ldbl-128ibm/test-totalorderl-ldbl-128ibm.c
> @@ -17,7 +17,7 @@
>     <http://www.gnu.org/licenses/>.  */
>  
>  #include <math.h>
> -#include <math_private.h>
> +#include <math_ldbl.h>
>  #include <stdbool.h>
>  #include <stdio.h>
>  
> diff --git a/sysdeps/ieee754/ldbl-96/math_ldbl.h b/sysdeps/ieee754/ldbl-96/math_ldbl.h
> index cca30657ce..74fbad6374 100644
> --- a/sysdeps/ieee754/ldbl-96/math_ldbl.h
> +++ b/sysdeps/ieee754/ldbl-96/math_ldbl.h
> @@ -1,11 +1,13 @@
> -#ifndef _MATH_PRIVATE_H_
> -#error "Never use <math_ldbl.h> directly; include <math_private.h> instead."
> -#endif
> +#ifndef _MATH_LDBL_H_
> +#define _MATH_LDBL_H_ 1
> +
> +#include <stdint.h>
> +#include <endian.h>
>  
>  /* A union which permits us to convert between a long double and
>     three 32 bit ints.  */
>  
> -#if __FLOAT_WORD_ORDER == BIG_ENDIAN
> +#if __FLOAT_WORD_ORDER == __BIG_ENDIAN
>  
>  typedef union
>  {
> @@ -14,22 +16,22 @@ typedef union
>    {
>      int sign_exponent:16;
>      unsigned int empty:16;
> -    u_int32_t msw;
> -    u_int32_t lsw;
> +    uint32_t msw;
> +    uint32_t lsw;
>    } parts;
>  } ieee_long_double_shape_type;
>  
>  #endif
>  
> -#if __FLOAT_WORD_ORDER == LITTLE_ENDIAN
> +#if __FLOAT_WORD_ORDER == __LITTLE_ENDIAN
>  
>  typedef union
>  {
>    long double value;
>    struct
>    {
> -    u_int32_t lsw;
> -    u_int32_t msw;
> +    uint32_t lsw;
> +    uint32_t msw;
>      int sign_exponent:16;
>      unsigned int empty:16;
>    } parts;
> @@ -96,3 +98,5 @@ do {								\
>    se_u.parts.sign_exponent = (exp);				\
>    (d) = se_u.value;						\
>  } while (0)
> +
> +#endif /* math_ldbl.h */
> diff --git a/sysdeps/ieee754/ldbl-96/test-canonical-ldbl-96.c b/sysdeps/ieee754/ldbl-96/test-canonical-ldbl-96.c
> index db9db04367..3254097754 100644
> --- a/sysdeps/ieee754/ldbl-96/test-canonical-ldbl-96.c
> +++ b/sysdeps/ieee754/ldbl-96/test-canonical-ldbl-96.c
> @@ -18,7 +18,7 @@
>  
>  #include <float.h>
>  #include <math.h>
> -#include <math_private.h>
> +#include <math_ldbl.h>
>  #include <stdbool.h>
>  #include <stdint.h>
>  #include <stdio.h>
> diff --git a/sysdeps/ieee754/ldbl-96/test-totalorderl-ldbl-96.c b/sysdeps/ieee754/ldbl-96/test-totalorderl-ldbl-96.c
> index eb21956e4f..4e01f15aa9 100644
> --- a/sysdeps/ieee754/ldbl-96/test-totalorderl-ldbl-96.c
> +++ b/sysdeps/ieee754/ldbl-96/test-totalorderl-ldbl-96.c
> @@ -18,7 +18,7 @@
>  
>  #include <float.h>
>  #include <math.h>
> -#include <math_private.h>
> +#include <math_ldbl.h>
>  #include <stdbool.h>
>  #include <stdint.h>
>  #include <stdio.h>
> diff --git a/sysdeps/powerpc/fpu/math_ldbl.h b/sysdeps/powerpc/fpu/math_ldbl.h
> index 36378c0239..597d216a07 100644
> --- a/sysdeps/powerpc/fpu/math_ldbl.h
> +++ b/sysdeps/powerpc/fpu/math_ldbl.h
> @@ -1,6 +1,5 @@
> -#ifndef _MATH_PRIVATE_H_
> -#error "Never use <math_ldbl.h> directly; include <math_private.h> instead."
> -#endif
> +#ifndef _MATH_LDBL_H_PPC_
> +#define _MATH_LDBL_H_PPC_ 1
>  
>  /* GCC does not optimize the default ldbl_pack code to not spill register
>     in the stack. The following optimization tells gcc that pack/unpack
> @@ -34,3 +33,5 @@ ldbl_unpack_ppc (long double l, double *a, double *aa)
>  #define ldbl_unpack ldbl_unpack_ppc
>  
>  #include <sysdeps/ieee754/ldbl-128ibm/math_ldbl.h>
> +
> +#endif /* math_ldbl.h */
> diff --git a/sysdeps/x86_64/fpu/math_ldbl.h b/sysdeps/x86_64/fpu/math_ldbl.h
> index b9ff8dadaf..cbc47c2935 100644
> --- a/sysdeps/x86_64/fpu/math_ldbl.h
> +++ b/sysdeps/x86_64/fpu/math_ldbl.h
> @@ -1,6 +1,7 @@
> -#ifndef _MATH_PRIVATE_H_
> -#error "Never use <math_ldbl.h> directly; include <math_private.h> instead."
> -#endif
> +#ifndef _MATH_LDBL_H_
> +#define _MATH_LDBL_H_ 1
> +
> +#include <stdint.h>
>  
>  /* A union which permits us to convert between a long double and
>     three 32 bit ints.  */
> @@ -10,8 +11,8 @@ typedef union
>    long double value;
>    struct
>    {
> -    u_int32_t lsw;
> -    u_int32_t msw;
> +    uint32_t lsw;
> +    uint32_t msw;
>      int sign_exponent:16;
>      unsigned int empty1:16;
>      unsigned int empty0:32;
> @@ -77,3 +78,5 @@ do {								\
>    se_u.parts.sign_exponent = (exp);				\
>    (d) = se_u.value;						\
>  } while (0)
> +
> +#endif /* math_ldbl.h */
> 


-- 
Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]