Re: [patch, fortran] Simplify constants which come from parameter arrays

Re: [patch, fortran] Simplify constants which come from parameter arrays

[Dominique d'Humières]

Hi Thomas,

If I am not mistaken, the patch causes:

FAIL: gfortran.dg/pr71935.f90   -O  (test for excess errors)
FAIL: gfortran.dg/substr_6.f90   -O0  execution test
FAIL: gfortran.dg/substr_6.f90   -O1  execution test
FAIL: gfortran.dg/substr_6.f90   -O2  execution test
FAIL: gfortran.dg/substr_6.f90   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/substr_6.f90   -O3 -g  execution test
FAIL: gfortran.dg/substr_6.f90   -Os  execution test

For gfortran.dg/pr71935.f90 I get two additional errors on top of the expected warnings:

/opt/gcc/_clean/gcc/testsuite/gfortran.dg/pr71935.f90:5:21:

    print *, sizeof(z(3))      ! { dg-warning "is out of bounds" }
                     1
Warning: Array reference at (1) is out of bounds (3 > 2) in dimension 1
/opt/gcc/_clean/gcc/testsuite/gfortran.dg/pr71935.f90:5:21:

    print *, sizeof(z(3))      ! { dg-warning "is out of bounds" }
                     1
Error: Index in dimension 1 is out of bounds at (1)
/opt/gcc/_clean/gcc/testsuite/gfortran.dg/pr71935.f90:6:23:

    print *, c_sizeof(z(3))    ! { dg-warning "is out of bounds" }
                       1
Warning: Array reference at (1) is out of bounds (3 > 2) in dimension 1
/opt/gcc/_clean/gcc/testsuite/gfortran.dg/pr71935.f90:6:23:

    print *, c_sizeof(z(3))    ! { dg-warning "is out of bounds" }
                       1
Error: Index in dimension 1 is out of bounds at (1)

For gfortran.dg/substr_6.f90, I reach STOP 1 when running the executable.

TIA

Dominique

Re: [patch, fortran] Simplify constants which come from parameter arrays

[Thomas König]

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

Hi Dominique,

> If I am not mistaken, the patch causes:
> 
> FAIL: gfortran.dg/pr71935.f90   -O  (test for excess errors)
> FAIL: gfortran.dg/substr_6.f90   -O0  execution test
> FAIL: gfortran.dg/substr_6.f90   -O1  execution test
> FAIL: gfortran.dg/substr_6.f90   -O2  execution test
> FAIL: gfortran.dg/substr_6.f90   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
> FAIL: gfortran.dg/substr_6.f90   -O3 -g  execution test
> FAIL: gfortran.dg/substr_6.f90   -Os  execution test

These have been resolved now - I have removed the invalid code
from substr_6.f90 (PR85130), and the error is now suppressed
in the attached patch.

Re-regression-tested. OK for trunk?

Regards

	Thomas

2018-03-25  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/51260
         * resolve.c (resolve_variable): Simplify cases where access to a
         parameter array results in a single constant.

2018-03-25  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/51260
         * gfortran.dg/parameter_array_element_3.f90: New test.

[-- Attachment #2: parameter_array_element_3.f90 --]
[-- Type: text/x-fortran, Size: 466 bytes --]

! { dg-do compile }
! PR 51260 - an unneeded parameter found its way into the
! assembly code. Original test case by Tobias Burnus.
module x
contains
  subroutine foo(i)
    integer, intent(in) :: i
  end subroutine foo
end module x

program main
  use x
  integer, parameter:: unneeded_parameter (10000)=(/(i,i=1,10000)/)
  call foo(unneeded_parameter (1))
  print *,unneeded_parameter (1)
end program
! { dg-final { scan-assembler-times "unneeded_parameter" 0 } }

[-- Attachment #3: p2.diff --]
[-- Type: text/x-patch, Size: 671 bytes --]

Index: resolve.c
===================================================================
--- resolve.c	(Revision 258845)
+++ resolve.c	(Arbeitskopie)
@@ -5577,6 +5577,16 @@ resolve_procedure:
   if (t && flag_coarray == GFC_FCOARRAY_LIB && gfc_is_coindexed (e))
     add_caf_get_intrinsic (e);
 
+  /* Simplify cases where access to a parameter array results in a
+     single constant.  Suppress errors since those will have been
+     issued before, as warnings.  */
+  if (e->rank == 0 && sym->as && sym->attr.flavor == FL_PARAMETER)
+    {
+      gfc_push_suppress_errors ();
+      gfc_simplify_expr (e, 1);
+      gfc_pop_suppress_errors ();
+    }
+
   return t;
 }
 

Re: [patch, fortran] Simplify constants which come from parameter arrays

[Dominique d'Humières]

Hi Thomas,

> Le 31 mars 2018 à 13:57, Thomas König <tk@tkoenig.net> a écrit :
> 
> Hi Dominique,
> 
> These have been resolved now - I have removed the invalid code
> from substr_6.f90 (PR85130), and the error is now suppressed
> in the attached patch.
> 
> Re-regression-tested. OK for trunk?
> 
> Regards

The new patch regtest fine now. However as said on IRC this looks as a kludge made necessary by a questionable (invalid) test.

IMO it would be more general (better) to call

gfc_simplify_expr (e, 1);

only when there is no pending error (warning?).

I have also a question about "is out of bounds": it is a warning in resolve.c, but an error in expr.c and simplify.c. Should not it be an error everywhere?

Cheers,

Dominique

Re: [patch, fortran] Simplify constants which come from parameter arrays

[Thomas König]

Am 03.04.2018 um 17:21 schrieb Dominique d'Humières:

Hi Dominique,

> The new patch regtest fine now. However as said on IRC this looks as a kludge made necessary by a questionable (invalid) test.

What I want to avoid is to have first an error and then a warning
for the same thing. If we say that doesn't matter, I am also fine
with that.

> IMO it would be more general (better) to call
> 
> gfc_simplify_expr (e, 1);
> 
> only when there is no pending error (warning?).

That makes less sense than the current approach. We do not want to
grow the size of an executable because the code has, let's say, a
conversion warning somewhere.

> I have also a question about "is out of bounds": it is a warning in > resolve.c, but an error in expr.c and simplify.c. Should not it be> 
an error everywhere?

It is inconsistent now. We had this general discussion last year, when
adding the -Wdo-subsript code, and I think we should fix this, but not
in time for gcc 8.

Suggested way forward: Apply the patch as is and open an PR to sort out
the warning/error stuff for out-of-bounds access to be resolved
consistently for gcc-9.

OK?

Regards

	Thomas

Re: [patch, fortran] Simplify constants which come from parameter arrays

[Thomas König]

Hello world,

> Suggested way forward: Apply the patch as is and open an PR to sort out
> the warning/error stuff for out-of-bounds access to be resolved
> consistently for gcc-9.
> 
> OK?

Ping?

Regards

	Thomas

Re: [patch, fortran] Simplify constants which come from parameter arrays

[Steve Kargl]

On Mon, Apr 09, 2018 at 10:25:59PM +0200, Thomas König wrote:
> Hello world,
> 
> > Suggested way forward: Apply the patch as is and open an PR to sort out
> > the warning/error stuff for out-of-bounds access to be resolved
> > consistently for gcc-9.
> > 
> > OK?
> 
> Ping?
> 

Yes.

-- 
Steve

Re: [patch, fortran] Simplify constants which come from parameter arrays

[Thomas König]

Hi Steve,

> On Mon, Apr 09, 2018 at 10:25:59PM +0200, Thomas König wrote:
>> Hello world,
>>
>>> Suggested way forward: Apply the patch as is and open an PR to sort out
>>> the warning/error stuff for out-of-bounds access to be resolved
>>> consistently for gcc-9.
>>>
>>> OK?
>>
>> Ping?
>>
> 
> Yes.

Thanks for the review, committed as r259256.

The promised PR is 85307.

Regards

	Thomas

[patch, fortran] Simplify constants which come from parameter arrays

[Thomas König]

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

Hello world,

the attached patch potentially saves some space in the object file by
simplifying access to individual elements of a parameter array, which
means that the original parameter may not be needed any more.

Regression-tested. OK for trunk?

Regards

	Thomas

2018-03-25  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/51260
         * resolve.c (resolve_variable): Simplify cases where access to a
         parameter array results in a single constant.

2018-03-25  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/51260
         * gfortran.dg/parameter_array_element_3.f90: New test.

[-- Attachment #2: p2.diff --]
[-- Type: text/x-patch, Size: 506 bytes --]

Index: resolve.c
===================================================================
--- resolve.c	(Revision 258501)
+++ resolve.c	(Arbeitskopie)
@@ -5577,6 +5577,11 @@ resolve_procedure:
   if (t && flag_coarray == GFC_FCOARRAY_LIB && gfc_is_coindexed (e))
     add_caf_get_intrinsic (e);
 
+  /* Simplify cases where access to a parameter array results in a
+     single constant.  */
+  if (e->rank == 0 && sym->as && sym->attr.flavor == FL_PARAMETER)
+    gfc_simplify_expr (e, 1);
+
   return t;
 }
 

[-- Attachment #3: parameter_array_element_3.f90 --]
[-- Type: text/x-fortran, Size: 466 bytes --]

! { dg-do compile }
! PR 51260 - an unneeded parameter found its way into the
! assembly code. Original test case by Tobias Burnus.
module x
contains
  subroutine foo(i)
    integer, intent(in) :: i
  end subroutine foo
end module x

program main
  use x
  integer, parameter:: unneeded_parameter (10000)=(/(i,i=1,10000)/)
  call foo(unneeded_parameter (1))
  print *,unneeded_parameter (1)
end program
! { dg-final { scan-assembler-times "unneeded_parameter" 0 } }