Re: [PATCH, OpenACC 2.5, libgomp] Add *_async versions of runtime library API functions

[Thomas Schwinge <thomas@codesourcery.com>]

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

Hi!

On 2021-06-08T19:32:22+0200, I wrote:
> Hi Chung-Lin!
>
> ;-) It's been a while:
>
> On 2018-09-10T23:04:18+0800, Chung-Lin Tang <chunglin_tang@mentor.com> wrote:
>>          * testsuite/libgomp.oacc-c-c++-common/lib-94.c: New test.
>>          * testsuite/libgomp.oacc-c-c++-common/lib-95.c: New test.
>>          * testsuite/libgomp.oacc-fortran/lib-16.f90: New test.
>
> Do you happen to remember why in these testcases you're using the
> following pattern:

Apparently not ;-) -- no answer/objection, I've thus now pushed
"Fix OpenACC 'async'/'wait' issues in
'libgomp.oacc-c-c++-common/lib-{94,95}.c',
'libgomp.oacc-fortran/lib-16{,-2}.f90'" to master branch in
commit 599e275d7e0b3fb79ff704d4cb2d8fdb0231116e, see attached.


Grüße
 Thomas


>> --- libgomp/testsuite/libgomp.oacc-c-c++-common/lib-94.c     (nonexistent)
>> +++ libgomp/testsuite/libgomp.oacc-c-c++-common/lib-94.c     (working copy)
>> @@ -0,0 +1,42 @@
>> +/* { dg-do run } */
>> +/* { dg-skip-if "" { *-*-* } { "*" } { "-DACC_MEM_SHARED=0" } } */
>> +
>> +#include <string.h>
>> +#include <stdlib.h>
>> +#include <openacc.h>
>> +
>> +int
>> +main (int argc, char **argv)
>> +{
>> +  const int N = 256;
>> +  int i;
>> +  int async = 8;
>> +  unsigned char *h;
>> +
>> +  h = (unsigned char *) malloc (N);
>> +
>> +  for (i = 0; i < N; i++)
>> +    {
>> +      h[i] = i;
>> +    }
>> +
>> +  acc_copyin_async (h, N, async);
>> +
>> +  memset (h, 0, N);
>> +
>> +  acc_wait (async);
>
> You first issue 'acc_copyin_async', then (while potentially that's still
> accessing 'h') already 'memset' 'h' (potentially overwriting data that
> 'acc_copyin_async' is still working on), and only then 'acc_wait'?
>
> My understanding of OpenACC would swap 'memset' and 'acc_wait', but maybe
> you have a specific reason to do it in this way?
>
> In particular, the GCC nvptx offloading implementation "doesn't seem to
> care" (as discussed elsewhere; 'OpenACC "ephemeral" asynchronous
> host-to-device copies', etc.) -- but I suppose if you meant to test such
> implementation traits here, you'd have commented that?
>
>> +
>> +  acc_copyout_async (h, N, async + 1);
>> +
>> +  acc_wait (async + 1);
>> +
>> +  for (i = 0; i < N; i++)
>> +    {
>> +      if (h[i] != i)
>> +    abort ();
>> +    }
>> +
>> +  free (h);
>> +
>> +  return 0;
>> +}
>
>> --- libgomp/testsuite/libgomp.oacc-c-c++-common/lib-95.c     (nonexistent)
>> +++ libgomp/testsuite/libgomp.oacc-c-c++-common/lib-95.c     (working copy)
>> @@ -0,0 +1,45 @@
>> +/* { dg-do run } */
>> +/* { dg-skip-if "" { *-*-* } { "*" } { "-DACC_MEM_SHARED=0" } } */
>> +
>> +#include <string.h>
>> +#include <stdlib.h>
>> +#include <openacc.h>
>> +
>> +int
>> +main (int argc, char **argv)
>> +{
>> +  const int N = 256;
>> +  int i, q = 5;
>> +  unsigned char *h, *g;
>> +  void *d;
>> +
>> +  h = (unsigned char *) malloc (N);
>> +  g = (unsigned char *) malloc (N);
>> +  for (i = 0; i < N; i++)
>> +    {
>> +      g[i] = i;
>> +    }
>> +
>> +  acc_create_async (h, N, q);
>> +
>> +  acc_memcpy_to_device_async (acc_deviceptr (h), g, N, q);
>> +  memset (&h[0], 0, N);
>> +
>> +  acc_wait (q);
>
> Similar here.
>
>> +  acc_update_self_async (h, N, q + 1);
>> +  acc_delete_async (h, N, q + 1);
>> +
>> +  acc_wait (q + 1);
>> +
>> +  for (i = 0; i < N; i++)
>> +    {
>> +      if (h[i] != i)
>> +    abort ();
>> +    }
>> +
>> +  free (h);
>> +  free (g);
>> +
>> +  return 0;
>> +}
>
>> --- libgomp/testsuite/libgomp.oacc-fortran/lib-16.f90        (nonexistent)
>> +++ libgomp/testsuite/libgomp.oacc-fortran/lib-16.f90        (working copy)
>
> (Later also similarly copied into 'libgomp.oacc-fortran/lib-16-2.f90'.)
>
> Similar:
>
>> @@ -0,0 +1,57 @@
>> +! { dg-do run }
>> +! { dg-skip-if "" { *-*-* } { "*" } { "-DACC_MEM_SHARED=0" } }
>> +
>> +program main
>> +  use openacc
>> +  implicit none
>> +
>> +  integer, parameter :: N = 256
>> +  integer, allocatable :: h(:)
>> +  integer :: i
>> +  integer :: async = 5
>> +
>> +  allocate (h(N))
>> +
>> +  do i = 1, N
>> +    h(i) = i
>> +  end do
>> +
>> +  call acc_copyin (h)
>> +
>> +  do i = 1, N
>> +    h(i) = i + i
>> +  end do
>> +
>> +  call acc_update_device_async (h, sizeof (h), async)
>> +
>> +  if (acc_is_present (h) .neqv. .TRUE.) call abort
>
> Don't we need 'acc_wait' here (while 'acc_update_device_async' may still
> be reading from 'h'), before overwriting 'h' here:
>
>> +
>> +  h(:) = 0
>> +
>> +  call acc_copyout_async (h, sizeof (h), async)
>> +
>> +  call acc_wait (async)
>> +
>> +  do i = 1, N
>> +    if (h(i) /= i + i) call abort
>> +  end do
>> +
>> +  call acc_copyin (h, sizeof (h))
>> +
>> +  h(:) = 0
>> +
>> +  call acc_update_self_async (h, sizeof (h), async)
>> +
>> +  if (acc_is_present (h) .neqv. .TRUE.) call abort
>
> Don't we need 'acc_wait' here (to make sure we finish device to host copy
> of 'h'), before evaluating 'h' here:
>
>> +
>> +  do i = 1, N
>> +    if (h(i) /= i + i) call abort
>> +  end do
>> +
>> +  call acc_delete_async (h, async)
>> +
>> +  call acc_wait (async)
>> +
>> +  if (acc_is_present (h) .neqv. .FALSE.) call abort
>> +
>> +end program
>
> Julian has patches for most of these (as part of other commits).


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-OpenACC-async-wait-issues-in-libgomp.oacc-c-c-co.patch --]
[-- Type: text/x-diff, Size: 3375 bytes --]

