[Bug libfortran/96613] New: SIGFPE on min1() with -ffpe-trap=invalid switch

[Bug libfortran/96613] New: SIGFPE on min1() with -ffpe-trap=invalid switch

[thomas.huxhorn at web dot de]

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

            Bug ID: 96613
           Summary: SIGFPE on min1() with -ffpe-trap=invalid switch
           Product: gcc
           Version: 10.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libfortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: thomas.huxhorn at web dot de
  Target Milestone: ---

The program fails if -ffpe-trap=invalid is present and no optimization flag is
set. It print 5000 with any optimization level set.

The expected behavior is that it print 5000 with no optimization.

Perhaps it is possible to first calculate the minimum of both real values and
then convert it to integer?

Testet with gcc 10.1 and 4.9
Compile with gfortran -ffpe-trap=invalid  -g -ggdb -fbacktrace   test.f90 

program test
  implicit none
  real :: X
  integer :: IX

  X = 7.7643945e+09
  IX = MIN1(5000.0, X)
  write(*,*) IX
end program

gdb ./a.out 
(gdb) run
Starting program: /home/thomas/a.out 
Program received signal SIGFPE, Arithmetic exception.
0x0000000000400794 in test () at test.f90:8
8         IX = MIN1(5000.0, X)

[Bug libfortran/96613] SIGFPE on min1() with -ffpe-trap=invalid switch

[kargl at gcc dot gnu.org]

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

kargl at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2020-08-14
     Ever confirmed|0                           |1
                 CC|                            |kargl at gcc dot gnu.org
           Priority|P3                          |P4

--- Comment #1 from kargl at gcc dot gnu.org ---
Compiling with -fdump-tree-original gives

  integer(kind=4) ix;
  real(kind=4) x;

  x = 7.7643945e+9;
  {
    integer(kind=4) M.0;

    M.0 = 5000;
    M.0 = MIN_EXPR <(integer(kind=4)) x, M.0>;
    ix = M.0;
  }

The MIN_EXPR is clealy a bad idea as x > huge(1).

The Fortran standard states, that for example,
MIN1(a,b) = INT(MIN(a,b)).  I would have expected
to see something along the lines of

  integer(kind=4) ix;
  real(kind=4) x;

  x = 7.7643945e+9;
  {
    real(kind=4) M.0;

    M.0 = 5000.0;
    M.0 = MIN_EXPR <x, M.0>;
    ix = (integer(kind=4)) M.0;
  }

If this is effecting your code, I suggest changing it to
min1() = int(min()) to work around the issue.  This might
get fixed someday when a gfortran contributor has time to
look at it.

[Bug fortran/96613] SIGFPE on min1() with -ffpe-trap=invalid switch

[kargl at gcc dot gnu.org]

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

kargl at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|libfortran                  |fortran

--- Comment #2 from kargl at gcc dot gnu.org ---
Change component to 'fortran' as MIN1() is in-lined.

[Bug fortran/96613] SIGFPE on min1() with -ffpe-trap=invalid switch

[anlauf at gcc dot gnu.org]

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

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |anlauf at gcc dot gnu.org

--- Comment #3 from anlauf at gcc dot gnu.org ---
Created attachment 49066
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49066&action=edit
WIP patch

The underlying issue is visible in the tree-dump, see also comment#1 by Steve.
Changing the type and kind of the temporary to a common argument one instead
of the result, the issue is fixed.  Playing around a little bit, one finds that
even the GNU extension to MIN/MAX, allowing different argument kinds, is fishy.

The attached patch appears regtesting fine, but the coverage of the borderline
cases is insufficient yet.

I'm not sure I got the GIMPLE magic right everywhere, so the patch may need
extended testing before submitting.

[Bug fortran/96613] SIGFPE on min1() with -ffpe-trap=invalid switch

[anlauf at gcc dot gnu.org]

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

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Keywords|                            |wrong-code
           Assignee|unassigned at gcc dot gnu.org      |anlauf at gcc dot gnu.org

[Bug fortran/96613] SIGFPE on min1() with -ffpe-trap=invalid switch

[kargl at gcc dot gnu.org]

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

