diff mbox series

Improve __LONG_DOUBLE_USES_FLOAT128 commentary

Message ID 20200214225416.28284-1-murphyp@linux.vnet.ibm.com
State New
Headers show
Series Improve __LONG_DOUBLE_USES_FLOAT128 commentary | expand

Commit Message

Paul E. Murphy Feb. 14, 2020, 10:54 p.m. UTC
Per the feedback from Joseph [1].  Good comments and a more
self-explanatory macro name will be very helpful when this
macro or its successor are able to assume a non-zero value.


[1] <https://sourceware.org/ml/libc-alpha/2020-02/msg00687.html>

---8<---

As noted, this is not the most ideal name for the conditional
used to determine whether long double ABI should be redirected
to _Float128 ABI.  Once the development effort settles down, I
will rename it.

Improve the commentary to aid future developers who will stumble
upon this novel, yet not always perfect, mechanism to support
alternative formats for long double.
---
 bits/long-double.h                            | 15 ++++++++++++
 .../ldbl-128ibm-compat/bits/long-double.h     | 24 +++++++++++++++----
 2 files changed, 35 insertions(+), 4 deletions(-)

Comments

Paul A. Clarke Feb. 17, 2020, 4:14 p.m. UTC | #1
On 2/14/20 4:54 PM, Paul E. Murphy wrote:
> Per the feedback from Joseph [1].  Good comments and a more
> self-explanatory macro name will be very helpful when this
> macro or its successor are able to assume a non-zero value.
> 
> 
> [1] <https://sourceware.org/ml/libc-alpha/2020-02/msg00687.html>
> 
> ---8<---
> 
> As noted, this is not the most ideal name for the conditional
> used to determine whether long double ABI should be redirected
> to _Float128 ABI.  Once the development effort settles down, I
> will rename it.
> 
> Improve the commentary to aid future developers who will stumble
> upon this novel, yet not always perfect, mechanism to support
> alternative formats for long double.
> ---
>  bits/long-double.h                            | 15 ++++++++++++
>  .../ldbl-128ibm-compat/bits/long-double.h     | 24 +++++++++++++++----
>  2 files changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/bits/long-double.h b/bits/long-double.h
> index 6e16447e65..29ba2506a2 100644
> --- a/bits/long-double.h
> +++ b/bits/long-double.h
> @@ -37,4 +37,19 @@
>  #ifndef __NO_LONG_DOUBLE_MATH
>  # define __NO_LONG_DOUBLE_MATH	1
>  #endif
> +
> +/* The macro __LONG_DOUBLE_USES_FLOAT128 is used to determine the
> +   choice of the underlying ABI of long double during a single compilation.

I'm not sure you need "during a single compilation".

> +
> +   If the value is non-zero, any API which is parameterized by the long
> +   double type (i.e the scanf/printf family of functions or the explicitly
> +   parameterized math.h functions) will be redirected to a compatible
> +   implementation using _Float128 ABI via symbols suffixed with ieee128.
> +
> +   These redirections usually take the form of an "ASM Label" as called out
> +   int Chapter 6.47.4 in the GCC 9 manual.  And in very rare cases, a

I don't see "ASM Label" in that section, although the page is called "Asm-Labels.html". hmm

> +   macro.  They do not alter or otherwise change the ABI stability of glibc.

I presume "They" refers to "redirections" and not "macro".  I wonder if the "And..." fragment should be joined with the sentence before it?  "GCC 9 manual, and..."

> +
> +   The mechanism this macro uses to acquire it's value is specific both a
> +   function of architecture, and options used to invoke compilation.  */

s/it's/its/

"value is specific both a function of architecture, and options used to invoke compilation" doesn't read well.

"value is a function of architecture and compilation options", perhaps?

>  #define __LONG_DOUBLE_USES_FLOAT128 0
> diff --git a/sysdeps/ieee754/ldbl-128ibm-compat/bits/long-double.h b/sysdeps/ieee754/ldbl-128ibm-compat/bits/long-double.h
> index 91dddbdc8b..6bec7d82a3 100644
> --- a/sysdeps/ieee754/ldbl-128ibm-compat/bits/long-double.h
> +++ b/sysdeps/ieee754/ldbl-128ibm-compat/bits/long-double.h
> @@ -22,8 +22,24 @@
>  #  define __NO_LONG_DOUBLE_MATH		1
>  # endif
>  #endif
> -/* On platforms that reuse the _Float128 implementation for IEEE long
> -   double, access to the correct long double functions is selected based
> -   on the long double mode being used during the compilation.  On
> -   powerpc64le, this is true when -mabi=ieeelongdouble is in use.  */
> +
> +/* The macro __LONG_DOUBLE_USES_FLOAT128 is used to determine the
> +   choice of the underlying ABI of long double during a single compilation.
> +
> +   If the value is non-zero, any API which is parameterized by the long
> +   double type (i.e the scanf/printf family of functions or the explicitly
> +   parameterized math.h functions) will be redirected to a compatible
> +   implementation using _Float128 ABI via symbols suffixed with ieee128.
> +
> +   These redirections usually take the form of an "ASM Label" as called out
> +   int Chapter 6.47.4 in the GCC 9 manual.  And in very rare cases, a
> +   macro.  They do not alter or otherwise change the ABI stability of glibc.
> +
> +   The mechanism this macro uses to acquire it's value is specific both a
> +   function of architecture, and options used to invoke compilation.
> +   
> +   On powerpc64le, the GCC >=7 compiler option -mabi=ieeelongdouble will
> +   change the underlying format of long double to IEEE 128 binary floating
> +   point.  We enable the redirects based how many mantissa digits GCC
> +   reports for long double.  For older GCC and non-IEEE, this is 106.  */
>  #define __LONG_DOUBLE_USES_FLOAT128 (__LDBL_MANT_DIG__ == 113)
> 

