public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
* Fix Fortran rounding issues, PR fortran/96983.
@ 2021-04-19 18:55 Michael Meissner
  2021-04-20  6:58 ` Richard Biener
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Meissner @ 2021-04-19 18:55 UTC (permalink / raw)
  To: gcc-patches, Michael Meissner, Segher Boessenkool,
	David Edelsohn, Bill Schmidt, Peter Bergner, Will Schmidt,
	fortran, Janne Blomqvist, Tobias Burnus,
	François-Xavier Coudert, Jerry DeLisle, Erik Edelmann,
	Daniel Franke, Thomas König, Daniel Kraft, Toon Moene,
	Mikael Morin, Tobias Schlüter, Paul Thomas, Janus Weil

Fix Fortran rounding issues, PR fortran/96983.

I was looking at Fortran PR 96983, which fails on the PowerPC when trying to
run the test PR96711.F90.  The compiler ICEs because the PowerPC does not have
a floating point type with a type precision of 128.  The reason is that the
PowerPC has 3 different 128 bit floating point types (__float128/_Float128,
__ibm128, and long double).  Currently long double uses the IBM extended double
type, but we would like to switch to using IEEE 128-bit long doubles in the
future.

In order to prevent the compiler from converting explicit __ibm128 types to
long double when long double uses the IEEE 128-bit representation, we have set
up the precision for __ibm128 to be 128, long double to be 127, and
__float128/_Float128 to be 126.

Originally, I was trying to see if for Fortran, I could change the precision of
long double to be 128 (Fortran doesn't access __ibm128), but it quickly became
hard to get the changes to work.

I looked at the Fortran code in build_round_expr, and I came to the conclusion
that there is no reason to promote the floating point type.  If you just do a
normal round of the value using the current floating point format and then
convert it to the integer type.  We don't have an appropriate built-in function
that provides the equivalent of llround for 128-bit integer types.

This patch fixes the compiler crash.

However, while with this patch, the PowerPC compiler will not crash when
building the test case, it will not run on the current default installation.
The failure is because the test is explicitly expecting 128-bit floating point
to handle 10384593717069655257060992658440192_16 (i.e. 2**113).

By default, the PowerPC uses IBM extended double used for 128-bit floating
point.  The IBM extended double format is a pair of doubles that provides more
mantissa bits but does not grow the expoenent range.  The value in the test is
fine for IEEE 128-bit floating point, but it is too large for the PowerPC
extended double setup.

I have built the following tests with this patch:

   * I have built a bootstrap compiler on a little endian power9 Linux system
     with the default long double format (IBM extended double).  The
     pr96711.f90 test builds, but it does not run due to the range of the
     real*16 exponent.  There were no other regressions in the C/C++/Fortran
     tests.

   * I have built a bootstrap compiler on a little endian power9 Linux system
     with the default long double format set to IEEE 128-bit. I used the
     Advance Toolchain 14.0-2 to provide the IEEE 128-bits.  The compiler was
     configured to build power9 code by default, so the test generated native
     power9 IEEE 128-bit instructions.  The pr96711.f90 test builds and runs
     correctly in this setup.

   * I have built a bootstrap compiler on a big endian power8 Linux system with
     the default long double format (IBM extended double).  Like the first
     case, the pr96711.f90 test does not crash the compiler, but the test fails
     due to the range of the real*16 exponent.    There were no other
     regressions in the C/C++/Fortran tests.

   * I built a bootstrap compiler on my x86_64 laptop.  There were no
     regressions in the tests.


Can I check this change into the GCC trunk?

I've not contributed to the Fortran front end before.  If the maintainers like
the patch, can somebody point out if I need to do additional things to commit
the patch?

gcc/fortran/
2021-04-19  Michael Meissner  <meissner@linux.ibm.com>

	PR gfortran/96983
	* trans-intrinsic.c (build_round_expr): If int type is larger than
	long long, do the round and convert to the integer type.  Do not
	try to find a floating point type the exact size of the integer
	type.
---
 gcc/fortran/trans-intrinsic.c | 26 ++++++++------------------
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 5e53d1162fa..cceef8f34ac 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -386,30 +386,20 @@ build_round_expr (tree arg, tree restype)
   argprec = TYPE_PRECISION (argtype);
   resprec = TYPE_PRECISION (restype);
 
-  /* Depending on the type of the result, choose the int intrinsic
-     (iround, available only as a builtin, therefore cannot use it for
-     __float128), long int intrinsic (lround family) or long long
-     intrinsic (llround).  We might also need to convert the result
-     afterwards.  */
+  /* Depending on the type of the result, choose the int intrinsic (iround,
+     available only as a builtin, therefore cannot use it for __float128), long
+     int intrinsic (lround family) or long long intrinsic (llround).  If we
+     don't have an appropriate function that converts directly to the integer
+     type (such as kind == 16), just use ROUND, and then convert the result to
+     an integer.  We might also need to convert the result afterwards.  */
   if (resprec <= INT_TYPE_SIZE && argprec <= LONG_DOUBLE_TYPE_SIZE)
     fn = builtin_decl_for_precision (BUILT_IN_IROUND, argprec);
   else if (resprec <= LONG_TYPE_SIZE)
     fn = builtin_decl_for_precision (BUILT_IN_LROUND, argprec);
   else if (resprec <= LONG_LONG_TYPE_SIZE)
     fn = builtin_decl_for_precision (BUILT_IN_LLROUND, argprec);
-  else if (resprec >= argprec && resprec == 128)
-    {
-      /* Search for a real kind suitable as temporary for conversion.  */
-      int kind = -1;
-      for (int i = 0; kind < 0 && gfc_real_kinds[i].kind != 0; i++)
-	if (gfc_real_kinds[i].mode_precision >= resprec)
-	  kind = gfc_real_kinds[i].kind;
-      if (kind < 0)
-	gfc_internal_error ("Could not find real kind with at least %d bits",
-			    resprec);
-      arg = fold_convert (gfc_get_real_type (kind), arg);
-      fn = gfc_builtin_decl_for_float_kind (BUILT_IN_ROUND, kind);
-    }
+  else if (resprec >= argprec)
+    fn = builtin_decl_for_precision (BUILT_IN_ROUND, argprec);
   else
     gcc_unreachable ();
 
-- 
2.22.0


-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Fix Fortran rounding issues, PR fortran/96983.
  2021-04-19 18:55 Fix Fortran rounding issues, PR fortran/96983 Michael Meissner
@ 2021-04-20  6:58 ` Richard Biener
  2021-04-21  8:10   ` Tobias Burnus
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Biener @ 2021-04-20  6:58 UTC (permalink / raw)
  To: Michael Meissner, GCC Patches, Segher Boessenkool,
	David Edelsohn, Bill Schmidt, Peter Bergner, Will Schmidt,
	fortran, Janne Blomqvist, Tobias Burnus,
	François-Xavier Coudert, Jerry DeLisle, Erik Edelmann,
	Daniel Franke, Thomas König, Daniel Kraft, Toon Moene,
	Mikael Morin, Tobias Schlüter, Paul Thomas, Janus Weil

On Mon, Apr 19, 2021 at 9:40 PM Michael Meissner via Fortran
<fortran@gcc.gnu.org> wrote:
>
> Fix Fortran rounding issues, PR fortran/96983.
>
> I was looking at Fortran PR 96983, which fails on the PowerPC when trying to
> run the test PR96711.F90.  The compiler ICEs because the PowerPC does not have
> a floating point type with a type precision of 128.  The reason is that the
> PowerPC has 3 different 128 bit floating point types (__float128/_Float128,
> __ibm128, and long double).  Currently long double uses the IBM extended double
> type, but we would like to switch to using IEEE 128-bit long doubles in the
> future.
>
> In order to prevent the compiler from converting explicit __ibm128 types to
> long double when long double uses the IEEE 128-bit representation, we have set
> up the precision for __ibm128 to be 128, long double to be 127, and
> __float128/_Float128 to be 126.
>
> Originally, I was trying to see if for Fortran, I could change the precision of
> long double to be 128 (Fortran doesn't access __ibm128), but it quickly became
> hard to get the changes to work.
>
> I looked at the Fortran code in build_round_expr, and I came to the conclusion
> that there is no reason to promote the floating point type.  If you just do a
> normal round of the value using the current floating point format and then
> convert it to the integer type.  We don't have an appropriate built-in function
> that provides the equivalent of llround for 128-bit integer types.
>
> This patch fixes the compiler crash.
>
> However, while with this patch, the PowerPC compiler will not crash when
> building the test case, it will not run on the current default installation.
> The failure is because the test is explicitly expecting 128-bit floating point
> to handle 10384593717069655257060992658440192_16 (i.e. 2**113).
>
> By default, the PowerPC uses IBM extended double used for 128-bit floating
> point.  The IBM extended double format is a pair of doubles that provides more
> mantissa bits but does not grow the expoenent range.  The value in the test is
> fine for IEEE 128-bit floating point, but it is too large for the PowerPC
> extended double setup.
>
> I have built the following tests with this patch:
>
>    * I have built a bootstrap compiler on a little endian power9 Linux system
>      with the default long double format (IBM extended double).  The
>      pr96711.f90 test builds, but it does not run due to the range of the
>      real*16 exponent.  There were no other regressions in the C/C++/Fortran
>      tests.
>
>    * I have built a bootstrap compiler on a little endian power9 Linux system
>      with the default long double format set to IEEE 128-bit. I used the
>      Advance Toolchain 14.0-2 to provide the IEEE 128-bits.  The compiler was
>      configured to build power9 code by default, so the test generated native
>      power9 IEEE 128-bit instructions.  The pr96711.f90 test builds and runs
>      correctly in this setup.
>
>    * I have built a bootstrap compiler on a big endian power8 Linux system with
>      the default long double format (IBM extended double).  Like the first
>      case, the pr96711.f90 test does not crash the compiler, but the test fails
>      due to the range of the real*16 exponent.    There were no other
>      regressions in the C/C++/Fortran tests.
>
>    * I built a bootstrap compiler on my x86_64 laptop.  There were no
>      regressions in the tests.
>
>
> Can I check this change into the GCC trunk?

The patch looks fine technically and is definitely an improvement since the
intermediate conversion looks odd.  But it might be that the standard
requires such dance though the preceeding cases handled don't seem to
care.  I'm thinking of a FP format where round(1.6) == 3 because of lack
of precision but using an intermediate FP format with higher precision
would "correctly" compute 2.

Of course the current code doesn't handle this correctly for the
case if llroundf either.

Richard.

> I've not contributed to the Fortran front end before.  If the maintainers like
> the patch, can somebody point out if I need to do additional things to commit
> the patch?
>
> gcc/fortran/
> 2021-04-19  Michael Meissner  <meissner@linux.ibm.com>
>
>         PR gfortran/96983
>         * trans-intrinsic.c (build_round_expr): If int type is larger than
>         long long, do the round and convert to the integer type.  Do not
>         try to find a floating point type the exact size of the integer
>         type.
> ---
>  gcc/fortran/trans-intrinsic.c | 26 ++++++++------------------
>  1 file changed, 8 insertions(+), 18 deletions(-)
>
> diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
> index 5e53d1162fa..cceef8f34ac 100644
> --- a/gcc/fortran/trans-intrinsic.c
> +++ b/gcc/fortran/trans-intrinsic.c
> @@ -386,30 +386,20 @@ build_round_expr (tree arg, tree restype)
>    argprec = TYPE_PRECISION (argtype);
>    resprec = TYPE_PRECISION (restype);
>
> -  /* Depending on the type of the result, choose the int intrinsic
> -     (iround, available only as a builtin, therefore cannot use it for
> -     __float128), long int intrinsic (lround family) or long long
> -     intrinsic (llround).  We might also need to convert the result
> -     afterwards.  */
> +  /* Depending on the type of the result, choose the int intrinsic (iround,
> +     available only as a builtin, therefore cannot use it for __float128), long
> +     int intrinsic (lround family) or long long intrinsic (llround).  If we
> +     don't have an appropriate function that converts directly to the integer
> +     type (such as kind == 16), just use ROUND, and then convert the result to
> +     an integer.  We might also need to convert the result afterwards.  */
>    if (resprec <= INT_TYPE_SIZE && argprec <= LONG_DOUBLE_TYPE_SIZE)
>      fn = builtin_decl_for_precision (BUILT_IN_IROUND, argprec);
>    else if (resprec <= LONG_TYPE_SIZE)
>      fn = builtin_decl_for_precision (BUILT_IN_LROUND, argprec);
>    else if (resprec <= LONG_LONG_TYPE_SIZE)
>      fn = builtin_decl_for_precision (BUILT_IN_LLROUND, argprec);
> -  else if (resprec >= argprec && resprec == 128)
> -    {
> -      /* Search for a real kind suitable as temporary for conversion.  */
> -      int kind = -1;
> -      for (int i = 0; kind < 0 && gfc_real_kinds[i].kind != 0; i++)
> -       if (gfc_real_kinds[i].mode_precision >= resprec)
> -         kind = gfc_real_kinds[i].kind;
> -      if (kind < 0)
> -       gfc_internal_error ("Could not find real kind with at least %d bits",
> -                           resprec);
> -      arg = fold_convert (gfc_get_real_type (kind), arg);
> -      fn = gfc_builtin_decl_for_float_kind (BUILT_IN_ROUND, kind);
> -    }
> +  else if (resprec >= argprec)
> +    fn = builtin_decl_for_precision (BUILT_IN_ROUND, argprec);
>    else
>      gcc_unreachable ();
>
> --
> 2.22.0
>
>
> --
> Michael Meissner, IBM
> IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
> email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Fix Fortran rounding issues, PR fortran/96983.
  2021-04-20  6:58 ` Richard Biener