--- Comment #4 from kargl at gcc dot gnu.org ---
(In reply to anlauf from comment #3)
> Created attachment 49066 [details]
> WIP patch
> 
> The underlying issue is visible in the tree-dump, see also comment#1 by
> Steve.
> Changing the type and kind of the temporary to a common argument one instead
> of the result, the issue is fixed.  Playing around a little bit, one finds
> that
> even the GNU extension to MIN/MAX, allowing different argument kinds, is
> fishy.
> 
> The attached patch appears regtesting fine, but the coverage of the
> borderline
> cases is insufficient yet.
> 
> I'm not sure I got the GIMPLE magic right everywhere, so the patch may need
> extended testing before submitting.

Hi Harald,

I started to think about doing a similar patch as you have done.
Then, after looking at F2018 where it states that MIN1() = INT(MIN()),
I thought that this might be a good candidate for frontend fix.  
Similar to thomas's frontend optimizations except except the 
transformation is always done.  That is, we should be able to do
substitutions based on Table 16.3 before we even get to backend.

[Bug fortran/96613] SIGFPE on min1() with -ffpe-trap=invalid switch

[anlauf at gcc dot gnu.org]

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

--- Comment #5 from anlauf at gcc dot gnu.org ---
(In reply to kargl from comment #4)
> I thought that this might be a good candidate for frontend fix.  
> Similar to thomas's frontend optimizations except except the 
> transformation is always done.  That is, we should be able to do
> substitutions based on Table 16.3 before we even get to backend.

If frontend optimization is preferred, I'll step out of the way.

Nevertheless, one also need to address issues like:

% cat maxmin.f90
program p
  implicit none
  print *, min (2.0, 1.d0)
  print *, min (2.d0, 1.0)
  print *, kind (min (2.0, 1.d0))
  print *, kind (min (2.d0, 1.0))
end program p

% gfc-11 maxmin.f90 && ./a.out 
   1.00000000    
   1.0000000000000000     
           4
           8

The only compiler I found having the same is PGI/NVIDIA.

OTOH ifort (and similarly sunf95, g95(!)) result in the expected:

   1.00000000000000     
   1.00000000000000     
           8
           8

[Bug fortran/96613] SIGFPE on min1() with -ffpe-trap=invalid switch

[sgk at troutmask dot apl.washington.edu]

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

--- Comment #6 from Steve Kargl <sgk at troutmask dot apl.washington.edu> ---
On Mon, Aug 17, 2020 at 06:03:31PM +0000, anlauf at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96613
> 
> --- Comment #5 from anlauf at gcc dot gnu.org ---
> (In reply to kargl from comment #4)
> > I thought that this might be a good candidate for frontend fix.  
> > Similar to thomas's frontend optimizations except except the 
> > transformation is always done.  That is, we should be able to do
> > substitutions based on Table 16.3 before we even get to backend.
> 
> If frontend optimization is preferred, I'll step out of the way.
> 
> Nevertheless, one also need to address issues like:
> 
> % cat maxmin.f90
> program p
>   implicit none
>   print *, min (2.0, 1.d0)
>   print *, min (2.d0, 1.0)
>   print *, kind (min (2.0, 1.d0))
>   print *, kind (min (2.d0, 1.0))
> end program p
> 
> % gfc-11 maxmin.f90 && ./a.out 
>    1.00000000    
>    1.0000000000000000     
>            4
>            8
> 
> The only compiler I found having the same is PGI/NVIDIA.
> 
> OTOH ifort (and similarly sunf95, g95(!)) result in the expected:
> 
>    1.00000000000000     
>    1.00000000000000     
>            8
>            8

Personally, I would rather issue an error if types of the
arguments are not the same, but that ship sailed years ago.
To fix, the above, you'll need to look at iresolve.c

static void
gfc_resolve_minmax (const char *name, gfc_expr *f, gfc_actual_arglist *args)
{
  gfc_actual_arglist *a;

  f->ts.type = args->expr->ts.type;
  f->ts.kind = args->expr->ts.kind;

and re-do the conversion stuff.  AFAICT, type conversion is
not handled correctly.  The largest kind is found regardless
of the type and this kind with the type of first argument is
used to to do conversion.

[Bug fortran/96613] SIGFPE on min1() with -ffpe-trap=invalid switch

[anlauf at gcc dot gnu.org]

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

--- Comment #7 from anlauf at gcc dot gnu.org ---
(In reply to Steve Kargl from comment #6)
> To fix, the above, you'll need to look at iresolve.c
> 
> static void
> gfc_resolve_minmax (const char *name, gfc_expr *f, gfc_actual_arglist *args)
> {
>   gfc_actual_arglist *a;
> 
>   f->ts.type = args->expr->ts.type;
>   f->ts.kind = args->expr->ts.kind;
> 
> and re-do the conversion stuff.  AFAICT, type conversion is
> not handled correctly.  The largest kind is found regardless
> of the type and this kind with the type of first argument is
> used to to do conversion.

I played around with other potential testcases.  So far it seems
sufficient to just "fix" the simplification:

diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index eb8b2afeb29..074b50c2e68 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -4924,6 +4924,8 @@ min_max_choose (gfc_expr *arg, gfc_expr *extremum, int
sign, bool back_val)
   switch (arg->ts.type)
     {
       case BT_INTEGER:
+       if (extremum->ts.kind < arg->ts.kind)
+         extremum->ts.kind = arg->ts.kind;
        ret = mpz_cmp (arg->value.integer,
                       extremum->value.integer) * sign;
        if (ret > 0)
@@ -4931,6 +4933,8 @@ min_max_choose (gfc_expr *arg, gfc_expr *extremum, int
sign, bool back_val)
        break;

       case BT_REAL:
+       if (extremum->ts.kind < arg->ts.kind)
+         extremum->ts.kind = arg->ts.kind;
        if (mpfr_nan_p (extremum->value.real))
          {
            ret = 1;

Not fully regtested though.

[Bug fortran/96613] SIGFPE on min1() with -ffpe-trap=invalid switch

[anlauf at gcc dot gnu.org]

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

--- Comment #8 from anlauf at gcc dot gnu.org ---
Patch: https://gcc.gnu.org/pipermail/fortran/2020-August/054884.html

[Bug fortran/96613] SIGFPE on min1() with -ffpe-trap=invalid switch

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Harald Anlauf <anlauf@gcc.gnu.org>:

https://gcc.gnu.org/g:3c04bd60e56da399a441f73ebb687b5039b9cf3f

commit r11-2746-g3c04bd60e56da399a441f73ebb687b5039b9cf3f
Author: Harald Anlauf <anlauf@gmx.de>
Date:   Tue Aug 18 21:48:56 2020 +0200

    PR fortran/96613,96686 - Fix type/kind issues, temporaries evaluating
MIN/MAX

    When evaluating functions of the MIN/MAX variety inline, use a temporary
    of appropriate type and kind, and convert to the result type at the end.
    In the case of allowing for the GNU extensions to MIN/MAX, derive the
    result kind consistently during simplificaton.

    Furthermore, the Fortran standard requires type and kind of arguments to
    the MIN/MAX intrinsics to all have the same type and kind.  While a GNU
    extension accepts kind differences for integer and real arguments which
    seems to have been used in legacy code, there is no reason to allow
    different character kinds.  We now reject the latter unconditionally.

    gcc/fortran/ChangeLog:

            * check.c (check_rest): Reject MIN/MAX character arguments of
            different kind.
            * simplify.c (min_max_choose): The simplification result shall
            have the highest kind value of the arguments.
            * trans-intrinsic.c (gfc_conv_intrinsic_minmax): Choose type and
            kind of intermediate by looking at all arguments, not the result.

    gcc/testsuite/ChangeLog:

            * gfortran.dg/minmax_char_3.f90: New test.
            * gfortran.dg/min_max_kind.f90: New test.
            * gfortran.dg/pr96613.f90: New test.

[Bug fortran/96613] SIGFPE on min1() with -ffpe-trap=invalid switch

[anlauf at gcc dot gnu.org]

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

anlauf at gcc dot gnu.org changed:

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

--- Comment #10 from anlauf at gcc dot gnu.org ---
Fixed on master for GCC-11.

Thanks for the report!

[Bug fortran/96613] SIGFPE on min1() with -ffpe-trap=invalid switch

[thomas.huxhorn at web dot de]

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

--- Comment #11 from Thomas Huxhorn <thomas.huxhorn at web dot de> ---
I compiled the latest git GCC and rerun the program, no more problems. 
Thank you all :)