[PATCH 0/2] IBM Z: Prepare cleanup of float express precision

[PATCH 0/2] IBM Z: Prepare cleanup of float express precision

[Marius Hillenbrand]

Hi,

gcc has special behavior for FLT_EVAL_METHOD on s390x that causes
performance impact in some scenarios and fails to align with float_t wrt
the C standard in others. These two patches prepare gcc for a cleanup to
get rid of that special case, to improve standard compliance and avoid
the overhead.

On s390 today, float_t is defined as double while gcc by default sets
FLT_EVAL_METHOD to 0 and evaluates float expressions in
single-precision. To mitigate that mismatch, with -std=c99 gcc emits
double precision instructions for evaluating float expressions -- at the
cost of additional conversion instructions. Earlier discussions favored
this behavior to maintain ABI compatibility and compliance with the C
standard (that is, for -std=c99), see the discussion around
https://gcc.gnu.org/legacy-ml/gcc-patches/2016-09/msg02392.html Given
the performance overhead, I have reevaluated the impact of cleaning up
the special behavior and changing float_t into float on s390, and now
think that option to be favorable.

The reason for float_t being defined as double is that the port of glibc
to s390 deferred to the generic definition, which back then defaulted to
double. Since then, that definition has not been changed to avoid
breaking ABIs that use float_t. I found only two affected packages,
clucene and ImageMagick, out of >130k Debian packages scanned, and
prepared patches to avoid impact. ImageMagick's ABI has become
independent of float_t on s390 since 7.0.10-39 (patch in
https://github.com/ImageMagick/ImageMagick/pull/2832); patch for clucene
in https://sourceforge.net/p/clucene/bugs/233/.

To smoothen the transition, the first patch makes gcc's behavior
configurable at compile-time with the flag
--enable-s390-excess-float-precision. When the flag is enabled, gcc
maintains the current behavior. By default, configure will test glibc's
behavior: if glibc ties float_t to double, configure will enable the
flag and maintain the current behavior.  Otherwise, it will disable the
flag and drop the special behavior.

Bootstrapped and regtested on s390x, both with a conventional and a
modified glibc. Bootstrapped and regtested on x86-64. Inspected the
generated headers on both targets.

Marius Hillenbrand (2):
  IBM Z: Configure excess precision for float at compile-time
  gcc/testsuite/s390: Add test cases for float_t

 gcc/config/s390/s390.c                    | 27 ++++++++++----
 gcc/configure.ac                          | 45 +++++++++++++++++++++++
 gcc/doc/install.texi                      | 10 +++++
 gcc/testsuite/gcc.target/s390/float_t-1.c | 15 ++++++++
 gcc/testsuite/gcc.target/s390/float_t-2.c | 13 +++++++
 5 files changed, 103 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/s390/float_t-1.c
 create mode 100644 gcc/testsuite/gcc.target/s390/float_t-2.c

-- 
2.26.2

[PATCH 1/2] IBM Z: Configure excess precision for float at compile-time

[Marius Hillenbrand]

Historically, float_t has been defined as double on s390 and gcc would
emit double precision insns for evaluating float expressions when in
standard-compliant mode. Configure that behavior at compile-time as prep
for changes in glibc: When glibc ties float_t to double, keep the old
behavior; when glibc derives float_t from FLT_EVAL_METHOD (as on most
other archs), revert to the default behavior (i.e.,
FLT_EVAL_METHOD_PROMOTE_TO_FLOAT). Provide a configure option
--enable-s390-excess-float-precision to override the check.

gcc/ChangeLog:

2020-11-25  Marius Hillenbrand  <mhillen@linux.ibm.com>

	* configure.ac: Add configure option
	--enable-s390-excess-float-precision and check to derive default
	from glibc.
	* config/s390/s390.c: Guard s390_excess_precision with an ifdef
	for ENABLE_S390_EXCESS_FLOAT_PRECISION.
	* doc/install.texi: Document --enable-s390-excess-float-precision.
	* configure: Regenerate.
	* config.in: Regenerate.
---
 gcc/config/s390/s390.c | 27 ++++++++++++++++++-------
 gcc/configure.ac       | 45 ++++++++++++++++++++++++++++++++++++++++++
 gcc/doc/install.texi   | 10 ++++++++++
 3 files changed, 75 insertions(+), 7 deletions(-)

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 6983e363252..02f18366aa1 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -16376,20 +16376,28 @@ s390_invalid_binary_op (int op ATTRIBUTE_UNUSED, const_tree type1, const_tree ty
   return NULL;
 }
 
-/* Implement TARGET_C_EXCESS_PRECISION.
+#if ENABLE_S390_EXCESS_FLOAT_PRECISION == 1
+/* Implement TARGET_C_EXCESS_PRECISION to maintain historic behavior with older
+   glibc versions
 
-   FIXME: For historical reasons, float_t and double_t are typedef'ed to
+   For historical reasons, float_t and double_t had been typedef'ed to
    double on s390, causing operations on float_t to operate in a higher
    precision than is necessary.  However, it is not the case that SFmode
    operations have implicit excess precision, and we generate more optimal
    code if we let the compiler know no implicit extra precision is added.
 
-   That means when we are compiling with -fexcess-precision=fast, the value
-   we set for FLT_EVAL_METHOD will be out of line with the actual precision of
-   float_t (though they would be correct for -fexcess-precision=standard).
+   With a glibc with that "historic" definition, configure will enable this hook
+   to set FLT_EVAL_METHOD to 1 for -fexcess-precision=standard (e.g., as implied
+   by -std=cXY).  That means when we are compiling with -fexcess-precision=fast,
+   the value we set for FLT_EVAL_METHOD will be out of line with the actual
+   precision of float_t.
 
-   A complete fix would modify glibc to remove the unnecessary typedef
-   of float_t to double.  */
+   Newer versions of glibc will be modified to derive the definition of float_t
+   from FLT_EVAL_METHOD on s390x, as on many other architectures.  There,
+   configure will disable this hook by default, so that we defer to the default
+   of FLT_EVAL_METHOD_PROMOTE_TO_FLOAT and a resulting typedef of float_t to
+   float.  Note that in that scenario, float_t and FLT_EVAL_METHOD will be in
+   line independent of -fexcess-precision. */
 
 static enum flt_eval_method
 s390_excess_precision (enum excess_precision_type type)
@@ -16412,6 +16420,7 @@ s390_excess_precision (enum excess_precision_type type)
     }
   return FLT_EVAL_METHOD_UNPREDICTABLE;
 }
