Hi!
On Sat, 8 Mar 2014 18:50:15 +0400, Ilya Verbin <iver...@gmail.com> wrote:
> --- a/libgomp/libgomp.map
> +++ b/libgomp/libgomp.map
> @@ -208,6 +208,7 @@ GOMP_3.0 {
>
> GOMP_4.0 {
> global:
> + GOMP_offload_register;
> GOMP_barrier_cancel;
> GOMP_cancel;
> GOMP_cancellation_point;
Now that the GOMP_4.0 symbol version is being used in GCC trunk, and will
be in the GCC 4.9 release, can we still add new symbols to it here?
(Jakub?)
> --- a/libgomp/plugin-host.c
> +++ b/libgomp/plugin-host.c
> +const int TARGET_TYPE_HOST = 0;
We'll have to see whether this (that is, libgomp/target.c:enum
target_type) should live in a shared header file, but OK for the moment.
> +void
> +device_run (void *fn_ptr, void *vars)
> +{
> +#ifdef DEBUG
> + printf ("libgomp plugin: %s:%s (%p, %p)\n", __FILE__, __FUNCTION__, fn_ptr,
> + vars);
> +#endif
> +
> + void (*fn)(void *) = (void (*)(void *)) fn_ptr;
> +
> + fn (vars);
> +}
Why not make fn_ptr a proper function pointer? Ah, because of
GOMP_target passing (void *) tgt_fn->tgt->tgt_start for the
!TARGET_TYPE_HOST case...
Would it make sense to have device_run return a value to make it able to
indicate to libgomp that the function cannot be run on the device (for
whatever reason), and libgomp should use host-fallback execution?
(Probably that needs more thought and discussion, OK to defer.)
> --- a/libgomp/target.c
> +++ b/libgomp/target.c
> +enum target_type {
> + TARGET_TYPE_HOST,
> + TARGET_TYPE_INTEL_MIC
> +};
(As discussed above, but OK to defer.)
> @@ -120,15 +140,26 @@ struct gomp_device_descr
> TARGET construct. */
> int id;
>
> + /* This is the TYPE of device. */
> + int type;
Use enum target_type instead of int?
> +/* This function should be called from every offload image. It gets the
> + descriptor of the host func and var tables HOST_TABLE, TYPE of the target,
> + and TARGET_DATA needed by target plugin (target tables, etc.) */
> +void
> +GOMP_offload_register (void *host_table, int type, void *target_data)
> +{
> + offload_images = realloc (offload_images,
> + (num_offload_images + 1)
> + * sizeof (struct offload_image_descr));
> +
> + if (offload_images == NULL)
> + return;
Fail silently, or use gomp_realloc to fail loudly?
> @@ -701,16 +836,25 @@ gomp_find_available_plugins (void)
> - out:
> +out:
Emacs wants the space to be there, so I assume that's the coding standard
to use. ;-)
> if (dir)
> closedir (dir);
> + free (offload_images);
I suggest to set offload_images = NULL, for clarity.
> + num_offload_images = 0;
> }
We may need to revisit this later: currently it's not possible to
register additional plugins after libgomp has initialized
(gomp_target_init, gomp_find_available_plugins just executed once), but
should that ever be made possible, we'd need to preserve offload_images.
OK to commit, thanks!
Grüße,
Thomas
pgpoKh9FyzF7I.pgp
Description: PGP signature
