[PATCH] Fix component mappings with derived types for OpenACC

[Julian Brown <julian@codesourcery.com>]

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

Hi,

This patch fixes a bug with mapping Fortran components (i.e. with the
manual deep-copy support) which themselves have derived types. I've
also added a couple of new tests to make sure such mappings are lowered
correctly, and to check for the case that Tobias found in the message:

  https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00215.html

The previous incorrect mapping was causing the error condition mentioned
in that message to fail to trigger, and I think (my!) code in libgomp
(goacc_exit_data_internal) to handle GOMP_MAP_STRUCT specially was
papering over the bad mapping also. Oops!

I haven't attempted to implement the (harder) sub-copy detection, if
that is indeed supposed to be forbidden by the spec. This patch should
get us to the same behaviour in Fortran as in C & C++ though.

Tested with offloading to nvptx, also with the (uncommitted)
reference-count self-checking patch enabled.

OK?

Thanks,

Julian

ChangeLog

    gcc/fortran/
    * trans-openmp.c (gfc_trans_omp_clauses): Use inner not decl for
    components with derived types.

    gcc/testsuite/
    * gfortran.dg/goacc/mapping-tests-3.f90: New test.
    * gfortran.dg/goacc/mapping-tests-4.f90: New test.

    libgomp/
    * oacc-mem.c (goacc_exit_data_internal): Remove special (no-copyback)
    behaviour for GOMP_MAP_STRUCT.

[-- Attachment #2: component-mappings-derived-types-1.diff --]
[-- Type: text/x-patch, Size: 3656 bytes --]

commit 5e9d8846bbaa33a9bdb08adcf1ee9f224a8e8fc0
Author: Julian Brown <julian@codesourcery.com>
Date:   Wed Jan 8 15:57:46 2020 -0800

    Fix component mappings with derived types for OpenACC
    
            gcc/fortran/
            * trans-openmp.c (gfc_trans_omp_clauses): Use inner not decl for
            components with derived types.
    
            gcc/testsuite/
            * gfortran.dg/goacc/mapping-tests-3.f90: New test.
            * gfortran.dg/goacc/mapping-tests-4.f90: New test.
    
            libgomp/
            * oacc-mem.c (goacc_exit_data_internal): Remove special (no-copyback)
            behaviour for GOMP_MAP_STRUCT.

diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index 9835d2aae3c..efc392d7fa6 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -2783,9 +2783,9 @@ gfc_trans_omp_clauses (stmtblock_t *block, gfc_omp_clauses *clauses,
 			}
 		      else
 			{
-			  OMP_CLAUSE_DECL (node) = decl;
+			  OMP_CLAUSE_DECL (node) = inner;
 			  OMP_CLAUSE_SIZE (node)
-			    = TYPE_SIZE_UNIT (TREE_TYPE (decl));
+			    = TYPE_SIZE_UNIT (TREE_TYPE (inner));
 			}
 		    }
 		  else if (lastcomp->next
diff --git a/gcc/testsuite/gfortran.dg/goacc/mapping-tests-3.f90 b/gcc/testsuite/gfortran.dg/goacc/mapping-tests-3.f90
new file mode 100644
index 00000000000..312f596461e
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/mapping-tests-3.f90
@@ -0,0 +1,15 @@
+! { dg-options "-fopenacc -fdump-tree-omplower" }
+
+subroutine foo
+  type one
+    integer i, j
+  end type
+  type two
+    type(one) A, B
+  end type
+
+  type(two) x
+
+  !$acc enter data copyin(x%A)
+! { dg-final { scan-tree-dump-times "omp target oacc_enter_exit_data map\\(struct:x \\\[len: 1\\\]\\) map\\(to:x.a \\\[len: \[0-9\]+\\\]\\)" 1 "omplower" } }
+end
diff --git a/gcc/testsuite/gfortran.dg/goacc/mapping-tests-4.f90 b/gcc/testsuite/gfortran.dg/goacc/mapping-tests-4.f90
new file mode 100644
index 00000000000..6257af942df
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/mapping-tests-4.f90
@@ -0,0 +1,17 @@
+subroutine foo
+  type one
+    integer i, j
+  end type
+  type two
+    type(one) A, B
+  end type
+
+  type(two) x
+
+! This is accepted at present, although it represents a probably-unintentional
+! overlapping subcopy.
+  !$acc enter data copyin(x%A, x%A%i)
+! But this raises an error.
+  !$acc enter data copyin(x%A, x%A%i, x%A%i)
+! { dg-error ".x.a.i. appears more than once in map clauses" "" { target "*-*-*" } 15 }
+end
diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c
index 2d4bba78efd..232683a85f0 100644
--- a/libgomp/oacc-mem.c
+++ b/libgomp/oacc-mem.c
@@ -1136,38 +1136,6 @@ goacc_exit_data_internal (struct gomp_device_descr *acc_dev, size_t mapnum,
 	  break;
 
 	case GOMP_MAP_STRUCT:
-	  {
-	    int elems = sizes[i];
-	    for (int j = 1; j <= elems; j++)
-	      {
-		struct splay_tree_key_s k;
-		k.host_start = (uintptr_t) hostaddrs[i + j];
-		k.host_end = k.host_start + sizes[i + j];
-		splay_tree_key str;
-		str = splay_tree_lookup (&acc_dev->mem_map, &k);
-		if (str)
-		  {
-		    if (finalize)
-		      {
-			if (str->refcount != REFCOUNT_INFINITY)
-			  str->refcount -= str->virtual_refcount;
-			str->virtual_refcount = 0;
-		      }
-		    if (str->virtual_refcount > 0)
-		      {
-			if (str->refcount != REFCOUNT_INFINITY)
-			  str->refcount--;
-			str->virtual_refcount--;
-		      }
-		    else if (str->refcount > 0
-			     && str->refcount != REFCOUNT_INFINITY)
-		      str->refcount--;
-		    if (str->refcount == 0)
-		      gomp_remove_var_async (acc_dev, str, aq);
-		  }
-	      }
-	    i += elems;
-	  }
 	  break;
 
 	default: