Re: libgomp nvptx plugin: rework initialisation and support the proposed load/unload hooks

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi!

On Mon, 23 Mar 2015 22:44:39 +0300, Ilya Verbin <iverbin@gmail.com> wrote:
> On Mon, Mar 09, 2015 at 14:45:55 +0000, Julian Brown wrote:
> > On Fri, 6 Mar 2015 17:01:13 +0300
> > Ilya Verbin <iverbin@gmail.com> wrote:
> > 
> > > On Thu, Feb 26, 2015 at 20:25:11 +0300, Ilya Verbin wrote:
> > > > On Wed, Feb 25, 2015 at 10:36:08 +0100, Thomas Schwinge wrote:
> > > > > > Julian Brown <julian@codesourcery.com> wrote:
> > > > > > This is a version of the previously-posted patch to rework
> > > > > > initialisation and support the proposed load/unload hooks,
> > > > > > merged to gomp4 branch and tested alongside the two patches
> > > > > > (from
> > > > 
> > > > Currently the 'struct gomp_memory_mapping' contains 'lock' and
> > > > 'is_initialized'. Do you still need them?  Or we can use
> > > > gomp_device_descr::lock and is_initialized instead?  If yes, then
> > > > we can replace the gomp_memory_mapping structure with a splay_tree,
> > > > as it was before the OpenACC merge.
> > > 
> > > Ping?
> > 
> > Apologies, I've been distracted with travel and other things. I
> > suspect, as you suggest, that the gomp_memory_mapping
> > lock/is_initialized fields may no longer be required. I haven't yet had
> > time to address that nor all of Thomas's comments on the patch (mostly
> > breakage with multiple devices), and I'm unlikely to have time this
> > week either due to vacation...
> 
> If it is too late for such global changes (rework initialization in libgomp,
> change mic and ptx plugins), then here is a small workaround patch to fix
> offloading from libraries.  Likely, it will not affect OpenACC programs with one
> image.  make check-target-libgomp passed.

Thanks!  Confirming that the nvptx offloading test cases are not affected
by this patch.  (But I can't formally approve the patch.)

> 	PR libgomp/65338
> libgomp/
> 	* libgomp.h (struct gomp_device_descr): Remove
> 	offload_regions_registered.
> 	* oacc-host.c (host_dispatch): Do not initialize
> 	offload_regions_registered.
> 	* target.c (gomp_register_image_for_device): Do not check for
> 	offload_regions_registered.
> 	(gomp_target_init): Do not initialize offload_regions_registered.
> 
> 
> diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
> index 3089401..f45fdba 100644
> --- a/libgomp/libgomp.h
> +++ b/libgomp/libgomp.h
> @@ -793,9 +793,6 @@ struct gomp_device_descr
>    /* Set to true when device is initialized.  */
>    bool is_initialized;
>  
> -  /* True when offload regions have been registered with this device.  */
> -  bool offload_regions_registered;
> -
>    /* OpenACC-specific data and functions.  */
>    /* This is mutable because of its mutable data_environ and target_data
>       members.  */
> diff --git a/libgomp/oacc-host.c b/libgomp/oacc-host.c
> index 6aeb1e7..2763f44 100644
> --- a/libgomp/oacc-host.c
> +++ b/libgomp/oacc-host.c
> @@ -56,7 +56,6 @@ static struct gomp_device_descr host_dispatch =
>      .mem_map.is_initialized = false,
>      .mem_map.splay_tree.root = NULL,
>      .is_initialized = false,
> -    .offload_regions_registered = false,
>  
>      .openacc = {
>        .open_device_func = GOMP_OFFLOAD_openacc_open_device,
> diff --git a/libgomp/target.c b/libgomp/target.c
> index 50baa4d..db1f509 100644
> --- a/libgomp/target.c
> +++ b/libgomp/target.c
> @@ -1035,13 +1035,8 @@ static void
>  gomp_register_image_for_device (struct gomp_device_descr *device,
>  				struct offload_image_descr *image)
>  {
> -  if (!device->offload_regions_registered
> -      && (device->type == image->type
> -	  || device->type == OFFLOAD_TARGET_TYPE_HOST))
> -    {
> -      device->register_image_func (image->host_table, image->target_data);
> -      device->offload_regions_registered = true;
> -    }
> +  if (device->type == image->type || device->type == OFFLOAD_TARGET_TYPE_HOST)
> +    device->register_image_func (image->host_table, image->target_data);
>  }
>  
>  /* This function initializes the runtime needed for offloading.
> @@ -1105,7 +1100,6 @@ gomp_target_init (void)
>  		current_device.mem_map.is_initialized = false;
>  		current_device.mem_map.splay_tree.root = NULL;
>  		current_device.is_initialized = false;
> -		current_device.offload_regions_registered = false;
>  		current_device.openacc.data_environ = NULL;
>  		current_device.openacc.target_data = NULL;
>  		for (i = 0; i < new_num_devices; i++)


GrÃÃe,
 Thomas

Attachment: pgpGr62dREntA.pgp
Description: PGP signature