[Bug c++/109359] New: Compile-time rounding of double literal to float is incorrect with -frounding-math

[Bug c++/109359] New: Compile-time rounding of double literal to float is incorrect with -frounding-math

[rcopley at gmail dot com]

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

            Bug ID: 109359
           Summary: Compile-time rounding of double literal to float is
                    incorrect with -frounding-math
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rcopley at gmail dot com
  Target Milestone: ---

The following program should print:
0.001914
0.630538

With "g++ -frounding-math", it prints instead:
-8023756970655744.000000
0.872496

This bug is present in trunk, and since gcc 12.1, and does not appear to be
platform specific.

Compiler explorer link: https://godbolt.org/z/aMhcYcY66



#include <cstdio>
float xs[] = {0.001914, 0.630539};
int main() {
  std::size_t size = sizeof xs / sizeof xs[0];
  for (std::size_t i = 0; i != size; ++i) {
    std::printf("%f\n", xs[i]);
  }
}

[Bug c++/109359] [12/13 Regression] Compile-time rounding of double literal to float is incorrect with -frounding-math

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2023-03-31
   Target Milestone|---                         |12.3
            Summary|Compile-time rounding of    |[12/13 Regression]
                   |double literal to float is  |Compile-time rounding of
                   |incorrect with              |double literal to float is
                   |-frounding-math             |incorrect with
                   |                            |-frounding-math
           Keywords|                            |needs-bisection, wrong-code
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
With -frounding-math:

xs:
        .long   -639366012
        .long   1063214053
        .long   536561674
        .long   1071918432

without:

xs:
        .long   989519663
        .long   1059154689

it looks like we fail to convert the double constant to single precision
and then end up outputting the double precision constants ...

The C frontend works.

[Bug c++/109359] [12/13 Regression] Compile-time rounding of double literal to float is incorrect with -frounding-math

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org
           Keywords|needs-bisection             |

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r12-4764-ga84b9d5373c7e67fd0ab2a

[Bug c++/109359] [12/13 Regression] Compile-time rounding of double literal to float is incorrect with -frounding-math

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2
                 CC|                            |jason at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think the fold-const.cc change is right though.
I wonder if for constant evaluation (constexpr, constinit) we shouldn't arrange
for those to be evaluated with temporarily -fno-rounding-math, I think C uses
fold_init and its START_FOLD_INIT ... END_FOLD_INIT for this purpose..
And otherwise perhaps we want dynamic initialization and do the conversion at
runtime?
Or disable the -frounding-math for all initializer folding?
What we emit is definitely wrong,
Variable which claims to have 8 bytes in size but actually has 16 under the
hood, with constants in different mode.

[Bug c++/109359] [12/13 Regression] Compile-time rounding of double literal to float is incorrect with -frounding-math

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)
> I think the fold-const.cc change is right though.
> I wonder if for constant evaluation (constexpr, constinit) we shouldn't
> arrange for those to be evaluated with temporarily -fno-rounding-math, I
> think C uses
> fold_init and its START_FOLD_INIT ... END_FOLD_INIT for this purpose..
> And otherwise perhaps we want dynamic initialization and do the conversion
> at runtime?
> Or disable the -frounding-math for all initializer folding?
> What we emit is definitely wrong,
> Variable which claims to have 8 bytes in size but actually has 16 under the
> hood, with constants in different mode.

We should have ICEd emitting the constant.  And yes, I think -frounding-math
should be disabled for constinit initializer folding (and possibly whether
it is const or not should not depend on -frounding-math).

[Bug c++/109359] [12/13 Regression] Compile-time rounding of double literal to float is incorrect with -frounding-math

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Ah, cp/constexpr.cc already uses fold_binary_initializer_loc if
-fconstexpr-fp-except.
That will turn the -frounding-math temporarily off for binary operations.
For this PR guess we need to use fold_init or fold_build1_initializer_loc
instead of fold or fold_build1 in the NOP_EXPR handling under the same
conditions.
Plus of course we need to figure out how to fix the issue if we didn't turn
those off (we shouldn't consider the initializer constant then but evaluate
dynamically).

[Bug c++/109359] [12/13 Regression] Compile-time rounding of double literal to float is incorrect with -frounding-math

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
output_constant gets called with

{(float) 1.91399999999999990279997419406754488591104745864868164062e-3, (float)
6.305389999999999606217215841752476990222930908203125e-1}

