public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc/devel/omp/gcc-10] gimple-fold: Don't optimize wierdo floating point value reads [PR95450]
@ 2020-09-01  9:59 Tobias Burnus
  0 siblings, 0 replies; only message in thread
From: Tobias Burnus @ 2020-09-01  9:59 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:7e53436da1902061af797d0aaa744c52bd9829ae

commit 7e53436da1902061af797d0aaa744c52bd9829ae
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Aug 25 07:17:10 2020 +0200

    gimple-fold: Don't optimize wierdo floating point value reads [PR95450]
    
    My patch to introduce native_encode_initializer to fold_ctor_reference
    apparently broke gnulib/m4 on powerpc64.
    There it uses a const union with two doubles and corresponding IBM double
    double long double which actually is the largest normalizable long double
    value (1 ulp higher than __LDBL_MAX__).  The reason our __LDBL_MAX__ is
    smaller is that we internally treat the double double type as one having
    106-bit precision, but it actually has a variable 53-bit to 2000-ish bit precision
    and for the
    0x1.fffffffffffff7ffffffffffffc000p+1023L
    value gnulib uses we need 107-bit precision, therefore for GCC __LDBL_MAX__
    is
    0x1.fffffffffffff7ffffffffffff8000p+1023L
    Before my changes, we wouldn't be able to fold_ctor_reference it and it
    worked fine at runtime, but with the change we are able to do that, but
    because it is larger than anything we can handle internally, we treat it
    weirdly.  Similar problem would be if somebody creates this way valid,
    but much more than 106 bit precision e.g. 1.0 + 1.0e-768.
    Now, I think similar problem could happen e.g. on i?86/x86_64 with long
    double there, it also has some weird values in the format, e.g. the
    unnormals, pseudo infinities and various other magic values.
    
    This patch for floating point types (including vector and complex types
    with such elements) will try to encode the returned value again and punt
    if it has different memory representation from the original.  Note, this
    is only done in the path where native_encode_initializer was used, in order
    not to affect e.g. just reading an unpunned long double value; the value
    should be compiler generated in that case and thus should be properly
    representable.  It will punt also if e.g. the padding bits are initialized
    to non-zero values.
    
    I think the verification that what we encode can be interpreted back
    woiuld be only an internal consistency check (so perhaps for ENABLE_CHECKING
    if flag_checking only, but if both directions perform it, then we need
    to avoid mutual recursion).
    While for the other direction (interpretation), at least for the broken by
    design long doubles we just know we can't represent in GCC all valid values.
    The other floating point formats are just theoretical case, perhaps we would
    canonicalize something to a value that wouldn't trigger invalid exception
    when without canonicalization it would trigger it at runtime, so let's just
    ignore those.
    
    Adjusted (so far untested) patch to do it in native_interpret_real instead
    and limit it to the MODE_COMPOSITE_P cases, for which e.g.
    fold-const.c/simplify-rtx.c punts in several other places too because we just
    know we can't represent everything.
    
    E.g.
          /* Don't constant fold this floating point operation if the
             result may dependent upon the run-time rounding mode and
             flag_rounding_math is set, or if GCC's software emulation
             is unable to accurately represent the result.  */
          if ((flag_rounding_math
               || (MODE_COMPOSITE_P (mode) && !flag_unsafe_math_optimizations))
              && (inexact || !real_identical (&result, &value)))
            return NULL_TREE;
    Or perhaps guard it with MODE_COMPOSITE_P (mode) && !flag_unsafe_math_optimizations
    too, thus break what gnulib / m4 does with -ffast-math, but not normally?
    
    2020-08-25  Jakub Jelinek  <jakub@redhat.com>
    
            PR target/95450
            * fold-const.c (native_interpret_real): For MODE_COMPOSITE_P modes
            punt if the to be returned REAL_CST does not encode to the bitwise
            same representation.
    
            * gcc.target/powerpc/pr95450.c: New test.
    
    (cherry picked from commit 9f2f79df19fbfaa1c4be313c2f2b5ce04646433e)

Diff:
---
 gcc/fold-const.c                           | 14 +++++++++++++-
 gcc/testsuite/gcc.target/powerpc/pr95450.c | 29 +++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 3744e4c2c2d..a128e3e2ec5 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -8212,7 +8212,19 @@ native_interpret_real (tree type, const unsigned char *ptr, int len)
     }
 
   real_from_target (&r, tmp, mode);
-  return build_real (type, r);
+  tree ret = build_real (type, r);
+  if (MODE_COMPOSITE_P (mode))
+    {
+      /* For floating point values in composite modes, punt if this folding
+	 doesn't preserve bit representation.  As the mode doesn't have fixed
+	 precision while GCC pretends it does, there could be valid values that
+	 GCC can't really represent accurately.  See PR95450.  */
+      unsigned char buf[24];
+      if (native_encode_expr (ret, buf, total_bytes, 0) != total_bytes
+	  || memcmp (ptr, buf, total_bytes) != 0)
+	ret = NULL_TREE;
+    }
+  return ret;
 }
 
 
diff --git a/gcc/testsuite/gcc.target/powerpc/pr95450.c b/gcc/testsuite/gcc.target/powerpc/pr95450.c
new file mode 100644
index 00000000000..569d2b2c536
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr95450.c
@@ -0,0 +1,29 @@
+/* PR target/95450 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump-not "return \[0-9.e+]\+;" "optimized" } } */
+
+/* Verify this is not optimized for double double into return floating_point_constant,
+   as while that constant is the maximum normalized floating point value, it needs
+   107 bit precision, which is more than GCC supports for this format.  */
+
+#if __LDBL_MANT_DIG__ == 106
+union U
+{
+  struct { double hi; double lo; } dd;
+  long double ld;
+};
+
+const union U g = { { __DBL_MAX__, __DBL_MAX__ / (double)134217728UL / (double)134217728UL } };
+#else
+struct S
+{
+  long double ld;
+} g;
+#endif
+
+long double
+foo (void)
+{
+  return g.ld;
+}


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2020-09-01  9:59 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-01  9:59 [gcc/devel/omp/gcc-10] gimple-fold: Don't optimize wierdo floating point value reads [PR95450] Tobias Burnus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).