[openacc] add a warning for non-contiguous data clauses

[openacc] add a warning for non-contiguous data clauses

[Cesar Philippidis]

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

OpenACC requires that fortran array pointers must represent contiguous
arrays. As of now, the fortran FE in trunk treats non-contiguous array
pointers as an errors, at least for explicit data clauses. This patch
relaxes this errors to warnings for two reasons:

  1. A lot of existing code don't set the contiguous pointer attribute
     so this error breaks working programs.

  2. Array pointers can be implicitly detected by the gimplifier, and
     prior to this patch, there were no diagnostics informing the user
     of any potential problems with those variables.

My solution for 2) is somewhat of a kludge. This patch using an unused
TREE_VISITED bit to mark whether a decl is used inside an OpenACC region
or not for gfc_omp_privatize_by_reference langhook. I didn't really want
to modify that hook because there are a couple of other users for it,
notably is_reference inside omp-low, which do not require any special
diagnostics to be generated.

After thinking about this some more, perhaps that hook should be
extended with a needs_target_diagnostics argument for both OpenACC and
OpenMP offloaded regions. I think OpenMP has similar contiguous array
restrictions as OpenACC, at least in target code.

This patch is for gomp-4_0-branch, but would something like this be OK
for trunk?

By the way, apparently the __gcc_gfc__ format attribute doesn't support
the 'D' specifier for tree nodes, so I had to access the decl's name
directly. Since I'm not if this patch is OK, I've decided to leave that
as-is for now.

Cesar

[-- Attachment #2: gomp4-contiguous-pointer-warnings.diff --]
[-- Type: text/x-patch, Size: 5830 bytes --]

2016-08-02  Cesar Philippidis  <cesar@codesourcery.com>

	gcc/fortran/
	* openmp.c (resolve_oacc_data_clauses): Emit a warning about a
	potentially non-contiguous array pointer.
	(resolve_omp_clauses): Extend coverage of OpenACC data clause
	validation.
	* trans-openmp.c (gfc_omp_privatize_by_reference): Use the
	TREE_VISITED bit on decl to determine if decl is used inside an
	OpenACC region.  If so, emit a warning whenever decl is an array
	pointer.

	gcc/
	* gimplify.c (oacc_default_clause): Use TREE_VISITED to denote that
	decl is used inside an OpenACC region.

	gcc/testsuite/
	* gfortran.dg/goacc/contiguous.f90: New test.
	* gfortran.dg/goacc/kernels-alias-4.f95:

diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index e463df7..f3ea9d8 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -3268,6 +3268,10 @@ resolve_oacc_data_clauses (gfc_symbol *sym, locus loc, const char *name)
 	  && CLASS_DATA (sym)->attr.allocatable))
     gfc_error ("ALLOCATABLE object %qs of polymorphic type "
 	       "in %s clause at %L", sym->name, name, &loc);
+  if (sym->as && sym->as->type == AS_DEFERRED && sym->attr.pointer
+      && !sym->attr.contiguous)
+    gfc_warning (0, "Potentially noncontiguous deferred shape array %qs in %s "
+		 "clause at %L", sym->name, name, &loc);
   check_symbol_not_pointer (sym, loc, name);
   check_array_not_assumed (sym, loc, name);
 }
@@ -3721,7 +3725,7 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses,
 			    }
 		      }
 		  }