it then eventually does

  /* Eliminate any conversions since we'll be outputting the underlying
     constant.  */
  while (CONVERT_EXPR_P (exp)
         || TREE_CODE (exp) == NON_LVALUE_EXPR
         || TREE_CODE (exp) == VIEW_CONVERT_EXPR)
    {
      HOST_WIDE_INT type_size = int_size_in_bytes (TREE_TYPE (exp));
      HOST_WIDE_INT op_size = int_size_in_bytes (TREE_TYPE (TREE_OPERAND (exp,
0)));

      /* Make sure eliminating the conversion is really a no-op, except with
         VIEW_CONVERT_EXPRs to allow for wild Ada unchecked conversions and
         union types to allow for Ada unchecked unions.  */
      if (type_size > op_size
          && TREE_CODE (exp) != VIEW_CONVERT_EXPR
          && TREE_CODE (TREE_TYPE (exp)) != UNION_TYPE)
        /* Keep the conversion. */
        break;
      else
        exp = TREE_OPERAND (exp, 0);
    }

where we strip conversions with type_size < op_size (aka float from double).
For float conversions not sure if just keying on type size is good enough
though (ibm double double vs long double 128 for example).

Fixing that "improves" the behavior to

t.ii:1:34: error: initializer for floating value is not a floating constant
    1 | float xs[] = {0.001914, 0.630539};
      |                                  ^
t.ii:1:34: error: initializer for floating value is not a floating constant

aka from wrong-code to rejects-valid.