+#endif
 
 /* Implement the TARGET_ASAN_SHADOW_OFFSET hook.  */
 
@@ -16708,8 +16717,12 @@ s390_shift_truncation_mask (machine_mode mode)
 #undef TARGET_ASM_CAN_OUTPUT_MI_THUNK
 #define TARGET_ASM_CAN_OUTPUT_MI_THUNK hook_bool_const_tree_hwi_hwi_const_tree_true
 
+#if ENABLE_S390_EXCESS_FLOAT_PRECISION == 1
+/* This hook is only needed to maintain the historic behavior with glibc
+   versions that typedef float_t to double. */
 #undef TARGET_C_EXCESS_PRECISION
 #define TARGET_C_EXCESS_PRECISION s390_excess_precision
+#endif
 
 #undef  TARGET_SCHED_ADJUST_PRIORITY
 #define TARGET_SCHED_ADJUST_PRIORITY s390_adjust_priority
diff --git a/gcc/configure.ac b/gcc/configure.ac
index b410428b4fc..24679a540c1 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -7318,6 +7318,51 @@ if test x"$ld_pushpopstate_support" = xyes; then
 fi
 AC_MSG_RESULT($ld_pushpopstate_support)
 
+# On s390, float_t has historically been statically defined as double for no
+# good reason. To comply with the C standard in the light of this definition,
+# gcc has evaluated float expressions in double precision when in
+# standards-compatible mode or when given -fexcess-precision=standard. To enable
+# a smooth transition towards the new model used by most architectures, where
+# gcc describes its behavior via the macro __FLT_EVAL_METHOD__ and glibc derives
+# float_t from that, this behavior can be configured with
+# --enable-s390-excess-float-precision. When given as enabled, that flag selects
+# the old model. When omitted, native builds will derive the flag from the
+# behavior of glibc. When glibc clamps float_t to double, gcc follows the old
+# model. In any other case, it defaults to the new model.
+AC_ARG_ENABLE(s390-excess-float-precision,
+  [AS_HELP_STRING([--enable-s390-excess-float-precision],
+		  [on s390 targets, evaluate float with double precision
+		   when in standards-conforming mode])],
+  [],[enable_s390_excess_float_precision=auto])
+
+case $target in
+  s390*-linux*)
+  if test "$target" = "$host" -a "$host" = "$build" -a \
+      x"$enable_s390_excess_float_precision" = xauto; then
+    AC_CACHE_CHECK([for glibc clamping float_t to double],
+      gcc_cv_float_t_clamped_to_double,
+      [AC_RUN_IFELSE([AC_LANG_SOURCE([
+#define __FLT_EVAL_METHOD__ 0
+#include <math.h>
+int main() {
+  return !(sizeof(float_t) == sizeof(double));
+}])],
+        [gcc_cv_float_t_clamped_to_double=yes],
+        [gcc_cv_float_t_clamped_to_double=no])])
+    if test x"$gcc_cv_float_t_clamped_to_double" = xyes; then
+      enable_s390_excess_float_precision=yes
+    fi
+  fi
+
+  GCC_TARGET_TEMPLATE(ENABLE_S390_EXCESS_FLOAT_PRECISION)
+  if test x"$enable_s390_excess_float_precision" = xyes; then
+    AC_DEFINE(ENABLE_S390_EXCESS_FLOAT_PRECISION, 1,
+[Define to enable evaluating float expressions with double precision in
+standards-compatible mode on s390 targets.])
+  fi
+  ;;
+esac
+
 # Configure the subdirectories
 # AC_CONFIG_SUBDIRS($subdirs)
 
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 8c55da373f8..546f79d2b8e 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -2271,6 +2271,16 @@ information in object.
 
 The option is disabled by default. It is enabled on RISC-V/ELF (bare-metal)
 target if target binutils supported.
