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
Related show

Commit Message

Paul E. Murphy Feb. 14, 2020, 10:54 p.m.
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(-)

-- 
2.21.1

Comments

Paul Clarke Feb. 17, 2020, 4:14 p.m. | #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. | #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.

-- 
Joseph S. Myers
joseph@codesourcery.com

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)