diff mbox series

[Fortran] PR87597 - fix off-by-one issue with inline matmul

Message ID 20181012143501.GB11980@physik.fu-berlin.de
State New
Headers show
Series [Fortran] PR87597 - fix off-by-one issue with inline matmul | expand

Commit Message

Tobias Burnus Oct. 12, 2018, 2:35 p.m. UTC 
In the front-end optimization for matmul, we call lbound() for each matrix
argument to obtain the shift to the 0-based loop variables.

If the first argument is a PARAMETER, it appears initially as EXPR_VARIABLE
and has associated ref->u.ar for the AR_FULL and ref->u.ar.as contains the
bounds.

Running gfc_simplify_expr() on the generated lbound() will simplify the
array argument to an EXPR_ARRAY, which always has a lower bound of 1.

The problem starts as soon as the PARAMETER array has bounds which do not
start with 1 but, e.g., with 0 as with the test case: gfortran generates
than code which is off by one (reads the wrong element and beyond array
bounds).

Fixed by explicitly preventing this during gfc_simplify_expr. I hope that's
the only place where matters and that it doesn't cause missed optimizations.

Build and regtested on x86-64-linux.
OK for the trunk and - after a grace time - for GCC 6, 7 and 8?

Tobias

Comments

Thomas Koenig Oct. 12, 2018, 4:07 p.m. UTC | #1 
Hi Tobias,

> Build and regtested on x86-64-linux.
> OK for the trunk and - after a grace time - for GCC 6, 7 and 8?

Thanks for taking this on so quickly!  I don't think that this will
result in any missed optimizations.

Ok.

	Thomas
diff mbox series

Patch

2018-10-12  Tobias Burnus  <burnus@net-b.de>

        PR fortran/87597
	* expr.c (gfc_simplify_expr): Avoid simplifying
	the 'array' argument to lbound/ubound/lcobound/
	ucobound.

        PR fortran/87597
	* gfortran.dg/inline_matmul_24.f90: New.

diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c
index 1cfda5fbfed..ca6f95d9d8e 100644
--- a/gcc/fortran/expr.c
+++ b/gcc/fortran/expr.c
@@ -1937,7 +1937,20 @@  gfc_simplify_expr (gfc_expr *p, int type)
       break;
 
     case EXPR_FUNCTION:
-      for (ap = p->value.function.actual; ap; ap = ap->next)
+      // For array-bound functions, we don't need to optimize
+      // the 'array' argument. In particular, if the argument
+      // is a PARAMETER, simplifying might convert an EXPR_VARIABLE
+      // into an EXPR_ARRAY; the latter has lbound = 1, the former
+      // can have any lbound.
+      ap = p->value.function.actual;
+      if (p->value.function.isym &&
+	  (p->value.function.isym->id == GFC_ISYM_LBOUND
+	   || p->value.function.isym->id == GFC_ISYM_UBOUND
+	   || p->value.function.isym->id == GFC_ISYM_LCOBOUND
+	   || p->value.function.isym->id == GFC_ISYM_UCOBOUND))
+	ap = ap->next;
+
+      for ( ; ap; ap = ap->next)
 	if (!gfc_simplify_expr (ap->expr, type))
 	  return false;
 
diff --git a/gcc/testsuite/gfortran.dg/inline_matmul_24.f90 b/gcc/testsuite/gfortran.dg/inline_matmul_24.f90
new file mode 100644
index 00000000000..067f6daf200
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/inline_matmul_24.f90
@@ -0,0 +1,42 @@ 
+! { dg-do run }
+! { dg-options "-ffrontend-optimize -fdump-tree-original" }
+!
+! PR fortran/87597
+!
+! Contributed by gallmeister
+!
+! Before, for the inlined matmul,
+! gamma5 was converted to an EXPR_ARRAY with lbound = 1
+! instead of the lbound = 0 as declared; leading to
+! an off-by-one problem.
+!
+program testMATMUL
+  implicit none
+    complex, dimension(0:3,0:3), parameter :: gamma5 = reshape((/ 0., 0., 1., 0., &
+                                                                  0., 0., 0., 1., &
+                                                                  1., 0., 0., 0., &
+                                                                  0., 1., 0., 0. /),(/4,4/))
+    complex, dimension(0:3,0:3) :: A, B, D
+    integer :: i
+
+    A = 0.0
+    do i=0,3
+       A(i,i) = i*1.0
+    end do
+
+    B = cmplx(7,-9)
+    B = matmul(A,gamma5)
+
+    D = reshape([0, 0, 2, 0, &
+                 0, 0, 0, 3, &
+                 0, 0, 0, 0, &
+                 0, 1, 0, 0], [4, 4])
+    write(*,*) B(0,:)
+    write(*,*) B(1,:)
+    write(*,*) B(2,:)
+    write(*,*) B(3,:)
+    if (any(B /= D)) then
+      call abort()
+    end if
+end program testMATMUL
+! { dg-final { scan-tree-dump-times "gamma5\[__var_1_do \\* 4 \\+ __var_2_do\]" 1 "optimized" } }