[PATCH] PR/fortran 96983 - ICE compiling gfortran.dg/pr96711.f90

[PATCH] PR/fortran 96983 - ICE compiling gfortran.dg/pr96711.f90

[Harald Anlauf]

[-- Attachment #1: Type: text/plain, Size: 1909 bytes --]

Dear all,

my fix for PR fortran/96711 adds a testcase that failed on powerpc64-*-*
as well as sparc*-sun-solaris2.11.  This is a consequence of the, say,
mess, on x86, where we have real kinds 4,8,10,16, with kind=10 being long
double and kind=16 being represented as float128, while on sparc real(16)
is mapped to long double.  I'm not going to comment on power, see PR for
details.

A minimal solution to the issue would extend an intermediate conversion
that is done for e.g. x86 to float128 to a conversion to long double
on platforms where the TYPE_PRECISION of long double equals 128.  This
solves the issue on sparc*-sun-solaris2.11, as confirmed by Rainer.

The attached patch does just this, and disables the testcase for target
powerpc*-*-*.  I expect a more sophisticated solution being needed for
that platform.

That said, the patch regtests cleanly on x86_64-pc-linux-gnu, and as
reported in the PR, sparc-sun-solaris2.11 and i386-pc-solaris2.11.

OK for master?  Or does anyone with better knowledge of the affected
platforms want to take over?  As this is a technical regression, it
should and could be fixed before the gcc-11 release.

My intention to either close the PR after committing, or changing it
into some appropriate state (SUSPENDED?) so that it can be handled
later, or to rather open a new PR that is finally a target issue.

Thanks,
Harald


PR/fortran 96983 - ICE compiling gfortran.dg/pr96711.f90

The fix for PR fortran/96711 introduced an intermediate conversion to
the float128 type that is used e.g. on x86 but not on some other
targets.  Use a conversion to long double on targets where this type
has a type precision of 128 bit.

gcc/fortran/ChangeLog:

	* trans-intrinsic.c (build_round_expr): Use conversion to long
	double when this type has a type precision of 128 bit.

gcc/testsuite/ChangeLog:

	* gfortran.dg/pr96711.f90: Skip testcase for target powerpc*-*-*.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pr96983.patch --]
[-- Type: text/x-patch, Size: 1433 bytes --]

diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 32fe9886c57..552cdf5f19c 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -395,6 +395,13 @@ build_round_expr (tree arg, tree restype)
     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 == TYPE_PRECISION (long_double_type_node)
+	   && resprec >= argprec)
+    {
+      int kind = TYPE_PRECISION (long_double_type_node) / 8;
+      arg = fold_convert (long_double_type_node, arg);
+      fn = gfc_builtin_decl_for_float_kind (BUILT_IN_ROUND, kind);
+    }
   else if (resprec >= argprec && resprec == 128)
     {
       /* Search for a real kind suitable as temporary for conversion.  */
diff --git a/gcc/testsuite/gfortran.dg/pr96711.f90 b/gcc/testsuite/gfortran.dg/pr96711.f90
index 3761a8ea416..13b6e829ed6 100644
--- a/gcc/testsuite/gfortran.dg/pr96711.f90
+++ b/gcc/testsuite/gfortran.dg/pr96711.f90
@@ -3,6 +3,8 @@
 ! { dg-require-effective-target fortran_real_16 }
 ! { dg-additional-options "-fdump-tree-original" }
 ! { dg-final { scan-tree-dump-times "_gfortran_stop_numeric" 2 "original" } }
+! { dg-skip-if "" { "powerpc*-*-*" } }
+! This test expects a target that supports IEEE128.
 !
 ! PR fortran/96711 - ICE on NINT() Function

Re: [PATCH] PR/fortran 96983 - ICE compiling gfortran.dg/pr96711.f90

[Tobias Burnus]

Dear Harald,

On 9/14/20 10:01 PM, Harald Anlauf wrote:
> OK for master?

I think we should fix this properly by using target
hooks as suggested by Joseph in the PR. Otherwise,
I have no idea how one could handle this properly
on the FE side for PowerPC (see below).

Tobias

PS: from gcc/config/rs6000/rs6000-modes.*:

/* IBM 128-bit floating point.  */
FRACTIONAL_FLOAT_MODE (IF, FLOAT_PRECISION_IFmode, 16, ibm_extended_format);

/* Explicit IEEE 128-bit floating point.  */
FRACTIONAL_FLOAT_MODE (KF, FLOAT_PRECISION_KFmode, 16, ieee_quad_format);

/* 128-bit floating point, either IBM 128-bit or IEEE 128-bit.  This is
    adjusted in rs6000_option_override_internal to be the appropriate floating
    point type.  */
FRACTIONAL_FLOAT_MODE (TF, FLOAT_PRECISION_TFmode, 16, ieee_quad_format);

/* We order the 3 128-bit floating point types so that IFmode (IBM 128-bit
    floating point) is the 128-bit floating point type with the highest
    precision (128 bits).  This so that machine independent parts of the
    compiler do not try to widen IFmode to TFmode on ISA 3.0 (power9) that has
    hardware support for IEEE 128-bit.  We set TFmode (long double mode) in
    between, and KFmode (explicit __float128) below it.

    We won't encounter conversion from IEEE 128-bit to IBM 128-bit because we
    don't have insns to support the IBM 128-bit aritmetic operations.  */

#ifndef RS6000_MODES_H
#define RS6000_MODES_H          1
#define FLOAT_PRECISION_IFmode  128
#define FLOAT_PRECISION_TFmode  127
#define FLOAT_PRECISION_KFmode  126

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter

Aw: Re: [PATCH] PR/fortran 96983 - ICE compiling gfortran.dg/pr96711.f90

[Harald Anlauf]

Dear Tobias,

I can see that you want a proper fix.

However, after having looked at all these comments about the powerpc
situation, I do not really think I'd want to ever touch that stuff.
It's clearly beyond my capabilities and ressources.

I do feel responsible for the regression introduced by my naive patch
for PR96711, which is a breakage for sparc and powerpc.

The patch was meant as a minimal solution, or rather a bandaid to fix
sparc, and skip the test for powerpc, and leave the powerpc situation
to someone with more knowledge of that target.

If this patch is rejected, but the regression is to be fixed before
gcc-11 release, I can therefore only offer to revert the patch for
PR96711, to reopen it and unassign.

Thanks,
Harald