public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
From: Cesar Philippidis <cesar@codesourcery.com>
To: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
	Fortran List	<fortran@gcc.gnu.org>,
	Jakub Jelinek <jakub@redhat.com>
Subject: [openacc] add a warning for non-contiguous data clauses
Date: Wed, 03 Aug 2016 04:16:00 -0000	[thread overview]
Message-ID: <57A17000.5030704@codesourcery.com> (raw)

[-- 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))

             reply	other threads:[~2016-08-03  4:16 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-03  4:16 Cesar Philippidis [this message]
2016-08-09 15:28 ` Jakub Jelinek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57A17000.5030704@codesourcery.com \
    --to=cesar@codesourcery.com \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).