@ 2021-04-21  8:10   ` Tobias Burnus
  2021-04-22 19:09     ` Michael Meissner
  0 siblings, 1 reply; 5+ messages in thread
From: Tobias Burnus @ 2021-04-21  8:10 UTC (permalink / raw)
  To: Michael Meissner, GCC Patches, Segher Boessenkool,
	David Edelsohn, Bill Schmidt, Peter Bergner, Will Schmidt,
	fortran

On 20.04.21 08:58, Richard Biener via Fortran wrote:
> On Mon, Apr 19, 2021 at 9:40 PM Michael Meissner via Fortran
> <fortran@gcc.gnu.org>  wrote:
Is there any reason to not only send the email to fortran@ _and_
gcc-patches@ but sending it to 13 Fortran maintainers explicitly? (Now
removed)
>> Fix Fortran rounding issues, PR fortran/96983.
>>
>> Can I check this change into the GCC trunk?
> The patch looks fine technically and is definitely an improvement since the
> intermediate conversion looks odd.  But it might be that the standard
> requires such dance though the preceeding cases handled don't seem to
> care.  I'm thinking of a FP format where round(1.6) == 3 because of lack
> of precision but using an intermediate FP format with higher precision
> would "correctly" compute 2.

The patched build_round_expr is only called by ANINT / NINT;
NINT is real → integer; ANINT is real → real
[And the modified code is only called for NINT, reason: see comment far below.]