+
+@item --enable-s390-excess-float-precision
+@itemx --disable-s390-excess-float-precision
+On s390(x) targets, enable treatment of float expressions with double precision
+when in standards-compliant mode (e.g., when @code{--std=c99} or
+@code{-fexcess-precision=standard} are given).
+
+For a native build, the option's default is derived from glibc's behavior. When
+glibc clamps float_t to double, gcc follows and enables the option. In all other
+cases, it defaults to off.
 @end table
 
 @subheading Cross-Compiler-Specific Options
-- 
2.26.2

[PATCH 2/2] gcc/testsuite/s390: Add test cases for float_t

[Marius Hillenbrand]

Add two test cases that check for acceptable combinations of float_t and
FLT_EVAL_METHOD on s390x.

Tested against an as-is glibc and one modified so that it derives
float_t from FLT_EVAL_METHOD.

gcc/testsuite/ChangeLog:

2020-11-25  Marius Hillenbrand  <mhillen@linux.ibm.com>

	* gcc.target/s390/float_t-1.c: New test.
	* gcc.target/s390/float_t-2.c: New test.
---
 gcc/testsuite/gcc.target/s390/float_t-1.c | 15 +++++++++++++++
 gcc/testsuite/gcc.target/s390/float_t-2.c | 13 +++++++++++++
 2 files changed, 28 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/s390/float_t-1.c
 create mode 100644 gcc/testsuite/gcc.target/s390/float_t-2.c

diff --git a/gcc/testsuite/gcc.target/s390/float_t-1.c b/gcc/testsuite/gcc.target/s390/float_t-1.c
new file mode 100644
index 00000000000..3455694250f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/float_t-1.c
@@ -0,0 +1,15 @@
+/* { dg-do run } */
+/* { dg-options "-std=c99" } */
+#include <math.h>
+#include <stdlib.h>
+
+int main()
+{
+  /* In standard-compliant mode, the size of float_t and FLT_EVAL_METHOD must
+     match. */
+  if (sizeof(float_t) == sizeof(double) && __FLT_EVAL_METHOD__ != 1)
+    abort();
+  if (sizeof(float_t) == sizeof(float) && __FLT_EVAL_METHOD__ != 0)
+    abort();
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/s390/float_t-2.c b/gcc/testsuite/gcc.target/s390/float_t-2.c
new file mode 100644
index 00000000000..ebeda28b6d2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/float_t-2.c
@@ -0,0 +1,13 @@
+/* { dg-do run } */
+/* { dg-options "-std=gnu99" } */
+#include <math.h>
+#include <stdlib.h>
+
+int main()
+{
+  /* In gnuXY mode, the size of float_t and FLT_EVAL_METHOD must
+     match, with the historic exception of permitting double and 0. */
+  if (sizeof(float_t) == sizeof(float) && __FLT_EVAL_METHOD__ == 1)
+    abort();
+  return 0;
+}
-- 
2.26.2

Re: [PATCH 1/2] IBM Z: Configure excess precision for float at compile-time

[Andreas Krebbel]

On 11/25/20 6:06 PM, Marius Hillenbrand wrote:
> Historically, float_t has been defined as double on s390 and gcc would
> emit double precision insns for evaluating float expressions when in
> standard-compliant mode. Configure that behavior at compile-time as prep
> for changes in glibc: When glibc ties float_t to double, keep the old
> behavior; when glibc derives float_t from FLT_EVAL_METHOD (as on most
> other archs), revert to the default behavior (i.e.,
> FLT_EVAL_METHOD_PROMOTE_TO_FLOAT). Provide a configure option
> --enable-s390-excess-float-precision to override the check.
> 
> gcc/ChangeLog:
> 
> 2020-11-25  Marius Hillenbrand  <mhillen@linux.ibm.com>
> 
> 	* configure.ac: Add configure option
> 	--enable-s390-excess-float-precision and check to derive default
> 	from glibc.
> 	* config/s390/s390.c: Guard s390_excess_precision with an ifdef
> 	for ENABLE_S390_EXCESS_FLOAT_PRECISION.
> 	* doc/install.texi: Document --enable-s390-excess-float-precision.
> 	* configure: Regenerate.
> 	* config.in: Regenerate.

Ok. Applied to mainline.

Thanks!

Andreas

Re: [PATCH 2/2] gcc/testsuite/s390: Add test cases for float_t

[Andreas Krebbel]

On 11/25/20 6:06 PM, Marius Hillenbrand wrote:
> Add two test cases that check for acceptable combinations of float_t and
> FLT_EVAL_METHOD on s390x.
> 
> Tested against an as-is glibc and one modified so that it derives
> float_t from FLT_EVAL_METHOD.
> 
> gcc/testsuite/ChangeLog:
> 
> 2020-11-25  Marius Hillenbrand  <mhillen@linux.ibm.com>
> 
> 	* gcc.target/s390/float_t-1.c: New test.
> 	* gcc.target/s390/float_t-2.c: New test.

Ok. Applied to mainline.

Thanks!

Andreas