As mentioned in the parent email, this patch adds diagnostics for
probably-broken code that updates (host/device) or copies-out blocks
that still have attached pointers.  Several new tests have been added.

OK?

Julian

ChangeLog

        libgomp/
        * oacc-mem.c (update_dev_host): Raise error on update of block with
        attached pointers.
        (goacc_exit_data_internal): Raise error on copyout of block with
        attached pointers.
        * target.c (gomp_unmap_vars_internal): Likewise.
        * testsuite/libgomp.oacc-c-c++-common/copyback-attached-dynamic-1.c:
        New test.
        * testsuite/libgomp.oacc-c-c++-common/delete-attached-dynamic-1.c:
        New test.
        * testsuite/libgomp.oacc-c-c++-common/copyback-attached-structural-1.c:
        New test.
        * testsuite/libgomp.oacc-c-c++-common/copyback-attached-structural-2.c:
        New test.
        * testsuite/libgomp.oacc-c-c++-common/copyback-attached-structural-3.c:
        New test.
        * testsuite/libgomp.oacc-c-c++-common/delete-attached-structural-1.c:
        New test.
        * testsuite/libgomp.oacc-c-c++-common/update-attached.c: New test.
        * testsuite/libgomp.oacc-fortran/deep-copy-6-no_finalize.F90: Update
        for new diagnostic.
---
 libgomp/oacc-mem.c                            | 42 ++++++++++++++++---
 libgomp/target.c                              | 27 +++++++++---
 .../copyback-attached-dynamic-1.c             | 31 ++++++++++++++
 .../copyback-attached-structural-1.c          | 30 +++++++++++++
 .../copyback-attached-structural-2.c          | 31 ++++++++++++++
 .../copyback-attached-structural-3.c          | 26 ++++++++++++
 .../delete-attached-dynamic-1.c               | 26 ++++++++++++
 .../delete-attached-structural-1.c            | 25 +++++++++++
 .../delete-attached-structural-2.c            | 26 ++++++++++++
 .../update-attached-1.c                       | 33 +++++++++++++++
 .../deep-copy-6-no_finalize.F90               |  6 +--
 11 files changed, 290 insertions(+), 13 deletions(-)
 create mode 100644 
libgomp/testsuite/libgomp.oacc-c-c++-common/copyback-attached-dynamic-1.c
 create mode 100644 
libgomp/testsuite/libgomp.oacc-c-c++-common/copyback-attached-structural-1.c
 create mode 100644 
libgomp/testsuite/libgomp.oacc-c-c++-common/copyback-attached-structural-2.c
 create mode 100644 
libgomp/testsuite/libgomp.oacc-c-c++-common/copyback-attached-structural-3.c
 create mode 100644 
libgomp/testsuite/libgomp.oacc-c-c++-common/delete-attached-dynamic-1.c
 create mode 100644 
libgomp/testsuite/libgomp.oacc-c-c++-common/delete-attached-structural-1.c
 create mode 100644 
libgomp/testsuite/libgomp.oacc-c-c++-common/delete-attached-structural-2.c
 create mode 100644 
libgomp/testsuite/libgomp.oacc-c-c++-common/update-attached-1.c

diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c
index 1816b06bf2d..cf054f14b12 100644
--- a/libgomp/oacc-mem.c
+++ b/libgomp/oacc-mem.c
@@ -865,6 +865,23 @@ update_dev_host (int is_dev, void *h, size_t s, int async)
       gomp_fatal ("[%p,%d] is not mapped", h, (int)s);
     }
 
+  if (n->aux && n->aux->attach_count)
+    {
+      size_t nptrs = (n->host_end - n->host_start + sizeof (void *) - 1)
+                    / sizeof (void *);
+      for (size_t i = 0; i < nptrs; i++)
+       if (n->aux->attach_count[i] > 0)
+         {
+           gomp_mutex_unlock (&acc_dev->lock);
+           if (is_dev)
+             gomp_fatal ("[%p,+%d] device update would overwrite attached "
+                         "pointers", h, (int) s);
+           else
+             gomp_fatal ("host update from block [%p,+%d] with attached "
+                         "pointers", h, (int) s);
+         }
+    }
+
   d = (void *) (n->tgt->tgt_start + n->tgt_offset
                + (uintptr_t) h - n->host_start);
 
@@ -1329,11 +1346,26 @@ goacc_exit_data_internal (struct gomp_device_descr 
*acc_dev, size_t mapnum,
            if (copyfrom
                && n->refcount != REFCOUNT_INFINITY
                && (kind != GOMP_MAP_FROM || n->refcount == 0))
-             gomp_copy_dev2host (acc_dev, aq, (void *) cur_node.host_start,
-                                 (void *) (n->tgt->tgt_start + n->tgt_offset
-                                           + cur_node.host_start
-                                           - n->host_start),
-                                 cur_node.host_end - cur_node.host_start);
+             {
+               if (n->aux && n->aux->attach_count)
+                 {
+                   size_t nptrs = (n->host_end - n->host_start
+                                   + sizeof (void *) - 1) / sizeof (void *);
+                   for (size_t j = 0; j < nptrs; j++)
+                     if (n->aux->attach_count[j] > 0)
+                       {
+                         gomp_mutex_unlock (&acc_dev->lock);
+                         gomp_fatal ("copyout of block [%p,+%d] with "
+                                     "attached pointers", hostaddrs[i],
+                                     (int) size);
+                       }
+                 }
+               gomp_copy_dev2host (acc_dev, aq, (void *) cur_node.host_start,
+                                   (void *) (n->tgt->tgt_start + n->tgt_offset
+                                             + cur_node.host_start
+                                             - n->host_start),
+                                   cur_node.host_end - cur_node.host_start);
+             }
 
            if (n->refcount == 0)
              {
diff --git a/libgomp/target.c b/libgomp/target.c
index badc254a777..db6f56a8ff8 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -1649,11 +1649,28 @@ gomp_unmap_vars_internal (struct target_mem_desc *tgt, 
bool do_copyfrom,
 
       if ((do_unmap && do_copyfrom && tgt->list[i].copy_from)
          || tgt->list[i].always_copy_from)
-       gomp_copy_dev2host (devicep, aq,
-                           (void *) (k->host_start + tgt->list[i].offset),
-                           (void *) (k->tgt->tgt_start + k->tgt_offset
-                                     + tgt->list[i].offset),
-                           tgt->list[i].length);
+       {
+         if (k->aux && k->aux->attach_count)
+           {
+             size_t nptrs = (k->host_end - k->host_start
+                             + sizeof (void *) - 1) / sizeof (void *);
+             for (size_t j = 0; j < nptrs; j++)
+               if (k->aux->attach_count[j] > 0)
+                 {
+                   gomp_mutex_unlock (&devicep->lock);
+                   gomp_fatal ("copyout of block [%p,+%d] with "
+                               "attached pointers",
+                               (void *) (k->host_start + tgt->list[i].offset),
+                               (int) (k->host_end - k->host_start));
+                 }
+           }
+         gomp_copy_dev2host (devicep, aq,
+                             (void *) (k->host_start + tgt->list[i].offset),
+                             (void *) (k->tgt->tgt_start + k->tgt_offset
+                                       + tgt->list[i].offset),
+                             tgt->list[i].length);
+       }
+
       if (do_unmap)
        {
          struct target_mem_desc *k_tgt = k->tgt;
diff --git 
a/libgomp/testsuite/libgomp.oacc-c-c++-common/copyback-attached-dynamic-1.c 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/copyback-attached-dynamic-1.c
new file mode 100644
index 00000000000..bc4e297fa6f
--- /dev/null
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/copyback-attached-dynamic-1.c
@@ -0,0 +1,31 @@
+/* { dg-skip-if "" { *-*-* } { "-DACC_MEM_SHARED=1" } } */
+
+#include <stdlib.h>
+
+struct mystruct {
+  int *arr;
+};
+
+int
+main (int argc, char *argv[])
+{
+  int localarray[1024];
+  struct mystruct s;
+  s.arr = localarray;
+
+  #pragma acc enter data copyin(s)
+
+  #pragma acc data copy(s.arr[0:1024])
+  {
+    /* This directive does one too many attachments: it should fail when we try
+       to do the copyout below.  */
+    #pragma acc enter data attach(s.arr)
+    /* { dg-output "copyout of block \\\[0x\[0-9a-f\]+,\\+\[0-9\]+\\\] with 
attached pointers" } */
+  }
+
+  #pragma acc exit data copyout(s)
+
+  return 0;
+}
+
+/* { dg-shouldfail "" } */
diff --git 
a/libgomp/testsuite/libgomp.oacc-c-c++-common/copyback-attached-structural-1.c 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/copyback-attached-structural-1.c
new file mode 100644
index 00000000000..7846c8c717c
--- /dev/null
+++ 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/copyback-attached-structural-1.c
@@ -0,0 +1,30 @@
+/* { dg-skip-if "" { *-*-* } { "-DACC_MEM_SHARED=1" } } */
+
+#include <stdlib.h>
+
+struct mystruct {
+  int *arr;
+};
+
+int
+main (int argc, char *argv[])
+{
+  int localarray[1024];
+  struct mystruct s;
+  s.arr = localarray;
+
+  #pragma acc data copy(s)
+  {
+    #pragma acc data copy(s.arr[0:1024])
+    {
+      /* This directive does one too many attachments: it should fail when we 
try
+        to do the copyout below.  */
+      #pragma acc enter data attach(s.arr)
+      /* { dg-output "copyout of block \\\[0x\[0-9a-f\]+,\\+\[0-9\]+\\\] with 
attached pointers" } */
+    }
+  }
+
+  return 0;
+}
+
+/* { dg-shouldfail "" } */
diff --git 
a/libgomp/testsuite/libgomp.oacc-c-c++-common/copyback-attached-structural-2.c 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/copyback-attached-structural-2.c
new file mode 100644
index 00000000000..bffa06eb725
--- /dev/null
+++ 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/copyback-attached-structural-2.c
@@ -0,0 +1,31 @@
+/* { dg-skip-if "" { *-*-* } { "-DACC_MEM_SHARED=1" } } */
+
+#include <stdlib.h>
+
+struct mystruct {
+  int *arr;
+};
+
+int
+main (int argc, char *argv[])
+{
+  int localarray[1024];
+  struct mystruct s;
+  s.arr = localarray;
+
+  #pragma acc enter data copyin(localarray[0:1024])
+
+  #pragma acc data copy(s)
+  {
+    /* This directive does one too many attachments: it should fail when we try
+       to do the copyout below.  */
+    #pragma acc enter data attach(s.arr)
+    /* { dg-output "copyout of block \\\[0x\[0-9a-f\]+,\\+\[0-9\]+\\\] with 
attached pointers" } */
+  }
+
+  #pragma acc exit data delete(localarray[0:1024])
+
+  return 0;
+}
+
+/* { dg-shouldfail "" } */
diff --git 
a/libgomp/testsuite/libgomp.oacc-c-c++-common/copyback-attached-structural-3.c 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/copyback-attached-structural-3.c
new file mode 100644
index 00000000000..4b21677af09
--- /dev/null
+++ 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/copyback-attached-structural-3.c
@@ -0,0 +1,26 @@
+#include <stdlib.h>
+
+struct mystruct {
+  int *arr;
+};
+
+int
+main (int argc, char *argv[])
+{
+  int localarray[1024];
+  struct mystruct s;
+  s.arr = localarray;
+
+  #pragma acc enter data copyin(localarray[0:1024])
+
+  #pragma acc data copy(s)
+  {
+    /* Here the attach and detach balance: this should work.  */
+    #pragma acc enter data attach(s.arr)
+    #pragma acc exit data detach(s.arr)
+  }
+
+  #pragma acc exit data delete(localarray[0:1024])
+
+  return 0;
+}
diff --git 
a/libgomp/testsuite/libgomp.oacc-c-c++-common/delete-attached-dynamic-1.c 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/delete-attached-dynamic-1.c
new file mode 100644
index 00000000000..e074d507fb2
--- /dev/null
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/delete-attached-dynamic-1.c
@@ -0,0 +1,26 @@
+#include <stdlib.h>
+
+struct mystruct {
+  int *arr;
+};
+
+int
+main (int argc, char *argv[])
+{
+  int localarray[1024];
+  struct mystruct s;
+  s.arr = localarray;
+
+  #pragma acc enter data copyin(s)
+
+  #pragma acc data copy(s.arr[0:1024])
+  {
+    /* We delete 's' from the target below: this extra attachment is not
+       dangerous and we do not raise an error.  */
+    #pragma acc enter data attach(s.arr)
+  }
+
+  #pragma acc exit data delete(s)
+
+  return 0;
+}
diff --git 
a/libgomp/testsuite/libgomp.oacc-c-c++-common/delete-attached-structural-1.c 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/delete-attached-structural-1.c
new file mode 100644
index 00000000000..e675762ecd8
--- /dev/null
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/delete-attached-structural-1.c
@@ -0,0 +1,25 @@
+#include <stdlib.h>
+
+struct mystruct {
+  int *arr;
+};
+
+int
+main (int argc, char *argv[])
+{
+  int localarray[1024];
+  struct mystruct s;
+  s.arr = localarray;
+
+  #pragma acc data copyin(s)
+  {
+    #pragma acc data copy(s.arr[0:1024])
+    {
+      /* This directive does one too many attachments: it should fail when we 
try
+        to do the copyout below.  */
+      #pragma acc enter data attach(s.arr)
+    }
+  }
+
+  return 0;
+}
diff --git 
a/libgomp/testsuite/libgomp.oacc-c-c++-common/delete-attached-structural-2.c 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/delete-attached-structural-2.c
new file mode 100644
index 00000000000..d2095255ad3
--- /dev/null
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/delete-attached-structural-2.c
@@ -0,0 +1,26 @@
+#include <stdlib.h>
+
+struct mystruct {
+  int *arr;
+};
+
+int
+main (int argc, char *argv[])
+{
+  int localarray[1024];
+  struct mystruct s;
+  s.arr = localarray;
+
+  #pragma acc enter data copyin(localarray[0:1024])
+
+  #pragma acc data copyin(s)
+  {
+    /* We only try to copy in: the extra attachment we're left over with is not
+       harmful and we don't raise an error.  */
+    #pragma acc enter data attach(s.arr)
+  }
+
+  #pragma acc exit data delete(localarray[0:1024])
+
+  return 0;
+}
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/update-attached-1.c 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/update-attached-1.c
new file mode 100644
index 00000000000..9f60bfa56f4
--- /dev/null
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/update-attached-1.c
@@ -0,0 +1,33 @@
+/* { dg-skip-if "" { *-*-* } { "-DACC_MEM_SHARED=1" } } */
+
+#include <stdlib.h>
+
+struct mystruct {
+  int *arr;
+};
+
+int
+main (int argc, char *argv[])
+{
+  int localarray[1024];
+  int localarray2[1024];
+  struct mystruct s;
+  s.arr = localarray;
+
+  #pragma acc enter data copyin(s)
+
+  #pragma acc data copy(s.arr[0:1024])
+  {
+    s.arr = localarray2;
+    /* This update is dangerous because we have attached pointers: raise an
+       error.  */
+    #pragma acc update device(s)
+    /* { dg-output "\\\[0x\[0-9a-f\]+,\\+\[0-9\]+\\\] device update would 
overwrite attached pointers" } */
+  }
+
+  #pragma acc exit data delete(s)
+
+  return 0;
+}
+
+/* { dg-shouldfail "" } */
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/deep-copy-6-no_finalize.F90 
b/libgomp/testsuite/libgomp.oacc-fortran/deep-copy-6-no_finalize.F90
index ad8da71d7c9..355a381b625 100644
--- a/libgomp/testsuite/libgomp.oacc-fortran/deep-copy-6-no_finalize.F90
+++ b/libgomp/testsuite/libgomp.oacc-fortran/deep-copy-6-no_finalize.F90
@@ -8,7 +8,7 @@
 ! { dg-output ".*CheCKpOInT1(\n|\r\n|\r)" }
 ! { dg-output ".*CheCKpOInT2(\n|\r\n|\r)" }
 
-! Without the finalize, we do not detach properly so the host sees a device
-! pointer, and fails with this STOP code.
-! { dg-output "STOP 7(\n|\r\n|\r)+" }
+! Without the finalize, we do not detach properly and raise an error on 
attempting
+! the copyout.
+! { dg-output ".*copyout of block \\\[0x\[0-9a-f\]+,\\+\[0-9\]+\\\] with 
attached pointers(\n|\r\n|\r)+" }
 ! { dg-shouldfail "" }
-- 
2.23.0

Reply via email to