[libgomp,openacc] Factor out async argument utility functions

Hi,

I've factored out 3 new functions to test properties of enum acc_async_t:
...
typedef enum acc_async_t {
   /* Keep in sync with include/gomp-constants.h.  */
   acc_async_noval = -1,
   acc_async_sync  = -2
} acc_async_t;
...


In order to understand what this means:
...
   if (async < acc_async_noval) 

...
you need to know the names and values of the enum.

Using the factored out functions, we get something that is easier to 
understand:
...
   if (async_synchronous_p (async)) 

...

Also I've changed the bodies of the functions to be robust against 
changes in values of acc_async_noval and acc_async_sync. No functional 
changes.

Build and tested on x86_64 with nvptx accelerator.

OK for trunk if bootstrap and reg-test on x86_64 succeeds?

Thanks,
- Tom

On 11/17/2017 02:18 PM, Tom de Vries wrote:
> Hi,
> 
> I've factored out 3 new functions to test properties of enum acc_async_t:
> ...
> typedef enum acc_async_t {
>    /* Keep in sync with include/gomp-constants.h.  */
>    acc_async_noval = -1,
>    acc_async_sync  = -2
> } acc_async_t;
> ...
> 
> 
> In order to understand what this means:
> ...
>    if (async < acc_async_noval)
> ...
> you need to know the names and values of the enum.
> 
> Using the factored out functions, we get something that is easier to 
> understand:
> ...
>    if (async_synchronous_p (async))
> ...
> 
> Also I've changed the bodies of the functions to be robust against 
> changes in values of acc_async_noval and acc_async_sync. No functional 
> changes.
> 
> Build and tested on x86_64 with nvptx accelerator.
> 
> OK for trunk if bootstrap and reg-test on x86_64 succeeds?
> 

Stage1 ping.

Thanks,- Tom

