Hi Julian,
the gfortran part is rather obvious and it and the test case look fine
to me → OK.
The oacc-mem.c also looks okay, but I assume Thomas needs to
rubber-stamp it.
Tobias
On 1/10/20 2:49 AM, Julian Brown wrote:
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.
component-mappings-derived-types-1.diff
commit 5e9d8846bbaa33a9bdb08adcf1ee9f224a8e8fc0
Author: Julian Brown<jul...@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:
