[fortran] Fix PR 85631, wrong size checking with allocatable arrays

Message ID 7a7a342c-c824-0acc-0966-63a82ec9de8a@netcologne.de
State New
Headers show
Series
  • [fortran] Fix PR 85631, wrong size checking with allocatable arrays
Related show

Commit Message

Thomas Koenig June 8, 2018, 7:06 p.m.
Hello world,

the attached patch fixes a bug which was uncovered by the PR in
a matmul regression.

The problem is that bounds checking on the LHS with reallocation on
assignment makes no sense, and the original flag was not set for
the case in question.

I added both the original test and the reduced test to the single test
case.

OK for trunk?

Regards

	Thomas

2018-06-08  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/85631
	* trans.h (gfc_ss): Add field no_bounds_check.
	* trans-array.c (gfc_conv_ss_startstride): If flag_realloc_lhs and
	ss->no_bounds_check is set, do not use runtime checks.
	* trans-expr.c (gfc_trans_assignment_1): Set lss->no_bounds_check
	for reallocatable lhs.

2018-06-08  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/85631
	* gfortran.dg/bounds_check_20.f90: New test.

Comments

Steve Kargl June 8, 2018, 7:15 p.m. | #1
On Fri, Jun 08, 2018 at 09:06:55PM +0200, Thomas Koenig wrote:
> 

> the attached patch fixes a bug which was uncovered by the PR in

> a matmul regression.

> 

> The problem is that bounds checking on the LHS with reallocation on

> assignment makes no sense, and the original flag was not set for

> the case in question.

> 

> I added both the original test and the reduced test to the single test

> case.

> 

> OK for trunk?

> 


OK.

-- 
steve
Thomas Koenig June 8, 2018, 10:05 p.m. | #2
Hi Steve,

> On Fri, Jun 08, 2018 at 09:06:55PM +0200, Thomas Koenig wrote:

>>

>> the attached patch fixes a bug which was uncovered by the PR in

>> a matmul regression.

>>

>> The problem is that bounds checking on the LHS with reallocation on

>> assignment makes no sense, and the original flag was not set for

>> the case in question.

>>

>> I added both the original test and the reduced test to the single test

>> case.

>>

>> OK for trunk?

>>

> 

> OK.


Committed as r261348.

Thanks!

	Thomas

Patch

Index: trans-array.c
===================================================================
--- trans-array.c	(Revision 261245)
+++ trans-array.c	(Arbeitskopie)
@@ -4304,7 +4304,7 @@  done:
 	}
     }
 
-  /* The rest is just runtime bound checking.  */
+  /* The rest is just runtime bounds checking.  */
   if (gfc_option.rtcheck & GFC_RTCHECK_BOUNDS)
     {
       stmtblock_t block;
@@ -4334,7 +4334,7 @@  done:
 	    continue;
 
 	  /* Catch allocatable lhs in f2003.  */
-	  if (flag_realloc_lhs && ss->is_alloc_lhs)
+	  if (flag_realloc_lhs && ss->no_bounds_check)
 	    continue;
 
 	  expr = ss_info->expr;
Index: trans-expr.c
===================================================================
--- trans-expr.c	(Revision 261245)
+++ trans-expr.c	(Arbeitskopie)
@@ -9982,12 +9982,15 @@  gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr
 
   /* Walk the lhs.  */
   lss = gfc_walk_expr (expr1);
-  if (gfc_is_reallocatable_lhs (expr1)
-      && !(expr2->expr_type == EXPR_FUNCTION
-	   && expr2->value.function.isym != NULL
-	   && !(expr2->value.function.isym->elemental
-		|| expr2->value.function.isym->conversion)))
-    lss->is_alloc_lhs = 1;
+  if (gfc_is_reallocatable_lhs (expr1))
+    {
+      lss->no_bounds_check = 1;
+      if (!(expr2->expr_type == EXPR_FUNCTION
+	    && expr2->value.function.isym != NULL
+	    && !(expr2->value.function.isym->elemental
+		 || expr2->value.function.isym->conversion)))
+	lss->is_alloc_lhs = 1;
+    }
 
   rss = NULL;
 
Index: trans.h
===================================================================
--- trans.h	(Revision 261245)
+++ trans.h	(Arbeitskopie)
@@ -330,6 +330,7 @@  typedef struct gfc_ss
   struct gfc_loopinfo *loop;
 
   unsigned is_alloc_lhs:1;
+  unsigned no_bounds_check:1;
 }
 gfc_ss;
 #define gfc_get_ss() XCNEW (gfc_ss)