> 
> 0001-Factor-out-async-argument-utility-functions.patch
> 
> 
> Factor out async argument utility functions
> 
> 2017-11-17  Tom de Vries  <tom@codesourcery.com>
> 
> 	* oacc-int.h (async_valid_stream_id_p, async_valid_p)
> 	(async_synchronous_p): New function.
> 	* oacc-async.c (acc_async_test, acc_wait, acc_wait_all_async): Use
> 	async_valid_p.
> 	* oacc-cuda.c (acc_get_cuda_stream, acc_set_cuda_stream): Use
> 	async_valid_stream_id_p.
> 	* oacc-mem.c (gomp_acc_remove_pointer): Use async_synchronous_p.
> 	* oacc-parallel.c (GOACC_parallel_keyed): Same.
> 
> ---
>   libgomp/oacc-async.c    |  6 +++---
>   libgomp/oacc-cuda.c     |  4 ++--
>   libgomp/oacc-int.h      | 22 ++++++++++++++++++++++
>   libgomp/oacc-mem.c      |  2 +-
>   libgomp/oacc-parallel.c |  2 +-
>   5 files changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/libgomp/oacc-async.c b/libgomp/oacc-async.c
> index 1334f99..f541b44 100644
> --- a/libgomp/oacc-async.c
> +++ b/libgomp/oacc-async.c
> @@ -34,7 +34,7 @@
>   int
>   acc_async_test (int async)
>   {
> -  if (async < acc_async_sync)
> +  if (!async_valid_p (async))
>       gomp_fatal ("invalid async argument: %d", async);
>   
>     struct goacc_thread *thr = goacc_thread ();
> @@ -59,7 +59,7 @@ acc_async_test_all (void)
>   void
>   acc_wait (int async)
>   {
> -  if (async < acc_async_sync)
> +  if (!async_valid_p (async))
>       gomp_fatal ("invalid async argument: %d", async);
>   
>     struct goacc_thread *thr = goacc_thread ();
> @@ -117,7 +117,7 @@ acc_async_wait_all (void)
>   void
>   acc_wait_all_async (int async)
>   {
> -  if (async < acc_async_sync)
> +  if (!async_valid_p (async))
>       gomp_fatal ("invalid async argument: %d", async);
>   
>     struct goacc_thread *thr = goacc_thread ();
> diff --git a/libgomp/oacc-cuda.c b/libgomp/oacc-cuda.c
> index ac9285f..97bc961 100644
> --- a/libgomp/oacc-cuda.c
> +++ b/libgomp/oacc-cuda.c
> @@ -58,7 +58,7 @@ acc_get_cuda_stream (int async)
>   {
>     struct goacc_thread *thr = goacc_thread ();
>   
> -  if (async < 0)
> +  if (!async_valid_stream_id_p (async))
>       return NULL;
>   
>     if (thr && thr->dev && thr->dev->openacc.cuda.get_stream_func)
> @@ -72,7 +72,7 @@ acc_set_cuda_stream (int async, void *stream)
>   {
>     struct goacc_thread *thr;
>   
> -  if (async < 0 || stream == NULL)
> +  if (!async_valid_stream_id_p (async) || stream == NULL)
>       return 0;
>   
>     goacc_lazy_initialize ();
> diff --git a/libgomp/oacc-int.h b/libgomp/oacc-int.h
> index cb14f09..f553267 100644
> --- a/libgomp/oacc-int.h
> +++ b/libgomp/oacc-int.h
> @@ -99,6 +99,28 @@ void goacc_restore_bind (void);
>   void goacc_lazy_initialize (void);
>   void goacc_host_init (void);
>   
> +static inline bool
> +async_valid_stream_id_p (int async)
> +{
> +  return async >= 0;
> +}
> +
> +static inline bool
> +async_valid_p (int async)
> +{
> +  return (async == acc_async_noval || async == acc_async_sync
> +	  || async_valid_stream_id_p (async));
> +}
> +
> +static inline bool
> +async_synchronous_p (int async)
> +{
> +  if (!async_valid_p (async))
> +    return true;
> +
> +  return async == acc_async_sync;
> +}
> +
>   #ifdef HAVE_ATTRIBUTE_VISIBILITY
>   # pragma GCC visibility pop
>   #endif
> diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c
> index ff3ed49..12558b8 100644
> --- a/libgomp/oacc-mem.c
> +++ b/libgomp/oacc-mem.c
> @@ -723,7 +723,7 @@ gomp_acc_remove_pointer (void *h, bool force_copyfrom, int async, int mapnum)
>     gomp_mutex_unlock (&acc_dev->lock);
>   
>     /* If running synchronously, unmap immediately.  */
> -  if (async < acc_async_noval)
> +  if (async_synchronous_p (async))
>       gomp_unmap_vars (t, true);
>     else
>       t->device_descr->openacc.register_async_cleanup_func (t, async);
> diff --git a/libgomp/oacc-parallel.c b/libgomp/oacc-parallel.c
> index a8cff9e..baf44ec 100644
> --- a/libgomp/oacc-parallel.c
> +++ b/libgomp/oacc-parallel.c
> @@ -183,7 +183,7 @@ GOACC_parallel_keyed (int device, void (*fn) (void *),
>   			      async, dims, tgt);
>   
>     /* If running synchronously, unmap immediately.  */
> -  if (async < acc_async_noval)
> +  if (async_synchronous_p (async))
>       gomp_unmap_vars (tgt, true);
>     else
>       tgt->device_descr->openacc.register_async_cleanup_func (tgt, async);
>

On 05/01/2018 10:50 PM, Tom de Vries wrote:
> On 11/17/2017 02:18 PM, Tom de Vries wrote:
>> Hi,
>>
>> I've factored out 3 new functions to test properties of enum acc_async_t:
>> ...
>> typedef enum acc_async_t {
>>    /* Keep in sync with include/gomp-constants.h.  */
>>    acc_async_noval = -1,
>>    acc_async_sync  = -2
>> } acc_async_t;
>> ...
>>
>>
>> In order to understand what this means:
>> ...
>>    if (async < acc_async_noval)
>> ...
>> you need to know the names and values of the enum.
>>
>> Using the factored out functions, we get something that is easier to 
>> understand:
>> ...
>>    if (async_synchronous_p (async))
>> ...
>>
>> Also I've changed the bodies of the functions to be robust against 
>> changes in values of acc_async_noval and acc_async_sync. No functional 
>> changes.
>>
>> Build and tested on x86_64 with nvptx accelerator.
>>
>> OK for trunk if bootstrap and reg-test on x86_64 succeeds?
>>
> 
> Stage1 ping.
> 

Assuming no objections, committed as attached.

Thanks,
- Tom
[openacc] Factor out async argument utility functions

2017-11-17  Tom de Vries  <tom@codesourcery.com>

	PR libgomp/83792
	* oacc-int.h (async_valid_stream_id_p, async_valid_p)
	(async_synchronous_p): New function.
	* oacc-async.c (acc_async_test, acc_wait, acc_wait_all_async): Use
	async_valid_p.
	* oacc-cuda.c (acc_get_cuda_stream, acc_set_cuda_stream): Use
	async_valid_stream_id_p.
	* oacc-mem.c (gomp_acc_remove_pointer): Use async_synchronous_p.
	* oacc-parallel.c (GOACC_parallel_keyed): Same.

---
 libgomp/oacc-async.c    |  6 +++---
 libgomp/oacc-cuda.c     |  4 ++--
 libgomp/oacc-int.h      | 22 ++++++++++++++++++++++
 libgomp/oacc-mem.c      |  2 +-
 libgomp/oacc-parallel.c |  2 +-
 5 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/libgomp/oacc-async.c b/libgomp/oacc-async.c
index 7cdb627..a4e1863 100644
--- a/libgomp/oacc-async.c
+++ b/libgomp/oacc-async.c
@@ -34,7 +34,7 @@
 int
 acc_async_test (int async)
 {
-  if (async < acc_async_sync)
+  if (!async_valid_p (async))
     gomp_fatal ("invalid async argument: %d", async);
 
   struct goacc_thread *thr = goacc_thread ();
@@ -59,7 +59,7 @@ acc_async_test_all (void)
 void
 acc_wait (int async)
 {
-  if (async < acc_async_sync)
+  if (!async_valid_p (async))
     gomp_fatal ("invalid async argument: %d", async);
 
   struct goacc_thread *thr = goacc_thread ();
@@ -117,7 +117,7 @@ acc_async_wait_all (void)
 void
 acc_wait_all_async (int async)
 {
-  if (async < acc_async_sync)
+  if (!async_valid_p (async))
     gomp_fatal ("invalid async argument: %d", async);
 
   struct goacc_thread *thr = goacc_thread ();
diff --git a/libgomp/oacc-cuda.c b/libgomp/oacc-cuda.c
index c388170..20774c1 100644
--- a/libgomp/oacc-cuda.c
+++ b/libgomp/oacc-cuda.c
@@ -58,7 +58,7 @@ acc_get_cuda_stream (int async)
 {
   struct goacc_thread *thr = goacc_thread ();
 
-  if (async < 0)
+  if (!async_valid_stream_id_p (async))
     return NULL;
 
   if (thr && thr->dev && thr->dev->openacc.cuda.get_stream_func)
@@ -72,7 +72,7 @@ acc_set_cuda_stream (int async, void *stream)
 {
   struct goacc_thread *thr;
 
-  if (async < 0 || stream == NULL)
+  if (!async_valid_stream_id_p (async) || stream == NULL)
     return 0;
 
   goacc_lazy_initialize ();
diff --git a/libgomp/oacc-int.h b/libgomp/oacc-int.h
index 912433a..cdd0f7f 100644
--- a/libgomp/oacc-int.h
+++ b/libgomp/oacc-int.h
@@ -99,6 +99,28 @@ void goacc_restore_bind (void);
 void goacc_lazy_initialize (void);
 void goacc_host_init (void);
 
+static inline bool
+async_valid_stream_id_p (int async)
+{
+  return async >= 0;
+}
+
+static inline bool
+async_valid_p (int async)
+{
+  return (async == acc_async_noval || async == acc_async_sync
+	  || async_valid_stream_id_p (async));
+}
+
+static inline bool
+async_synchronous_p (int async)
+{
+  if (!async_valid_p (async))
+    return true;
+
+  return async == acc_async_sync;
+}
+
 #ifdef HAVE_ATTRIBUTE_VISIBILITY
 # pragma GCC visibility pop
 #endif
diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c
index 5cc8fcf..158f086 100644
--- a/libgomp/oacc-mem.c
+++ b/libgomp/oacc-mem.c
@@ -723,7 +723,7 @@ gomp_acc_remove_pointer (void *h, bool force_copyfrom, int async, int mapnum)
   gomp_mutex_unlock (&acc_dev->lock);
 
   /* If running synchronously, unmap immediately.  */
-  if (async < acc_async_noval)
+  if (async_synchronous_p (async))
     gomp_unmap_vars (t, true);
   else
     t->device_descr->openacc.register_async_cleanup_func (t, async);
diff --git a/libgomp/oacc-parallel.c b/libgomp/oacc-parallel.c
index a71b399..cfba581 100644
--- a/libgomp/oacc-parallel.c
+++ b/libgomp/oacc-parallel.c
@@ -183,7 +183,7 @@ GOACC_parallel_keyed (int device, void (*fn) (void *),
 			      async, dims, tgt);
 
   /* If running synchronously, unmap immediately.  */
-  if (async < acc_async_noval)
+  if (async_synchronous_p (async))
     gomp_unmap_vars (tgt, true);
   else
     tgt->device_descr->openacc.register_async_cleanup_func (tgt, async);