diff mbox series

[fortran] Some corrections for DO loop index warnings

Message ID 93ba61ec-0db4-1888-9a3f-856a4c59d9ba@netcologne.de
State New
Headers show
Series [fortran] Some corrections for DO loop index warnings | expand

Commit Message

Thomas Koenig Aug. 12, 2019, 3:18 p.m. UTC
Hello world,

the attached patch fixes three problems with DO loop index warnings:

- DO loops in contained procedures were not checked

- Zero-trip loops gave a false positive

- DO loops in blocks gave the same warning twice

plus it fixes the resulting fallout from the test suite.

Regression-tested.  OK for trunk?  I also think that this is something
that could be safely backported to gcc-9.


2019-08-12  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/91424
	* frontend-passes.c (do_subscript): Do not warn for an
	expression a second time.  Do not warn about a zero-trip loop.
	(doloop_warn): Also look at contained namespaces.

2019-08-12  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/91424
	* gfortran.dg/do_subscript_3.f90: New test.
	* gfortran.dg/do_subscript_4.f90: New test.
	* gfortran.dg/pr70754.f90: Use indices that to not overflow.

2019-08-12  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/91422
	* testsuite/libgomp.oacc-fortran/routine-7.f90: Correct array
	dimension.

Comments

Steve Kargl Aug. 12, 2019, 7:59 p.m. UTC | #1
On Mon, Aug 12, 2019 at 05:18:18PM +0200, Thomas Koenig wrote:
> Hello world,
> 
> the attached patch fixes three problems with DO loop index warnings:
> 
> - DO loops in contained procedures were not checked
> 
> - Zero-trip loops gave a false positive
> 
> - DO loops in blocks gave the same warning twice
> 
> plus it fixes the resulting fallout from the test suite.
> 
> Regression-tested.  OK for trunk?  I also think that this is something
> that could be safely backported to gcc-9.
> 

Ok.  If it regression cleanly on gcc9, go ahead an commit there as well.
Thomas Koenig Aug. 13, 2019, 10:30 a.m. UTC | #2
Hi Steve,

> Ok.  If it regression cleanly on gcc9, go ahead an commit there as well.

Committed to both (after doing a regression test on gcc 9 and also
waiting for gcc-testresults containing the revision).

Thanks for the review!
diff mbox series

Patch

Index: gcc/fortran/frontend-passes.c
===================================================================
--- gcc/fortran/frontend-passes.c	(revision 273855)
+++ gcc/fortran/frontend-passes.c	(working copy)
@@ -2556,6 +2556,12 @@  do_subscript (gfc_expr **e)
   if (in_assoc_list)
     return 0;
 
+  /* We already warned about this.  */
+  if (v->do_not_warn)
+    return 0;
+
+  v->do_not_warn = 1;
+
   for (ref = v->ref; ref; ref = ref->next)
     {
       if (ref->type == REF_ARRAY && ref->u.ar.type == AR_ELEMENT)
@@ -2608,7 +2614,6 @@  do_subscript (gfc_expr **e)
 	      else
 		have_do_start = false;
 
-
 	      if (dl->ext.iterator->end->expr_type == EXPR_CONSTANT)
 		{
 		  have_do_end = true;
@@ -2620,6 +2625,17 @@  do_subscript (gfc_expr **e)
 	      if (!have_do_start && !have_do_end)
 		return 0;
 
+	      /* No warning inside a zero-trip loop.  */
+	      if (have_do_start && have_do_end)
+		{
+		  int sgn, cmp;
+
+		  sgn = mpz_cmp_ui (do_step, 0);
+		  cmp = mpz_cmp (do_end, do_start);
+		  if ((sgn > 0 && cmp < 0) || (sgn < 0 && cmp > 0))
+		    break;
+		}
+
 	      /* May have to correct the end value if the step does not equal
 		 one.  */
 	      if (have_do_start && have_do_end && mpz_cmp_ui (do_step, 1) != 0)
@@ -2761,6 +2777,12 @@  static void
 doloop_warn (gfc_namespace *ns)
 {
   gfc_code_walker (&ns->code, doloop_code, do_function, NULL);
+
+  for (ns = ns->contained; ns; ns = ns->sibling)
+    {
+      if (ns->code == NULL || ns->code->op != EXEC_BLOCK)
+	doloop_warn (ns);
+    }
 }
 
 /* This selction deals with inlining calls to MATMUL.  */
Index: gcc/testsuite/gfortran.dg/pr70754.f90
===================================================================
--- gcc/testsuite/gfortran.dg/pr70754.f90	(revision 273855)
+++ gcc/testsuite/gfortran.dg/pr70754.f90	(working copy)
@@ -18,12 +18,13 @@  contains
     integer (ii4), dimension(40,40) :: c
     integer  i, j
 
-    do i=1,20
-      b(i,j) = 123 * a(i,j) + 34 * a(i,j+1) &
-             + 34 * a(i,j-1) + a(i+1,j+1) &
-             + a(i+1,j-1) + a(i-1,j+1) &
-             + a(i-1,j-1)
-      c(i,j) = 123
+    j = 10
+    do i=11,30
+       b(i,j) = 123 * a(i,j) + 34 * a(i,j+1) &
+            + 34 * a(i,j-1) + a(i+1,j+1) &
+            + a(i+1,j-1) + a(i-1,j+1) &
+            + a(i-1,j-1)
+       c(i,j) = 123
     end do
 
     where ((xyz(:,:,2) /= 0) .and. (c /= 0))
Index: libgomp/testsuite/libgomp.oacc-fortran/routine-7.f90
===================================================================
--- libgomp/testsuite/libgomp.oacc-fortran/routine-7.f90	(revision 273855)
+++ libgomp/testsuite/libgomp.oacc-fortran/routine-7.f90	(working copy)
@@ -109,7 +109,7 @@  end subroutine gang
 
 subroutine seq (a)
   !$acc routine seq
-  integer, intent (inout) :: a(M)
+  integer, intent (inout) :: a(N)
   integer :: i
 
   do i = 1, N