Hi Chung-Lin!
On Wed, 2 Jan 2019 20:46:12 +0800, Chung-Lin Tang <chunglin_t...@mentor.com>
wrote:
> Hi Thomas, Happy New Year,
Thanks! If I remember right, you still have a few weeks until "your" New
Year/Spring Festival, right?
> On 2018/12/19 5:03 AM, Thomas Schwinge wrote:
> >> +
> >> + if (!dev->openacc.async.asyncqueue[async])
> >> + {
> >> + dev->openacc.async.asyncqueue[async] =
> >> dev->openacc.async.construct_func ();
> >> +
> >> + if (!dev->openacc.async.asyncqueue[async])
> >> + {
> >> + gomp_mutex_unlock (&dev->openacc.async.lock);
> >> + gomp_fatal ("async %d creation failed", async);
> >> + }
> > That will now always fail for host fallback, where
> > "host_openacc_async_construct" just always does "return NULL".
> >
> > Actually, if the device doesn't support asyncqueues, this whole function
> > should turn into some kind of no-op, so that we don't again and again try
> > to create a new one for every call to "lookup_goacc_asyncqueue".
> >
> > I'm attaching one possible solution. I think it's fine to assume that
> > the majority of devices will support asyncqueues, and for those that
> > don't, this is just a one-time overhead per async-argument. So, no
> > special handling required in "lookup_goacc_asyncqueue".
>
> > --- a/libgomp/oacc-host.c
> > +++ b/libgomp/oacc-host.c
> > @@ -212,7 +212,8 @@ host_openacc_async_queue_callback (struct
> > goacc_asyncqueue *aq
> > static struct goacc_asyncqueue *
> > host_openacc_async_construct (void)
> > {
> > - return NULL;
> > + /* We have to return non-NULL here, but it's OK to use a dummy. */
> > + return (struct goacc_asyncqueue *) -1;
> > }
>
> I'm not sure I understand the meaning of this? Is there any use to
> segfaulting somewhere else
> due to this 0xffff... pointer?
There will be no such dereferencing (and thus no such segfault), as you
(quite nicely!) made this is an opaque data type to the generic code.
The concrete type is specific to, and only ever dereferenced inside each
plugin, and the "host plugin" never dereferences it, so returning minus
one here only serves as a non-NULL value/identifier to the generic code.
> A feature of a NULL asyncqueue should mean that it is simply synchronous
OK, then that should be documented, and as I mentioned above, the
"lookup" code be adjusted so that it doesn't again and again try to
create an asyncqueue when the "construct" function returns NULL.
> however this does somewhat
> conflict with the case of async.construct_func() returning NULL on error...
>
> Perhaps, again using an explicit success code as the return value (and return
> asyncqueue using
> an out parameter)?
Sure, that's also fine. I just did it as presented above, because of its
simplicity, and to avoid adjusting the "lookup" code, as mentioned above.
Grüße
Thomas
