On 09/28/2015 10:52 AM, Thomas Schwinge wrote:
On Fri, 25 Sep 2015 19:49:50 +0300, Ilya Verbin <iver...@gmail.com> wrote:
Looks like you're right, and this scenario is possible.
Thanks for your review! Jakub, OK to commit the patch I had posted?
Then, in context of a similar scenario, I think we'll also want the
following. Please confirm that my reasoning in gomp_get_num_devices and
resolve_device is correct. OK for trunk?
I've looked at that for a while. I don't see anything immediately wrong
with the patches, but I think it would still be good for Jakub to have a
look.
One oddity I noticed in target.c is that there are two different
num_devices variables:
/* Total number of available devices. */
static int num_devices;
/* Number of GOMP_OFFLOAD_CAP_OPENMP_400 devices. */
static int num_devices_openmp;
Confusingly, the get_num_devices function returns num_devices_openmp.
That function includes a pthread_once call to gomp_target_init, which
sets up these variables. References to num_devices_openmp through
get_num_devices are thereforce guaranteed to be initialized. However,
there are direct references to num_devices, in GOMP_offload_register_ver
and GOMP_offload_unregister_ver, and they don't seem to enforce any kind
of initialization:
/* Load image to all initialized devices. */
for (i = 0; i < num_devices; i++)
{
struct gomp_device_descr *devicep = &devices[i];
gomp_mutex_lock (&devicep->lock);
if (devicep->type == target_type && devicep->is_initialized)
gomp_load_image_to_device (devicep, version,
host_table, target_data, true);
gomp_mutex_unlock (&devicep->lock);
}
I'm guessing this only triggers when dlopening something with an offload
image after devices have been initialized already, and it looks like we
have symmetrical code in gomp_init_device. Wouldn't it be
possible/better to force a gomp_target_init before referencing
num_devices, and then relying on the code I quoted and deleting the
image loading from gomp_init_device?
Bernd
