Hi Kwok!
On Wed, 30 Jan 2019 22:18:43 +0000, Kwok Cheung Yeung <k...@codesourcery.com>
wrote:
> A non-present passed-by-reference Fortran optional argument is
> represented by a null pointer. When passed to an update directive, it
> should be ignored as variable mappings are not created for null
> pointers. This should be safe as it is not possible to change a
> non-present argument into a present one (or vice-versa) in Fortran.
>
> libgomp/
> * oacc-mem.c (update_dev_host): Return early if the host address
> is NULL.
>
> Reviewed-by: Julian Brown <jul...@codesourcery.com>
> Reviewed-by: Thomas Schwinge <tho...@codesourcery.com>
As I'd mentioned a few weeks ago internally, this change:
> --- a/libgomp/oacc-mem.c
> +++ b/libgomp/oacc-mem.c
> @@ -819,6 +819,12 @@ update_dev_host (int is_dev, void *h, size_t s, int
> async)
> if (acc_dev->capabilities & GOMP_OFFLOAD_CAP_SHARED_MEM)
> return;
>
> + /* Fortran optional arguments that are non-present result in a
> + null host address here. This can safely be ignored as it is
> + not possible to 'update' a non-present optional argument. */
> + if (h == NULL)
> + return;
... doesn't just affect the Fortran "update" directive but also the C/C++
one, and the "acc_update_device", "acc_update_self" runtime library
functions, and thus invalidates the C/C++
"libgomp.oacc-c-c++-common/lib-43.c", and
"libgomp.oacc-c-c++-common/lib-47.c" test cases, so these will have to be
adjusted (that is, removed). I pushed the attached to
openacc-gcc-8-branch in commit 745d3a19c63ec9c1de3f44e0dd5ee3ff126e2828
"Remove libgomp.oacc-c-c++-common/lib-43.c,
libgomp.oacc-c-c++-common/lib-47.c".
For avoidance of doubt: I'm not objecting to your code change. On
contrary, I do have an issue open to verify whether a host-NULL should
actually be accepted in even more places, as a no-op. But we'll have to
review/adjust/test all the code, not just "update_dev_host". (That's for
later, of course.)
A (very) quick check with the PGI compiler shows that they're also
handling such NULL pointers as no-ops.
Grüße
Thomas
>From 745d3a19c63ec9c1de3f44e0dd5ee3ff126e2828 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tho...@codesourcery.com>
Date: Thu, 31 Jan 2019 18:08:09 +0100
Subject: [PATCH] Remove libgomp.oacc-c-c++-common/lib-43.c,
libgomp.oacc-c-c++-common/lib-47.c
They're invalid after commit 550afd2b2b47091f43780f0fbf5ead87f73d9b1e "[og8]
Allow NULL for update directives in OpenACC 2.6".
libgomp/
* testsuite/libgomp.oacc-c-c++-common/lib-43.c: Remove.
* testsuite/libgomp.oacc-c-c++-common/lib-47.c: Likewise.
---
libgomp/ChangeLog.openacc | 3 ++
.../libgomp.oacc-c-c++-common/lib-43.c | 51 -------------------
.../libgomp.oacc-c-c++-common/lib-47.c | 49 ------------------
3 files changed, 3 insertions(+), 100 deletions(-)
delete mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/lib-43.c
delete mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/lib-47.c
diff --git a/libgomp/ChangeLog.openacc b/libgomp/ChangeLog.openacc
index d558f0df97b..edb7d3f76eb 100644
--- a/libgomp/ChangeLog.openacc
+++ b/libgomp/ChangeLog.openacc
@@ -1,5 +1,8 @@
2019-01-31 Thomas Schwinge <tho...@codesourcery.com>
+ * testsuite/libgomp.oacc-c-c++-common/lib-43.c: Remove.
+ * testsuite/libgomp.oacc-c-c++-common/lib-47.c: Likewise.
+
* testsuite/libgomp.oacc-c-c++-common/avoid-offloading-1.c:
Update.
* testsuite/libgomp.oacc-c-c++-common/avoid-offloading-2.c:
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-43.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-43.c
deleted file mode 100644
index 5db29124e9e..00000000000
--- a/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-43.c
+++ /dev/null
@@ -1,51 +0,0 @@
-/* Exercise acc_update_device with a NULL data address on nvidia targets. */
-
-/* { dg-do run { target openacc_nvidia_accel_selected } } */
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <openacc.h>
-
-int
-main (int argc, char **argv)
-{
- const int N = 256;
- int i;
- unsigned char *h;
- void *d;
-
- h = (unsigned char *) malloc (N);
-
- for (i = 0; i < N; i++)
- {
- h[i] = i;
- }
-
- d = acc_copyin (h, N);
- if (!d)
- abort ();
-
- for (i = 0; i < N; i++)
- {
- h[i] = 0xab;
- }
-
- fprintf (stderr, "CheCKpOInT\n");
- acc_update_device (0, N);
-
- acc_copyout (h, N);
-
- for (i = 0; i < N; i++)
- {
- if (h[i] != 0xab)
- abort ();
- }
-
- free (h);
-
- return 0;
-}
-
-/* { dg-output "CheCKpOInT(\n|\r\n|\r).*" } */
-/* { dg-output "\\\[\[^\n\r]*,256\\\] is not mapped" } */
-/* { dg-shouldfail "" } */
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-47.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-47.c
deleted file mode 100644
index c2140429cb1..00000000000
--- a/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-47.c
+++ /dev/null
@@ -1,49 +0,0 @@
-/* Exercise acc_update_self with a NULL data mapping on nvidia targets. */
-
-/* { dg-do run { target openacc_nvidia_accel_selected } } */
-
-#include <stdio.h>
-#include <string.h>
-#include <stdlib.h>
-#include <openacc.h>
-
-int
-main (int argc, char **argv)
-{
- const int N = 256;
- int i;
- unsigned char *h;
- void *d;
-
- h = (unsigned char *) malloc (N);
-
- for (i = 0; i < N; i++)
- {
- h[i] = i;
- }
-
- d = acc_copyin (h, N);
- if (!d)
- abort ();
-
- memset (&h[0], 0, N);
-
- fprintf (stderr, "CheCKpOInT\n");
- acc_update_self (0, N);
-
- for (i = 0; i < N; i++)
- {
- if (h[i] != i)
- abort ();
- }
-
- acc_delete (h, N);
-
- free (h);
-
- return 0;
-}
-
-/* { dg-output "CheCKpOInT(\n|\r\n|\r).*" } */
-/* { dg-output "\\\[\[^\n\r]*,256\\\] is not mapped" } */
-/* { dg-shouldfail "" } */
--
2.17.1