-		else if (openacc)
+		if (openacc)
 		  {
 		    if (list == OMP_LIST_MAP
 			&& n->u.map_op == OMP_MAP_FORCE_DEVICEPTR)
diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index 2c28a31..5773337 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -39,17 +39,27 @@ along with GCC; see the file COPYING3.  If not see
 #include "gomp-constants.h"
 #include "hash-set.h"
 #include "tree-iterator.h"
+#include "diagnostic-core.h"
 
 int ompws_flags;
 
 /* True if OpenMP should privatize what this DECL points to rather
-   than the DECL itself.  */
+   than the DECL itself.  TREE_VISITED is set on the decl if it is
+   used inside an OpenACC region and requires additional array pointer
+   diagnostics.  */
 
 bool
 gfc_omp_privatize_by_reference (const_tree decl)
 {
   tree type = TREE_TYPE (decl);
 
+  /* Warn on non-contiguous array pointers.  */
+  if (TREE_VISITED (decl) && GFC_DESCRIPTOR_TYPE_P (type)
+      && (GFC_TYPE_ARRAY_AKIND (type) == GFC_ARRAY_POINTER
+	  || GFC_TYPE_ARRAY_AKIND (type) == GFC_ARRAY_POINTER_CONT))
+    warning (0, "Potentially noncontiguous deferred shape array %qs",
+	     IDENTIFIER_POINTER (DECL_NAME (decl)));
+
   if (TREE_CODE (type) == REFERENCE_TYPE
       && (!DECL_ARTIFICIAL (decl) || TREE_CODE (decl) == PARM_DECL))
     return true;
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 717b25f..9f21b52 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -6063,8 +6063,12 @@ oacc_default_clause (struct gimplify_omp_ctx *ctx, tree decl, unsigned flags)
   bool on_device = false;
   tree type = TREE_TYPE (decl);
 
+  /* Use TREE_VISITED to mark that this decl is used inside an OpenACC
+     context.  */
+  TREE_VISITED (decl) = 1;
   if (lang_hooks.decls.omp_privatize_by_reference (decl))
     type = TREE_TYPE (type);
+  TREE_VISITED (decl) = 0;
 
   if ((ctx->region_type & (ORT_ACC_PARALLEL | ORT_ACC_KERNELS)) != 0
       && is_global_var (decl)
diff --git a/gcc/testsuite/gfortran.dg/goacc/contiguous.f90 b/gcc/testsuite/gfortran.dg/goacc/contiguous.f90
new file mode 100644
index 0000000..84fbbbd
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/contiguous.f90
@@ -0,0 +1,81 @@
+subroutine allocated_array
+  integer i
+  integer, target, allocatable :: a(:)
+  integer, pointer :: b(:)
+
+  allocate (a(100))
+  b => a(20:30:5)
+
+  !$acc parallel loop
+  do i = 1, 3
+     b(i) = i ! { dg-warning "noncontiguous" }
+  end do
+  !$acc end parallel loop
+
+  print *, b
+  deallocate(a)
+end subroutine allocated_array
+
+subroutine dummy_array(a)
+  integer, target :: a(100)
+  integer, pointer :: b(:)
+
+  b => a(20:30:5)
+
+  !$acc parallel loop
+  do i = 1, 3
+     b(i) = i ! { dg-warning "noncontiguous" }
+  end do
+  !$acc end parallel loop
+
+  print *, b
+end subroutine dummy_array
+
+subroutine local_array
+  integer i
+  integer, target :: a(100)
+  integer, pointer :: b(:)
+
+  b => a(20:30:5)
+
+  !$acc parallel loop
+  do i = 1, 3
+     b(i) = i ! { dg-warning "noncontiguous" }
+  end do
+  !$acc end parallel loop
+
+  print *, b
+end subroutine local_array
+
+module d
+  integer, allocatable, target :: arr(:,:)
+  integer, pointer :: ptr(:,:)
+end module d
+
+subroutine module_array
+  use d
+  integer i
+
+  allocate (arr(100, 100))
+  ptr => arr(20:30:5,1:2)
+
+  !$acc parallel loop copy (ptr(1:3,1:2)) ! { dg-warning "noncontiguous" }
+  do i = 1, 3
+     ptr(i,1) = i
+  end do
+  !$acc end parallel loop
+
+  !$acc parallel loop copy (ptr) ! { dg-warning "noncontiguous" }
+  do i = 1, 3
+     ptr(i,1) = i
+  end do
+  !$acc end parallel loop
+
+  !$acc parallel loop
+  do i = 1, 3
+     ptr(i,1) = i ! { dg-warning "noncontiguous" }
+  end do
+  !$acc end parallel loop
+
+  print *, ptr(:,1)
+end subroutine module_array
diff --git a/gcc/testsuite/gfortran.dg/goacc/kernels-alias-4.f95 b/gcc/testsuite/gfortran.dg/goacc/kernels-alias-4.f95
index 93a3a03..61a8d63 100644
--- a/gcc/testsuite/gfortran.dg/goacc/kernels-alias-4.f95
+++ b/gcc/testsuite/gfortran.dg/goacc/kernels-alias-4.f95
@@ -5,8 +5,8 @@
 program main
   implicit none
   integer, parameter :: n = 2
-  integer, target, dimension (0:n-1) :: a
-  integer, pointer :: ptr(:)
+  integer, target, dimension (0:n-1):: a
+  integer, pointer, contiguous :: ptr(:)
   ptr => a
 
   !$acc kernels pcopyin (a, ptr(0:2))

Re: [openacc] add a warning for non-contiguous data clauses

[Jakub Jelinek]

On Tue, Aug 02, 2016 at 09:16:00PM -0700, Cesar Philippidis wrote:
> 2016-08-02  Cesar Philippidis  <cesar@codesourcery.com>
> 
> 	gcc/fortran/
> 	* openmp.c (resolve_oacc_data_clauses): Emit a warning about a
> 	potentially non-contiguous array pointer.
> 	(resolve_omp_clauses): Extend coverage of OpenACC data clause
> 	validation.
> 	* trans-openmp.c (gfc_omp_privatize_by_reference): Use the
> 	TREE_VISITED bit on decl to determine if decl is used inside an
> 	OpenACC region.  If so, emit a warning whenever decl is an array
> 	pointer.
> 
> 	gcc/
> 	* gimplify.c (oacc_default_clause): Use TREE_VISITED to denote that
> 	decl is used inside an OpenACC region.
> 
> 	gcc/testsuite/
> 	* gfortran.dg/goacc/contiguous.f90: New test.
> 	* gfortran.dg/goacc/kernels-alias-4.f95:
> 
> diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
> index e463df7..f3ea9d8 100644
> --- a/gcc/fortran/openmp.c
> +++ b/gcc/fortran/openmp.c
> @@ -3268,6 +3268,10 @@ resolve_oacc_data_clauses (gfc_symbol *sym, locus loc, const char *name)
>  	  && CLASS_DATA (sym)->attr.allocatable))
>      gfc_error ("ALLOCATABLE object %qs of polymorphic type "
>  	       "in %s clause at %L", sym->name, name, &loc);
> +  if (sym->as && sym->as->type == AS_DEFERRED && sym->attr.pointer
> +      && !sym->attr.contiguous)
> +    gfc_warning (0, "Potentially noncontiguous deferred shape array %qs in %s "
> +		 "clause at %L", sym->name, name, &loc);
>    check_symbol_not_pointer (sym, loc, name);
>    check_array_not_assumed (sym, loc, name);
>  }

That doesn't make sense.
From what I can see, the OpenACC 2.0 standard refers to Fortran 2003, which
doesn't have CONTIGUOUS attribute.  That attribute has been only introduced
in Fortran 2008, IMNSHO you can't force OpenACC users even through such
warnings (which can't be disabled except for -w even) to use newer Fortran
standard in their sources.
I'm pretty sure the standard requires something similar to OpenMP, i.e. that
the array sections are contiguous.  The attribute just asserts something is
contiguous, it doesn't mean if the attribute is missing, it isn't
contiguous.

So, IMNSHO if you want to warn about something, only warn when you provably
detect at compile time a non-contiguous array section.

	Jakub