NINT (A[, KIND]) is described (F2018) as "Nearest integer":
* Result Characteristics. Integer. If KIND is present, the kind type parameter
   is that specified by the value of KIND;
   otherwise, the kind type parameter is that of default integer type.
* The result is the integer nearest A, or if there are two
   integers equally near A, the result is whichever such integer has the greater
   magnitude.
* Example. NINT (2.783) has the value 3.

ANINT (A[, KIND]) as "Nearest whole number":
* The result is of type real. If KIND is present, the kind type parameter is that
   specified by the value of KIND; otherwise, the kind type parameter is that of A.
* The result is the integer nearest A, or if there are two integers equally near A,
   the result is whichever such integer has the greater magnitude.
* Examples. ANINT (2.783) has the value 3.0. ANINT (−2.783) has the value −3.0.

> Of course the current code doesn't handle this correctly for the
> case if llroundf either.
>> I've not contributed to the Fortran front end before.  If the maintainers like
>> the patch, can somebody point out if I need to do additional things to commit
>> the patch?
Nothing special: a testcase already exists, committing is done as usual
and a PR to update you have as well.
>> gcc/fortran/
>> 2021-04-19  Michael Meissner<meissner@linux.ibm.com>
>>
>>          PR gfortran/96983
>>          * trans-intrinsic.c (build_round_expr): If int type is larger than
>>          long long, do the round and convert to the integer type.  Do not
>>          try to find a floating point type the exact size of the integer
>>          type.
LGTM. (Minor remark below.)
>> ---
>>   gcc/fortran/trans-intrinsic.c | 26 ++++++++------------------
>>   1 file changed, 8 insertions(+), 18 deletions(-)
>>
>> diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
>> index 5e53d1162fa..cceef8f34ac 100644
>> --- a/gcc/fortran/trans-intrinsic.c
>> +++ b/gcc/fortran/trans-intrinsic.c
>> @@ -386,30 +386,20 @@ build_round_expr (tree arg, tree restype)
>>     argprec = TYPE_PRECISION (argtype);
>>     resprec = TYPE_PRECISION (restype);
>>
>> -  /* Depending on the type of the result, choose the int intrinsic
>> -     (iround, available only as a builtin, therefore cannot use it for
>> -     __float128), long int intrinsic (lround family) or long long
>> -     intrinsic (llround).  We might also need to convert the result
>> -     afterwards.  */
>> +  /* Depending on the type of the result, choose the int intrinsic (iround,
>> +     available only as a builtin, therefore cannot use it for __float128), long
>> +     int intrinsic (lround family) or long long intrinsic (llround).  If we
>> +     don't have an appropriate function that converts directly to the integer
>> +     type (such as kind == 16), just use ROUND, and then convert the result to
>> +     an integer.  We might also need to convert the result afterwards.  */

