Hi Tobias!
On 2024-11-18T14:23:24+0100, Tobias Burnus <tbur...@baylibre.com> wrote:
> This fixes a C23 error, causing a build fail: 'false'
> should have been 'NULL'.
ACK.
> The NULL value is not really handled as the code calling
> maybe_init_omp_async assumes that agent->omp_async_queue can be
> dereferenced. Hence, besides fixing the false/NULL issue, it switches to
> a 'fatal' error. Comments before I commit it after lunch?
(Please don't bundle up unrelated changes...)
That's a bug in 'libgomp/plugin/plugin-gcn.c:maybe_init_omp_async' (or
its users); the real user of 'GOMP_OFFLOAD_openacc_async_exec' does
handle the error condition:
libgomp/oacc-async.c- if (!dev->openacc.async.asyncqueue[async])
libgomp/oacc-async.c- {
libgomp/oacc-async.c- dev->openacc.async.asyncqueue[async]
libgomp/oacc-async.c: = dev->openacc.async.construct_func
(dev->target_id);
libgomp/oacc-async.c-
libgomp/oacc-async.c- if (!dev->openacc.async.asyncqueue[async])
libgomp/oacc-async.c- {
libgomp/oacc-async.c- gomp_mutex_unlock (&dev->openacc.async.lock);
libgomp/oacc-async.c- gomp_fatal ("async %d creation failed", async);
libgomp/oacc-async.c- }
..., but needs to 'gomp_mutex_unlock' before 'gomp_fatal', which your
change now circumvents. Therefore, please revert these 's%error%fatal'
changes, and instead fix up the libgomp GCN plugin-internal usage of
'GOMP_OFFLOAD_openacc_async_construct'.
Grüße
Thomas
> --- a/libgomp/plugin/plugin-gcn.c
> +++ b/libgomp/plugin/plugin-gcn.c
> @@ -4388,7 +4388,8 @@ GOMP_OFFLOAD_openacc_async_exec (void (*fn_ptr) (void
> *),
> gcn_exec (kernel, devaddrs, dims, targ_mem_desc, true, aq);
> }
>
> -/* Create a new asynchronous thread and queue for running future kernels. */
> +/* Create a new asynchronous thread and queue for running future kernels;
> + fails with a fatal error as all callers expect the queue to exist. */
>
> struct goacc_asyncqueue *
> GOMP_OFFLOAD_openacc_async_construct (int device)
> @@ -4416,18 +4417,18 @@ GOMP_OFFLOAD_openacc_async_construct (int device)
>
> if (pthread_mutex_init (&aq->mutex, NULL))
> {
> - GOMP_PLUGIN_error ("Failed to initialize a GCN agent queue mutex");
> - return false;
> + GOMP_PLUGIN_fatal ("Failed to initialize a GCN agent queue mutex");
> + return NULL;
> }
> if (pthread_cond_init (&aq->queue_cond_in, NULL))
> {
> - GOMP_PLUGIN_error ("Failed to initialize a GCN agent queue cond");
> - return false;
> + GOMP_PLUGIN_fatal ("Failed to initialize a GCN agent queue cond");
> + return NULL;
> }
> if (pthread_cond_init (&aq->queue_cond_out, NULL))
> {
> - GOMP_PLUGIN_error ("Failed to initialize a GCN agent queue cond");
> - return false;
> + GOMP_PLUGIN_fatal ("Failed to initialize a GCN agent queue cond");
> + return NULL;
> }
>
> hsa_status_t status = hsa_fns.hsa_queue_create_fn (agent->id,