From 599e275d7e0b3fb79ff704d4cb2d8fdb0231116e Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Tue, 8 Jun 2021 19:32:22 +0200
Subject: [PATCH] Fix OpenACC 'async'/'wait' issues in
 'libgomp.oacc-c-c++-common/lib-{94,95}.c',
 'libgomp.oacc-fortran/lib-16{,-2}.f90'

Fix-up for r265842 (commit 58168bbf6f8fb456280cca13343a498ad94878c7)
"[OpenACC 2.5, libgomp] Add *_async versions of runtime library API functions".

	libgomp/
	* testsuite/libgomp.oacc-c-c++-common/lib-94.c: Fix OpenACC
	'async'/'wait' issue.
	* testsuite/libgomp.oacc-c-c++-common/lib-95.c: Likewise.
	* testsuite/libgomp.oacc-fortran/lib-16-2.f90: Likewise.
	* testsuite/libgomp.oacc-fortran/lib-16.f90: Likewise.

Co-Authored-By: Julian Brown <julian@codesourcery.com>
---
 libgomp/testsuite/libgomp.oacc-c-c++-common/lib-94.c | 4 ++--
 libgomp/testsuite/libgomp.oacc-c-c++-common/lib-95.c | 3 ++-
 libgomp/testsuite/libgomp.oacc-fortran/lib-16-2.f90  | 4 ++++
 libgomp/testsuite/libgomp.oacc-fortran/lib-16.f90    | 4 ++++
 4 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-94.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-94.c
index 54497237b0c..baa3ac83f04 100644
--- a/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-94.c
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-94.c
@@ -22,10 +22,10 @@ main (int argc, char **argv)
 
   acc_copyin_async (h, N, async);
 
-  memset (h, 0, N);
-
   acc_wait (async);
 
+  memset (h, 0, N);
+
   acc_copyout_async (h, N, async + 1);
 
   acc_wait (async + 1);
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-95.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-95.c
index 85b238d78c8..842fb849e79 100644
--- a/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-95.c
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-95.c
@@ -23,10 +23,11 @@ main (int argc, char **argv)
   acc_create_async (h, N, q);
 
   acc_memcpy_to_device_async (acc_deviceptr (h), g, N, q);
-  memset (&h[0], 0, N);
 
   acc_wait (q);
 
+  memset (h, 0, N);
+
   acc_update_self_async (h, N, q + 1);
   acc_delete_async (h, N, q + 1);
 
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/lib-16-2.f90 b/libgomp/testsuite/libgomp.oacc-fortran/lib-16-2.f90
index ddd557d3be0..2be75dca98c 100644
--- a/libgomp/testsuite/libgomp.oacc-fortran/lib-16-2.f90
+++ b/libgomp/testsuite/libgomp.oacc-fortran/lib-16-2.f90
@@ -27,6 +27,8 @@ program main
 
   if (acc_is_present (h) .neqv. .TRUE.) stop 1
 
+  call acc_wait (async)
+
   h(:) = 0
 
   call acc_copyout_async (h, sizeof (h), async)
@@ -45,6 +47,8 @@ program main
   
   if (acc_is_present (h) .neqv. .TRUE.) stop 3
 
+  call acc_wait (async)
+
   do i = 1, N
     if (h(i) /= i + i) stop 4
   end do 
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/lib-16.f90 b/libgomp/testsuite/libgomp.oacc-fortran/lib-16.f90
index ccd1ce6ee18..fae0d1031ed 100644
--- a/libgomp/testsuite/libgomp.oacc-fortran/lib-16.f90
+++ b/libgomp/testsuite/libgomp.oacc-fortran/lib-16.f90
@@ -27,6 +27,8 @@ program main
 
   if (acc_is_present (h) .neqv. .TRUE.) stop 1
 
+  call acc_wait (async)
+
   h(:) = 0
 
   call acc_copyout_async (h, sizeof (h), async)
@@ -45,6 +47,8 @@ program main
   
   if (acc_is_present (h) .neqv. .TRUE.) stop 3
 
+  call acc_wait (async)
+
   do i = 1, N
     if (h(i) /= i + i) stop 4
   end do 
-- 
2.30.2