diff --git a/gcc/varasm.cc b/gcc/varasm.cc
index cd0cd88321c..e6ab581dc5f 100644
--- a/gcc/varasm.cc
+++ b/gcc/varasm.cc
@@ -5202,7 +5202,7 @@ output_constant (tree exp, unsigned HOST_WIDE_INT size,
unsigned int align,
       /* Make sure eliminating the conversion is really a no-op, except with
         VIEW_CONVERT_EXPRs to allow for wild Ada unchecked conversions and
         union types to allow for Ada unchecked unions.  */
-      if (type_size > op_size
+      if (type_size != op_size
          && TREE_CODE (exp) != VIEW_CONVERT_EXPR
          && TREE_CODE (TREE_TYPE (exp)) != UNION_TYPE)
        /* Keep the conversion. */

note that for integral and pointer types we do

      cst = expand_expr (exp, NULL_RTX, VOIDmode, EXPAND_INITIALIZER);
      if (reverse)
        cst = flip_storage_order (TYPE_MODE (TREE_TYPE (exp)), cst);
      if (!assemble_integer (cst, MIN (size, thissize), align, 0))
        error ("initializer for integer/fixed-point value is too complicated");

so we handle "narrowing" in a weird way.  So in case FEs leave around
nop-casts the following should be safer

diff --git a/gcc/varasm.cc b/gcc/varasm.cc
index cd0cd88321c..81f7288449c 100644
--- a/gcc/varasm.cc
+++ b/gcc/varasm.cc
@@ -5196,13 +5196,17 @@ output_constant (tree exp, unsigned HOST_WIDE_INT size,
unsigned int align,
         || TREE_CODE (exp) == NON_LVALUE_EXPR
         || TREE_CODE (exp) == VIEW_CONVERT_EXPR)
     {
-      HOST_WIDE_INT type_size = int_size_in_bytes (TREE_TYPE (exp));
-      HOST_WIDE_INT op_size = int_size_in_bytes (TREE_TYPE (TREE_OPERAND (exp,
0)));
+      tree type = TREE_TYPE (exp);
+      tree op_type = TREE_TYPE (TREE_OPERAND (exp, 0));
+      HOST_WIDE_INT type_size = int_size_in_bytes (type);
+      HOST_WIDE_INT op_size = int_size_in_bytes (op_type);

       /* Make sure eliminating the conversion is really a no-op, except with
         VIEW_CONVERT_EXPRs to allow for wild Ada unchecked conversions and
         union types to allow for Ada unchecked unions.  */
-      if (type_size > op_size
+      if ((type_size > op_size
+          || (TYPE_MAIN_VARIANT (type) != TYPE_MAIN_VARIANT (op_type)
+              && FLOAT_TYPE_P (type)))
          && TREE_CODE (exp) != VIEW_CONVERT_EXPR
          && TREE_CODE (TREE_TYPE (exp)) != UNION_TYPE)
        /* Keep the conversion. */

The real fix is of course in the frontend, the above is just a safety net.

[Bug c++/109359] [12/13 Regression] Compile-time rounding of double literal to float is incorrect with -frounding-math

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
For the safety net I'd compare TYPE_MODE of the SCALAR_FLOAT_TYPE_Ps, that is
what matters for those whether it is a noop conversion or needs actually some
runtime adjustment.

[Bug c++/109359] [12/13/14 Regression] Compile-time rounding of double literal to float is incorrect with -frounding-math

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|12.3                        |12.4

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 12.3 is being released, retargeting bugs to GCC 12.4.

[Bug c++/109359] [12/13/14 Regression] Compile-time rounding of double literal to float is incorrect with -frounding-math

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |jason at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

[Bug c++/109359] [12/13 Regression] Compile-time rounding of double literal to float is incorrect with -frounding-math

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|jason at gcc dot gnu.org           |unassigned at gcc dot gnu.org
             Status|ASSIGNED                    |NEW
            Summary|[12/13/14 Regression]       |[12/13 Regression]
                   |Compile-time rounding of    |Compile-time rounding of
                   |double literal to float is  |double literal to float is
                   |incorrect with              |incorrect with
                   |-frounding-math             |-frounding-math

--- Comment #9 from Jason Merrill <jason at gcc dot gnu.org> ---
This seems to be fixed on trunk.  Jakub, can you bisect what fixed it?

[Bug c++/109359] [12/13 Regression] Compile-time rounding of double literal to float is incorrect with -frounding-math

[jakub at gcc dot gnu.org]

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

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
r14-1498-ge7cc4d703bceb9095316c106eba0d1939c6c8044

[Bug c++/109359] [12/13 Regression] Compile-time rounding of double literal to float is incorrect with -frounding-math

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |jason at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

[Bug c++/109359] [12/13 Regression] Compile-time rounding of double literal to float is incorrect with -frounding-math

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

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

commit r14-8813-gd49780c08aade447953bfe4e877d386f5757f165
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Feb 5 15:59:45 2024 -0500

    c++: -frounding-math test [PR109359]

    This test was fixed by the patch for PR95226, but that patch had no
    testcase so let's add this one.

            PR c++/109359

    gcc/testsuite/ChangeLog:

            * g++.dg/ext/frounding-math1.C: New test.

[Bug c++/109359] [12/13 Regression] Compile-time rounding of double literal to float is incorrect with -frounding-math

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Jason Merrill
<jason@gcc.gnu.org>:

https://gcc.gnu.org/g:470f501f31a4bdb9fa04c691ca7db2915ac3ae5b

commit r12-10136-g470f501f31a4bdb9fa04c691ca7db2915ac3ae5b
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Jun 1 14:41:07 2023 -0400

    varasm: check float size [PR109359]

    In PR95226, the testcase was failing because we tried to output_constant a
    NOP_EXPR to float from a double REAL_CST, and so we output a double where
    the caller wanted a float.  That doesn't happen anymore, but with the
    output_constant hunk we will ICE in that situation rather than emit the
    wrong number of bytes.

    Part of the problem was that initializer_constant_valid_p_1 returned true
    for that NOP_EXPR, because it compared the sizes of integer types but not
    floating-point types.  So the C++ front end assumed it didn't need to fold
    the initializer.

    This also fixed the test for PR109359.

            PR c++/95226
            PR c++/109359

    gcc/ChangeLog:

            * varasm.cc (output_constant) [REAL_TYPE]: Check that sizes match.
            (initializer_constant_valid_p_1): Compare float precision.

    gcc/testsuite/ChangeLog:

            * g++.dg/ext/frounding-math1.C: New test.

    (cherry picked from commit e7cc4d703bceb9095316c106eba0d1939c6c8044)

[Bug c++/109359] [12/13 Regression] Compile-time rounding of double literal to float is incorrect with -frounding-math

[cvs-commit at gcc dot gnu.org]

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

--- Comment #13 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Jason Merrill
<jason@gcc.gnu.org>:

https://gcc.gnu.org/g:9b8e82ab45d1ad976a824cfd7c9bd2640c8bc8e3

commit r13-8282-g9b8e82ab45d1ad976a824cfd7c9bd2640c8bc8e3
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Jun 1 14:41:07 2023 -0400

    varasm: check float size [PR109359]

    In PR95226, the testcase was failing because we tried to output_constant a
    NOP_EXPR to float from a double REAL_CST, and so we output a double where
    the caller wanted a float.  That doesn't happen anymore, but with the
    output_constant hunk we will ICE in that situation rather than emit the
    wrong number of bytes.

    Part of the problem was that initializer_constant_valid_p_1 returned true
    for that NOP_EXPR, because it compared the sizes of integer types but not
    floating-point types.  So the C++ front end assumed it didn't need to fold
    the initializer.

    This also fixed the test for PR109359.

            PR c++/95226
            PR c++/109359

    gcc/ChangeLog:

            * varasm.cc (output_constant) [REAL_TYPE]: Check that sizes match.
            (initializer_constant_valid_p_1): Compare float precision.

    gcc/testsuite/ChangeLog:

            * g++.dg/ext/frounding-math1.C: New test.

    (cherry picked from commit e7cc4d703bceb9095316c106eba0d1939c6c8044)

[Bug c++/109359] [12/13 Regression] Compile-time rounding of double literal to float is incorrect with -frounding-math

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

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

--- Comment #14 from Jason Merrill <jason at gcc dot gnu.org> ---
Fixed for 12.4/13.3/14.0.