I think that's fine.

Small comment for completeness:
   "and convert the result to an integer"
Technically, this function can be called for both 'anint' and 'nint'
and for 'anint' it needs a real – and wouldn't convert to an integer.
However, as  gfc_conv_intrinsic_aint  only calls this function when
   gfc_builtin_decl_for_float_kind (BUILT_IN_ROUND, kind);
is NULL_TREE. I think the new code can not be reached – and if, it
would give an ICE as the same call is used.

As effectively the comment is correct, you can leave it as is.

Tobias

>>     if (resprec <= INT_TYPE_SIZE && argprec <= LONG_DOUBLE_TYPE_SIZE)
>>       fn = builtin_decl_for_precision (BUILT_IN_IROUND, argprec);
>>     else if (resprec <= LONG_TYPE_SIZE)
>>       fn = builtin_decl_for_precision (BUILT_IN_LROUND, argprec);
>>     else if (resprec <= LONG_LONG_TYPE_SIZE)
>>       fn = builtin_decl_for_precision (BUILT_IN_LLROUND, argprec);
>> -  else if (resprec >= argprec && resprec == 128)
>> -    {
>> -      /* Search for a real kind suitable as temporary for conversion.  */
>> -      int kind = -1;
>> -      for (int i = 0; kind < 0 && gfc_real_kinds[i].kind != 0; i++)
>> -       if (gfc_real_kinds[i].mode_precision >= resprec)
>> -         kind = gfc_real_kinds[i].kind;
>> -      if (kind < 0)
>> -       gfc_internal_error ("Could not find real kind with at least %d bits",
>> -                           resprec);
>> -      arg = fold_convert (gfc_get_real_type (kind), arg);
>> -      fn = gfc_builtin_decl_for_float_kind (BUILT_IN_ROUND, kind);
>> -    }
>> +  else if (resprec >= argprec)
>> +    fn = builtin_decl_for_precision (BUILT_IN_ROUND, argprec);
>>     else
>>       gcc_unreachable ();
>>
>> --
>> 2.22.0
>>
>>
>> --
>> Michael Meissner, IBM
>> IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
>> email:meissner@linux.ibm.com, phone: +1 (978) 899-4797
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Fix Fortran rounding issues, PR fortran/96983.
  2021-04-21  8:10   ` Tobias Burnus
@ 2021-04-22 19:09     ` Michael Meissner
  2021-04-22 19:21       ` Richard Biener
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Meissner @ 2021-04-22 19:09 UTC (permalink / raw)
  To: Tobias Burnus
  Cc: Michael Meissner, GCC Patches, Segher Boessenkool,
	David Edelsohn, Bill Schmidt, Peter Bergner, Will Schmidt,
	fortran, Richard Biener

On Wed, Apr 21, 2021 at 10:10:07AM +0200, Tobias Burnus wrote:
> On 20.04.21 08:58, Richard Biener via Fortran wrote:
> >On Mon, Apr 19, 2021 at 9:40 PM Michael Meissner via Fortran
> ><fortran@gcc.gnu.org>  wrote:
> Is there any reason to not only send the email to fortran@ _and_
> gcc-patches@ but sending it to 13 Fortran maintainers explicitly? (Now
> removed)

Sorry about that.  With PowerPC backend changes, I generally do explicitly add
the maintainers so things don't get lost.


> >>Fix Fortran rounding issues, PR fortran/96983.
> >>
> >>Can I check this change into the GCC trunk?
> >The patch looks fine technically and is definitely an improvement since the
> >intermediate conversion looks odd.  But it might be that the standard
> >requires such dance though the preceeding cases handled don't seem to
> >care.  I'm thinking of a FP format where round(1.6) == 3 because of lack
> >of precision but using an intermediate FP format with higher precision
> >would "correctly" compute 2.
> 
> The patched build_round_expr is only called by ANINT / NINT;
> NINT is real → integer; ANINT is real → real
> [And the modified code is only called for NINT, reason: see comment far below.]
> 
> NINT (A[, KIND]) is described (F2018) as "Nearest integer":
> * Result Characteristics. Integer. If KIND is present, the kind type parameter
>   is that specified by the value of KIND;
>   otherwise, the kind type parameter is that of default integer type.
> * The result is the integer nearest A, or if there are two
>   integers equally near A, the result is whichever such integer has the greater
>   magnitude.
> * Example. NINT (2.783) has the value 3.
> 
> ANINT (A[, KIND]) as "Nearest whole number":
> * The result is of type real. If KIND is present, the kind type parameter is that
>   specified by the value of KIND; otherwise, the kind type parameter is that of A.
> * The result is the integer nearest A, or if there are two integers equally near A,
>   the result is whichever such integer has the greater magnitude.
> * Examples. ANINT (2.783) has the value 3.0. ANINT (−2.783) has the value −3.0.
> 
> >Of course the current code doesn't handle this correctly for the
> >case if llroundf either.
> >>I've not contributed to the Fortran front end before.  If the maintainers like
> >>the patch, can somebody point out if I need to do additional things to commit
> >>the patch?
> Nothing special: a testcase already exists, committing is done as usual
> and a PR to update you have as well.

Given GCC 11 has branched, is it ok to backport the patch to the GCC 11 branch
as well?  I assume it is, since it fixes a regression in the compiler.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Fix Fortran rounding issues, PR fortran/96983.
  2021-04-22 19:09     ` Michael Meissner
