[Bug target/106831] New: [13 Regression] mpfr-4.1.0 started failing 2 tests: tget_set_d64 and tget_set_d128

[Bug target/106831] New: [13 Regression] mpfr-4.1.0 started failing 2 tests: tget_set_d64 and tget_set_d128

[slyfox at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106831

            Bug ID: 106831
           Summary: [13 Regression] mpfr-4.1.0 started failing 2 tests:
                    tget_set_d64 and tget_set_d128
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: slyfox at gcc dot gnu.org
  Target Milestone: ---

The test failures came up on this week's gcc snapshot updates:

- works: gcc-13.0.0.20220828 -O2
- fails: gcc-13.0.0.20220904 -O2
- works: gcc-13.0.0.20220904 -O0

Against https://ftp.gnu.org/gnu/mpfr/mpfr-4.1.0.tar.xz the following seems to
be enough:

    $ ./configure CC=gcc-13 && make && make check

Did not extract details yet to double-check if it's really a compiler error
(and not an mpfr deficiency). But -O0 / -O2 difference might be a hint.

Two test failures reported:

FAIL: tget_set_d64
==================

Error in check_misc for +0.
  mpfr_get_decimal64() returned:
|1011000110100000000000000000000000000000000000000000000000000000|
  mpfr_set_decimal64() set x to: -0 approx.
    = -0
FAIL tget_set_d64 (exit status: 1)

FAIL: tget_set_d128
===================

Error in check_misc for +0.
  mpfr_get_decimal128() returned: 0.0
  mpfr_set_decimal128() set x to: -0 approx.
    = -0
FAIL tget_set_d128 (exit status: 1)

$ gcc -v
Using built-in specs.
COLLECT_GCC=/<<NIX>>/gcc-13.0.0/bin/gcc
COLLECT_LTO_WRAPPER=/<<NIX>>/gcc-13.0.0/libexec/gcc/x86_64-unknown-linux-gnu/13.0.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with:
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 13.0.0 20220904 (experimental) (GCC)

[Bug target/106831] [13 Regression] mpfr-4.1.0 started failing 2 tests: tget_set_d64 and tget_set_d128

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106831

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0
                 CC|                            |aldyh at gcc dot gnu.org,
                   |                            |jakub at gcc dot gnu.org

[Bug target/106831] [13 Regression] mpfr-4.1.0 started failing 2 tests: tget_set_d64 and tget_set_d128

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106831

Aldy Hernandez <aldyh at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #1 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
Should be fixed with:

commit 8293a9632c46c8f3f9d531c09194aa8738944927
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Sun Sep 4 08:00:02 2022 +0200

    Do not clobber signbit when unioning a NAN.

    When unioning a known NAN and something else, we're dropping the
    properties of the NAN, particularly the sign.  This fixes the
    oversight.

    With this patch, we should be keeping the sign bit up to date, even in
    the presence of NANs.

and this:

commit dae8b9e2bbb6017bf90d68c7b720c500125c8295
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Sat Sep 3 15:41:06 2022 +0200

    [PR/middle-end 106819] NANs can never be a singleton

    Possible NANs can never be a singleton, so they will never be
    propagated.  This was the intent, and then the signed zero code crept
    in, and was mistakenly checked before the NAN.

[Bug target/106831] [13 Regression] mpfr-4.1.0 started failing 2 tests: tget_set_d64 and tget_set_d128

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106831

--- Comment #2 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
And yes, I've started testing mpfr now for my frange patches.

[Bug target/106831] [13 Regression] mpfr-4.1.0 started failing 2 tests: tget_set_d64 and tget_set_d128

[slyfox at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106831

--- Comment #3 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
Hm, mpfr-4.1.0 still fails these 2 tests for me from current gcc-master.

Comparing -O2 build from last week and this week snapshot code generation
changed seemingly only for `mpfr_get_decimal64()` at src/.libs/get_d64.o.

I'll re-check current gcc-master, extract self-contained example from
mpfr-4.1.0 and reopen the bug if I succeed.

[Bug target/106831] [13 Regression] mpfr-4.1.0 started failing 2 tests: tget_set_d64 and tget_set_d128

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106831

Aldy Hernandez <aldyh at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-09-05
             Status|RESOLVED                    |NEW
         Resolution|FIXED                       |---
     Ever confirmed|0                           |1

--- Comment #4 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
reproduced on x86-64.  Will take a look.  Sorry for closing it prematurely.

[Bug target/106831] [13 Regression] mpfr-4.1.0 started failing 2 tests: tget_set_d64 and tget_set_d128

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106831

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
BTW, I admit I don't know much about decimal{32,64,128}, but
https://en.wikipedia.org/wiki/Decimal32_floating-point_format
says:
Because the significand is not normalized (there is no implicit leading "1"),
most values with less than 7 significant digits have multiple possible
representations; 1 × 10^2=0.1 × 10^3=0.01 × 10^4, etc. Zero has 192 possible
representations (384 when both signed zeros are included).
So, I think singleton_p should just always return false for
DECIMAL_FLOAT_TYPE_P (at least for now).

[Bug target/106831] [13 Regression] mpfr-4.1.0 started failing 2 tests: tget_set_d64 and tget_set_d128

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106831

--- Comment #6 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #5)
> BTW, I admit I don't know much about decimal{32,64,128}, but
> https://en.wikipedia.org/wiki/Decimal32_floating-point_format
> says:
> Because the significand is not normalized (there is no implicit leading
> "1"), most values with less than 7 significant digits have multiple possible
> representations; 1 × 10^2=0.1 × 10^3=0.01 × 10^4, etc. Zero has 192 possible
> representations (384 when both signed zeros are included).
> So, I think singleton_p should just always return false for
> DECIMAL_FLOAT_TYPE_P (at least for now).

Interestingly, frange_drop_*inf() bails on DECIMAL_FLOAT_MODE_P because
build_real() will ultimately ICE when trying to make a tree out of the max/min
representable number for a type:

  /* dconst{0,1,2,m1,half} are used in various places in
     the middle-end and optimizers, allow them here
     even for decimal floating point types as an exception
     by converting them to decimal.  */
  if (DECIMAL_FLOAT_MODE_P (TYPE_MODE (type))
      && (d.cl == rvc_normal || d.cl == rvc_zero)
      && !d.decimal)
...
...

I know even less about decimal floats.  Jakub, should we disable them
altogether from the frange infrastructure, or is that too big of a hammer?  I'm
just afraid we'll keep running into limitations when we start implementing
floating operations in range-op-float.cc.  Or worse, have to special case them
all over.

bool frange::supports_p (const_tree type) 
{ 
  return SCALAR_FLOAT_TYPE_P (type) && !DECIMAL_FLOAT_MODE_P (type);
}

??

[Bug middle-end/106831] [13 Regression] mpfr-4.1.0 started failing 2 tests: tget_set_d64 and tget_set_d128

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106831

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I guess disabling them at least for now could be fine.
If somebody involved with dfp wants to extend it for dfp, it can be done
incrementally.

BTW, thinking further about the singletons, I wonder if MODE_COMPOSITE_P
don't have similar problems.  For the case where the values are exactly equal
to their MSB (i.e. the __ibm128 value is equal to that value cast to double and
back), then the low part can be either +0.0 or -0.0.  Ditto for +Inf and -Inf.
So there can be quite a few non-singleton values.
libgcc/config/rs6000/ibm-ldouble-format describes it.

[Bug middle-end/106831] [13 Regression] mpfr-4.1.0 started failing 2 tests: tget_set_d64 and tget_set_d128

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106831

--- Comment #8 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #7)
> I guess disabling them at least for now could be fine.
> If somebody involved with dfp wants to extend it for dfp, it can be done
> incrementally.
> 
> BTW, thinking further about the singletons, I wonder if MODE_COMPOSITE_P
> don't have similar problems.  For the case where the values are exactly equal
> to their MSB (i.e. the __ibm128 value is equal to that value cast to double
> and back), then the low part can be either +0.0 or -0.0.  Ditto for +Inf and
> -Inf.
> So there can be quite a few non-singleton values.
> libgcc/config/rs6000/ibm-ldouble-format describes it.

Eeech, could you come up with a patch for that one?  Consider it preapproved
:-/.  I'm afraid to mess it up.

In the meantime I think I'll disable decimal floats for frange.

[Bug middle-end/106831] [13 Regression] mpfr-4.1.0 started failing 2 tests: tget_set_d64 and tget_set_d128

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106831

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Aldy Hernandez from comment #8)
> (In reply to Jakub Jelinek from comment #7)
> > I guess disabling them at least for now could be fine.
> > If somebody involved with dfp wants to extend it for dfp, it can be done
> > incrementally.
> > 
> > BTW, thinking further about the singletons, I wonder if MODE_COMPOSITE_P
> > don't have similar problems.  For the case where the values are exactly equal
> > to their MSB (i.e. the __ibm128 value is equal to that value cast to double
> > and back), then the low part can be either +0.0 or -0.0.  Ditto for +Inf and
> > -Inf.
> > So there can be quite a few non-singleton values.
> > libgcc/config/rs6000/ibm-ldouble-format describes it.
> 
> Eeech, could you come up with a patch for that one?  Consider it preapproved
> :-/.  I'm afraid to mess it up.

I think it would be:
--- gcc/value-range.cc.jj       2022-09-05 16:50:51.443419082 +0200
+++ gcc/value-range.cc  2022-09-05 16:52:04.880434594 +0200
@@ -639,6 +639,19 @@ frange::singleton_p (tree *result) const
       if (HONOR_NANS (m_type) && !get_nan ().no_p ())
        return false;

+      if (MODE_COMPOSITE_P (TYPE_MODE (m_type)))
+       {
+         // In IBM extended format, if value is +-Inf or
+         // is exactly representable in double, the other
+         // double could be +0.0 or -0.0.
+         if (real_isinf (&m_min))
+           return false;
+         REAL_VALUE_TYPE v;
+         real_convert (&v, DFmode, &m_min);
+         if (real_identical (&v, &m_min))
+           return false;
+       }
+
       // Return the appropriate zero if known.
       if (HONOR_SIGNED_ZEROS (m_type) && zero_p ())
        {
It changes when
long double foo (long double x)
{
  if (x != 25.0L)
    return 16.0L;
  return x + 17.0L;
}
is simplified from evrp to dom2, but perhaps it is ok doing it in dom2 because
the result of the addition would at runtime go through libgcc which would most
likely use 0.0 as the other half.
But perhaps it could matter in other cases.

[Bug middle-end/106831] [13 Regression] mpfr-4.1.0 started failing 2 tests: tget_set_d64 and tget_set_d128

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106831

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Aldy Hernandez <aldyh@gcc.gnu.org>:

https://gcc.gnu.org/g:b4d8a56a4c62ba8bca55469ae2b841fb4e1334a4

commit r13-2445-gb4d8a56a4c62ba8bca55469ae2b841fb4e1334a4
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Mon Sep 5 15:41:39 2022 +0200

    Disable decimal floating point in frange.

    As Jakub mentioned in the PR, because many numbers have multiple
    possible representations, we can't reliably return true for singleton_p.
    For that matter, we may not be capable of modeling them just yet.
    Disabling them until someone with DFP knowledge can opine or extend
    frange.

            PR middle-end/106831

    gcc/ChangeLog:

            * value-range.h (frange::supports_p): Disable decimal floats.
            * range-op-float.cc (frange_drop_inf): Remove DECIMAL_FLOAT_MODE_P
            check.
            (frange_drop_ninf): Same.

[Bug middle-end/106831] [13 Regression] mpfr-4.1.0 started failing 2 tests: tget_set_d64 and tget_set_d128

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106831

Aldy Hernandez <aldyh at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #11 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
Fixed in trunk.  Verified by doing a make check-mpfr with in-tree libmpfr
before and after patch.  Thanks Jakub.

[Bug middle-end/106831] [13 Regression] mpfr-4.1.0 started failing 2 tests: tget_set_d64 and tget_set_d128

[joseph at codesourcery dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106831

--- Comment #12 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
The difference with __ibm128 is that in that case there is no semantic 
significance to whether the low part is +0 or -0, or what the low part is 
at all when the high part is a NaN.  At the C level, such __ibm128 
representations should be considered different representations of the same 
value, not different values.  Whereas different DFP quantum exponents for 
the same real number correspond to different values that compare equal.  
(Noncanonical DFP encodings might be more closely analogous to the 
__ibm128 variants, except that most operations aren't supposed to return a 
noncanonical encoding even if inputs have such an encoding.)

[Bug middle-end/106831] [13 Regression] mpfr-4.1.0 started failing 2 tests: tget_set_d64 and tget_set_d128

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106831

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Aldy Hernandez <aldyh@gcc.gnu.org>:

https://gcc.gnu.org/g:5dba8b2a91376d0518eb21c69a0700f099aa9cc4

commit r13-2714-g5dba8b2a91376d0518eb21c69a0700f099aa9cc4
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Sep 17 08:50:22 2022 +0200

    [PR106831] Avoid propagating long doubles that may have multiple
representations.

    Long doubles are tricky when it comes to considering singletons
    because small numbers and +-INF can have multiple representations for
    the same number.  So we need to be very careful not to treat those as
    singletons, lest they be incorrectly propagated by VRP.  This is
    similar to the -0.0 and +0.0 duality.

    In long doubles +INF can be represented with +INF in the MSB and
    either -0.0 or +0.0 in the LSB.  Similarly for numbers that are exactly
    representable in DF.  For example, 1.0 can be represented as either
    (1.0, +0.0) or (1.0, -0.0).

    This patch avoids treating these numbers as singletons.

    Note that NANs in long double format have a LSB of don't care, but
    this is irrelevant for singleton_p, because NANs are never considered
    singletons.  Also, internally in the frange we store NANs as a pair of
    boolean flags indicating whether they are +NAN or -NAN, so we don't need
    any special treatment here for comparing range equality etc.  We never
    see anything but the boolean flags.

            PR middle-end/106831

    gcc/ChangeLog:

            * value-range.cc (frange::singleton_p): Avoid propagating long
            doubles that may have multiple representations.