PC
Joseph Myers March 24, 2020, 11:35 p.m. UTC | #2
On Fri, 14 Feb 2020, Paul E. Murphy wrote:

> +/* The macro __LONG_DOUBLE_USES_FLOAT128 is used to determine the
> +   choice of the underlying ABI of long double during a single compilation.
> +
> +   If the value is non-zero, any API which is parameterized by the long
> +   double type (i.e the scanf/printf family of functions or the explicitly
> +   parameterized math.h functions) will be redirected to a compatible
> +   implementation using _Float128 ABI via symbols suffixed with ieee128.
> +
> +   These redirections usually take the form of an "ASM Label" as called out
> +   int Chapter 6.47.4 in the GCC 9 manual.  And in very rare cases, a
> +   macro.  They do not alter or otherwise change the ABI stability of glibc.

I don't think this paragraph about how the redirections are implemented 
should be present at all; it's simply a description of how the redirection 
macros work, which belongs with the implementation of those macros if 
anywhere.  (And referencing particular section numbers in the GCC manual 
is a bad idea; section names may be somewhat stable, numbers are very 
unstable.)

> diff --git a/sysdeps/ieee754/ldbl-128ibm-compat/bits/long-double.h b/sysdeps/ieee754/ldbl-128ibm-compat/bits/long-double.h

I think it's best for the detailed comment defining the semantics of the 
macro to be only in the default bits/long-double.h file, with 
architecture-specific files having comments that only discuss the 
architecture-specific details for that architecture.  That avoids having 
multiple places that need updating with any improvement to the 
documentation.
diff mbox series

Patch

diff --git a/bits/long-double.h b/bits/long-double.h
index 6e16447e65..29ba2506a2 100644
--- a/bits/long-double.h
+++ b/bits/long-double.h
@@ -37,4 +37,19 @@ 
 #ifndef __NO_LONG_DOUBLE_MATH
 # define __NO_LONG_DOUBLE_MATH	1
 #endif
+
+/* The macro __LONG_DOUBLE_USES_FLOAT128 is used to determine the
+   choice of the underlying ABI of long double during a single compilation.
+
+   If the value is non-zero, any API which is parameterized by the long
+   double type (i.e the scanf/printf family of functions or the explicitly
+   parameterized math.h functions) will be redirected to a compatible
+   implementation using _Float128 ABI via symbols suffixed with ieee128.
+
+   These redirections usually take the form of an "ASM Label" as called out
+   int Chapter 6.47.4 in the GCC 9 manual.  And in very rare cases, a
+   macro.  They do not alter or otherwise change the ABI stability of glibc.
+
+   The mechanism this macro uses to acquire it's value is specific both a
+   function of architecture, and options used to invoke compilation.  */
 #define __LONG_DOUBLE_USES_FLOAT128 0
diff --git a/sysdeps/ieee754/ldbl-128ibm-compat/bits/long-double.h b/sysdeps/ieee754/ldbl-128ibm-compat/bits/long-double.h
index 91dddbdc8b..6bec7d82a3 100644
--- a/sysdeps/ieee754/ldbl-128ibm-compat/bits/long-double.h
+++ b/sysdeps/ieee754/ldbl-128ibm-compat/bits/long-double.h
@@ -22,8 +22,24 @@ 
 #  define __NO_LONG_DOUBLE_MATH		1
 # endif
 #endif
-/* On platforms that reuse the _Float128 implementation for IEEE long
-   double, access to the correct long double functions is selected based
-   on the long double mode being used during the compilation.  On
-   powerpc64le, this is true when -mabi=ieeelongdouble is in use.  */
+
+/* The macro __LONG_DOUBLE_USES_FLOAT128 is used to determine the
+   choice of the underlying ABI of long double during a single compilation.
+
+   If the value is non-zero, any API which is parameterized by the long
+   double type (i.e the scanf/printf family of functions or the explicitly
+   parameterized math.h functions) will be redirected to a compatible
+   implementation using _Float128 ABI via symbols suffixed with ieee128.
+
+   These redirections usually take the form of an "ASM Label" as called out
+   int Chapter 6.47.4 in the GCC 9 manual.  And in very rare cases, a
+   macro.  They do not alter or otherwise change the ABI stability of glibc.
+
+   The mechanism this macro uses to acquire it's value is specific both a
+   function of architecture, and options used to invoke compilation.
+   
+   On powerpc64le, the GCC >=7 compiler option -mabi=ieeelongdouble will
+   change the underlying format of long double to IEEE 128 binary floating
+   point.  We enable the redirects based how many mantissa digits GCC
+   reports for long double.  For older GCC and non-IEEE, this is 106.  */
 #define __LONG_DOUBLE_USES_FLOAT128 (__LDBL_MANT_DIG__ == 113)