@ 2021-04-22 19:21       ` Richard Biener
  0 siblings, 0 replies; 5+ messages in thread
From: Richard Biener @ 2021-04-22 19:21 UTC (permalink / raw)
  To: Michael Meissner, Tobias Burnus
  Cc: GCC Patches, Segher Boessenkool, David Edelsohn, Bill Schmidt,
	Peter Bergner, Will Schmidt, fortran

On April 22, 2021 9:09:28 PM GMT+02:00, Michael Meissner <meissner@linux.ibm.com> wrote:
>On Wed, Apr 21, 2021 at 10:10:07AM +0200, Tobias Burnus wrote:
>> On 20.04.21 08:58, Richard Biener via Fortran wrote:
>> >On Mon, Apr 19, 2021 at 9:40 PM Michael Meissner via Fortran
>> ><fortran@gcc.gnu.org>  wrote:
>> Is there any reason to not only send the email to fortran@ _and_
>> gcc-patches@ but sending it to 13 Fortran maintainers explicitly?
>(Now
>> removed)
>
>Sorry about that.  With PowerPC backend changes, I generally do
>explicitly add
>the maintainers so things don't get lost.
>
>
>> >>Fix Fortran rounding issues, PR fortran/96983.
>> >>
>> >>Can I check this change into the GCC trunk?
>> >The patch looks fine technically and is definitely an improvement
>since the
>> >intermediate conversion looks odd.  But it might be that the
>standard
>> >requires such dance though the preceeding cases handled don't seem
>to
>> >care.  I'm thinking of a FP format where round(1.6) == 3 because of
>lack
>> >of precision but using an intermediate FP format with higher
>precision
>> >would "correctly" compute 2.
>> 
>> The patched build_round_expr is only called by ANINT / NINT;
>> NINT is real → integer; ANINT is real → real
>> [And the modified code is only called for NINT, reason: see comment
>far below.]
>> 
>> NINT (A[, KIND]) is described (F2018) as "Nearest integer":
>> * Result Characteristics. Integer. If KIND is present, the kind type
>parameter
>>   is that specified by the value of KIND;
>>   otherwise, the kind type parameter is that of default integer type.
>> * The result is the integer nearest A, or if there are two
>>   integers equally near A, the result is whichever such integer has
>the greater
>>   magnitude.
>> * Example. NINT (2.783) has the value 3.
>> 
>> ANINT (A[, KIND]) as "Nearest whole number":
>> * The result is of type real. If KIND is present, the kind type
>parameter is that
>>   specified by the value of KIND; otherwise, the kind type parameter
>is that of A.
>> * The result is the integer nearest A, or if there are two integers
>equally near A,
>>   the result is whichever such integer has the greater magnitude.
>> * Examples. ANINT (2.783) has the value 3.0. ANINT (−2.783) has the
>value −3.0.
>> 
>> >Of course the current code doesn't handle this correctly for the
>> >case if llroundf either.
>> >>I've not contributed to the Fortran front end before.  If the
>maintainers like
>> >>the patch, can somebody point out if I need to do additional things
>to commit
>> >>the patch?
>> Nothing special: a testcase already exists, committing is done as
>usual
>> and a PR to update you have as well.
>
>Given GCC 11 has branched, is it ok to backport the patch to the GCC 11
>branch
>as well?  I assume it is, since it fixes a regression in the compiler.

Please wait until after 11.1 is released. 

Thanks, 
Richard. 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-04-22 19:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-19 18:55 Fix Fortran rounding issues, PR fortran/96983 Michael Meissner
2021-04-20  6:58 ` Richard Biener
2021-04-21  8:10   ` Tobias Burnus
2021-04-22 19:09     ` Michael Meissner
2021-04-22 19:21       ` Richard Biener

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).