On Mon, Nov 16, 2015 at 08:33:28PM +0300, Ilya Verbin wrote:
> diff --git a/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
> b/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
> index 772e198..6ee585e 100644
> --- a/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
> +++ b/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
> @@ -65,6 +65,17 @@ typedef std::vector<AddrVect> DevAddrVect;
> /* Addresses for all images and all devices. */
> typedef std::map<const void *, DevAddrVect> ImgDevAddrMap;
>
> +/* Image descriptor needed by __offload_[un]register_image. */
> +struct TargetImageDesc {
> + int64_t size;
> + /* 10 characters is enough for max int value. */
> + char name[sizeof ("lib0000000000.so")];
> + char data[];
> +} __attribute__ ((packed));
Why the packed attribute? I know it is preexisting, but with int64_t
being the first and then just char, there is no padding in between fields.
And to determine the size without data, you can just use offsetof.
> @@ -217,13 +231,27 @@ offload (const char *file, uint64_t line, int device,
> const char *name,
> }
>
> static void
> +unregister_main_image ()
> +{
> + __offload_unregister_image (&main_target_image);
> +}
> +
> +static void
> register_main_image ()
> {
> + /* Do not check the return value, because old versions of liboffloadmic did
> + not have return values. */
> __offload_register_image (&main_target_image);
>
> /* liboffloadmic will call GOMP_PLUGIN_target_task_completion when
> asynchronous task on target is completed. */
> __offload_register_task_callback (GOMP_PLUGIN_target_task_completion);
> +
> + if (atexit (unregister_main_image) != 0)
> + {
> + fprintf (stderr, "%s: atexit failed\n", __FILE__);
> + exit (1);
> + }
> }
What is the point of this hunk? Is there any point in unregistering the
main target image? I mean at that point the process is exiting anyway.
The importance of unregistering target images registered from shared
libraries is that they should be unregistered when they are dlclosed.
Other than that LGTM.
Jakub
