Re: [PATCH 6/6, OpenACC, libgomp] Async re-work, nvptx changes

[Chung-Lin Tang <chunglin_tang@mentor.com>]

On 2018/12/7 04:57 AM, Thomas Schwinge wrote>> --- 
a/libgomp/plugin/plugin-nvptx.c
>> +++ b/libgomp/plugin/plugin-nvptx.c
> 
>> +struct goacc_asyncqueue *
>> +GOMP_OFFLOAD_openacc_async_construct (void)
>> +{
>> +  struct goacc_asyncqueue *aq
>> +    = GOMP_PLUGIN_malloc (sizeof (struct goacc_asyncqueue));
>> +  aq->cuda_stream = NULL;
>> +  CUDA_CALL_ASSERT (cuStreamCreate, &aq->cuda_stream, CU_STREAM_DEFAULT);
> 
> Curiously (this was the same in the code before): does this have to be
> "CU_STREAM_DEFAULT" instead of "CU_STREAM_NON_BLOCKING", because we want
> to block anything from running in parallel with "acc_async_sync" GPU
> kernels, that use the "NULL" stream?  (Not asking you to change this now,
> but I wonder if this is overly strict?)

IIUC, this non-blocking only pertains to the "Legacy Default Stream" in 
CUDA, which we're pretty much ignoring; we should be using the newer 
per-thread default stream model. We could review this issue later.

>> +  if (aq->cuda_stream == NULL)
>> +    GOMP_PLUGIN_fatal ("CUDA stream create NULL\n");
> 
> Can this actually happen, given the "CUDA_CALL_ASSERT" usage above?

Hmm, yeah I think this is superfluous too...

>> +  CUDA_CALL_ASSERT (cuStreamSynchronize, aq->cuda_stream);
> 
> Why is the synchronization needed here?

I don't remember, could likely be something added during debug.
I'll remove this and test if things are okay.

Thanks,
Chung-Lin