public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* offload data version number
@ 2015-07-21 13:28 Nathan Sidwell
  2015-07-21 16:02 ` Nathan Sidwell
  0 siblings, 1 reply; 22+ messages in thread
From: Nathan Sidwell @ 2015-07-21 13:28 UTC (permalink / raw)
  To: Jakub Jelinek, ilya.verbin, GCC Patches

[-- Attachment #1: Type: text/plain, Size: 1203 bytes --]

This trunk patch implements new register and unregister entry points to allow 
specifying data version information.  (I'll shortly be posting patches changing 
the PTX offload data format.)

We now have GOMP_offload_{,un}register_2, which take an additional unsigned int 
version number.  The version number is composed of two parts.  16 bits for the 
libgomp version and 16 bits for the device-specific plugin.  Currently both are 
zero.  When the PTX data changes, that device-specific value will increment.

The existing register/unregister calls forward to the new ones, providing zero 
for the version information.

On the plugin side I've added 2 new entry points GOMP_OFFLOAD_{,un}load_image_2, 
which also take an additional version number argument.  These entry points are 
optional, and only added to the PTX plugin.  The existing plugin entrypoints 
forward to the new ones.

libgomp  will use these new entry points if they exist, otherwise use the 
original entry points, provided the incoming version is zero.

I added the GOMP_offload_{,un}register_2 routines to the libgomp map file as 
version 4.0.2 -- I wasn't sure whether to increment it more than that. Advice 
sought.

ok?

nathan

[-- Attachment #2: trunk-version.patch --]
[-- Type: text/x-patch, Size: 12696 bytes --]

2015-07-21  Nathan Sidwell  <nathan@acm.org>

	include/
	* gomp-constants.h (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX): New.
	(GOMP_VERSION_PACK, GOMP_VERSION_LIB, GOMP_VERSION_DEV): New.

	libgomp/
	* plugin/plugin-nvptx.c: Include gomp-constants.h.
	(GOMP_OFFLOAD_load_image): Replace with stub function.  Move bulk
	to ....
	(GOMP_OFFLOAD_load_image_2): ... here.  Add version argument and
	check it.
	(GOMP_OFFLOAD_unload_image): Replace with stub function.  Move bulk
	to ....
	(GOMP_OFFLOAD_unload_image_2): ... here.  Add version argument and
	check it.
	* libgomp.map: Add 4.0.2 version.
	* target.c (offload_image_descr): Add versionn field.
	(gomp_offload_image_to_device): Add version argument.  Forward to
	load_image_func_2 if available.  Improve load mismatch diagnostic.
	(GOMP_offload_regster): Make stub function, move bulk to ...
	(GOMP_offload_register_2): ... here.  Process version argument.
	(GOMP_offload_unregister): Make stub function, move bulk to ...
	(GOMP_offload_unregister_2): ... here.  Process version argument.
	(gomp_init_device): Process version field.
	(gomp_load_plugin_for_device): Search for load_image_2 and
	unload_image_2 entry points.
	* libgomp.h (gomp_device_descr) Add load_image_2 and
	unload_image_2 fields.

	gcc/
	* config/nvptx/mkoffload.c (process): Replace
	GOMP_offload_register with GOMP_offload_register_2.

Index: libgomp/libgomp.map
===================================================================
--- libgomp/libgomp.map	(revision 226017)
+++ libgomp/libgomp.map	(working copy)
@@ -234,6 +234,12 @@ GOMP_4.0.1 {
 	GOMP_offload_unregister;
 } GOMP_4.0;
 
+GOMP_4.0.2 {
+  global:
+	GOMP_offload_register_2;
+	GOMP_offload_unregister_2;
+} GOMP_4.0.1;
+
 OACC_2.0 {
   global:
 	acc_get_num_devices;
Index: libgomp/target.c
===================================================================
--- libgomp/target.c	(revision 226017)
+++ libgomp/target.c	(working copy)
@@ -56,6 +56,7 @@ static gomp_mutex_t register_lock;
    It contains type of the target device, pointer to host table descriptor, and
    pointer to target data.  */
 struct offload_image_descr {
+  unsigned version;
   enum offload_target_type type;
   const void *host_table;
   const void *target_data;
@@ -641,7 +642,8 @@ gomp_update (struct gomp_device_descr *d
    from loaded target image.  */
 
 static void
-gomp_offload_image_to_device (struct gomp_device_descr *devicep,
+gomp_offload_image_to_device (unsigned version,
+			      struct gomp_device_descr *devicep,
 			      const void *host_table, const void *target_data,
 			      bool is_register_lock)
 {
@@ -657,15 +659,28 @@ gomp_offload_image_to_device (struct gom
 
   /* Load image to device and get target addresses for the image.  */
   struct addr_pair *target_table = NULL;
-  int i, num_target_entries
-    = devicep->load_image_func (devicep->target_id, target_data, &target_table);
+  int i, num_target_entries;
+
+  if (devicep->load_image_2_func)
+    num_target_entries
+      = devicep->load_image_2_func (version, devicep->target_id,
+				    target_data, &target_table);
+  else if (GOMP_VERSION_DEV (version))
+    gomp_fatal ("Plugin too old for offload data (0 < %u)",
+		GOMP_VERSION_DEV (version));
+  else
+    num_target_entries
+      = devicep->load_image_func (devicep->target_id,
+				  target_data, &target_table);
 
   if (num_target_entries != num_funcs + num_vars)
     {
       gomp_mutex_unlock (&devicep->lock);
       if (is_register_lock)
 	gomp_mutex_unlock (&register_lock);
-      gomp_fatal ("Can't map target functions or variables");
+      gomp_fatal ("Cannot map target functions or variables"
+		  " (expected %u, have %u)", num_funcs + num_vars,
+		  num_target_entries);
     }
 
   /* Insert host-target address mapping into splay tree.  */
@@ -730,10 +745,15 @@ gomp_offload_image_to_device (struct gom
    the target, and TARGET_DATA needed by target plugin.  */
 
 void
-GOMP_offload_register (const void *host_table, int target_type,
-		       const void *target_data)
+GOMP_offload_register_2 (unsigned version, const void *host_table,
+			 int target_type, const void *target_data)
 {
   int i;
+
+  if (GOMP_VERSION_LIB (version) > GOMP_VERSION)
+    gomp_fatal ("Library too old for offload (version %u < %u)",
+		GOMP_VERSION, GOMP_VERSION_LIB (version));
+  
   gomp_mutex_lock (&register_lock);
 
   /* Load image to all initialized devices.  */
@@ -742,7 +762,8 @@ GOMP_offload_register (const void *host_
       struct gomp_device_descr *devicep = &devices[i];
       gomp_mutex_lock (&devicep->lock);
       if (devicep->type == target_type && devicep->is_initialized)
-	gomp_offload_image_to_device (devicep, host_table, target_data, true);
+	gomp_offload_image_to_device (version, devicep,
+				      host_table, target_data, true);
       gomp_mutex_unlock (&devicep->lock);
     }
 
@@ -751,6 +772,7 @@ GOMP_offload_register (const void *host_
     = gomp_realloc_unlock (offload_images,
 			   (num_offload_images + 1)
 			   * sizeof (struct offload_image_descr));
+  offload_images[num_offload_images].version = version;
   offload_images[num_offload_images].type = target_type;
   offload_images[num_offload_images].host_table = host_table;
   offload_images[num_offload_images].target_data = target_data;
@@ -759,13 +781,20 @@ GOMP_offload_register (const void *host_
   gomp_mutex_unlock (&register_lock);
 }
 
+void
+GOMP_offload_register (const void *host_table, int target_type,
+		       const void *target_data)
+{
+  GOMP_offload_register_2 (0, host_table, target_type, target_data);
+}
+
 /* This function should be called from every offload image while unloading.
    It gets the descriptor of the host func and var tables HOST_TABLE, TYPE of
    the target, and TARGET_DATA needed by target plugin.  */
 
 void
-GOMP_offload_unregister (const void *host_table, int target_type,
-			 const void *target_data)
+GOMP_offload_unregister_2 (unsigned version, const void *host_table,
+			   int target_type, const void *target_data)
 {
   void **host_func_table = ((void ***) host_table)[0];
   void **host_funcs_end  = ((void ***) host_table)[1];
@@ -792,7 +821,11 @@ GOMP_offload_unregister (const void *hos
 	  continue;
 	}
 
-      devicep->unload_image_func (devicep->target_id, target_data);
+      if (devicep->unload_image_2_func)
+	devicep->unload_image_2_func (version,
+				      devicep->target_id, target_data);
+      else
+	devicep->unload_image_func (devicep->target_id, target_data);
 
       /* Remove mapping from splay tree.  */
       struct splay_tree_key_s k;
@@ -844,6 +877,13 @@ GOMP_offload_unregister (const void *hos
   gomp_mutex_unlock (&register_lock);
 }
 
+void
+GOMP_offload_unregister (const void *host_table, int target_type,
+			 const void *target_data)
+{
+  GOMP_offload_unregister_2 (0, host_table, target_type, target_data);
+}
+
 /* This function initializes the target device, specified by DEVICEP.  DEVICEP
    must be locked on entry, and remains locked on return.  */
 
@@ -858,8 +898,9 @@ gomp_init_device (struct gomp_device_des
     {
       struct offload_image_descr *image = &offload_images[i];
       if (image->type == devicep->type)
-	gomp_offload_image_to_device (devicep, image->host_table,
-				      image->target_data, false);
+	gomp_offload_image_to_device (image->version, devicep,
+				      image->host_table, image->target_data,
+				      false);
     }
 
   devicep->is_initialized = true;
@@ -1113,6 +1154,8 @@ gomp_load_plugin_for_device (struct gomp
   DLSYM (free);
   DLSYM (dev2host);
   DLSYM (host2dev);
+  DLSYM_OPT (load_image_2, load_image_2);
+  DLSYM_OPT (unload_image_2, unload_image_2);
   device->capabilities = device->get_caps_func ();
   if (device->capabilities & GOMP_OFFLOAD_CAP_OPENMP_400)
     DLSYM (run);
Index: libgomp/libgomp.h
===================================================================
--- libgomp/libgomp.h	(revision 226017)
+++ libgomp/libgomp.h	(working copy)
@@ -749,7 +749,9 @@ struct gomp_device_descr
   void (*init_device_func) (int);
   void (*fini_device_func) (int);
   int (*load_image_func) (int, const void *, struct addr_pair **);
+  int (*load_image_2_func) (unsigned, int, const void *, struct addr_pair **);
   void (*unload_image_func) (int, const void *);
+  void (*unload_image_2_func) (unsigned, int, const void *);
   void *(*alloc_func) (int, size_t);
   void (*free_func) (int, void *);
   void *(*dev2host_func) (int, void *, const void *, size_t);
Index: libgomp/plugin/plugin-nvptx.c
===================================================================
--- libgomp/plugin/plugin-nvptx.c	(revision 226017)
+++ libgomp/plugin/plugin-nvptx.c	(working copy)
@@ -36,6 +36,7 @@
 #include "libgomp-plugin.h"
 #include "oacc-ptx.h"
 #include "oacc-plugin.h"
+#include "gomp-constants.h"
 
 #include <pthread.h>
 #include <cuda.h>
@@ -1648,8 +1649,8 @@ typedef struct nvptx_tdata
    number ORD.  Allocate and return TARGET_TABLE.  */
 
 int
-GOMP_OFFLOAD_load_image (int ord, const void *target_data,
-			 struct addr_pair **target_table)
+GOMP_OFFLOAD_load_image_2 (unsigned version, int ord, const void *target_data,
+			   struct addr_pair **target_table)
 {
   CUmodule module;
   const char *const *fn_names, *const *var_names;
@@ -1661,6 +1662,11 @@ GOMP_OFFLOAD_load_image (int ord, const
   struct ptx_image_data *new_image;
   struct ptx_device *dev;
 
+  if (GOMP_VERSION_DEV (version) != GOMP_VERSION_NVIDIA_PTX)
+    GOMP_PLUGIN_fatal ("Offload data incompatible with PTX plugin"
+		       " (version %u ! %u)",
+		       GOMP_VERSION_NVIDIA_PTX, GOMP_VERSION_DEV (version));
+  
   GOMP_OFFLOAD_init_device (ord);
 
   dev = ptx_devices[ord];
@@ -1726,15 +1732,26 @@ GOMP_OFFLOAD_load_image (int ord, const
   return fn_entries + var_entries;
 }
 
+int
+GOMP_OFFLOAD_load_image (int device,
+			 const void *target_image, struct addr_pair **result)
+{
+  return GOMP_OFFLOAD_load_image_2 (0, device, target_image, result);
+}
+
 /* Unload the program described by TARGET_DATA.  DEV_DATA is the
    function descriptors allocated by G_O_load_image.  */
 
 void
-GOMP_OFFLOAD_unload_image (int ord, const void *target_data)
+GOMP_OFFLOAD_unload_image_2 (unsigned version, int ord,
+			     const void *target_data)
 {
   struct ptx_image_data *image, **prev_p;
   struct ptx_device *dev = ptx_devices[ord];
 
+  if (GOMP_VERSION_DEV (version) != GOMP_VERSION_NVIDIA_PTX)
+    return;
+  
   pthread_mutex_lock (&dev->image_lock);
   for (prev_p = &dev->images; (image = *prev_p) != 0; prev_p = &image->next)
     if (image->target_data == target_data)
@@ -1748,6 +1765,12 @@ GOMP_OFFLOAD_unload_image (int ord, cons
   pthread_mutex_unlock (&dev->image_lock);
 }
 
+void
+GOMP_OFFLOAD_unload_image (int ord, const void *target_data)
+{
+  GOMP_OFFLOAD_unload_image_2 (0, ord, target_data);
+}
+
 void *
 GOMP_OFFLOAD_alloc (int ord, size_t size)
 {
Index: include/gomp-constants.h
===================================================================
--- include/gomp-constants.h	(revision 226017)
+++ include/gomp-constants.h	(working copy)
@@ -113,4 +113,12 @@ enum gomp_map_kind
 #define GOMP_DEVICE_ICV			-1
 #define GOMP_DEVICE_HOST_FALLBACK	-2
 
+/* Versions of libgomp and device-specific plugins.  */
+#define GOMP_VERSION	0
+#define GOMP_VERSION_NVIDIA_PTX 1
+
+#define GOMP_VERSION_PACK(LIB, DEV) (((LIB) << 16) | (DEV))
+#define GOMP_VERSION_LIB(PACK) (((PACK) >> 16) & 0xffff)
+#define GOMP_VERSION_DEV(PACK) ((PACK) & 0xffff)
+
 #endif
Index: gcc/config/nvptx/mkoffload.c
===================================================================
--- gcc/config/nvptx/mkoffload.c	(revision 226017)
+++ gcc/config/nvptx/mkoffload.c	(working copy)
@@ -880,18 +880,21 @@ process (FILE *in, FILE *out)
   fprintf (out, "#ifdef __cplusplus\n"
 	   "extern \"C\" {\n"
 	   "#endif\n");
-  fprintf (out, "extern void GOMP_offload_register"
-	   " (const void *, int, const void *);\n");
+  fprintf (out, "extern void GOMP_offload_register_2"
+	   " (unsigned,  const void *, int, const void *);\n");
   fprintf (out, "#ifdef __cplusplus\n"
 	   "}\n"
 	   "#endif\n");
 
   fprintf (out, "extern const void *const __OFFLOAD_TABLE__[];\n\n");
-  fprintf (out, "static __attribute__((constructor)) void init (void)\n{\n");
-  fprintf (out, "  GOMP_offload_register (__OFFLOAD_TABLE__, %d,\n",
+
+  fprintf (out, "static __attribute__((constructor)) void init (void)\n"
+	   "{\n"
+	   "  GOMP_offload_register_2 (%#x, __OFFLOAD_TABLE__,"
+	   "		%d/*NVIDIA_PTX*/, &target_data);\n"
+	   "};\n",
+	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
 	   GOMP_DEVICE_NVIDIA_PTX);
-  fprintf (out, "                         &target_data);\n");
-  fprintf (out, "};\n");
 }
 
 static void

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-07-21 13:28 offload data version number Nathan Sidwell
@ 2015-07-21 16:02 ` Nathan Sidwell
  2015-07-24 14:21   ` Nathan Sidwell
  0 siblings, 1 reply; 22+ messages in thread
From: Nathan Sidwell @ 2015-07-21 16:02 UTC (permalink / raw)
  To: Jakub Jelinek, ilya.verbin, GCC Patches

[-- Attachment #1: Type: text/plain, Size: 1374 bytes --]

On 07/21/15 09:25, Nathan Sidwell wrote:
> This trunk patch implements new register and unregister entry points to allow
> specifying data version information.  (I'll shortly be posting patches changing
> the PTX offload data format.)
>
> We now have GOMP_offload_{,un}register_2, which take an additional unsigned int
> version number.  The version number is composed of two parts.  16 bits for the
> libgomp version and 16 bits for the device-specific plugin.  Currently both are
> zero.  When the PTX data changes, that device-specific value will increment.
>
> The existing register/unregister calls forward to the new ones, providing zero
> for the version information.
>
> On the plugin side I've added 2 new entry points GOMP_OFFLOAD_{,un}load_image_2,
> which also take an additional version number argument.  These entry points are
> optional, and only added to the PTX plugin.  The existing plugin entrypoints
> forward to the new ones.
>
> libgomp  will use these new entry points if they exist, otherwise use the
> original entry points, provided the incoming version is zero.
>
> I added the GOMP_offload_{,un}register_2 routines to the libgomp map file as
> version 4.0.2 -- I wasn't sure whether to increment it more than that. Advice
> sought.

this version is updated following committing the unload patch.  there were a few 
(expected) collisions.

nathan

[-- Attachment #2: trunk-version-2.patch --]
[-- Type: text/x-patch, Size: 14375 bytes --]

2015-07-21  Nathan Sidwell  <nathan@codesourcery.com>

	include/
	* gomp-constants.h (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX): New.
	(GOMP_VERSION_PACK, GOMP_VERSION_LIB, GOMP_VERSION_DEV): New.

	libgomp/
	* plugin/plugin-nvptx.c: Include gomp-constants.h.
	(GOMP_OFFLOAD_load_image): Replace with stub function.  Move bulk
	to ....
	(GOMP_OFFLOAD_load_image_2): ... here.  Add version argument and
	check it.
	(GOMP_OFFLOAD_unload_image): Replace with stub function.  Move bulk
	to ....
	(GOMP_OFFLOAD_unload_image_2): ... here.  Add version argument and
	check it.
	* libgomp.map: Add 4.0.2 version.
	* target.c (offload_image_descr): Add versionn field.
	(gomp_offload_image_to_device): Add version argument.  Forward to
	load_image_func_2 if available.  Improve load mismatch diagnostic.
	(GOMP_offload_regster): Make stub function, move bulk to ...
	(GOMP_offload_register_2): ... here.  Process version argument.
	(GOMP_offload_unregister): Make stub function, move bulk to ...
	(GOMP_offload_unregister_2): ... here.  Process version argument.
	(gomp_init_device): Process version field.
	(gomp_load_plugin_for_device): Search for load_image_2 and
	unload_image_2 entry points.
	* libgomp.h (gomp_device_descr) Add load_image_2 and
	unload_image_2 fields.

	gcc/
	* config/nvptx/mkoffload.c (process): Replace
	GOMP_offload_register with GOMP_offload_register_2.

Index: libgomp/target.c
===================================================================
--- libgomp/target.c	(revision 226039)
+++ libgomp/target.c	(working copy)
@@ -56,6 +56,7 @@ static gomp_mutex_t register_lock;
    It contains type of the target device, pointer to host table descriptor, and
    pointer to target data.  */
 struct offload_image_descr {
+  unsigned version;
   enum offload_target_type type;
   const void *host_table;
   const void *target_data;
@@ -642,7 +643,8 @@ gomp_update (struct gomp_device_descr *d
    emitting variable and functions in the same order.  */
 
 static void
-gomp_load_image_to_device (struct gomp_device_descr *devicep,
+gomp_load_image_to_device (unsigned version,
+			   struct gomp_device_descr *devicep,
 			   const void *host_table, const void *target_data,
 			   bool is_register_lock)
 {
@@ -658,16 +660,28 @@ gomp_load_image_to_device (struct gomp_d
 
   /* Load image to device and get target addresses for the image.  */
   struct addr_pair *target_table = NULL;
-  int i, num_target_entries
-    = devicep->load_image_func (devicep->target_id, target_data,
-				&target_table);
+  int i, num_target_entries;
+
+  if (devicep->load_image_2_func)
+    num_target_entries
+      = devicep->load_image_2_func (version, devicep->target_id,
+				    target_data, &target_table);
+  else if (GOMP_VERSION_DEV (version))
+    gomp_fatal ("Plugin too old for offload data (0 < %u)",
+		GOMP_VERSION_DEV (version));
+  else
+    num_target_entries
+      = devicep->load_image_func (devicep->target_id,
+				  target_data, &target_table);
 
   if (num_target_entries != num_funcs + num_vars)
     {
       gomp_mutex_unlock (&devicep->lock);
       if (is_register_lock)
 	gomp_mutex_unlock (&register_lock);
-      gomp_fatal ("Can't map target functions or variables");
+      gomp_fatal ("Cannot map target functions or variables"
+		  " (expected %u, have %u)", num_funcs + num_vars,
+		  num_target_entries);
     }
 
   /* Insert host-target address mapping into splay tree.  */
@@ -731,7 +745,8 @@ gomp_load_image_to_device (struct gomp_d
    The device must be locked.   */
 
 static void
-gomp_unload_image_from_device (struct gomp_device_descr *devicep,
+gomp_unload_image_from_device (unsigned version,
+			       struct gomp_device_descr *devicep,
 			       const void *host_table, const void *target_data)
 {
   void **host_func_table = ((void ***) host_table)[0];
@@ -756,8 +771,12 @@ gomp_unload_image_from_device (struct go
       k.host_end = k.host_start + 1;
       node = splay_tree_lookup (&devicep->mem_map, &k);
     }
-  
-  devicep->unload_image_func (devicep->target_id, target_data);
+
+  if (devicep->unload_image_2_func)
+    devicep->unload_image_2_func (version,
+				  devicep->target_id, target_data);
+  else
+    devicep->unload_image_func (devicep->target_id, target_data);
 
   /* Remove mappings from splay tree.  */
   for (j = 0; j < num_funcs; j++)
@@ -786,10 +805,15 @@ gomp_unload_image_from_device (struct go
    the target, and TARGET_DATA needed by target plugin.  */
 
 void
-GOMP_offload_register (const void *host_table, int target_type,
-		       const void *target_data)
+GOMP_offload_register_2 (unsigned version, const void *host_table,
+			 int target_type, const void *target_data)
 {
   int i;
+
+  if (GOMP_VERSION_LIB (version) > GOMP_VERSION)
+    gomp_fatal ("Library too old for offload (version %u < %u)",
+		GOMP_VERSION, GOMP_VERSION_LIB (version));
+  
   gomp_mutex_lock (&register_lock);
 
   /* Load image to all initialized devices.  */
@@ -798,7 +822,8 @@ GOMP_offload_register (const void *host_
       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, host_table, target_data, true);
+	gomp_load_image_to_device (version, devicep,
+				   host_table, target_data, true);
       gomp_mutex_unlock (&devicep->lock);
     }
 
@@ -807,6 +832,7 @@ GOMP_offload_register (const void *host_
     = gomp_realloc_unlock (offload_images,
 			   (num_offload_images + 1)
 			   * sizeof (struct offload_image_descr));
+  offload_images[num_offload_images].version = version;
   offload_images[num_offload_images].type = target_type;
   offload_images[num_offload_images].host_table = host_table;
   offload_images[num_offload_images].target_data = target_data;
@@ -815,13 +841,20 @@ GOMP_offload_register (const void *host_
   gomp_mutex_unlock (&register_lock);
 }
 
+void
+GOMP_offload_register (const void *host_table, int target_type,
+		       const void *target_data)
+{
+  GOMP_offload_register_2 (0, host_table, target_type, target_data);
+}
+
 /* This function should be called from every offload image while unloading.
    It gets the descriptor of the host func and var tables HOST_TABLE, TYPE of
    the target, and TARGET_DATA needed by target plugin.  */
 
 void
-GOMP_offload_unregister (const void *host_table, int target_type,
-			 const void *target_data)
+GOMP_offload_unregister_2 (unsigned version, const void *host_table,
+			   int target_type, const void *target_data)
 {
   int i;
 
@@ -833,7 +866,8 @@ GOMP_offload_unregister (const void *hos
       struct gomp_device_descr *devicep = &devices[i];
       gomp_mutex_lock (&devicep->lock);
       if (devicep->type == target_type && devicep->is_initialized)
-	gomp_unload_image_from_device (devicep, host_table, target_data);
+	gomp_unload_image_from_device (version, devicep,
+				       host_table, target_data);
       gomp_mutex_unlock (&devicep->lock);
     }
 
@@ -848,6 +882,13 @@ GOMP_offload_unregister (const void *hos
   gomp_mutex_unlock (&register_lock);
 }
 
+void
+GOMP_offload_unregister (const void *host_table, int target_type,
+			 const void *target_data)
+{
+  GOMP_offload_unregister_2 (0, host_table, target_type, target_data);
+}
+
 /* This function initializes the target device, specified by DEVICEP.  DEVICEP
    must be locked on entry, and remains locked on return.  */
 
@@ -862,8 +903,9 @@ gomp_init_device (struct gomp_device_des
     {
       struct offload_image_descr *image = &offload_images[i];
       if (image->type == devicep->type)
-	gomp_load_image_to_device (devicep, image->host_table,
-				   image->target_data, false);
+	gomp_load_image_to_device (image->version, devicep,
+				   image->host_table, image->target_data,
+				   false);
     }
 
   devicep->is_initialized = true;
@@ -881,7 +923,8 @@ gomp_unload_device (struct gomp_device_d
 	{
 	  struct offload_image_descr *image = &offload_images[i];
 	  if (image->type == devicep->type)
-	    gomp_unload_image_from_device (devicep, image->host_table,
+	    gomp_unload_image_from_device (image->version, devicep,
+					   image->host_table,
 					   image->target_data);
 	}
     }
@@ -1135,6 +1178,8 @@ gomp_load_plugin_for_device (struct gomp
   DLSYM (free);
   DLSYM (dev2host);
   DLSYM (host2dev);
+  DLSYM_OPT (load_image_2, load_image_2);
+  DLSYM_OPT (unload_image_2, unload_image_2);
   device->capabilities = device->get_caps_func ();
   if (device->capabilities & GOMP_OFFLOAD_CAP_OPENMP_400)
     DLSYM (run);
Index: libgomp/libgomp.h
===================================================================
--- libgomp/libgomp.h	(revision 226039)
+++ libgomp/libgomp.h	(working copy)
@@ -749,7 +749,9 @@ struct gomp_device_descr
   void (*init_device_func) (int);
   void (*fini_device_func) (int);
   int (*load_image_func) (int, const void *, struct addr_pair **);
+  int (*load_image_2_func) (unsigned, int, const void *, struct addr_pair **);
   void (*unload_image_func) (int, const void *);
+  void (*unload_image_2_func) (unsigned, int, const void *);
   void *(*alloc_func) (int, size_t);
   void (*free_func) (int, void *);
   void *(*dev2host_func) (int, void *, const void *, size_t);
Index: libgomp/plugin/plugin-nvptx.c
===================================================================
--- libgomp/plugin/plugin-nvptx.c	(revision 226039)
+++ libgomp/plugin/plugin-nvptx.c	(working copy)
@@ -36,6 +36,7 @@
 #include "libgomp-plugin.h"
 #include "oacc-ptx.h"
 #include "oacc-plugin.h"
+#include "gomp-constants.h"
 
 #include <pthread.h>
 #include <cuda.h>
@@ -1648,8 +1649,8 @@ typedef struct nvptx_tdata
    number ORD.  Allocate and return TARGET_TABLE.  */
 
 int
-GOMP_OFFLOAD_load_image (int ord, const void *target_data,
-			 struct addr_pair **target_table)
+GOMP_OFFLOAD_load_image_2 (unsigned version, int ord, const void *target_data,
+			   struct addr_pair **target_table)
 {
   CUmodule module;
   const char *const *fn_names, *const *var_names;
@@ -1661,6 +1662,11 @@ GOMP_OFFLOAD_load_image (int ord, const
   struct ptx_image_data *new_image;
   struct ptx_device *dev;
 
+  if (GOMP_VERSION_DEV (version) != GOMP_VERSION_NVIDIA_PTX)
+    GOMP_PLUGIN_fatal ("Offload data incompatible with PTX plugin"
+		       " (version %u ! %u)",
+		       GOMP_VERSION_NVIDIA_PTX, GOMP_VERSION_DEV (version));
+  
   GOMP_OFFLOAD_init_device (ord);
 
   dev = ptx_devices[ord];
@@ -1726,15 +1732,26 @@ GOMP_OFFLOAD_load_image (int ord, const
   return fn_entries + var_entries;
 }
 
+int
+GOMP_OFFLOAD_load_image (int device,
+			 const void *target_image, struct addr_pair **result)
+{
+  return GOMP_OFFLOAD_load_image_2 (0, device, target_image, result);
+}
+
 /* Unload the program described by TARGET_DATA.  DEV_DATA is the
    function descriptors allocated by G_O_load_image.  */
 
 void
-GOMP_OFFLOAD_unload_image (int ord, const void *target_data)
+GOMP_OFFLOAD_unload_image_2 (unsigned version, int ord,
+			     const void *target_data)
 {
   struct ptx_image_data *image, **prev_p;
   struct ptx_device *dev = ptx_devices[ord];
 
+  if (GOMP_VERSION_DEV (version) != GOMP_VERSION_NVIDIA_PTX)
+    return;
+  
   pthread_mutex_lock (&dev->image_lock);
   for (prev_p = &dev->images; (image = *prev_p) != 0; prev_p = &image->next)
     if (image->target_data == target_data)
@@ -1748,6 +1765,12 @@ GOMP_OFFLOAD_unload_image (int ord, cons
   pthread_mutex_unlock (&dev->image_lock);
 }
 
+void
+GOMP_OFFLOAD_unload_image (int ord, const void *target_data)
+{
+  GOMP_OFFLOAD_unload_image_2 (0, ord, target_data);
+}
+
 void *
 GOMP_OFFLOAD_alloc (int ord, size_t size)
 {
Index: libgomp/libgomp.map
===================================================================
--- libgomp/libgomp.map	(revision 226039)
+++ libgomp/libgomp.map	(working copy)
@@ -234,6 +234,12 @@ GOMP_4.0.1 {
 	GOMP_offload_unregister;
 } GOMP_4.0;
 
+GOMP_4.0.2 {
+  global:
+	GOMP_offload_register_2;
+	GOMP_offload_unregister_2;
+} GOMP_4.0.1;
+
 OACC_2.0 {
   global:
 	acc_get_num_devices;
Index: include/gomp-constants.h
===================================================================
--- include/gomp-constants.h	(revision 226039)
+++ include/gomp-constants.h	(working copy)
@@ -113,4 +113,12 @@ enum gomp_map_kind
 #define GOMP_DEVICE_ICV			-1
 #define GOMP_DEVICE_HOST_FALLBACK	-2
 
+/* Versions of libgomp and device-specific plugins.  */
+#define GOMP_VERSION	0
+#define GOMP_VERSION_NVIDIA_PTX 1
+
+#define GOMP_VERSION_PACK(LIB, DEV) (((LIB) << 16) | (DEV))
+#define GOMP_VERSION_LIB(PACK) (((PACK) >> 16) & 0xffff)
+#define GOMP_VERSION_DEV(PACK) ((PACK) & 0xffff)
+
 #endif
Index: gcc/config/nvptx/mkoffload.c
===================================================================
--- gcc/config/nvptx/mkoffload.c	(revision 226039)
+++ gcc/config/nvptx/mkoffload.c	(working copy)
@@ -881,10 +881,10 @@ process (FILE *in, FILE *out)
 	   "extern \"C\" {\n"
 	   "#endif\n");
 
-  fprintf (out, "extern void GOMP_offload_register"
-	   " (const void *, int, const void *);\n");
-  fprintf (out, "extern void GOMP_offload_unregister"
-	   " (const void *, int, const void *);\n");
+  fprintf (out, "extern void GOMP_offload_register_2"
+	   " (unsigned, const void *, int, const void *);\n");
+  fprintf (out, "extern void GOMP_offload_unregister_2"
+	   " (unsigned, const void *, int, const void *);\n");
 
   fprintf (out, "#ifdef __cplusplus\n"
 	   "}\n"
@@ -894,15 +894,19 @@ process (FILE *in, FILE *out)
 
   fprintf (out, "static __attribute__((constructor)) void init (void)\n"
 	   "{\n"
-	   "  GOMP_offload_register (__OFFLOAD_TABLE__, %d/*NVIDIA_PTX*/,\n"
-	   "                         &target_data);\n"
-	   "};\n", GOMP_DEVICE_NVIDIA_PTX);
+	   "  GOMP_offload_register_2 (%#x, __OFFLOAD_TABLE__,"
+	   "%d/*NVIDIA_PTX*/, &target_data);\n"
+	   "};\n",
+	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
+	   GOMP_DEVICE_NVIDIA_PTX);
 
   fprintf (out, "static __attribute__((destructor)) void fini (void)\n"
 	   "{\n"
-	   "  GOMP_offload_unregister (__OFFLOAD_TABLE__, %d/*NVIDIA_PTX*/,\n"
-	   "                           &target_data);\n"
-	   "};\n", GOMP_DEVICE_NVIDIA_PTX);
+	   "  GOMP_offload_unregister_2 (%#x, __OFFLOAD_TABLE__,"
+	   "%d/*NVIDIA_PTX*/, &target_data);\n"
+	   "};\n",
+	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
+	   GOMP_DEVICE_NVIDIA_PTX);
 }
 
 static void

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-07-21 16:02 ` Nathan Sidwell
@ 2015-07-24 14:21   ` Nathan Sidwell
  2015-07-24 16:35     ` Jakub Jelinek
  2015-07-27 15:21     ` Nathan Sidwell
  0 siblings, 2 replies; 22+ messages in thread
From: Nathan Sidwell @ 2015-07-24 14:21 UTC (permalink / raw)
  To: Jakub Jelinek, ilya.verbin, GCC Patches

On 07/21/15 11:21, Nathan Sidwell wrote:
> On 07/21/15 09:25, Nathan Sidwell wrote:
>> This trunk patch implements new register and unregister entry points to allow
>> specifying data version information.  (I'll shortly be posting patches changing
>> the PTX offload data format.)
>>
>> We now have GOMP_offload_{,un}register_2, which take an additional unsigned int
>> version number.  The version number is composed of two parts.  16 bits for the
>> libgomp version and 16 bits for the device-specific plugin.  Currently both are
>> zero.  When the PTX data changes, that device-specific value will increment.
>>
>> The existing register/unregister calls forward to the new ones, providing zero
>> for the version information.
>>
>> On the plugin side I've added 2 new entry points GOMP_OFFLOAD_{,un}load_image_2,
>> which also take an additional version number argument.  These entry points are
>> optional, and only added to the PTX plugin.  The existing plugin entrypoints
>> forward to the new ones.
>>
>> libgomp  will use these new entry points if they exist, otherwise use the
>> original entry points, provided the incoming version is zero.
>>
>> I added the GOMP_offload_{,un}register_2 routines to the libgomp map file as
>> version 4.0.2 -- I wasn't sure whether to increment it more than that. Advice
>> sought.
>
> this version is updated following committing the unload patch.  there were a few
> (expected) collisions.

I committed a version to gomp4 branch, but would still like to get this to trunk 
ASAP.

nathan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-07-24 14:21   ` Nathan Sidwell
@ 2015-07-24 16:35     ` Jakub Jelinek
  2015-07-24 16:46       ` Ilya Verbin
  2015-07-24 17:22       ` Nathan Sidwell
  2015-07-27 15:21     ` Nathan Sidwell
  1 sibling, 2 replies; 22+ messages in thread
From: Jakub Jelinek @ 2015-07-24 16:35 UTC (permalink / raw)
  To: Nathan Sidwell; +Cc: ilya.verbin, GCC Patches

On Fri, Jul 24, 2015 at 09:32:04AM -0400, Nathan Sidwell wrote:
> On 07/21/15 11:21, Nathan Sidwell wrote:
> >On 07/21/15 09:25, Nathan Sidwell wrote:
> >>This trunk patch implements new register and unregister entry points to allow
> >>specifying data version information.  (I'll shortly be posting patches changing
> >>the PTX offload data format.)
> >>
> >>We now have GOMP_offload_{,un}register_2, which take an additional unsigned int
> >>version number.  The version number is composed of two parts.  16 bits for the
> >>libgomp version and 16 bits for the device-specific plugin.  Currently both are
> >>zero.  When the PTX data changes, that device-specific value will increment.
> >>
> >>The existing register/unregister calls forward to the new ones, providing zero
> >>for the version information.
> >>
> >>On the plugin side I've added 2 new entry points GOMP_OFFLOAD_{,un}load_image_2,
> >>which also take an additional version number argument.  These entry points are
> >>optional, and only added to the PTX plugin.  The existing plugin entrypoints
> >>forward to the new ones.
> >>
> >>libgomp  will use these new entry points if they exist, otherwise use the
> >>original entry points, provided the incoming version is zero.
> >>
> >>I added the GOMP_offload_{,un}register_2 routines to the libgomp map file as
> >>version 4.0.2 -- I wasn't sure whether to increment it more than that. Advice
> >>sought.
> >
> >this version is updated following committing the unload patch.  there were a few
> >(expected) collisions.
> 
> I committed a version to gomp4 branch, but would still like to get this to
> trunk ASAP.

So there is no version anywhere?  I remember in the design ideas the plan
was that the data section containing the target info (that originally has
been meant to be passed as GOMP_target parameter, but later on has been
changed to the register/unregister approach) will contain some header that
would include version number, some flags and details on the payload.
Do you mean that right now the data section (or pointer passed to the
register functions) only contains the raw bits (ELF DSO for Intel MIC and
PTX text files for NVPTX), rather than some header?
How do you determine the size of the bits?

In any case, I must say I don't like the GOMP_offload_{,un}register_2 names,
if they are really needed, then it should be GOMP_offload_{,un}register_ver
or something similar, _2 is just weird.
And I'd say that we don't really need to maintain support for mixing libgomp
from one GCC version and libgomp plugins from another version, worst case
there should be some GOMP_OFFLOAD_get_version function that libgomp could
use to verify it is talking to the right version of the plugin and
completely ignore it if it gives wrong version.

	Jakub

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-07-24 16:35     ` Jakub Jelinek
@ 2015-07-24 16:46       ` Ilya Verbin
  2015-07-24 17:22       ` Nathan Sidwell
  1 sibling, 0 replies; 22+ messages in thread
From: Ilya Verbin @ 2015-07-24 16:46 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Nathan Sidwell, ilya.verbin, GCC Patches

On Fri, Jul 24, 2015 at 18:30:16 +0200, Jakub Jelinek wrote:
> On Fri, Jul 24, 2015 at 09:32:04AM -0400, Nathan Sidwell wrote:
> > On 07/21/15 11:21, Nathan Sidwell wrote:
> > >On 07/21/15 09:25, Nathan Sidwell wrote:
> > >>This trunk patch implements new register and unregister entry points to allow
> > >>specifying data version information.  (I'll shortly be posting patches changing
> > >>the PTX offload data format.)
> > >>
> > >>We now have GOMP_offload_{,un}register_2, which take an additional unsigned int
> > >>version number.  The version number is composed of two parts.  16 bits for the
> > >>libgomp version and 16 bits for the device-specific plugin.  Currently both are
> > >>zero.  When the PTX data changes, that device-specific value will increment.
> > >>
> > >>The existing register/unregister calls forward to the new ones, providing zero
> > >>for the version information.
> > >>
> > >>On the plugin side I've added 2 new entry points GOMP_OFFLOAD_{,un}load_image_2,
> > >>which also take an additional version number argument.  These entry points are
> > >>optional, and only added to the PTX plugin.  The existing plugin entrypoints
> > >>forward to the new ones.
> > >>
> > >>libgomp  will use these new entry points if they exist, otherwise use the
> > >>original entry points, provided the incoming version is zero.
> > >>
> > >>I added the GOMP_offload_{,un}register_2 routines to the libgomp map file as
> > >>version 4.0.2 -- I wasn't sure whether to increment it more than that. Advice
> > >>sought.
> > >
> > >this version is updated following committing the unload patch.  there were a few
> > >(expected) collisions.
> > 
> > I committed a version to gomp4 branch, but would still like to get this to
> > trunk ASAP.
> 
> So there is no version anywhere?  I remember in the design ideas the plan
> was that the data section containing the target info (that originally has
> been meant to be passed as GOMP_target parameter, but later on has been
> changed to the register/unregister approach) will contain some header that
> would include version number, some flags and details on the payload.
> Do you mean that right now the data section (or pointer passed to the
> register functions) only contains the raw bits (ELF DSO for Intel MIC and
> PTX text files for NVPTX), rather than some header?
> How do you determine the size of the bits?

Yes, currently there is no version in target info, which is passed to register
function.  In case of MIC, this header contains only 2 fields: start and end of
the target image.

  -- Ilya

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-07-24 16:35     ` Jakub Jelinek
  2015-07-24 16:46       ` Ilya Verbin
@ 2015-07-24 17:22       ` Nathan Sidwell
  2015-07-24 19:51         ` Nathan Sidwell
  1 sibling, 1 reply; 22+ messages in thread
From: Nathan Sidwell @ 2015-07-24 17:22 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: ilya.verbin, GCC Patches

On 07/24/15 12:30, Jakub Jelinek wrote:

> So there is no version anywhere?  I remember in the design ideas the plan
> was that the data section containing the target info (that originally has
> been meant to be passed as GOMP_target parameter, but later on has been
> changed to the register/unregister approach) will contain some header that
> would include version number, some flags and details on the payload.
> Do you mean that right now the data section (or pointer passed to the
> register functions) only contains the raw bits (ELF DSO for Intel MIC and
> PTX text files for NVPTX), rather than some header?
> How do you determine the size of the bits?

There is no header information in the target data.  Certainly for PTX, and 
AFAICT for intel  mic too.  One cannot tell the format (or size) of the data 
just by looking at it.

I had thought about adding such a header, but that would be problematic in 
forwarding from the unversioned entry points to the versioned ones.

> In any case, I must say I don't like the GOMP_offload_{,un}register_2 names,
> if they are really needed, then it should be GOMP_offload_{,un}register_ver
> or something similar, _2 is just weird.

sure, I'm not very imaginative with names :)

> And I'd say that we don't really need to maintain support for mixing libgomp
> from one GCC version and libgomp plugins from another version, worst case
> there should be some GOMP_OFFLOAD_get_version function that libgomp could
> use to verify it is talking to the right version of the plugin and
> completely ignore it if it gives wrong version.

Ok, I wondered about making such an assumption.  Implementation-wise, I don't 
think another entry point's needed.  libgomp can observe the register_2 entry 
point and that contains the smarts to check the incoming 2-part version number 
from the target data.  I'm  pretty sure that'll be sufficient.

thanks for the review.  will work on updating patch.

nathan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-07-24 17:22       ` Nathan Sidwell
@ 2015-07-24 19:51         ` Nathan Sidwell
  2015-07-24 20:02           ` Ilya Verbin
  2015-07-31 12:30           ` Nathan Sidwell
  0 siblings, 2 replies; 22+ messages in thread
From: Nathan Sidwell @ 2015-07-24 19:51 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: ilya.verbin, GCC Patches

[-- Attachment #1: Type: text/plain, Size: 857 bytes --]

Jakub,
this version makes the following changes to the earlier version.

*) Renames things to FOO_ver, rather than FOO_2

*) No attempt to deal with cross-version plugins and libgomp.

*) Adds GOMP_OFFLOAD_version function to plugin. (I went with your approach). 
Returns the GOMP_VERSION used to build the plugin, which libgomp checks matches 
the value for its build.  When we make incompatible changes to the plugin 
interface, that value can be incremented.

*) While working on gomp_load_plugin_for_device, I noticed the DLSYM  and 
DLSYM_OPT macros were somewhat funky.  We're loading functions, so don't expect 
a NULL value.  We can simply check the returned value and only need dlerror when 
we get NULL.  The counting that DLSYM_OPT does was somewhat funky too.  IMHO 
better for that macro to simply return a truth value.

ok for trunk?

nathan


[-- Attachment #2: trunk-version-3.patch --]
[-- Type: text/x-patch, Size: 21408 bytes --]

2015-07-24  Nathan Sidwell  <nathan@codesourcery.com>

	include/
	* gomp-constants.h (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX): New.
	(GOMP_VERSION_PACK, GOMP_VERSION_LIB, GOMP_VERSION_DEV): New.

	gcc/
	* config/nvptx/mkoffload.c (process): Replace
	GOMP_offload_{,un}register with GOMP_offload_{,un}register_ver.

	libgomp/
	* libgomp.map: Add 4.0.2 version.
	* target.c (offload_image_descr): Add version field.
	(gomp_load_image_to_device): Add version argument.  Forward to
	versioning loader if available.  Improve load mismatch diagnostic.
	(gomp_unload_image_from_device): Add version argument.  Forward to
	versioning unloader if available.
	(GOMP_offload_regster): Make stub function, move bulk to ...
	(GOMP_offload_register_ver): ... here.  Process version argument.
	(GOMP_offload_unregister): Make stub function, move bulk to ...
	(GOMP_offload_unregister_ver): ... here.  Process version argument.
	(gomp_init_device): Process version field.
	(gomp_unload_device): Process version field.o
	(gomp_load_plugin_for_device): Reimplement DLSYM & DLSYM_OPT
	macros.  Look for versioning function and check it.  Fetch
	versioning loader and unloader if avaulable.
	* libgomp.h (gomp_device_descr): Add version function field.  Put
	loader and unloader fields in unions.
	* oacc-host.c (host_dispatch): Adjust.
	* plugin/plugin-nvptx.c: Include gomp-constants.h.
	(GOMP_OFFLOAD_version): New.
	(GOMP_OFFLOAD_load_image): Replace with ...
	(GOMP_OFFLOAD_load_image_ver): ... this.  Add version argument and
	check it.
	(GOMP_OFFLOAD_unload_image): Replace with ....
	(GOMP_OFFLOAD_unload_image_ver): ... this.  Add version argument and
	check it.

Index: include/gomp-constants.h
===================================================================
--- include/gomp-constants.h	(revision 226039)
+++ include/gomp-constants.h	(working copy)
@@ -113,4 +113,12 @@ enum gomp_map_kind
 #define GOMP_DEVICE_ICV			-1
 #define GOMP_DEVICE_HOST_FALLBACK	-2
 
+/* Versions of libgomp and device-specific plugins.  */
+#define GOMP_VERSION	0
+#define GOMP_VERSION_NVIDIA_PTX 0
+
+#define GOMP_VERSION_PACK(LIB, DEV) (((LIB) << 16) | (DEV))
+#define GOMP_VERSION_LIB(PACK) (((PACK) >> 16) & 0xffff)
+#define GOMP_VERSION_DEV(PACK) ((PACK) & 0xffff)
+
 #endif
Index: gcc/config/nvptx/mkoffload.c
===================================================================
--- gcc/config/nvptx/mkoffload.c	(revision 226039)
+++ gcc/config/nvptx/mkoffload.c	(working copy)
@@ -881,10 +881,10 @@ process (FILE *in, FILE *out)
 	   "extern \"C\" {\n"
 	   "#endif\n");
 
-  fprintf (out, "extern void GOMP_offload_register"
-	   " (const void *, int, const void *);\n");
-  fprintf (out, "extern void GOMP_offload_unregister"
-	   " (const void *, int, const void *);\n");
+  fprintf (out, "extern void GOMP_offload_register_ver"
+	   " (unsigned, const void *, int, const void *);\n");
+  fprintf (out, "extern void GOMP_offload_unregister_ver"
+	   " (unsigned, const void *, int, const void *);\n");
 
   fprintf (out, "#ifdef __cplusplus\n"
 	   "}\n"
@@ -894,15 +894,19 @@ process (FILE *in, FILE *out)
 
   fprintf (out, "static __attribute__((constructor)) void init (void)\n"
 	   "{\n"
-	   "  GOMP_offload_register (__OFFLOAD_TABLE__, %d/*NVIDIA_PTX*/,\n"
-	   "                         &target_data);\n"
-	   "};\n", GOMP_DEVICE_NVIDIA_PTX);
+	   "  GOMP_offload_register_ver (%#x, __OFFLOAD_TABLE__,"
+	   "%d/*NVIDIA_PTX*/, &target_data);\n"
+	   "};\n",
+	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
+	   GOMP_DEVICE_NVIDIA_PTX);
 
   fprintf (out, "static __attribute__((destructor)) void fini (void)\n"
 	   "{\n"
-	   "  GOMP_offload_unregister (__OFFLOAD_TABLE__, %d/*NVIDIA_PTX*/,\n"
-	   "                           &target_data);\n"
-	   "};\n", GOMP_DEVICE_NVIDIA_PTX);
+	   "  GOMP_offload_unregister_ver (%#x, __OFFLOAD_TABLE__,"
+	   "%d/*NVIDIA_PTX*/, &target_data);\n"
+	   "};\n",
+	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
+	   GOMP_DEVICE_NVIDIA_PTX);
 }
 
 static void
Index: libgomp/libgomp.map
===================================================================
--- libgomp/libgomp.map	(revision 226039)
+++ libgomp/libgomp.map	(working copy)
@@ -234,6 +234,12 @@ GOMP_4.0.1 {
 	GOMP_offload_unregister;
 } GOMP_4.0;
 
+GOMP_4.0.2 {
+  global:
+	GOMP_offload_register_ver;
+	GOMP_offload_unregister_ver;
+} GOMP_4.0.1;
+
 OACC_2.0 {
   global:
 	acc_get_num_devices;
Index: libgomp/target.c
===================================================================
--- libgomp/target.c	(revision 226039)
+++ libgomp/target.c	(working copy)
@@ -56,6 +56,7 @@ static gomp_mutex_t register_lock;
    It contains type of the target device, pointer to host table descriptor, and
    pointer to target data.  */
 struct offload_image_descr {
+  unsigned version;
   enum offload_target_type type;
   const void *host_table;
   const void *target_data;
@@ -642,7 +643,8 @@ gomp_update (struct gomp_device_descr *d
    emitting variable and functions in the same order.  */
 
 static void
-gomp_load_image_to_device (struct gomp_device_descr *devicep,
+gomp_load_image_to_device (unsigned version,
+			   struct gomp_device_descr *devicep,
 			   const void *host_table, const void *target_data,
 			   bool is_register_lock)
 {
@@ -658,16 +660,28 @@ gomp_load_image_to_device (struct gomp_d
 
   /* Load image to device and get target addresses for the image.  */
   struct addr_pair *target_table = NULL;
-  int i, num_target_entries
-    = devicep->load_image_func (devicep->target_id, target_data,
-				&target_table);
+  int i, num_target_entries;
+
+  if (devicep->version_func)
+    num_target_entries
+      = devicep->load_image.ver_func (version, devicep->target_id,
+					target_data, &target_table);
+  else if (GOMP_VERSION_DEV (version))
+    gomp_fatal ("Plugin too old for offload data (0 < %u)",
+		GOMP_VERSION_DEV (version));
+  else
+    num_target_entries
+      = devicep->load_image.unver_func (devicep->target_id,
+					  target_data, &target_table);
 
   if (num_target_entries != num_funcs + num_vars)
     {
       gomp_mutex_unlock (&devicep->lock);
       if (is_register_lock)
 	gomp_mutex_unlock (&register_lock);
-      gomp_fatal ("Can't map target functions or variables");
+      gomp_fatal ("Cannot map target functions or variables"
+		  " (expected %u, have %u)", num_funcs + num_vars,
+		  num_target_entries);
     }
 
   /* Insert host-target address mapping into splay tree.  */
@@ -731,7 +745,8 @@ gomp_load_image_to_device (struct gomp_d
    The device must be locked.   */
 
 static void
-gomp_unload_image_from_device (struct gomp_device_descr *devicep,
+gomp_unload_image_from_device (unsigned version,
+			       struct gomp_device_descr *devicep,
 			       const void *host_table, const void *target_data)
 {
   void **host_func_table = ((void ***) host_table)[0];
@@ -756,8 +771,12 @@ gomp_unload_image_from_device (struct go
       k.host_end = k.host_start + 1;
       node = splay_tree_lookup (&devicep->mem_map, &k);
     }
-  
-  devicep->unload_image_func (devicep->target_id, target_data);
+
+  if (devicep->version_func)
+    devicep->unload_image.ver_func (version,
+				    devicep->target_id, target_data);
+  else
+    devicep->unload_image.unver_func (devicep->target_id, target_data);
 
   /* Remove mappings from splay tree.  */
   for (j = 0; j < num_funcs; j++)
@@ -786,10 +805,15 @@ gomp_unload_image_from_device (struct go
    the target, and TARGET_DATA needed by target plugin.  */
 
 void
-GOMP_offload_register (const void *host_table, int target_type,
-		       const void *target_data)
+GOMP_offload_register_ver (unsigned version, const void *host_table,
+			   int target_type, const void *target_data)
 {
   int i;
+
+  if (GOMP_VERSION_LIB (version) > GOMP_VERSION)
+    gomp_fatal ("Library too old for offload (version %u < %u)",
+		GOMP_VERSION, GOMP_VERSION_LIB (version));
+  
   gomp_mutex_lock (&register_lock);
 
   /* Load image to all initialized devices.  */
@@ -798,7 +822,8 @@ GOMP_offload_register (const void *host_
       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, host_table, target_data, true);
+	gomp_load_image_to_device (version, devicep,
+				   host_table, target_data, true);
       gomp_mutex_unlock (&devicep->lock);
     }
 
@@ -807,6 +832,7 @@ GOMP_offload_register (const void *host_
     = gomp_realloc_unlock (offload_images,
 			   (num_offload_images + 1)
 			   * sizeof (struct offload_image_descr));
+  offload_images[num_offload_images].version = version;
   offload_images[num_offload_images].type = target_type;
   offload_images[num_offload_images].host_table = host_table;
   offload_images[num_offload_images].target_data = target_data;
@@ -815,13 +841,20 @@ GOMP_offload_register (const void *host_
   gomp_mutex_unlock (&register_lock);
 }
 
+void
+GOMP_offload_register (const void *host_table, int target_type,
+		       const void *target_data)
+{
+  GOMP_offload_register_ver (0, host_table, target_type, target_data);
+}
+
 /* This function should be called from every offload image while unloading.
    It gets the descriptor of the host func and var tables HOST_TABLE, TYPE of
    the target, and TARGET_DATA needed by target plugin.  */
 
 void
-GOMP_offload_unregister (const void *host_table, int target_type,
-			 const void *target_data)
+GOMP_offload_unregister_ver (unsigned version, const void *host_table,
+			     int target_type, const void *target_data)
 {
   int i;
 
@@ -833,7 +866,8 @@ GOMP_offload_unregister (const void *hos
       struct gomp_device_descr *devicep = &devices[i];
       gomp_mutex_lock (&devicep->lock);
       if (devicep->type == target_type && devicep->is_initialized)
-	gomp_unload_image_from_device (devicep, host_table, target_data);
+	gomp_unload_image_from_device (version, devicep,
+				       host_table, target_data);
       gomp_mutex_unlock (&devicep->lock);
     }
 
@@ -848,6 +882,13 @@ GOMP_offload_unregister (const void *hos
   gomp_mutex_unlock (&register_lock);
 }
 
+void
+GOMP_offload_unregister (const void *host_table, int target_type,
+			 const void *target_data)
+{
+  GOMP_offload_unregister_ver (0, host_table, target_type, target_data);
+}
+
 /* This function initializes the target device, specified by DEVICEP.  DEVICEP
    must be locked on entry, and remains locked on return.  */
 
@@ -862,8 +903,9 @@ gomp_init_device (struct gomp_device_des
     {
       struct offload_image_descr *image = &offload_images[i];
       if (image->type == devicep->type)
-	gomp_load_image_to_device (devicep, image->host_table,
-				   image->target_data, false);
+	gomp_load_image_to_device (image->version, devicep,
+				   image->host_table, image->target_data,
+				   false);
     }
 
   devicep->is_initialized = true;
@@ -881,7 +923,8 @@ gomp_unload_device (struct gomp_device_d
 	{
 	  struct offload_image_descr *image = &offload_images[i];
 	  if (image->type == devicep->type)
-	    gomp_unload_image_from_device (devicep, image->host_table,
+	    gomp_unload_image_from_device (image->version, devicep,
+					   image->host_table,
 					   image->target_data);
 	}
     }
@@ -1085,43 +1128,41 @@ gomp_load_plugin_for_device (struct gomp
 			     const char *plugin_name)
 {
   const char *err = NULL, *last_missing = NULL;
-  int optional_present, optional_total;
-
-  /* Clear any existing error.  */
-  dlerror ();
 
   void *plugin_handle = dlopen (plugin_name, RTLD_LAZY);
   if (!plugin_handle)
-    {
-      err = dlerror ();
-      goto out;
-    }
+    goto dl_fail;
 
   /* Check if all required functions are available in the plugin and store
-     their handlers.  */
+     their handlers.  None of the symbols can legitimately be NULL,
+     so we don't need to check dlerror all the time.  */
 #define DLSYM(f)							\
-  do									\
-    {									\
-      device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #f);	\
-      err = dlerror ();							\
-      if (err != NULL)							\
-	goto out;							\
-    }									\
-  while (0)
-  /* Similar, but missing functions are not an error.  */
-#define DLSYM_OPT(f, n)						\
-  do									\
-    {									\
-      const char *tmp_err;							\
-      device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #n);	\
-      tmp_err = dlerror ();						\
-      if (tmp_err == NULL)						\
-        optional_present++;						\
-      else								\
-        last_missing = #n;						\
-      optional_total++;							\
-    }									\
-  while (0)
+  if (!(device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #f)))	\
+    goto dl_fail
+  /* Similar, but missing functions are not an error.  Return false if
+     failed, true otherwise.  */
+#define DLSYM_OPT(f, n)							\
+  ((device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #n))	\
+   || (last_missing = #n, 0))
+
+  if (DLSYM_OPT (version, version))
+    {
+      unsigned v = device->version_func ();
+      if (v != GOMP_VERSION)
+	{
+	  err = "plugin version mismatch";
+	  goto fail;
+	}
+      if (!DLSYM_OPT (load_image.ver, load_image_ver)
+	  || !DLSYM_OPT (unload_image.ver, unload_image_ver))
+	goto  dl_fail;
+    }
+  else
+    {
+      if (!DLSYM_OPT (load_image.unver, load_image)
+	  || !DLSYM_OPT (unload_image.unver, unload_image))
+	goto dl_fail;
+    }
 
   DLSYM (get_name);
   DLSYM (get_caps);
@@ -1129,8 +1170,6 @@ gomp_load_plugin_for_device (struct gomp
   DLSYM (get_num_devices);
   DLSYM (init_device);
   DLSYM (fini_device);
-  DLSYM (load_image);
-  DLSYM (unload_image);
   DLSYM (alloc);
   DLSYM (free);
   DLSYM (dev2host);
@@ -1140,53 +1179,57 @@ gomp_load_plugin_for_device (struct gomp
     DLSYM (run);
   if (device->capabilities & GOMP_OFFLOAD_CAP_OPENACC_200)
     {
-      optional_present = optional_total = 0;
-      DLSYM_OPT (openacc.exec, openacc_parallel);
-      DLSYM_OPT (openacc.register_async_cleanup,
-		 openacc_register_async_cleanup);
-      DLSYM_OPT (openacc.async_test, openacc_async_test);
-      DLSYM_OPT (openacc.async_test_all, openacc_async_test_all);
-      DLSYM_OPT (openacc.async_wait, openacc_async_wait);
-      DLSYM_OPT (openacc.async_wait_async, openacc_async_wait_async);
-      DLSYM_OPT (openacc.async_wait_all, openacc_async_wait_all);
-      DLSYM_OPT (openacc.async_wait_all_async, openacc_async_wait_all_async);
-      DLSYM_OPT (openacc.async_set_async, openacc_async_set_async);
-      DLSYM_OPT (openacc.create_thread_data, openacc_create_thread_data);
-      DLSYM_OPT (openacc.destroy_thread_data, openacc_destroy_thread_data);
-      /* Require all the OpenACC handlers if we have
-	 GOMP_OFFLOAD_CAP_OPENACC_200.  */
-      if (optional_present != optional_total)
+      if (!DLSYM_OPT (openacc.exec, openacc_parallel)
+	  || !DLSYM_OPT (openacc.register_async_cleanup,
+			 openacc_register_async_cleanup)
+	  || !DLSYM_OPT (openacc.async_test, openacc_async_test)
+	  || !DLSYM_OPT (openacc.async_test_all, openacc_async_test_all)
+	  || !DLSYM_OPT (openacc.async_wait, openacc_async_wait)
+	  || !DLSYM_OPT (openacc.async_wait_async, openacc_async_wait_async)
+	  || !DLSYM_OPT (openacc.async_wait_all, openacc_async_wait_all)
+	  || !DLSYM_OPT (openacc.async_wait_all_async,
+			 openacc_async_wait_all_async)
+	  || !DLSYM_OPT (openacc.async_set_async, openacc_async_set_async)
+	  || !DLSYM_OPT (openacc.create_thread_data,
+			 openacc_create_thread_data)
+	  || !DLSYM_OPT (openacc.destroy_thread_data,
+			 openacc_destroy_thread_data))
 	{
+	  /* Require all the OpenACC handlers if we have
+	     GOMP_OFFLOAD_CAP_OPENACC_200.  */
 	  err = "plugin missing OpenACC handler function";
-	  goto out;
+	  goto fail;
 	}
-      optional_present = optional_total = 0;
-      DLSYM_OPT (openacc.cuda.get_current_device,
-		 openacc_get_current_cuda_device);
-      DLSYM_OPT (openacc.cuda.get_current_context,
-		 openacc_get_current_cuda_context);
-      DLSYM_OPT (openacc.cuda.get_stream, openacc_get_cuda_stream);
-      DLSYM_OPT (openacc.cuda.set_stream, openacc_set_cuda_stream);
-      /* Make sure all the CUDA functions are there if any of them are.  */
-      if (optional_present && optional_present != optional_total)
+
+      unsigned cuda = 0;
+      cuda += DLSYM_OPT (openacc.cuda.get_current_device,
+			 openacc_get_current_cuda_device);
+      cuda += DLSYM_OPT (openacc.cuda.get_current_context,
+			 openacc_get_current_cuda_context);
+      cuda += DLSYM_OPT (openacc.cuda.get_stream, openacc_get_cuda_stream);
+      cuda += DLSYM_OPT (openacc.cuda.set_stream, openacc_set_cuda_stream);
+      if (cuda && cuda != 4)
 	{
+	  /* Make sure all the CUDA functions are there if any of them are.  */
 	  err = "plugin missing OpenACC CUDA handler function";
-	  goto out;
+	  goto fail;
 	}
     }
 #undef DLSYM
 #undef DLSYM_OPT
 
- out:
-  if (err != NULL)
-    {
-      gomp_error ("while loading %s: %s", plugin_name, err);
-      if (last_missing)
-        gomp_error ("missing function was %s", last_missing);
-      if (plugin_handle)
-	dlclose (plugin_handle);
-    }
-  return err == NULL;
+  return 1;
+
+ dl_fail:
+  err = dlerror ();
+ fail:
+  gomp_error ("while loading %s: %s", plugin_name, err);
+  if (last_missing)
+    gomp_error ("missing function was %s", last_missing);
+  if (plugin_handle)
+    dlclose (plugin_handle);
+
+  return 0;
 }
 
 /* This function initializes the runtime needed for offloading.
Index: libgomp/libgomp.h
===================================================================
--- libgomp/libgomp.h	(revision 226039)
+++ libgomp/libgomp.h	(working copy)
@@ -748,8 +748,22 @@ struct gomp_device_descr
   int (*get_num_devices_func) (void);
   void (*init_device_func) (int);
   void (*fini_device_func) (int);
-  int (*load_image_func) (int, const void *, struct addr_pair **);
-  void (*unload_image_func) (int, const void *);
+
+  unsigned (*version_func) (void);
+  
+  /* When all plugins updated, we can remove these unions and just
+     have the versioned entry points.  */
+  union 
+  {
+    int (*unver_func) (int, const void *, struct addr_pair **);
+    int (*ver_func) (unsigned, int, const void *, struct addr_pair **);
+  } load_image;
+  union
+  {
+    void (*unver_func) (int, const void *);
+    void (*ver_func) (unsigned, int, const void *);
+  } unload_image;
+  
   void *(*alloc_func) (int, size_t);
   void (*free_func) (int, void *);
   void *(*dev2host_func) (int, void *, const void *, size_t);
Index: libgomp/plugin/plugin-nvptx.c
===================================================================
--- libgomp/plugin/plugin-nvptx.c	(revision 226039)
+++ libgomp/plugin/plugin-nvptx.c	(working copy)
@@ -36,6 +36,7 @@
 #include "libgomp-plugin.h"
 #include "oacc-ptx.h"
 #include "oacc-plugin.h"
+#include "gomp-constants.h"
 
 #include <pthread.h>
 #include <cuda.h>
@@ -1644,12 +1645,22 @@ typedef struct nvptx_tdata
   size_t fn_num;
 } nvptx_tdata_t;
 
+/* Return the libgomp version number we're compatible with.  There is
+   no requirement for cross-version compatibility.  */
+
+unsigned
+GOMP_OFFLOAD_version (void)
+{
+  return GOMP_VERSION;
+}
+
 /* Load the (partial) program described by TARGET_DATA to device
    number ORD.  Allocate and return TARGET_TABLE.  */
 
 int
-GOMP_OFFLOAD_load_image (int ord, const void *target_data,
-			 struct addr_pair **target_table)
+GOMP_OFFLOAD_load_image_ver (unsigned version, int ord,
+			     const void *target_data,
+			     struct addr_pair **target_table)
 {
   CUmodule module;
   const char *const *fn_names, *const *var_names;
@@ -1661,6 +1672,11 @@ GOMP_OFFLOAD_load_image (int ord, const
   struct ptx_image_data *new_image;
   struct ptx_device *dev;
 
+  if (GOMP_VERSION_DEV (version) != GOMP_VERSION_NVIDIA_PTX)
+    GOMP_PLUGIN_fatal ("Offload data incompatible with PTX plugin"
+		       " (version %u != %u)",
+		       GOMP_VERSION_NVIDIA_PTX, GOMP_VERSION_DEV (version));
+  
   GOMP_OFFLOAD_init_device (ord);
 
   dev = ptx_devices[ord];
@@ -1730,11 +1746,15 @@ GOMP_OFFLOAD_load_image (int ord, const
    function descriptors allocated by G_O_load_image.  */
 
 void
-GOMP_OFFLOAD_unload_image (int ord, const void *target_data)
+GOMP_OFFLOAD_unload_image_ver (unsigned version, int ord,
+			       const void *target_data)
 {
   struct ptx_image_data *image, **prev_p;
   struct ptx_device *dev = ptx_devices[ord];
 
+  if (GOMP_VERSION_DEV (version) != GOMP_VERSION_NVIDIA_PTX)
+    return;
+  
   pthread_mutex_lock (&dev->image_lock);
   for (prev_p = &dev->images; (image = *prev_p) != 0; prev_p = &image->next)
     if (image->target_data == target_data)
Index: libgomp/oacc-host.c
===================================================================
--- libgomp/oacc-host.c	(revision 226039)
+++ libgomp/oacc-host.c	(working copy)
@@ -45,8 +45,9 @@ static struct gomp_device_descr host_dis
     .get_num_devices_func = GOMP_OFFLOAD_get_num_devices,
     .init_device_func = GOMP_OFFLOAD_init_device,
     .fini_device_func = GOMP_OFFLOAD_fini_device,
-    .load_image_func = GOMP_OFFLOAD_load_image,
-    .unload_image_func = GOMP_OFFLOAD_unload_image,
+    .version_func = NULL,
+    .load_image = {.unver_func = GOMP_OFFLOAD_load_image},
+    .unload_image = {.unver_func = GOMP_OFFLOAD_unload_image},
     .alloc_func = GOMP_OFFLOAD_alloc,
     .free_func = GOMP_OFFLOAD_free,
     .dev2host_func = GOMP_OFFLOAD_dev2host,

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-07-24 19:51         ` Nathan Sidwell
@ 2015-07-24 20:02           ` Ilya Verbin
  2015-07-24 20:05             ` Nathan Sidwell
  2015-07-31 12:30           ` Nathan Sidwell
  1 sibling, 1 reply; 22+ messages in thread
From: Ilya Verbin @ 2015-07-24 20:02 UTC (permalink / raw)
  To: Nathan Sidwell; +Cc: Jakub Jelinek, GCC Patches

On Fri, Jul 24, 2015 at 15:26:38 -0400, Nathan Sidwell wrote:
> this version makes the following changes to the earlier version.
> 
> *) Renames things to FOO_ver, rather than FOO_2
> 
> *) No attempt to deal with cross-version plugins and libgomp.
> 
> *) Adds GOMP_OFFLOAD_version function to plugin. (I went with your
> approach). Returns the GOMP_VERSION used to build the plugin, which
> libgomp checks matches the value for its build.  When we make
> incompatible changes to the plugin interface, that value can be
> incremented.
> 
> *) While working on gomp_load_plugin_for_device, I noticed the DLSYM
> and DLSYM_OPT macros were somewhat funky.  We're loading functions,
> so don't expect a NULL value.  We can simply check the returned
> value and only need dlerror when we get NULL.  The counting that
> DLSYM_OPT does was somewhat funky too.  IMHO better for that macro
> to simply return a truth value.

I do not know whether this is a good idea, but it's possible to add some magic
number into mkoffload:process () like:

865   fprintf (out, "static const void *target_data[] = {\n");
866   fprintf (out, "  MAGIC, VERSION, ptx_code, (void*) %u, var_mappings, (void*) %u, "
867                 "func_mappings\n", nvars, nfuncs);
868   fprintf (out, "};\n\n");

So, libgomp will be able to check target_data in GOMP_offload_register.
If MAGIC is present, it can check the VERSION, the plugin also can check the
version in a similar way.  This hack allows to avoid new versions of
GOMP_*_ver in libgomp and GOMP_OFFLOAD_*_ver in plugins.

  -- Ilya

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-07-24 20:02           ` Ilya Verbin
@ 2015-07-24 20:05             ` Nathan Sidwell
  0 siblings, 0 replies; 22+ messages in thread
From: Nathan Sidwell @ 2015-07-24 20:05 UTC (permalink / raw)
  To: Ilya Verbin; +Cc: Jakub Jelinek, GCC Patches

On 07/24/15 15:51, Ilya Verbin wrote:
> On Fri, Jul 24, 2015 at 15:26:38 -0400, Nathan Sidwell wrote:
mber into mkoffload:process () like:
>
> 865   fprintf (out, "static const void *target_data[] = {\n");
> 866   fprintf (out, "  MAGIC, VERSION, ptx_code, (void*) %u, var_mappings, (void*) %u, "
> 867                 "func_mappings\n", nvars, nfuncs);
> 868   fprintf (out, "};\n\n");
>
> So, libgomp will be able to check target_data in GOMP_offload_register.
> If MAGIC is present, it can check the VERSION, the plugin also can check the

I do not like this approach.  It relies on finding a MAGIC number.

> version in a similar way.  This hack allows to avoid new versions of
> GOMP_*_ver in libgomp and GOMP_OFFLOAD_*_ver in plugins.

true, but I don't think that's where the complexity lies.

nathan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-07-24 14:21   ` Nathan Sidwell
  2015-07-24 16:35     ` Jakub Jelinek
@ 2015-07-27 15:21     ` Nathan Sidwell
  1 sibling, 0 replies; 22+ messages in thread
From: Nathan Sidwell @ 2015-07-27 15:21 UTC (permalink / raw)
  To: Jakub Jelinek, ilya.verbin, GCC Patches

[-- Attachment #1: Type: text/plain, Size: 296 bytes --]

On 07/24/15 09:32, Nathan Sidwell wrote:
> On 07/21/15 11:21, Nathan Sidwell wrote:

>
> I committed a version to gomp4 branch, but would still like to get this to trunk
> ASAP.

I committed this update to the gomp4 branch to match the updated version 
currently under review for trunk.

nathan


[-- Attachment #2: gomp4-version-2.patch --]
[-- Type: text/x-patch, Size: 16956 bytes --]

2015-07-27  Nathan Sidwell  <nathan@codesourcery.com>

	include/
	* gomp-constants.h (GOMP_VERSION_NVIDIA_PTX): Restore to zero.

	libgomp/
	* libgomp.map: Rename GOMP_offload_{,un}register_2 to
	GOMP_offload_{,un}register_ver.
	* target.c (gomp_load_image_to_device): Check version function to
	determine interface to plugin.
	(gomp_unload_image_from_device): Likewise.
	(GOMP_offload_register_2): Rename to ...
	(GOMP_offload_register_ver): ... here.
	(GOMP_offload_unregister_2): Rename to ...
	(GOMP_offload_unregister_ver): ... here.
	(GOMP_offload_register, GOMP_offload_unregister): Adjust
	forwarding.
	(gomp_load_plugin_for_device): Reimplement DLSYM & DLSYM_OPT
	macros.  Look for versioning function and check it.  Fetch
	versioning loader and unloader if available.
	* libgomp.h (gomp_device_descr): Add version function field.  Put
	loader and unloader fields in unions.
	* plugin/plugin-nvptx.c (GOMP_OFFLOAD_version): New.
	(GOMP_OFFLOAD_load_image_2): Renamed to ...
	(GOMP_OFFLOAD_load_image_ver): ... here.
	(GOMP_OFFLOAD_unload_image_2): Renamed to ...
	(GOMP_OFFLOAD_unload_image_ver): ... here.
	(GOMP_OFFLOAD_load_image, GOMP_OFFLOAD_unload_image): Delete.
	* oacc-host.c (host_dispatch): Adjust.

	gcc/
	* config/nvptx/mkoffload.c (process): Rename _2 functions to _ver.

Index: libgomp/libgomp.map
===================================================================
--- libgomp/libgomp.map	(revision 226237)
+++ libgomp/libgomp.map	(working copy)
@@ -236,8 +236,8 @@ GOMP_4.0.1 {
 
 GOMP_4.0.2 {
   global:
-	GOMP_offload_register_2;
-	GOMP_offload_unregister_2;
+	GOMP_offload_register_ver;
+	GOMP_offload_unregister_ver;
 } GOMP_4.0.1;
 
 OACC_2.0 {
Index: libgomp/target.c
===================================================================
--- libgomp/target.c	(revision 226237)
+++ libgomp/target.c	(working copy)
@@ -671,17 +671,17 @@ gomp_load_image_to_device (unsigned vers
   struct addr_pair *target_table = NULL;
   int i, num_target_entries;
 
-  if (devicep->load_image_2_func)
+  if (devicep->version_func)
     num_target_entries
-      = devicep->load_image_2_func (version, devicep->target_id,
-				    target_data, &target_table);
+      = devicep->load_image.ver_func (version, devicep->target_id,
+					target_data, &target_table);
   else if (GOMP_VERSION_DEV (version))
     gomp_fatal ("Plugin too old for offload data (0 < %u)",
 		GOMP_VERSION_DEV (version));
   else
     num_target_entries
-      = devicep->load_image_func (devicep->target_id,
-				  target_data, &target_table);
+      = devicep->load_image.unver_func (devicep->target_id,
+					target_data, &target_table);
 
   if (num_target_entries != num_funcs + num_vars)
     {
@@ -790,11 +790,11 @@ gomp_unload_image_from_device (unsigned
       node = splay_tree_lookup (&devicep->mem_map, &k);
     }
 
-  if (devicep->unload_image_2_func)
-    devicep->unload_image_2_func (version,
-				  devicep->target_id, target_data);
+  if (devicep->version_func)
+    devicep->unload_image.ver_func (version,
+				    devicep->target_id, target_data);
   else
-    devicep->unload_image_func (devicep->target_id, target_data);
+    devicep->unload_image.unver_func (devicep->target_id, target_data);
 
   /* Remove mappings from splay tree.  */
   for (j = 0; j < num_funcs; j++)
@@ -823,8 +823,8 @@ gomp_unload_image_from_device (unsigned
    the target, and TARGET_DATA needed by target plugin.  */
 
 void
-GOMP_offload_register_2 (unsigned version, const void *host_table,
-			 int target_type, const void *target_data)
+GOMP_offload_register_ver (unsigned version, const void *host_table,
+			   int target_type, const void *target_data)
 {
   int i;
 
@@ -863,7 +863,7 @@ void
 GOMP_offload_register (const void *host_table, int target_type,
 		       const void *target_data)
 {
-  GOMP_offload_register_2 (0, host_table, target_type, target_data);
+  GOMP_offload_register_ver (0, host_table, target_type, target_data);
 }
 
 /* This function should be called from every offload image while unloading.
@@ -871,8 +871,8 @@ GOMP_offload_register (const void *host_
    the target, and TARGET_DATA needed by target plugin.  */
 
 void
-GOMP_offload_unregister_2 (unsigned version, const void *host_table,
-			   int target_type, const void *target_data)
+GOMP_offload_unregister_ver (unsigned version, const void *host_table,
+			     int target_type, const void *target_data)
 {
   int i;
 
@@ -904,7 +904,7 @@ void
 GOMP_offload_unregister (const void *host_table, int target_type,
 			 const void *target_data)
 {
-  GOMP_offload_unregister_2 (0, host_table, target_type, target_data);
+  GOMP_offload_unregister_ver (0, host_table, target_type, target_data);
 }
 
 /* This function initializes the target device, specified by DEVICEP.  DEVICEP
@@ -1130,43 +1130,41 @@ gomp_load_plugin_for_device (struct gomp
 			     const char *plugin_name)
 {
   const char *err = NULL, *last_missing = NULL;
-  int optional_present = 0, optional_total = 0;
-
-  /* Clear any existing error.  */
-  dlerror ();
 
   void *plugin_handle = dlopen (plugin_name, RTLD_LAZY);
   if (!plugin_handle)
-    {
-      err = dlerror ();
-      goto out;
-    }
+    goto dl_fail;
 
   /* Check if all required functions are available in the plugin and store
-     their handlers.  */
+     their handlers.  None of the symbols can legitimately be NULL,
+     so we don't need to check dlerror all the time.  */
 #define DLSYM(f)							\
-  do									\
-    {									\
-      device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #f);	\
-      err = dlerror ();							\
-      if (err != NULL)							\
-	goto out;							\
-    }									\
-  while (0)
-  /* Similar, but missing functions are not an error.  */
-#define DLSYM_OPT(f, n)						\
-  do									\
-    {									\
-      const char *tmp_err;							\
-      device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #n);	\
-      tmp_err = dlerror ();						\
-      if (tmp_err == NULL)						\
-        optional_present++;						\
-      else								\
-        last_missing = #n;						\
-      optional_total++;							\
-    }									\
-  while (0)
+  if (!(device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #f)))	\
+    goto dl_fail
+  /* Similar, but missing functions are not an error.  Return false if
+     failed, true otherwise.  */
+#define DLSYM_OPT(f, n)							\
+  ((device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #n))	\
+   || (last_missing = #n, 0))
+
+  if (DLSYM_OPT (version, version))
+    {
+      unsigned v = device->version_func ();
+      if (v != GOMP_VERSION)
+	{
+	  err = "plugin version mismatch";
+	  goto fail;
+	}
+      if (!DLSYM_OPT (load_image.ver, load_image_ver)
+	  || !DLSYM_OPT (unload_image.ver, unload_image_ver))
+	goto  dl_fail;
+    }
+  else
+    {
+      if (!DLSYM_OPT (load_image.unver, load_image)
+	  || !DLSYM_OPT (unload_image.unver, unload_image))
+	goto dl_fail;
+    }
 
   DLSYM (get_name);
   DLSYM (get_caps);
@@ -1174,66 +1172,67 @@ gomp_load_plugin_for_device (struct gomp
   DLSYM (get_num_devices);
   DLSYM (init_device);
   DLSYM (fini_device);
-  DLSYM (load_image);
-  DLSYM (unload_image);
   DLSYM (alloc);
   DLSYM (free);
   DLSYM (dev2host);
   DLSYM (host2dev);
-  DLSYM_OPT (load_image_2, load_image_2);
-  DLSYM_OPT (unload_image_2, unload_image_2);
   device->capabilities = device->get_caps_func ();
   if (device->capabilities & GOMP_OFFLOAD_CAP_OPENMP_400)
     DLSYM (run);
   if (device->capabilities & GOMP_OFFLOAD_CAP_OPENACC_200)
     {
-      optional_present = optional_total = 0;
-      DLSYM_OPT (openacc.exec, openacc_parallel);
-      DLSYM_OPT (openacc.register_async_cleanup,
-		 openacc_register_async_cleanup);
-      DLSYM_OPT (openacc.async_test, openacc_async_test);
-      DLSYM_OPT (openacc.async_test_all, openacc_async_test_all);
-      DLSYM_OPT (openacc.async_wait, openacc_async_wait);
-      DLSYM_OPT (openacc.async_wait_async, openacc_async_wait_async);
-      DLSYM_OPT (openacc.async_wait_all, openacc_async_wait_all);
-      DLSYM_OPT (openacc.async_wait_all_async, openacc_async_wait_all_async);
-      DLSYM_OPT (openacc.async_set_async, openacc_async_set_async);
-      DLSYM_OPT (openacc.create_thread_data, openacc_create_thread_data);
-      DLSYM_OPT (openacc.destroy_thread_data, openacc_destroy_thread_data);
-      /* Require all the OpenACC handlers if we have
-	 GOMP_OFFLOAD_CAP_OPENACC_200.  */
-      if (optional_present != optional_total)
+      if (!DLSYM_OPT (openacc.exec, openacc_parallel)
+	  || !DLSYM_OPT (openacc.register_async_cleanup,
+			 openacc_register_async_cleanup)
+	  || !DLSYM_OPT (openacc.async_test, openacc_async_test)
+	  || !DLSYM_OPT (openacc.async_test_all, openacc_async_test_all)
+	  || !DLSYM_OPT (openacc.async_wait, openacc_async_wait)
+	  || !DLSYM_OPT (openacc.async_wait_async, openacc_async_wait_async)
+	  || !DLSYM_OPT (openacc.async_wait_all, openacc_async_wait_all)
+	  || !DLSYM_OPT (openacc.async_wait_all_async,
+			 openacc_async_wait_all_async)
+	  || !DLSYM_OPT (openacc.async_set_async, openacc_async_set_async)
+	  || !DLSYM_OPT (openacc.create_thread_data,
+			 openacc_create_thread_data)
+	  || !DLSYM_OPT (openacc.destroy_thread_data,
+			 openacc_destroy_thread_data))
 	{
+	  /* Require all the OpenACC handlers if we have
+	     GOMP_OFFLOAD_CAP_OPENACC_200.  */
 	  err = "plugin missing OpenACC handler function";
-	  goto out;
+	  goto fail;
 	}
-      optional_present = optional_total = 0;
-      DLSYM_OPT (openacc.cuda.get_current_device,
-		 openacc_get_current_cuda_device);
-      DLSYM_OPT (openacc.cuda.get_current_context,
-		 openacc_get_current_cuda_context);
-      DLSYM_OPT (openacc.cuda.get_stream, openacc_get_cuda_stream);
-      DLSYM_OPT (openacc.cuda.set_stream, openacc_set_cuda_stream);
-      /* Make sure all the CUDA functions are there if any of them are.  */
-      if (optional_present && optional_present != optional_total)
+
+      unsigned cuda = 0;
+      cuda += DLSYM_OPT (openacc.cuda.get_current_device,
+			 openacc_get_current_cuda_device);
+      cuda += DLSYM_OPT (openacc.cuda.get_current_context,
+			 openacc_get_current_cuda_context);
+      cuda += DLSYM_OPT (openacc.cuda.get_stream, openacc_get_cuda_stream);
+      cuda += DLSYM_OPT (openacc.cuda.set_stream, openacc_set_cuda_stream);
+      if (cuda && cuda != 4)
 	{
+	  /* Make sure all the CUDA functions are there if any of them are.  */
 	  err = "plugin missing OpenACC CUDA handler function";
-	  goto out;
+	  goto fail;
 	}
     }
 #undef DLSYM
 #undef DLSYM_OPT
 
- out:
-  if (err != NULL)
-    {
-      gomp_error ("while loading %s: %s", plugin_name, err);
-      if (last_missing)
-        gomp_error ("missing function was %s", last_missing);
-      if (plugin_handle)
-	dlclose (plugin_handle);
-    }
-  return err == NULL;
+
+  return 1;
+
+ dl_fail:
+  err = dlerror ();
+ fail:
+  gomp_error ("while loading %s: %s", plugin_name, err);
+  if (last_missing)
+    gomp_error ("missing function was %s", last_missing);
+  if (plugin_handle)
+    dlclose (plugin_handle);
+
+  return 0;
 }
 
 /* This function initializes the runtime needed for offloading.
Index: libgomp/libgomp.h
===================================================================
--- libgomp/libgomp.h	(revision 226237)
+++ libgomp/libgomp.h	(working copy)
@@ -750,10 +750,22 @@ struct gomp_device_descr
   int (*get_num_devices_func) (void);
   void (*init_device_func) (int);
   void (*fini_device_func) (int);
-  int (*load_image_func) (int, const void *, struct addr_pair **);
-  int (*load_image_2_func) (unsigned, int, const void *, struct addr_pair **);
-  void (*unload_image_func) (int, const void *);
-  void (*unload_image_2_func) (unsigned, int, const void *);
+
+  unsigned (*version_func) (void);
+  
+  /* When all plugins updated, we can remove these unions and just
+     have the versioned entry points.  */
+  union 
+  {
+    int (*unver_func) (int, const void *, struct addr_pair **);
+    int (*ver_func) (unsigned, int, const void *, struct addr_pair **);
+  } load_image;
+  union
+  {
+    void (*unver_func) (int, const void *);
+    void (*ver_func) (unsigned, int, const void *);
+  } unload_image;
+  
   void *(*alloc_func) (int, size_t);
   void (*free_func) (int, void *);
   void *(*dev2host_func) (int, void *, const void *, size_t);
Index: libgomp/plugin/plugin-nvptx.c
===================================================================
--- libgomp/plugin/plugin-nvptx.c	(revision 226237)
+++ libgomp/plugin/plugin-nvptx.c	(working copy)
@@ -1615,12 +1615,22 @@ typedef struct nvptx_tdata
   size_t fn_num;
 } nvptx_tdata_t;
 
+/* Return the libgomp version number we're compatible with.  There is
+   no requirement for cross-version compatibility.  */
+
+unsigned
+GOMP_OFFLOAD_version (void)
+{
+  return GOMP_VERSION;
+}
+
 /* Load the (partial) program described by TARGET_DATA to device
    number ORD.  Allocate and return TARGET_TABLE.  */
 
 int
-GOMP_OFFLOAD_load_image_2 (unsigned version, int ord, const void *target_data,
-			   struct addr_pair **target_table)
+GOMP_OFFLOAD_load_image_ver (unsigned version, int ord,
+			     const void *target_data,
+			     struct addr_pair **target_table)
 {
   CUmodule module;
   const char *const *fn_names, *const *var_names;
@@ -1702,19 +1712,12 @@ GOMP_OFFLOAD_load_image_2 (unsigned vers
   return fn_entries + var_entries;
 }
 
-int
-GOMP_OFFLOAD_load_image (int device,
-			 const void *target_image, struct addr_pair **result)
-{
-  return GOMP_OFFLOAD_load_image_2 (0, device, target_image, result);
-}
-
 /* Unload the program described by TARGET_DATA.  DEV_DATA is the
    function descriptors allocated by G_O_load_image.  */
 
 void
-GOMP_OFFLOAD_unload_image_2 (unsigned version, int ord,
-			     const void *target_data)
+GOMP_OFFLOAD_unload_image_ver (unsigned version, int ord,
+			       const void *target_data)
 {
   struct ptx_image_data *image, **prev_p;
   struct ptx_device *dev = ptx_devices[ord];
@@ -1735,12 +1738,6 @@ GOMP_OFFLOAD_unload_image_2 (unsigned ve
   pthread_mutex_unlock (&dev->image_lock);
 }
 
-void
-GOMP_OFFLOAD_unload_image (int ord, const void *target_data)
-{
-  GOMP_OFFLOAD_unload_image_2 (0, ord, target_data);
-}
-
 void *
 GOMP_OFFLOAD_alloc (int ord, size_t size)
 {
Index: libgomp/oacc-host.c
===================================================================
--- libgomp/oacc-host.c	(revision 226237)
+++ libgomp/oacc-host.c	(working copy)
@@ -45,8 +45,9 @@ static struct gomp_device_descr host_dis
     .get_num_devices_func = GOMP_OFFLOAD_get_num_devices,
     .init_device_func = GOMP_OFFLOAD_init_device,
     .fini_device_func = GOMP_OFFLOAD_fini_device,
-    .load_image_func = GOMP_OFFLOAD_load_image,
-    .unload_image_func = GOMP_OFFLOAD_unload_image,
+    .version_func = NULL,
+    .load_image = {.unver_func = GOMP_OFFLOAD_load_image},
+    .unload_image = {.unver_func = GOMP_OFFLOAD_unload_image},
     .alloc_func = GOMP_OFFLOAD_alloc,
     .free_func = GOMP_OFFLOAD_free,
     .dev2host_func = GOMP_OFFLOAD_dev2host,
Index: include/gomp-constants.h
===================================================================
--- include/gomp-constants.h	(revision 226237)
+++ include/gomp-constants.h	(working copy)
@@ -124,7 +124,7 @@ enum gomp_map_kind
 
 /* Versions of libgomp and device-specific plugins.  */
 #define GOMP_VERSION	0
-#define GOMP_VERSION_NVIDIA_PTX 1
+#define GOMP_VERSION_NVIDIA_PTX 0
 
 #define GOMP_VERSION_PACK(LIB, DEV) (((LIB) << 16) | (DEV))
 #define GOMP_VERSION_LIB(PACK) (((PACK) >> 16) & 0xffff)
Index: gcc/config/nvptx/mkoffload.c
===================================================================
--- gcc/config/nvptx/mkoffload.c	(revision 226237)
+++ gcc/config/nvptx/mkoffload.c	(working copy)
@@ -296,9 +296,9 @@ process (FILE *in, FILE *out)
 	   "extern \"C\" {\n"
 	   "#endif\n");
 
-  fprintf (out, "extern void GOMP_offload_register_2"
+  fprintf (out, "extern void GOMP_offload_register_ver"
 	   " (unsigned, const void *, int, const void *);\n");
-  fprintf (out, "extern void GOMP_offload_unregister_2"
+  fprintf (out, "extern void GOMP_offload_unregister_ver"
 	   " (unsigned, const void *, int, const void *);\n");
 
   fprintf (out, "#ifdef __cplusplus\n"
@@ -309,7 +309,7 @@ process (FILE *in, FILE *out)
 
   fprintf (out, "static __attribute__((constructor)) void init (void)\n"
 	   "{\n"
-	   "  GOMP_offload_register_2 (%#x, __OFFLOAD_TABLE__,"
+	   "  GOMP_offload_register_ver (%#x, __OFFLOAD_TABLE__,"
 	   "%d/*NVIDIA_PTX*/, &target_data);\n"
 	   "};\n",
 	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
@@ -317,7 +317,7 @@ process (FILE *in, FILE *out)
 
   fprintf (out, "static __attribute__((destructor)) void fini (void)\n"
 	   "{\n"
-	   "  GOMP_offload_unregister_2 (%#x, __OFFLOAD_TABLE__,"
+	   "  GOMP_offload_unregister_ver (%#x, __OFFLOAD_TABLE__,"
 	   "%d/*NVIDIA_PTX*/, &target_data);\n"
 	   "};\n",
 	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-07-24 19:51         ` Nathan Sidwell
  2015-07-24 20:02           ` Ilya Verbin
@ 2015-07-31 12:30           ` Nathan Sidwell
  2015-07-31 14:08             ` Nathan Sidwell
  1 sibling, 1 reply; 22+ messages in thread
From: Nathan Sidwell @ 2015-07-31 12:30 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: ilya.verbin, GCC Patches

On 07/24/15 15:26, Nathan Sidwell wrote:
> Jakub,
> this version makes the following changes to the earlier version.
>
> *) Renames things to FOO_ver, rather than FOO_2
>
> *) No attempt to deal with cross-version plugins and libgomp.
>
> *) Adds GOMP_OFFLOAD_version function to plugin. (I went with your approach).
> Returns the GOMP_VERSION used to build the plugin, which libgomp checks matches
> the value for its build.  When we make incompatible changes to the plugin
> interface, that value can be incremented.
>
> *) While working on gomp_load_plugin_for_device, I noticed the DLSYM  and
> DLSYM_OPT macros were somewhat funky.  We're loading functions, so don't expect
> a NULL value.  We can simply check the returned value and only need dlerror when
> we get NULL.  The counting that DLSYM_OPT does was somewhat funky too.  IMHO
> better for that macro to simply return a truth value.
>
> ok for trunk?

ping?



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-07-31 12:30           ` Nathan Sidwell
@ 2015-07-31 14:08             ` Nathan Sidwell
  2015-07-31 16:20               ` Jakub Jelinek
  0 siblings, 1 reply; 22+ messages in thread
From: Nathan Sidwell @ 2015-07-31 14:08 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: ilya.verbin, GCC Patches

[-- Attachment #1: Type: text/plain, Size: 1184 bytes --]

On 07/31/15 08:16, Nathan Sidwell wrote:
> On 07/24/15 15:26, Nathan Sidwell wrote:
>> Jakub,
>> this version makes the following changes to the earlier version.
>>
>> *) Renames things to FOO_ver, rather than FOO_2
>>
>> *) No attempt to deal with cross-version plugins and libgomp.
>>
>> *) Adds GOMP_OFFLOAD_version function to plugin. (I went with your approach).
>> Returns the GOMP_VERSION used to build the plugin, which libgomp checks matches
>> the value for its build.  When we make incompatible changes to the plugin
>> interface, that value can be incremented.
>>
>> *) While working on gomp_load_plugin_for_device, I noticed the DLSYM  and
>> DLSYM_OPT macros were somewhat funky.  We're loading functions, so don't expect
>> a NULL value.  We can simply check the returned value and only need dlerror when
>> we get NULL.  The counting that DLSYM_OPT does was somewhat funky too.  IMHO
>> better for that macro to simply return a truth value.
>>
>> ok for trunk?
>
> ping?

Hm, apparently the original update didn't make it to the maiiling list and is 
missing from as a reply to:

https://gcc.gnu.org/ml/gcc-patches/2015-07/msg02085.html

attaching patch here.

nathan

[-- Attachment #2: trunk-version-3.patch --]
[-- Type: text/x-patch, Size: 21408 bytes --]

2015-07-24  Nathan Sidwell  <nathan@codesourcery.com>

	include/
	* gomp-constants.h (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX): New.
	(GOMP_VERSION_PACK, GOMP_VERSION_LIB, GOMP_VERSION_DEV): New.

	gcc/
	* config/nvptx/mkoffload.c (process): Replace
	GOMP_offload_{,un}register with GOMP_offload_{,un}register_ver.

	libgomp/
	* libgomp.map: Add 4.0.2 version.
	* target.c (offload_image_descr): Add version field.
	(gomp_load_image_to_device): Add version argument.  Forward to
	versioning loader if available.  Improve load mismatch diagnostic.
	(gomp_unload_image_from_device): Add version argument.  Forward to
	versioning unloader if available.
	(GOMP_offload_regster): Make stub function, move bulk to ...
	(GOMP_offload_register_ver): ... here.  Process version argument.
	(GOMP_offload_unregister): Make stub function, move bulk to ...
	(GOMP_offload_unregister_ver): ... here.  Process version argument.
	(gomp_init_device): Process version field.
	(gomp_unload_device): Process version field.o
	(gomp_load_plugin_for_device): Reimplement DLSYM & DLSYM_OPT
	macros.  Look for versioning function and check it.  Fetch
	versioning loader and unloader if avaulable.
	* libgomp.h (gomp_device_descr): Add version function field.  Put
	loader and unloader fields in unions.
	* oacc-host.c (host_dispatch): Adjust.
	* plugin/plugin-nvptx.c: Include gomp-constants.h.
	(GOMP_OFFLOAD_version): New.
	(GOMP_OFFLOAD_load_image): Replace with ...
	(GOMP_OFFLOAD_load_image_ver): ... this.  Add version argument and
	check it.
	(GOMP_OFFLOAD_unload_image): Replace with ....
	(GOMP_OFFLOAD_unload_image_ver): ... this.  Add version argument and
	check it.

Index: include/gomp-constants.h
===================================================================
--- include/gomp-constants.h	(revision 226039)
+++ include/gomp-constants.h	(working copy)
@@ -113,4 +113,12 @@ enum gomp_map_kind
 #define GOMP_DEVICE_ICV			-1
 #define GOMP_DEVICE_HOST_FALLBACK	-2
 
+/* Versions of libgomp and device-specific plugins.  */
+#define GOMP_VERSION	0
+#define GOMP_VERSION_NVIDIA_PTX 0
+
+#define GOMP_VERSION_PACK(LIB, DEV) (((LIB) << 16) | (DEV))
+#define GOMP_VERSION_LIB(PACK) (((PACK) >> 16) & 0xffff)
+#define GOMP_VERSION_DEV(PACK) ((PACK) & 0xffff)
+
 #endif
Index: gcc/config/nvptx/mkoffload.c
===================================================================
--- gcc/config/nvptx/mkoffload.c	(revision 226039)
+++ gcc/config/nvptx/mkoffload.c	(working copy)
@@ -881,10 +881,10 @@ process (FILE *in, FILE *out)
 	   "extern \"C\" {\n"
 	   "#endif\n");
 
-  fprintf (out, "extern void GOMP_offload_register"
-	   " (const void *, int, const void *);\n");
-  fprintf (out, "extern void GOMP_offload_unregister"
-	   " (const void *, int, const void *);\n");
+  fprintf (out, "extern void GOMP_offload_register_ver"
+	   " (unsigned, const void *, int, const void *);\n");
+  fprintf (out, "extern void GOMP_offload_unregister_ver"
+	   " (unsigned, const void *, int, const void *);\n");
 
   fprintf (out, "#ifdef __cplusplus\n"
 	   "}\n"
@@ -894,15 +894,19 @@ process (FILE *in, FILE *out)
 
   fprintf (out, "static __attribute__((constructor)) void init (void)\n"
 	   "{\n"
-	   "  GOMP_offload_register (__OFFLOAD_TABLE__, %d/*NVIDIA_PTX*/,\n"
-	   "                         &target_data);\n"
-	   "};\n", GOMP_DEVICE_NVIDIA_PTX);
+	   "  GOMP_offload_register_ver (%#x, __OFFLOAD_TABLE__,"
+	   "%d/*NVIDIA_PTX*/, &target_data);\n"
+	   "};\n",
+	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
+	   GOMP_DEVICE_NVIDIA_PTX);
 
   fprintf (out, "static __attribute__((destructor)) void fini (void)\n"
 	   "{\n"
-	   "  GOMP_offload_unregister (__OFFLOAD_TABLE__, %d/*NVIDIA_PTX*/,\n"
-	   "                           &target_data);\n"
-	   "};\n", GOMP_DEVICE_NVIDIA_PTX);
+	   "  GOMP_offload_unregister_ver (%#x, __OFFLOAD_TABLE__,"
+	   "%d/*NVIDIA_PTX*/, &target_data);\n"
+	   "};\n",
+	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
+	   GOMP_DEVICE_NVIDIA_PTX);
 }
 
 static void
Index: libgomp/libgomp.map
===================================================================
--- libgomp/libgomp.map	(revision 226039)
+++ libgomp/libgomp.map	(working copy)
@@ -234,6 +234,12 @@ GOMP_4.0.1 {
 	GOMP_offload_unregister;
 } GOMP_4.0;
 
+GOMP_4.0.2 {
+  global:
+	GOMP_offload_register_ver;
+	GOMP_offload_unregister_ver;
+} GOMP_4.0.1;
+
 OACC_2.0 {
   global:
 	acc_get_num_devices;
Index: libgomp/target.c
===================================================================
--- libgomp/target.c	(revision 226039)
+++ libgomp/target.c	(working copy)
@@ -56,6 +56,7 @@ static gomp_mutex_t register_lock;
    It contains type of the target device, pointer to host table descriptor, and
    pointer to target data.  */
 struct offload_image_descr {
+  unsigned version;
   enum offload_target_type type;
   const void *host_table;
   const void *target_data;
@@ -642,7 +643,8 @@ gomp_update (struct gomp_device_descr *d
    emitting variable and functions in the same order.  */
 
 static void
-gomp_load_image_to_device (struct gomp_device_descr *devicep,
+gomp_load_image_to_device (unsigned version,
+			   struct gomp_device_descr *devicep,
 			   const void *host_table, const void *target_data,
 			   bool is_register_lock)
 {
@@ -658,16 +660,28 @@ gomp_load_image_to_device (struct gomp_d
 
   /* Load image to device and get target addresses for the image.  */
   struct addr_pair *target_table = NULL;
-  int i, num_target_entries
-    = devicep->load_image_func (devicep->target_id, target_data,
-				&target_table);
+  int i, num_target_entries;
+
+  if (devicep->version_func)
+    num_target_entries
+      = devicep->load_image.ver_func (version, devicep->target_id,
+					target_data, &target_table);
+  else if (GOMP_VERSION_DEV (version))
+    gomp_fatal ("Plugin too old for offload data (0 < %u)",
+		GOMP_VERSION_DEV (version));
+  else
+    num_target_entries
+      = devicep->load_image.unver_func (devicep->target_id,
+					  target_data, &target_table);
 
   if (num_target_entries != num_funcs + num_vars)
     {
       gomp_mutex_unlock (&devicep->lock);
       if (is_register_lock)
 	gomp_mutex_unlock (&register_lock);
-      gomp_fatal ("Can't map target functions or variables");
+      gomp_fatal ("Cannot map target functions or variables"
+		  " (expected %u, have %u)", num_funcs + num_vars,
+		  num_target_entries);
     }
 
   /* Insert host-target address mapping into splay tree.  */
@@ -731,7 +745,8 @@ gomp_load_image_to_device (struct gomp_d
    The device must be locked.   */
 
 static void
-gomp_unload_image_from_device (struct gomp_device_descr *devicep,
+gomp_unload_image_from_device (unsigned version,
+			       struct gomp_device_descr *devicep,
 			       const void *host_table, const void *target_data)
 {
   void **host_func_table = ((void ***) host_table)[0];
@@ -756,8 +771,12 @@ gomp_unload_image_from_device (struct go
       k.host_end = k.host_start + 1;
       node = splay_tree_lookup (&devicep->mem_map, &k);
     }
-  
-  devicep->unload_image_func (devicep->target_id, target_data);
+
+  if (devicep->version_func)
+    devicep->unload_image.ver_func (version,
+				    devicep->target_id, target_data);
+  else
+    devicep->unload_image.unver_func (devicep->target_id, target_data);
 
   /* Remove mappings from splay tree.  */
   for (j = 0; j < num_funcs; j++)
@@ -786,10 +805,15 @@ gomp_unload_image_from_device (struct go
    the target, and TARGET_DATA needed by target plugin.  */
 
 void
-GOMP_offload_register (const void *host_table, int target_type,
-		       const void *target_data)
+GOMP_offload_register_ver (unsigned version, const void *host_table,
+			   int target_type, const void *target_data)
 {
   int i;
+
+  if (GOMP_VERSION_LIB (version) > GOMP_VERSION)
+    gomp_fatal ("Library too old for offload (version %u < %u)",
+		GOMP_VERSION, GOMP_VERSION_LIB (version));
+  
   gomp_mutex_lock (&register_lock);
 
   /* Load image to all initialized devices.  */
@@ -798,7 +822,8 @@ GOMP_offload_register (const void *host_
       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, host_table, target_data, true);
+	gomp_load_image_to_device (version, devicep,
+				   host_table, target_data, true);
       gomp_mutex_unlock (&devicep->lock);
     }
 
@@ -807,6 +832,7 @@ GOMP_offload_register (const void *host_
     = gomp_realloc_unlock (offload_images,
 			   (num_offload_images + 1)
 			   * sizeof (struct offload_image_descr));
+  offload_images[num_offload_images].version = version;
   offload_images[num_offload_images].type = target_type;
   offload_images[num_offload_images].host_table = host_table;
   offload_images[num_offload_images].target_data = target_data;
@@ -815,13 +841,20 @@ GOMP_offload_register (const void *host_
   gomp_mutex_unlock (&register_lock);
 }
 
+void
+GOMP_offload_register (const void *host_table, int target_type,
+		       const void *target_data)
+{
+  GOMP_offload_register_ver (0, host_table, target_type, target_data);
+}
+
 /* This function should be called from every offload image while unloading.
    It gets the descriptor of the host func and var tables HOST_TABLE, TYPE of
    the target, and TARGET_DATA needed by target plugin.  */
 
 void
-GOMP_offload_unregister (const void *host_table, int target_type,
-			 const void *target_data)
+GOMP_offload_unregister_ver (unsigned version, const void *host_table,
+			     int target_type, const void *target_data)
 {
   int i;
 
@@ -833,7 +866,8 @@ GOMP_offload_unregister (const void *hos
       struct gomp_device_descr *devicep = &devices[i];
       gomp_mutex_lock (&devicep->lock);
       if (devicep->type == target_type && devicep->is_initialized)
-	gomp_unload_image_from_device (devicep, host_table, target_data);
+	gomp_unload_image_from_device (version, devicep,
+				       host_table, target_data);
       gomp_mutex_unlock (&devicep->lock);
     }
 
@@ -848,6 +882,13 @@ GOMP_offload_unregister (const void *hos
   gomp_mutex_unlock (&register_lock);
 }
 
+void
+GOMP_offload_unregister (const void *host_table, int target_type,
+			 const void *target_data)
+{
+  GOMP_offload_unregister_ver (0, host_table, target_type, target_data);
+}
+
 /* This function initializes the target device, specified by DEVICEP.  DEVICEP
    must be locked on entry, and remains locked on return.  */
 
@@ -862,8 +903,9 @@ gomp_init_device (struct gomp_device_des
     {
       struct offload_image_descr *image = &offload_images[i];
       if (image->type == devicep->type)
-	gomp_load_image_to_device (devicep, image->host_table,
-				   image->target_data, false);
+	gomp_load_image_to_device (image->version, devicep,
+				   image->host_table, image->target_data,
+				   false);
     }
 
   devicep->is_initialized = true;
@@ -881,7 +923,8 @@ gomp_unload_device (struct gomp_device_d
 	{
 	  struct offload_image_descr *image = &offload_images[i];
 	  if (image->type == devicep->type)
-	    gomp_unload_image_from_device (devicep, image->host_table,
+	    gomp_unload_image_from_device (image->version, devicep,
+					   image->host_table,
 					   image->target_data);
 	}
     }
@@ -1085,43 +1128,41 @@ gomp_load_plugin_for_device (struct gomp
 			     const char *plugin_name)
 {
   const char *err = NULL, *last_missing = NULL;
-  int optional_present, optional_total;
-
-  /* Clear any existing error.  */
-  dlerror ();
 
   void *plugin_handle = dlopen (plugin_name, RTLD_LAZY);
   if (!plugin_handle)
-    {
-      err = dlerror ();
-      goto out;
-    }
+    goto dl_fail;
 
   /* Check if all required functions are available in the plugin and store
-     their handlers.  */
+     their handlers.  None of the symbols can legitimately be NULL,
+     so we don't need to check dlerror all the time.  */
 #define DLSYM(f)							\
-  do									\
-    {									\
-      device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #f);	\
-      err = dlerror ();							\
-      if (err != NULL)							\
-	goto out;							\
-    }									\
-  while (0)
-  /* Similar, but missing functions are not an error.  */
-#define DLSYM_OPT(f, n)						\
-  do									\
-    {									\
-      const char *tmp_err;							\
-      device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #n);	\
-      tmp_err = dlerror ();						\
-      if (tmp_err == NULL)						\
-        optional_present++;						\
-      else								\
-        last_missing = #n;						\
-      optional_total++;							\
-    }									\
-  while (0)
+  if (!(device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #f)))	\
+    goto dl_fail
+  /* Similar, but missing functions are not an error.  Return false if
+     failed, true otherwise.  */
+#define DLSYM_OPT(f, n)							\
+  ((device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #n))	\
+   || (last_missing = #n, 0))
+
+  if (DLSYM_OPT (version, version))
+    {
+      unsigned v = device->version_func ();
+      if (v != GOMP_VERSION)
+	{
+	  err = "plugin version mismatch";
+	  goto fail;
+	}
+      if (!DLSYM_OPT (load_image.ver, load_image_ver)
+	  || !DLSYM_OPT (unload_image.ver, unload_image_ver))
+	goto  dl_fail;
+    }
+  else
+    {
+      if (!DLSYM_OPT (load_image.unver, load_image)
+	  || !DLSYM_OPT (unload_image.unver, unload_image))
+	goto dl_fail;
+    }
 
   DLSYM (get_name);
   DLSYM (get_caps);
@@ -1129,8 +1170,6 @@ gomp_load_plugin_for_device (struct gomp
   DLSYM (get_num_devices);
   DLSYM (init_device);
   DLSYM (fini_device);
-  DLSYM (load_image);
-  DLSYM (unload_image);
   DLSYM (alloc);
   DLSYM (free);
   DLSYM (dev2host);
@@ -1140,53 +1179,57 @@ gomp_load_plugin_for_device (struct gomp
     DLSYM (run);
   if (device->capabilities & GOMP_OFFLOAD_CAP_OPENACC_200)
     {
-      optional_present = optional_total = 0;
-      DLSYM_OPT (openacc.exec, openacc_parallel);
-      DLSYM_OPT (openacc.register_async_cleanup,
-		 openacc_register_async_cleanup);
-      DLSYM_OPT (openacc.async_test, openacc_async_test);
-      DLSYM_OPT (openacc.async_test_all, openacc_async_test_all);
-      DLSYM_OPT (openacc.async_wait, openacc_async_wait);
-      DLSYM_OPT (openacc.async_wait_async, openacc_async_wait_async);
-      DLSYM_OPT (openacc.async_wait_all, openacc_async_wait_all);
-      DLSYM_OPT (openacc.async_wait_all_async, openacc_async_wait_all_async);
-      DLSYM_OPT (openacc.async_set_async, openacc_async_set_async);
-      DLSYM_OPT (openacc.create_thread_data, openacc_create_thread_data);
-      DLSYM_OPT (openacc.destroy_thread_data, openacc_destroy_thread_data);
-      /* Require all the OpenACC handlers if we have
-	 GOMP_OFFLOAD_CAP_OPENACC_200.  */
-      if (optional_present != optional_total)
+      if (!DLSYM_OPT (openacc.exec, openacc_parallel)
+	  || !DLSYM_OPT (openacc.register_async_cleanup,
+			 openacc_register_async_cleanup)
+	  || !DLSYM_OPT (openacc.async_test, openacc_async_test)
+	  || !DLSYM_OPT (openacc.async_test_all, openacc_async_test_all)
+	  || !DLSYM_OPT (openacc.async_wait, openacc_async_wait)
+	  || !DLSYM_OPT (openacc.async_wait_async, openacc_async_wait_async)
+	  || !DLSYM_OPT (openacc.async_wait_all, openacc_async_wait_all)
+	  || !DLSYM_OPT (openacc.async_wait_all_async,
+			 openacc_async_wait_all_async)
+	  || !DLSYM_OPT (openacc.async_set_async, openacc_async_set_async)
+	  || !DLSYM_OPT (openacc.create_thread_data,
+			 openacc_create_thread_data)
+	  || !DLSYM_OPT (openacc.destroy_thread_data,
+			 openacc_destroy_thread_data))
 	{
+	  /* Require all the OpenACC handlers if we have
+	     GOMP_OFFLOAD_CAP_OPENACC_200.  */
 	  err = "plugin missing OpenACC handler function";
-	  goto out;
+	  goto fail;
 	}
-      optional_present = optional_total = 0;
-      DLSYM_OPT (openacc.cuda.get_current_device,
-		 openacc_get_current_cuda_device);
-      DLSYM_OPT (openacc.cuda.get_current_context,
-		 openacc_get_current_cuda_context);
-      DLSYM_OPT (openacc.cuda.get_stream, openacc_get_cuda_stream);
-      DLSYM_OPT (openacc.cuda.set_stream, openacc_set_cuda_stream);
-      /* Make sure all the CUDA functions are there if any of them are.  */
-      if (optional_present && optional_present != optional_total)
+
+      unsigned cuda = 0;
+      cuda += DLSYM_OPT (openacc.cuda.get_current_device,
+			 openacc_get_current_cuda_device);
+      cuda += DLSYM_OPT (openacc.cuda.get_current_context,
+			 openacc_get_current_cuda_context);
+      cuda += DLSYM_OPT (openacc.cuda.get_stream, openacc_get_cuda_stream);
+      cuda += DLSYM_OPT (openacc.cuda.set_stream, openacc_set_cuda_stream);
+      if (cuda && cuda != 4)
 	{
+	  /* Make sure all the CUDA functions are there if any of them are.  */
 	  err = "plugin missing OpenACC CUDA handler function";
-	  goto out;
+	  goto fail;
 	}
     }
 #undef DLSYM
 #undef DLSYM_OPT
 
- out:
-  if (err != NULL)
-    {
-      gomp_error ("while loading %s: %s", plugin_name, err);
-      if (last_missing)
-        gomp_error ("missing function was %s", last_missing);
-      if (plugin_handle)
-	dlclose (plugin_handle);
-    }
-  return err == NULL;
+  return 1;
+
+ dl_fail:
+  err = dlerror ();
+ fail:
+  gomp_error ("while loading %s: %s", plugin_name, err);
+  if (last_missing)
+    gomp_error ("missing function was %s", last_missing);
+  if (plugin_handle)
+    dlclose (plugin_handle);
+
+  return 0;
 }
 
 /* This function initializes the runtime needed for offloading.
Index: libgomp/libgomp.h
===================================================================
--- libgomp/libgomp.h	(revision 226039)
+++ libgomp/libgomp.h	(working copy)
@@ -748,8 +748,22 @@ struct gomp_device_descr
   int (*get_num_devices_func) (void);
   void (*init_device_func) (int);
   void (*fini_device_func) (int);
-  int (*load_image_func) (int, const void *, struct addr_pair **);
-  void (*unload_image_func) (int, const void *);
+
+  unsigned (*version_func) (void);
+  
+  /* When all plugins updated, we can remove these unions and just
+     have the versioned entry points.  */
+  union 
+  {
+    int (*unver_func) (int, const void *, struct addr_pair **);
+    int (*ver_func) (unsigned, int, const void *, struct addr_pair **);
+  } load_image;
+  union
+  {
+    void (*unver_func) (int, const void *);
+    void (*ver_func) (unsigned, int, const void *);
+  } unload_image;
+  
   void *(*alloc_func) (int, size_t);
   void (*free_func) (int, void *);
   void *(*dev2host_func) (int, void *, const void *, size_t);
Index: libgomp/plugin/plugin-nvptx.c
===================================================================
--- libgomp/plugin/plugin-nvptx.c	(revision 226039)
+++ libgomp/plugin/plugin-nvptx.c	(working copy)
@@ -36,6 +36,7 @@
 #include "libgomp-plugin.h"
 #include "oacc-ptx.h"
 #include "oacc-plugin.h"
+#include "gomp-constants.h"
 
 #include <pthread.h>
 #include <cuda.h>
@@ -1644,12 +1645,22 @@ typedef struct nvptx_tdata
   size_t fn_num;
 } nvptx_tdata_t;
 
+/* Return the libgomp version number we're compatible with.  There is
+   no requirement for cross-version compatibility.  */
+
+unsigned
+GOMP_OFFLOAD_version (void)
+{
+  return GOMP_VERSION;
+}
+
 /* Load the (partial) program described by TARGET_DATA to device
    number ORD.  Allocate and return TARGET_TABLE.  */
 
 int
-GOMP_OFFLOAD_load_image (int ord, const void *target_data,
-			 struct addr_pair **target_table)
+GOMP_OFFLOAD_load_image_ver (unsigned version, int ord,
+			     const void *target_data,
+			     struct addr_pair **target_table)
 {
   CUmodule module;
   const char *const *fn_names, *const *var_names;
@@ -1661,6 +1672,11 @@ GOMP_OFFLOAD_load_image (int ord, const
   struct ptx_image_data *new_image;
   struct ptx_device *dev;
 
+  if (GOMP_VERSION_DEV (version) != GOMP_VERSION_NVIDIA_PTX)
+    GOMP_PLUGIN_fatal ("Offload data incompatible with PTX plugin"
+		       " (version %u != %u)",
+		       GOMP_VERSION_NVIDIA_PTX, GOMP_VERSION_DEV (version));
+  
   GOMP_OFFLOAD_init_device (ord);
 
   dev = ptx_devices[ord];
@@ -1730,11 +1746,15 @@ GOMP_OFFLOAD_load_image (int ord, const
    function descriptors allocated by G_O_load_image.  */
 
 void
-GOMP_OFFLOAD_unload_image (int ord, const void *target_data)
+GOMP_OFFLOAD_unload_image_ver (unsigned version, int ord,
+			       const void *target_data)
 {
   struct ptx_image_data *image, **prev_p;
   struct ptx_device *dev = ptx_devices[ord];
 
+  if (GOMP_VERSION_DEV (version) != GOMP_VERSION_NVIDIA_PTX)
+    return;
+  
   pthread_mutex_lock (&dev->image_lock);
   for (prev_p = &dev->images; (image = *prev_p) != 0; prev_p = &image->next)
     if (image->target_data == target_data)
Index: libgomp/oacc-host.c
===================================================================
--- libgomp/oacc-host.c	(revision 226039)
+++ libgomp/oacc-host.c	(working copy)
@@ -45,8 +45,9 @@ static struct gomp_device_descr host_dis
     .get_num_devices_func = GOMP_OFFLOAD_get_num_devices,
     .init_device_func = GOMP_OFFLOAD_init_device,
     .fini_device_func = GOMP_OFFLOAD_fini_device,
-    .load_image_func = GOMP_OFFLOAD_load_image,
-    .unload_image_func = GOMP_OFFLOAD_unload_image,
+    .version_func = NULL,
+    .load_image = {.unver_func = GOMP_OFFLOAD_load_image},
+    .unload_image = {.unver_func = GOMP_OFFLOAD_unload_image},
     .alloc_func = GOMP_OFFLOAD_alloc,
     .free_func = GOMP_OFFLOAD_free,
     .dev2host_func = GOMP_OFFLOAD_dev2host,

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-07-31 14:08             ` Nathan Sidwell
@ 2015-07-31 16:20               ` Jakub Jelinek
  2015-08-02  0:06                 ` Nathan Sidwell
  0 siblings, 1 reply; 22+ messages in thread
From: Jakub Jelinek @ 2015-07-31 16:20 UTC (permalink / raw)
  To: Nathan Sidwell; +Cc: ilya.verbin, GCC Patches

> Index: libgomp/libgomp.map
> ===================================================================
> --- libgomp/libgomp.map	(revision 226039)
> +++ libgomp/libgomp.map	(working copy)
> @@ -234,6 +234,12 @@ GOMP_4.0.1 {
>  	GOMP_offload_unregister;
>  } GOMP_4.0;
>  
> +GOMP_4.0.2 {
> +  global:
> +	GOMP_offload_register_ver;
> +	GOMP_offload_unregister_ver;
> +} GOMP_4.0.1;
> +

This will hopefully be just GOMP_4.1 instead in the end, but it can
change when gomp-4_1-branch is merged to trunk, we don't guarantee
ABI stability at this point.

> @@ -642,7 +643,8 @@ gomp_update (struct gomp_device_descr *d
>     emitting variable and functions in the same order.  */
>  
>  static void
> -gomp_load_image_to_device (struct gomp_device_descr *devicep,
> +gomp_load_image_to_device (unsigned version,
> +			   struct gomp_device_descr *devicep,

I'd prefer version to go after devicep argument rather than before.

>  			   const void *host_table, const void *target_data,
>  			   bool is_register_lock)
>  {
> @@ -658,16 +660,28 @@ gomp_load_image_to_device (struct gomp_d
>  
>    /* Load image to device and get target addresses for the image.  */
>    struct addr_pair *target_table = NULL;
> -  int i, num_target_entries
> -    = devicep->load_image_func (devicep->target_id, target_data,
> -				&target_table);
> +  int i, num_target_entries;
> +
> +  if (devicep->version_func)
> +    num_target_entries
> +      = devicep->load_image.ver_func (version, devicep->target_id,
> +					target_data, &target_table);
> +  else if (GOMP_VERSION_DEV (version))
> +    gomp_fatal ("Plugin too old for offload data (0 < %u)",
> +		GOMP_VERSION_DEV (version));
> +  else
> +    num_target_entries
> +      = devicep->load_image.unver_func (devicep->target_id,
> +					  target_data, &target_table);

And really don't have ver_func vs. unver_func, just a single
callback that will take the version argument too (again, if possible
after target_id).

>  static void
> -gomp_unload_image_from_device (struct gomp_device_descr *devicep,
> +gomp_unload_image_from_device (unsigned version,
> +			       struct gomp_device_descr *devicep,
>  			       const void *host_table, const void *target_data)
>  {
>    void **host_func_table = ((void ***) host_table)[0];

Likewise.

> @@ -756,8 +771,12 @@ gomp_unload_image_from_device (struct go
>        k.host_end = k.host_start + 1;
>        node = splay_tree_lookup (&devicep->mem_map, &k);
>      }
> -  
> -  devicep->unload_image_func (devicep->target_id, target_data);
> +
> +  if (devicep->version_func)
> +    devicep->unload_image.ver_func (version,
> +				    devicep->target_id, target_data);
> +  else
> +    devicep->unload_image.unver_func (devicep->target_id, target_data);

And here too.

> @@ -815,13 +841,20 @@ GOMP_offload_register (const void *host_
>    gomp_mutex_unlock (&register_lock);
>  }
>  
> +void
> +GOMP_offload_register (const void *host_table, int target_type,
> +		       const void *target_data)
> +{
> +  GOMP_offload_register_ver (0, host_table, target_type, target_data);
> +}

This is ok.

> @@ -862,8 +903,9 @@ gomp_init_device (struct gomp_device_des
>      {
>        struct offload_image_descr *image = &offload_images[i];
>        if (image->type == devicep->type)
> -	gomp_load_image_to_device (devicep, image->host_table,
> -				   image->target_data, false);
> +	gomp_load_image_to_device (image->version, devicep,
> +				   image->host_table, image->target_data,
> +				   false);
>      }
>  
>    devicep->is_initialized = true;
> @@ -881,7 +923,8 @@ gomp_unload_device (struct gomp_device_d
>  	{
>  	  struct offload_image_descr *image = &offload_images[i];
>  	  if (image->type == devicep->type)
> -	    gomp_unload_image_from_device (devicep, image->host_table,
> +	    gomp_unload_image_from_device (image->version, devicep,
> +					   image->host_table,
>  					   image->target_data);

Again, please swap the first two arguments.

> +
> +  if (DLSYM_OPT (version, version))

I'd prefer requiring version always (i.e. DLSYM (version);
plus the v != GOMP_VERSION checking).

	Jakub

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-07-31 16:20               ` Jakub Jelinek
@ 2015-08-02  0:06                 ` Nathan Sidwell
  2015-08-02  0:20                   ` Nathan Sidwell
                                     ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: Nathan Sidwell @ 2015-08-02  0:06 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: ilya.verbin, GCC Patches

[-- Attachment #1: Type: text/plain, Size: 1012 bytes --]

On 07/31/15 12:10, Jakub Jelinek wrote:

> This will hopefully be just GOMP_4.1 instead in the end, but it can
> change when gomp-4_1-branch is merged to trunk, we don't guarantee
> ABI stability at this point.

Sure.

> I'd prefer version to go after devicep argument rather than before.
Fixed.

> And really don't have ver_func vs. unver_func, just a single
> callback that will take the version argument too (again, if possible
> after target_id).

Fixed (& elsewhere).  The patch should be checked for intelmic if possible 
(Ilya?).  The  changes there are very mechanical so I'm not expecting a problem.

We don't need to  make the initial value of GOMP_VERSION non-zero, because the 
absence of the GOMP_OFFLOAD_version func will distinguish out of date plugins at 
this point.


>> +
>> +  if (DLSYM_OPT (version, version))
>
> I'd prefer requiring version always (i.e. DLSYM (version);
> plus the v != GOMP_VERSION checking).

Fixed.

ok?  I'll commit a version of this to gomp4 branch shortly.

nathan


[-- Attachment #2: trunk-version-4.patch --]
[-- Type: text/x-patch, Size: 22540 bytes --]

2015-08-01  Nathan Sidwell  <nathan@codesourcery.com>

	gcc/
	* config/nvptx/mkoffload.c (process): Replace
	GOMP_offload_{,un}register with GOMP_offload_{,un}register_ver.

	libgomp/
	* libgomp.map: Add 4.0.2 version.
	* target.c (offload_image_descr): Add version field.
	(gomp_load_image_to_device): Add version argument.  Adjust plugin
	call.  Improve load mismatch diagnostic.
	(gomp_unload_image_from_device): Add version argument.  Adjust plugin
	call.
	(GOMP_offload_regster): Make stub function, move bulk to ...
	(GOMP_offload_register_ver): ... here.  Process version argument.
	(GOMP_offload_unregister): Make stub function, move bulk to ...
	(GOMP_offload_unregister_ver): ... here.  Process version argument.
	(gomp_init_device): Process version field.
	(gomp_unload_device): Process version field.
	(gomp_load_plugin_for_device): Reimplement DLSYM & DLSYM_OPT
	macros.  Check plugin version.
	* libgomp.h (gomp_device_descr): Add version function field.  Adjust
	loader and unloader types.
	* oacc-host.c (host_dispatch): Adjust.
	* plugin/plugin-nvptx.c: Include gomp-constants.h.
	(GOMP_OFFLOAD_version): New.
	(GOMP_OFFLOAD_load_image): Add version arg and check it.
	(GOMP_OFFLOAD_unload_image): Likewise.
	* plugin/plugin-host.c: Include gomp-constants.h.
	(GOMP_OFFLOAD_version): New.
	(GOMP_OFFLOAD_load_image): Add version arg.
	(GOMP_OFFLOAD_unload_image): Likewise.
	* oacc-host.c (host_dispatch): Init version field.

	liboffloadmic/
	* plugin/libgomp-plugin-intelmic.cpp (GOMP_OFFLOAD_version): New.
	(GOMP_OFFLOAD_load_image): Add version arg and check it.
	(GOMP_OFFLOAD_unload_image): Likewise.

	include/
	* gomp-constants.h (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX,
	GOMP_VERSION_INTEL_MIC): New.
	(GOMP_VERSION_PACK, GOMP_VERSION_LIB, GOMP_VERSION_DEV): New.

Index: gcc/config/nvptx/mkoffload.c
===================================================================
--- gcc/config/nvptx/mkoffload.c	(revision 226462)
+++ gcc/config/nvptx/mkoffload.c	(working copy)
@@ -881,10 +881,10 @@ process (FILE *in, FILE *out)
 	   "extern \"C\" {\n"
 	   "#endif\n");
 
-  fprintf (out, "extern void GOMP_offload_register"
-	   " (const void *, int, const void *);\n");
-  fprintf (out, "extern void GOMP_offload_unregister"
-	   " (const void *, int, const void *);\n");
+  fprintf (out, "extern void GOMP_offload_register_ver"
+	   " (unsigned, const void *, int, const void *);\n");
+  fprintf (out, "extern void GOMP_offload_unregister_ver"
+	   " (unsigned, const void *, int, const void *);\n");
 
   fprintf (out, "#ifdef __cplusplus\n"
 	   "}\n"
@@ -894,15 +894,19 @@ process (FILE *in, FILE *out)
 
   fprintf (out, "static __attribute__((constructor)) void init (void)\n"
 	   "{\n"
-	   "  GOMP_offload_register (__OFFLOAD_TABLE__, %d/*NVIDIA_PTX*/,\n"
-	   "                         &target_data);\n"
-	   "};\n", GOMP_DEVICE_NVIDIA_PTX);
+	   "  GOMP_offload_register_ver (%#x, __OFFLOAD_TABLE__,"
+	   "%d/*NVIDIA_PTX*/, &target_data);\n"
+	   "};\n",
+	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
+	   GOMP_DEVICE_NVIDIA_PTX);
 
   fprintf (out, "static __attribute__((destructor)) void fini (void)\n"
 	   "{\n"
-	   "  GOMP_offload_unregister (__OFFLOAD_TABLE__, %d/*NVIDIA_PTX*/,\n"
-	   "                           &target_data);\n"
-	   "};\n", GOMP_DEVICE_NVIDIA_PTX);
+	   "  GOMP_offload_unregister_ver (%#x, __OFFLOAD_TABLE__,"
+	   "%d/*NVIDIA_PTX*/, &target_data);\n"
+	   "};\n",
+	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
+	   GOMP_DEVICE_NVIDIA_PTX);
 }
 
 static void
Index: libgomp/libgomp.map
===================================================================
--- libgomp/libgomp.map	(revision 226462)
+++ libgomp/libgomp.map	(working copy)
@@ -234,6 +234,12 @@ GOMP_4.0.1 {
 	GOMP_offload_unregister;
 } GOMP_4.0;
 
+GOMP_4.0.2 {
+  global:
+	GOMP_offload_register_ver;
+	GOMP_offload_unregister_ver;
+} GOMP_4.0.1;
+
 OACC_2.0 {
   global:
 	acc_get_num_devices;
Index: libgomp/target.c
===================================================================
--- libgomp/target.c	(revision 226462)
+++ libgomp/target.c	(working copy)
@@ -56,6 +56,7 @@ static gomp_mutex_t register_lock;
    It contains type of the target device, pointer to host table descriptor, and
    pointer to target data.  */
 struct offload_image_descr {
+  unsigned version;
   enum offload_target_type type;
   const void *host_table;
   const void *target_data;
@@ -642,7 +643,7 @@ gomp_update (struct gomp_device_descr *d
    emitting variable and functions in the same order.  */
 
 static void
-gomp_load_image_to_device (struct gomp_device_descr *devicep,
+gomp_load_image_to_device (struct gomp_device_descr *devicep, unsigned version,
 			   const void *host_table, const void *target_data,
 			   bool is_register_lock)
 {
@@ -658,16 +659,20 @@ gomp_load_image_to_device (struct gomp_d
 
   /* Load image to device and get target addresses for the image.  */
   struct addr_pair *target_table = NULL;
-  int i, num_target_entries
-    = devicep->load_image_func (devicep->target_id, target_data,
-				&target_table);
+  int i, num_target_entries;
+
+  num_target_entries
+    = devicep->load_image_func (devicep->target_id, version,
+				target_data, &target_table);
 
   if (num_target_entries != num_funcs + num_vars)
     {
       gomp_mutex_unlock (&devicep->lock);
       if (is_register_lock)
 	gomp_mutex_unlock (&register_lock);
-      gomp_fatal ("Can't map target functions or variables");
+      gomp_fatal ("Cannot map target functions or variables"
+		  " (expected %u, have %u)", num_funcs + num_vars,
+		  num_target_entries);
     }
 
   /* Insert host-target address mapping into splay tree.  */
@@ -732,6 +737,7 @@ gomp_load_image_to_device (struct gomp_d
 
 static void
 gomp_unload_image_from_device (struct gomp_device_descr *devicep,
+			       unsigned version,
 			       const void *host_table, const void *target_data)
 {
   void **host_func_table = ((void ***) host_table)[0];
@@ -756,8 +762,8 @@ gomp_unload_image_from_device (struct go
       k.host_end = k.host_start + 1;
       node = splay_tree_lookup (&devicep->mem_map, &k);
     }
-  
-  devicep->unload_image_func (devicep->target_id, target_data);
+
+  devicep->unload_image_func (devicep->target_id, version, target_data);
 
   /* Remove mappings from splay tree.  */
   for (j = 0; j < num_funcs; j++)
@@ -786,10 +792,15 @@ gomp_unload_image_from_device (struct go
    the target, and TARGET_DATA needed by target plugin.  */
 
 void
-GOMP_offload_register (const void *host_table, int target_type,
-		       const void *target_data)
+GOMP_offload_register_ver (unsigned version, const void *host_table,
+			   int target_type, const void *target_data)
 {
   int i;
+
+  if (GOMP_VERSION_LIB (version) > GOMP_VERSION)
+    gomp_fatal ("Library too old for offload (version %u < %u)",
+		GOMP_VERSION, GOMP_VERSION_LIB (version));
+  
   gomp_mutex_lock (&register_lock);
 
   /* Load image to all initialized devices.  */
@@ -798,7 +809,8 @@ GOMP_offload_register (const void *host_
       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, host_table, target_data, true);
+	gomp_load_image_to_device (devicep, version,
+				   host_table, target_data, true);
       gomp_mutex_unlock (&devicep->lock);
     }
 
@@ -807,6 +819,7 @@ GOMP_offload_register (const void *host_
     = gomp_realloc_unlock (offload_images,
 			   (num_offload_images + 1)
 			   * sizeof (struct offload_image_descr));
+  offload_images[num_offload_images].version = version;
   offload_images[num_offload_images].type = target_type;
   offload_images[num_offload_images].host_table = host_table;
   offload_images[num_offload_images].target_data = target_data;
@@ -815,13 +828,20 @@ GOMP_offload_register (const void *host_
   gomp_mutex_unlock (&register_lock);
 }
 
+void
+GOMP_offload_register (const void *host_table, int target_type,
+		       const void *target_data)
+{
+  GOMP_offload_register_ver (0, host_table, target_type, target_data);
+}
+
 /* This function should be called from every offload image while unloading.
    It gets the descriptor of the host func and var tables HOST_TABLE, TYPE of
    the target, and TARGET_DATA needed by target plugin.  */
 
 void
-GOMP_offload_unregister (const void *host_table, int target_type,
-			 const void *target_data)
+GOMP_offload_unregister_ver (unsigned version, const void *host_table,
+			     int target_type, const void *target_data)
 {
   int i;
 
@@ -833,7 +853,8 @@ GOMP_offload_unregister (const void *hos
       struct gomp_device_descr *devicep = &devices[i];
       gomp_mutex_lock (&devicep->lock);
       if (devicep->type == target_type && devicep->is_initialized)
-	gomp_unload_image_from_device (devicep, host_table, target_data);
+	gomp_unload_image_from_device (devicep, version,
+				       host_table, target_data);
       gomp_mutex_unlock (&devicep->lock);
     }
 
@@ -848,6 +869,13 @@ GOMP_offload_unregister (const void *hos
   gomp_mutex_unlock (&register_lock);
 }
 
+void
+GOMP_offload_unregister (const void *host_table, int target_type,
+			 const void *target_data)
+{
+  GOMP_offload_unregister_ver (0, host_table, target_type, target_data);
+}
+
 /* This function initializes the target device, specified by DEVICEP.  DEVICEP
    must be locked on entry, and remains locked on return.  */
 
@@ -862,8 +890,9 @@ gomp_init_device (struct gomp_device_des
     {
       struct offload_image_descr *image = &offload_images[i];
       if (image->type == devicep->type)
-	gomp_load_image_to_device (devicep, image->host_table,
-				   image->target_data, false);
+	gomp_load_image_to_device (devicep, image->version,
+				   image->host_table, image->target_data,
+				   false);
     }
 
   devicep->is_initialized = true;
@@ -881,7 +910,8 @@ gomp_unload_device (struct gomp_device_d
 	{
 	  struct offload_image_descr *image = &offload_images[i];
 	  if (image->type == devicep->type)
-	    gomp_unload_image_from_device (devicep, image->host_table,
+	    gomp_unload_image_from_device (devicep, image->version,
+					   image->host_table,
 					   image->target_data);
 	}
     }
@@ -1085,43 +1115,29 @@ gomp_load_plugin_for_device (struct gomp
 			     const char *plugin_name)
 {
   const char *err = NULL, *last_missing = NULL;
-  int optional_present, optional_total;
-
-  /* Clear any existing error.  */
-  dlerror ();
 
   void *plugin_handle = dlopen (plugin_name, RTLD_LAZY);
   if (!plugin_handle)
-    {
-      err = dlerror ();
-      goto out;
-    }
+    goto dl_fail;
 
   /* Check if all required functions are available in the plugin and store
-     their handlers.  */
+     their handlers.  None of the symbols can legitimately be NULL,
+     so we don't need to check dlerror all the time.  */
 #define DLSYM(f)							\
-  do									\
-    {									\
-      device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #f);	\
-      err = dlerror ();							\
-      if (err != NULL)							\
-	goto out;							\
-    }									\
-  while (0)
-  /* Similar, but missing functions are not an error.  */
-#define DLSYM_OPT(f, n)						\
-  do									\
-    {									\
-      const char *tmp_err;							\
-      device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #n);	\
-      tmp_err = dlerror ();						\
-      if (tmp_err == NULL)						\
-        optional_present++;						\
-      else								\
-        last_missing = #n;						\
-      optional_total++;							\
-    }									\
-  while (0)
+  if (!(device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #f)))	\
+    goto dl_fail
+  /* Similar, but missing functions are not an error.  Return false if
+     failed, true otherwise.  */
+#define DLSYM_OPT(f, n)							\
+  ((device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #n))	\
+   || (last_missing = #n, 0))
+
+  DLSYM (version);
+  if (device->version_func () != GOMP_VERSION)
+    {
+      err = "plugin version mismatch";
+      goto fail;
+    }
 
   DLSYM (get_name);
   DLSYM (get_caps);
@@ -1140,53 +1156,57 @@ gomp_load_plugin_for_device (struct gomp
     DLSYM (run);
   if (device->capabilities & GOMP_OFFLOAD_CAP_OPENACC_200)
     {
-      optional_present = optional_total = 0;
-      DLSYM_OPT (openacc.exec, openacc_parallel);
-      DLSYM_OPT (openacc.register_async_cleanup,
-		 openacc_register_async_cleanup);
-      DLSYM_OPT (openacc.async_test, openacc_async_test);
-      DLSYM_OPT (openacc.async_test_all, openacc_async_test_all);
-      DLSYM_OPT (openacc.async_wait, openacc_async_wait);
-      DLSYM_OPT (openacc.async_wait_async, openacc_async_wait_async);
-      DLSYM_OPT (openacc.async_wait_all, openacc_async_wait_all);
-      DLSYM_OPT (openacc.async_wait_all_async, openacc_async_wait_all_async);
-      DLSYM_OPT (openacc.async_set_async, openacc_async_set_async);
-      DLSYM_OPT (openacc.create_thread_data, openacc_create_thread_data);
-      DLSYM_OPT (openacc.destroy_thread_data, openacc_destroy_thread_data);
-      /* Require all the OpenACC handlers if we have
-	 GOMP_OFFLOAD_CAP_OPENACC_200.  */
-      if (optional_present != optional_total)
+      if (!DLSYM_OPT (openacc.exec, openacc_parallel)
+	  || !DLSYM_OPT (openacc.register_async_cleanup,
+			 openacc_register_async_cleanup)
+	  || !DLSYM_OPT (openacc.async_test, openacc_async_test)
+	  || !DLSYM_OPT (openacc.async_test_all, openacc_async_test_all)
+	  || !DLSYM_OPT (openacc.async_wait, openacc_async_wait)
+	  || !DLSYM_OPT (openacc.async_wait_async, openacc_async_wait_async)
+	  || !DLSYM_OPT (openacc.async_wait_all, openacc_async_wait_all)
+	  || !DLSYM_OPT (openacc.async_wait_all_async,
+			 openacc_async_wait_all_async)
+	  || !DLSYM_OPT (openacc.async_set_async, openacc_async_set_async)
+	  || !DLSYM_OPT (openacc.create_thread_data,
+			 openacc_create_thread_data)
+	  || !DLSYM_OPT (openacc.destroy_thread_data,
+			 openacc_destroy_thread_data))
 	{
+	  /* Require all the OpenACC handlers if we have
+	     GOMP_OFFLOAD_CAP_OPENACC_200.  */
 	  err = "plugin missing OpenACC handler function";
-	  goto out;
+	  goto fail;
 	}
-      optional_present = optional_total = 0;
-      DLSYM_OPT (openacc.cuda.get_current_device,
-		 openacc_get_current_cuda_device);
-      DLSYM_OPT (openacc.cuda.get_current_context,
-		 openacc_get_current_cuda_context);
-      DLSYM_OPT (openacc.cuda.get_stream, openacc_get_cuda_stream);
-      DLSYM_OPT (openacc.cuda.set_stream, openacc_set_cuda_stream);
-      /* Make sure all the CUDA functions are there if any of them are.  */
-      if (optional_present && optional_present != optional_total)
+
+      unsigned cuda = 0;
+      cuda += DLSYM_OPT (openacc.cuda.get_current_device,
+			 openacc_get_current_cuda_device);
+      cuda += DLSYM_OPT (openacc.cuda.get_current_context,
+			 openacc_get_current_cuda_context);
+      cuda += DLSYM_OPT (openacc.cuda.get_stream, openacc_get_cuda_stream);
+      cuda += DLSYM_OPT (openacc.cuda.set_stream, openacc_set_cuda_stream);
+      if (cuda && cuda != 4)
 	{
+	  /* Make sure all the CUDA functions are there if any of them are.  */
 	  err = "plugin missing OpenACC CUDA handler function";
-	  goto out;
+	  goto fail;
 	}
     }
 #undef DLSYM
 #undef DLSYM_OPT
 
- out:
-  if (err != NULL)
-    {
-      gomp_error ("while loading %s: %s", plugin_name, err);
-      if (last_missing)
-        gomp_error ("missing function was %s", last_missing);
-      if (plugin_handle)
-	dlclose (plugin_handle);
-    }
-  return err == NULL;
+  return 1;
+
+ dl_fail:
+  err = dlerror ();
+ fail:
+  gomp_error ("while loading %s: %s", plugin_name, err);
+  if (last_missing)
+    gomp_error ("missing function was %s", last_missing);
+  if (plugin_handle)
+    dlclose (plugin_handle);
+
+  return 0;
 }
 
 /* This function initializes the runtime needed for offloading.
Index: libgomp/libgomp.h
===================================================================
--- libgomp/libgomp.h	(revision 226462)
+++ libgomp/libgomp.h	(working copy)
@@ -748,8 +748,9 @@ struct gomp_device_descr
   int (*get_num_devices_func) (void);
   void (*init_device_func) (int);
   void (*fini_device_func) (int);
-  int (*load_image_func) (int, const void *, struct addr_pair **);
-  void (*unload_image_func) (int, const void *);
+  unsigned (*version_func) (void);
+  int (*load_image_func) (int, unsigned, const void *, struct addr_pair **);
+  void (*unload_image_func) (int, unsigned, const void *);
   void *(*alloc_func) (int, size_t);
   void (*free_func) (int, void *);
   void *(*dev2host_func) (int, void *, const void *, size_t);
Index: libgomp/plugin/plugin-nvptx.c
===================================================================
--- libgomp/plugin/plugin-nvptx.c	(revision 226462)
+++ libgomp/plugin/plugin-nvptx.c	(working copy)
@@ -36,6 +36,7 @@
 #include "libgomp-plugin.h"
 #include "oacc-ptx.h"
 #include "oacc-plugin.h"
+#include "gomp-constants.h"
 
 #include <pthread.h>
 #include <cuda.h>
@@ -1644,11 +1645,20 @@ typedef struct nvptx_tdata
   size_t fn_num;
 } nvptx_tdata_t;
 
+/* Return the libgomp version number we're compatible with.  There is
+   no requirement for cross-version compatibility.  */
+
+unsigned
+GOMP_OFFLOAD_version (void)
+{
+  return GOMP_VERSION;
+}
+
 /* Load the (partial) program described by TARGET_DATA to device
    number ORD.  Allocate and return TARGET_TABLE.  */
 
 int
-GOMP_OFFLOAD_load_image (int ord, const void *target_data,
+GOMP_OFFLOAD_load_image (int ord, unsigned version, const void *target_data,
 			 struct addr_pair **target_table)
 {
   CUmodule module;
@@ -1661,6 +1671,11 @@ GOMP_OFFLOAD_load_image (int ord, const
   struct ptx_image_data *new_image;
   struct ptx_device *dev;
 
+  if (GOMP_VERSION_DEV (version) > GOMP_VERSION_NVIDIA_PTX)
+    GOMP_PLUGIN_fatal ("Offload data incompatible with PTX plugin"
+		       " (expected %u, received %u)",
+		       GOMP_VERSION_NVIDIA_PTX, GOMP_VERSION_DEV (version));
+  
   GOMP_OFFLOAD_init_device (ord);
 
   dev = ptx_devices[ord];
@@ -1730,11 +1745,14 @@ GOMP_OFFLOAD_load_image (int ord, const
    function descriptors allocated by G_O_load_image.  */
 
 void
-GOMP_OFFLOAD_unload_image (int ord, const void *target_data)
+GOMP_OFFLOAD_unload_image (int ord, unsigned version, const void *target_data)
 {
   struct ptx_image_data *image, **prev_p;
   struct ptx_device *dev = ptx_devices[ord];
 
+  if (GOMP_VERSION_DEV (version) > GOMP_VERSION_NVIDIA_PTX)
+    return;
+  
   pthread_mutex_lock (&dev->image_lock);
   for (prev_p = &dev->images; (image = *prev_p) != 0; prev_p = &image->next)
     if (image->target_data == target_data)
Index: libgomp/plugin/plugin-host.c
===================================================================
--- libgomp/plugin/plugin-host.c	(revision 226462)
+++ libgomp/plugin/plugin-host.c	(working copy)
@@ -39,6 +39,7 @@
 #include "libgomp.h"
 #include "oacc-int.h"
 #endif
+#include "gomp-constants.h"
 
 #include <stdint.h>
 #include <stdlib.h>
@@ -109,8 +110,15 @@ GOMP_OFFLOAD_fini_device (int n __attrib
 {
 }
 
+STATIC unsigned
+GOMP_OFFLOAD_version (void)
+{
+  return GOMP_VERSION;
+}
+
 STATIC int
 GOMP_OFFLOAD_load_image (int n __attribute__ ((unused)),
+			 unsigned  v __attribute__ ((unused)),
 			 const void *t __attribute__ ((unused)),
 			 struct addr_pair **r __attribute__ ((unused)))
 {
@@ -119,6 +127,7 @@ GOMP_OFFLOAD_load_image (int n __attribu
 
 STATIC void
 GOMP_OFFLOAD_unload_image (int n __attribute__ ((unused)),
+			   unsigned  v __attribute__ ((unused)),
 			   const void *t __attribute__ ((unused)))
 {
 }
Index: libgomp/oacc-host.c
===================================================================
--- libgomp/oacc-host.c	(revision 226462)
+++ libgomp/oacc-host.c	(working copy)
@@ -45,6 +45,7 @@ static struct gomp_device_descr host_dis
     .get_num_devices_func = GOMP_OFFLOAD_get_num_devices,
     .init_device_func = GOMP_OFFLOAD_init_device,
     .fini_device_func = GOMP_OFFLOAD_fini_device,
+    .version_func = GOMP_OFFLOAD_version,
     .load_image_func = GOMP_OFFLOAD_load_image,
     .unload_image_func = GOMP_OFFLOAD_unload_image,
     .alloc_func = GOMP_OFFLOAD_alloc,
Index: liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
===================================================================
--- liboffloadmic/plugin/libgomp-plugin-intelmic.cpp	(revision 226462)
+++ liboffloadmic/plugin/libgomp-plugin-intelmic.cpp	(working copy)
@@ -327,12 +327,26 @@ offload_image (const void *target_image)
   free (image);
 }
 
+/* Return the libgomp version number we're compatible with.  There is
+   no requirement for cross-version compatibility.  */
+
+extern "C" unsigned
+GOMP_OFFLOAD_version (void)
+{
+  return GOMP_VERSION;
+}
+
 extern "C" int
-GOMP_OFFLOAD_load_image (int device, const void *target_image,
-			 addr_pair **result)
+GOMP_OFFLOAD_load_image (int device, const unsigned version,
+			 void *target_image, addr_pair **result)
 {
   TRACE ("(device = %d, target_image = %p)", device, target_image);
 
+  if (GOMP_VERSION_DEV (version) > GOMP_VERSION_INTEL_MIC)
+    GOMP_PLUGIN_fatal ("Offload data incompatible with intelmic plugin"
+		       " (expected %u, received %u)",
+		       GOMP_VERSION_INTEL_MIC, GOMP_VERSION_DEV (version));
+
   /* If target_image is already present in address_table, then there is no need
      to offload it.  */
   if (address_table->count (target_image) == 0)
@@ -353,8 +367,12 @@ GOMP_OFFLOAD_load_image (int device, con
 }
 
 extern "C" void
-GOMP_OFFLOAD_unload_image (int device, const void *target_image)
+GOMP_OFFLOAD_unload_image (int device, unsigned version,
+			   const void *target_image)
 {
+  if (GOMP_VERSION_DEV (version) > GOMP_VERSION_INTEL_MIC)
+    return;
+
   TRACE ("(device = %d, target_image = %p)", device, target_image);
 
   /* TODO: Currently liboffloadmic doesn't support __offload_unregister_image
Index: include/gomp-constants.h
===================================================================
--- include/gomp-constants.h	(revision 226462)
+++ include/gomp-constants.h	(working copy)
@@ -113,4 +113,13 @@ enum gomp_map_kind
 #define GOMP_DEVICE_ICV			-1
 #define GOMP_DEVICE_HOST_FALLBACK	-2
 
+/* Versions of libgomp and device-specific plugins.  */
+#define GOMP_VERSION	0
+#define GOMP_VERSION_NVIDIA_PTX 0
+#define GOMP_VERSION_INTEL_MIC 0
+
+#define GOMP_VERSION_PACK(LIB, DEV) (((LIB) << 16) | (DEV))
+#define GOMP_VERSION_LIB(PACK) (((PACK) >> 16) & 0xffff)
+#define GOMP_VERSION_DEV(PACK) ((PACK) & 0xffff)
+
 #endif

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-08-02  0:06                 ` Nathan Sidwell
@ 2015-08-02  0:20                   ` Nathan Sidwell
  2015-08-03 11:16                     ` Thomas Schwinge
  2015-08-04 16:19                   ` Thomas Schwinge
  2015-08-06 19:52                   ` Nathan Sidwell
  2 siblings, 1 reply; 22+ messages in thread
From: Nathan Sidwell @ 2015-08-02  0:20 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: ilya.verbin, GCC Patches

[-- Attachment #1: Type: text/plain, Size: 151 bytes --]

On 08/01/15 20:06, Nathan Sidwell wrote:

> ok?  I'll commit a version of this to gomp4 branch shortly.

this is the gomp4 committed version.

nathan


[-- Attachment #2: gomp4-version-3.patch --]
[-- Type: text/x-patch, Size: 11793 bytes --]

2015-08-01  Nathan Sidwell  <nathan@codesourcery.com>

	include/
	* gomp-constants.h (GOMP_VERSION_INTEL_MIC): New.

	libgomp/
	* plugin/plugin-nvptx.c (GOMP_OFFLOAD_load_image_ver): Rename to ...
	(GOMP_OFFLOAD_load_image): ... here.  Swap version and ord args.
	Adjust version check and mesage.
	(GOMP_OFFLOAD_unload_image_ver): Rename to ...
	(GOMP_OFFLOAD_unload_image): ... here.  Swap version and ord args.
	* oacc-host.c: Include gomp-constants.h.
	(host_version): New.
	(host_load_image, host_unload_image): Add version arg.
	(host_dispatch): Adjust.
	* target.c (gomp_load_image_to_device): Swap version and devicep
	args. Adjust plugin load calling.  Adjust all callers.
	* target.c (gomp_unload_image_from_device): Swap version and devicep
	args. Adjust plugin unload calling. Adjust all callers.
	(load_plugin_for_device): Always look for version function and
	check it.  Adjust load and unload call lookup.
	* libgomp.h: Adjustt load and unload plugin fields.

	liboffloadmic/
	* plugin/libgomp-plugin-intelmic.cpp (GOMP_OFFLOAD_version): New.
	(GOMP_OFFLOAD_load_image): Add version arg and check it.
	(GOMP_OFFLOAD_unload_image): Likewise.

Index: libgomp/plugin/plugin-nvptx.c
===================================================================
--- libgomp/plugin/plugin-nvptx.c	(revision 226462)
+++ libgomp/plugin/plugin-nvptx.c	(working copy)
@@ -1650,9 +1650,9 @@ GOMP_OFFLOAD_version (void)
    number ORD.  Allocate and return TARGET_TABLE.  */
 
 int
-GOMP_OFFLOAD_load_image_ver (unsigned version, int ord,
-			     const void *target_data,
-			     struct addr_pair **target_table)
+GOMP_OFFLOAD_load_image (int ord, unsigned version,
+			 const void *target_data,
+			 struct addr_pair **target_table)
 {
   CUmodule module;
   const char *const *var_names;
@@ -1665,9 +1665,9 @@ GOMP_OFFLOAD_load_image_ver (unsigned ve
   struct ptx_image_data *new_image;
   struct ptx_device *dev;
 
-  if (GOMP_VERSION_DEV (version) != GOMP_VERSION_NVIDIA_PTX)
+  if (GOMP_VERSION_DEV (version) > GOMP_VERSION_NVIDIA_PTX)
     GOMP_PLUGIN_fatal ("Offload data incompatible with PTX plugin"
-		       " (version %u ! %u)",
+		       " (expected %u, received %u)",
 		       GOMP_VERSION_NVIDIA_PTX, GOMP_VERSION_DEV (version));
   
   GOMP_OFFLOAD_init_device (ord);
@@ -1739,13 +1739,12 @@ GOMP_OFFLOAD_load_image_ver (unsigned ve
    function descriptors allocated by G_O_load_image.  */
 
 void
-GOMP_OFFLOAD_unload_image_ver (unsigned version, int ord,
-			       const void *target_data)
+GOMP_OFFLOAD_unload_image (int ord, unsigned version, const void *target_data)
 {
   struct ptx_image_data *image, **prev_p;
   struct ptx_device *dev = ptx_devices[ord];
 
-  if (GOMP_VERSION_DEV (version) != GOMP_VERSION_NVIDIA_PTX)
+  if (GOMP_VERSION_DEV (version) > GOMP_VERSION_NVIDIA_PTX)
     return;
   
   pthread_mutex_lock (&dev->image_lock);
Index: libgomp/oacc-host.c
===================================================================
--- libgomp/oacc-host.c	(revision 226462)
+++ libgomp/oacc-host.c	(working copy)
@@ -28,6 +28,7 @@
 
 #include "libgomp.h"
 #include "oacc-int.h"
+#include "gomp-constants.h"
 
 #include <stdbool.h>
 #include <stddef.h>
@@ -69,8 +70,15 @@ host_fini_device (int n __attribute__ ((
 {
 }
 
+static unsigned
+host_version (void)
+{
+  return GOMP_VERSION;
+}
+
 static int
 host_load_image (int n __attribute__ ((unused)),
+		 unsigned v __attribute__ ((unused)),
 		 const void *t __attribute__ ((unused)),
 		 struct addr_pair **r __attribute__ ((unused)))
 {
@@ -79,6 +87,7 @@ host_load_image (int n __attribute__ ((u
 
 static void
 host_unload_image (int n __attribute__ ((unused)),
+		   unsigned v __attribute__ ((unused)),
 		   const void *t __attribute__ ((unused)))
 {
 }
@@ -203,9 +212,9 @@ static struct gomp_device_descr host_dis
     .get_num_devices_func = host_get_num_devices,
     .init_device_func = host_init_device,
     .fini_device_func = host_fini_device,
-    .version_func = NULL,
-    .load_image = {.unver_func = host_load_image},
-    .unload_image = {.unver_func = host_unload_image},
+    .version_func = host_version,
+    .load_image_func = host_load_image,
+    .unload_image_func = host_unload_image,
     .alloc_func = host_alloc,
     .free_func = host_free,
     .dev2host_func = host_dev2host,
Index: libgomp/target.c
===================================================================
--- libgomp/target.c	(revision 226462)
+++ libgomp/target.c	(working copy)
@@ -652,8 +652,7 @@ gomp_update (struct gomp_device_descr *d
    emitting variable and functions in the same order.  */
 
 static void
-gomp_load_image_to_device (unsigned version,
-			   struct gomp_device_descr *devicep,
+gomp_load_image_to_device (struct gomp_device_descr *devicep, unsigned version,
 			   const void *host_table, const void *target_data,
 			   bool is_register_lock)
 {
@@ -671,17 +670,9 @@ gomp_load_image_to_device (unsigned vers
   struct addr_pair *target_table = NULL;
   int i, num_target_entries;
 
-  if (devicep->version_func)
-    num_target_entries
-      = devicep->load_image.ver_func (version, devicep->target_id,
-					target_data, &target_table);
-  else if (GOMP_VERSION_DEV (version))
-    gomp_fatal ("Plugin too old for offload data (0 < %u)",
-		GOMP_VERSION_DEV (version));
-  else
-    num_target_entries
-      = devicep->load_image.unver_func (devicep->target_id,
-					target_data, &target_table);
+  num_target_entries
+    = devicep->load_image_func (devicep->target_id, version,
+				target_data, &target_table);
 
   if (num_target_entries != num_funcs + num_vars)
     {
@@ -763,8 +754,8 @@ gomp_load_image_to_device (unsigned vers
    The device must be locked.   */
 
 static void
-gomp_unload_image_from_device (unsigned version,
-			       struct gomp_device_descr *devicep,
+gomp_unload_image_from_device (struct gomp_device_descr *devicep,
+			       unsigned version,
 			       const void *host_table, const void *target_data)
 {
   void **host_func_table = ((void ***) host_table)[0];
@@ -790,11 +781,7 @@ gomp_unload_image_from_device (unsigned
       node = splay_tree_lookup (&devicep->mem_map, &k);
     }
 
-  if (devicep->version_func)
-    devicep->unload_image.ver_func (version,
-				    devicep->target_id, target_data);
-  else
-    devicep->unload_image.unver_func (devicep->target_id, target_data);
+  devicep->unload_image_func (devicep->target_id, version, target_data);
 
   /* Remove mappings from splay tree.  */
   for (j = 0; j < num_funcs; j++)
@@ -840,7 +827,7 @@ GOMP_offload_register_ver (unsigned vers
       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 (version, devicep,
+	gomp_load_image_to_device (devicep, version,
 				   host_table, target_data, true);
       gomp_mutex_unlock (&devicep->lock);
     }
@@ -884,7 +871,7 @@ GOMP_offload_unregister_ver (unsigned ve
       struct gomp_device_descr *devicep = &devices[i];
       gomp_mutex_lock (&devicep->lock);
       if (devicep->type == target_type && devicep->is_initialized)
-	gomp_unload_image_from_device (version, devicep,
+	gomp_unload_image_from_device (devicep, version,
 				       host_table, target_data);
       gomp_mutex_unlock (&devicep->lock);
     }
@@ -921,7 +908,7 @@ gomp_init_device (struct gomp_device_des
     {
       struct offload_image_descr *image = &offload_images[i];
       if (image->type == devicep->type)
-	gomp_load_image_to_device (image->version, devicep,
+	gomp_load_image_to_device (devicep, image->version,
 				   image->host_table, image->target_data,
 				   false);
     }
@@ -941,7 +928,7 @@ gomp_unload_device (struct gomp_device_d
 	{
 	  struct offload_image_descr *image = &offload_images[i];
 	  if (image->type == devicep->type)
-	    gomp_unload_image_from_device (image->version, devicep,
+	    gomp_unload_image_from_device (devicep, image->version,
 					   image->host_table,
 					   image->target_data);
 	}
@@ -1147,23 +1134,11 @@ gomp_load_plugin_for_device (struct gomp
   ((device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #n))	\
    || (last_missing = #n, 0))
 
-  if (DLSYM_OPT (version, version))
-    {
-      unsigned v = device->version_func ();
-      if (v != GOMP_VERSION)
-	{
-	  err = "plugin version mismatch";
-	  goto fail;
-	}
-      if (!DLSYM_OPT (load_image.ver, load_image_ver)
-	  || !DLSYM_OPT (unload_image.ver, unload_image_ver))
-	goto  dl_fail;
-    }
-  else
+  DLSYM (version);
+  if (device->version_func () != GOMP_VERSION)
     {
-      if (!DLSYM_OPT (load_image.unver, load_image)
-	  || !DLSYM_OPT (unload_image.unver, unload_image))
-	goto dl_fail;
+      err = "plugin version mismatch";
+      goto fail;
     }
 
   DLSYM (get_name);
@@ -1172,6 +1147,8 @@ gomp_load_plugin_for_device (struct gomp
   DLSYM (get_num_devices);
   DLSYM (init_device);
   DLSYM (fini_device);
+  DLSYM (load_image);
+  DLSYM (unload_image);
   DLSYM (alloc);
   DLSYM (free);
   DLSYM (dev2host);
Index: libgomp/libgomp.h
===================================================================
--- libgomp/libgomp.h	(revision 226462)
+++ libgomp/libgomp.h	(working copy)
@@ -750,22 +750,9 @@ struct gomp_device_descr
   int (*get_num_devices_func) (void);
   void (*init_device_func) (int);
   void (*fini_device_func) (int);
-
   unsigned (*version_func) (void);
-  
-  /* When all plugins updated, we can remove these unions and just
-     have the versioned entry points.  */
-  union 
-  {
-    int (*unver_func) (int, const void *, struct addr_pair **);
-    int (*ver_func) (unsigned, int, const void *, struct addr_pair **);
-  } load_image;
-  union
-  {
-    void (*unver_func) (int, const void *);
-    void (*ver_func) (unsigned, int, const void *);
-  } unload_image;
-  
+  int (*load_image_func) (int, unsigned, const void *, struct addr_pair **);
+  void (*unload_image_func) (int, unsigned, const void *);
   void *(*alloc_func) (int, size_t);
   void (*free_func) (int, void *);
   void *(*dev2host_func) (int, void *, const void *, size_t);
Index: liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
===================================================================
--- liboffloadmic/plugin/libgomp-plugin-intelmic.cpp	(revision 226462)
+++ liboffloadmic/plugin/libgomp-plugin-intelmic.cpp	(working copy)
@@ -327,12 +327,26 @@ offload_image (const void *target_image)
   free (image);
 }
 
+/* Return the libgomp version number we're compatible with.  There is
+   no requirement for cross-version compatibility.  */
+
+extern "C" unsigned
+GOMP_OFFLOAD_version (void)
+{
+  return GOMP_VERSION;
+}
+
 extern "C" int
-GOMP_OFFLOAD_load_image (int device, const void *target_image,
-			 addr_pair **result)
+GOMP_OFFLOAD_load_image (int device, const unsigned version,
+			 void *target_image, addr_pair **result)
 {
   TRACE ("(device = %d, target_image = %p)", device, target_image);
 
+  if (GOMP_VERSION_DEV (version) > GOMP_VERSION_INTEL_MIC)
+    GOMP_PLUGIN_fatal ("Offload data incompatible with intelmic plugin"
+		       " (expected %u, received %u)",
+		       GOMP_VERSION_INTEL_MIC, GOMP_VERSION_DEV (version));
+
   /* If target_image is already present in address_table, then there is no need
      to offload it.  */
   if (address_table->count (target_image) == 0)
@@ -353,8 +367,12 @@ GOMP_OFFLOAD_load_image (int device, con
 }
 
 extern "C" void
-GOMP_OFFLOAD_unload_image (int device, const void *target_image)
+GOMP_OFFLOAD_unload_image (int device, unsigned version,
+			   const void *target_image)
 {
+  if (GOMP_VERSION_DEV (version) > GOMP_VERSION_INTEL_MIC)
+    return;
+
   TRACE ("(device = %d, target_image = %p)", device, target_image);
 
   /* TODO: Currently liboffloadmic doesn't support __offload_unregister_image

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-08-02  0:20                   ` Nathan Sidwell
@ 2015-08-03 11:16                     ` Thomas Schwinge
  2015-08-03 13:26                       ` Nathan Sidwell
  0 siblings, 1 reply; 22+ messages in thread
From: Thomas Schwinge @ 2015-08-03 11:16 UTC (permalink / raw)
  To: Nathan Sidwell, GCC Patches; +Cc: ilya.verbin, Jakub Jelinek

[-- Attachment #1: Type: text/plain, Size: 9501 bytes --]

Hi!

On Sat, 1 Aug 2015 20:20:49 -0400, Nathan Sidwell <nathan@acm.org> wrote:
> this is the gomp4 committed version.

> --- liboffloadmic/plugin/libgomp-plugin-intelmic.cpp	(revision 226462)
> +++ liboffloadmic/plugin/libgomp-plugin-intelmic.cpp	(working copy)
> @@ -327,12 +327,26 @@ offload_image (const void *target_image)
>    free (image);
>  }
>  
> +/* Return the libgomp version number we're compatible with.  There is
> +   no requirement for cross-version compatibility.  */
> +
> +extern "C" unsigned
> +GOMP_OFFLOAD_version (void)
> +{
> +  return GOMP_VERSION;
> +}
> +
>  extern "C" int
> -GOMP_OFFLOAD_load_image (int device, const void *target_image,
> -			 addr_pair **result)
> +GOMP_OFFLOAD_load_image (int device, const unsigned version,
> +			 void *target_image, addr_pair **result)
>  {
>    TRACE ("(device = %d, target_image = %p)", device, target_image);
>  
> +  if (GOMP_VERSION_DEV (version) > GOMP_VERSION_INTEL_MIC)
> +    GOMP_PLUGIN_fatal ("Offload data incompatible with intelmic plugin"
> +		       " (expected %u, received %u)",
> +		       GOMP_VERSION_INTEL_MIC, GOMP_VERSION_DEV (version));
> +
>    /* If target_image is already present in address_table, then there is no need
>       to offload it.  */
>    if (address_table->count (target_image) == 0)
> @@ -353,8 +367,12 @@ GOMP_OFFLOAD_load_image (int device, con
>  }
>  
>  extern "C" void
> -GOMP_OFFLOAD_unload_image (int device, const void *target_image)
> +GOMP_OFFLOAD_unload_image (int device, unsigned version,
> +			   const void *target_image)
>  {
> +  if (GOMP_VERSION_DEV (version) > GOMP_VERSION_INTEL_MIC)
> +    return;
> +
>    TRACE ("(device = %d, target_image = %p)", device, target_image);
>  
>    /* TODO: Currently liboffloadmic doesn't support __offload_unregister_image

Committed to gomp-4_0-branch in r226497:

commit 4e0158f41a00d6c4d09ca8a48eb63832abdd2f84
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Mon Aug 3 11:14:24 2015 +0000

    Fix intelmic libgomp plugin build
    
    ... which got broken in r226469:
    
        [...]/source-gcc/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp: In function 'unsigned int GOMP_OFFLOAD_version()':
        [...]/source-gcc/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp:336:10: error: 'GOMP_VERSION' was not declared in this scope
           return GOMP_VERSION;
                  ^
        [...]/source-gcc/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp: In function 'int GOMP_OFFLOAD_load_image(int, unsigned int, void*, addr_pair**)':
        [...]/source-gcc/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp:345:32: error: 'GOMP_VERSION_DEV' was not declared in this scope
           if (GOMP_VERSION_DEV (version) > GOMP_VERSION_INTEL_MIC)
                                        ^
        [...]/source-gcc/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp:345:36: error: 'GOMP_VERSION_INTEL_MIC' was not declared in this scope
           if (GOMP_VERSION_DEV (version) > GOMP_VERSION_INTEL_MIC)
                                            ^
        [...]/source-gcc/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp: In function 'void GOMP_OFFLOAD_unload_image(int, unsigned int, const void*)':
        [...]/source-gcc/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp:373:32: error: 'GOMP_VERSION_DEV' was not declared in this scope
           if (GOMP_VERSION_DEV (version) > GOMP_VERSION_INTEL_MIC)
                                        ^
        [...]/source-gcc/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp:373:36: error: 'GOMP_VERSION_INTEL_MIC' was not declared in this scope
           if (GOMP_VERSION_DEV (version) > GOMP_VERSION_INTEL_MIC)
                                            ^
        make[6]: *** [libgomp_plugin_intelmic_la-libgomp-plugin-intelmic.lo] Error 1
    
    	liboffloadmic/
    	* plugin/Makefile.am (include_src_dir): Set.
    	[PLUGIN_HOST] (libgomp_plugin_intelmic_la_CPPFLAGS): Use it.
    	* plugin/Makefile.in: Regenerate.
    	* plugin/libgomp-plugin-intelmic.cpp: Include "gomp-constants.h".
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@226497 138bc75d-0d04-0410-961f-82ee72b054a4
---
 liboffloadmic/ChangeLog.gomp                     |    7 +++++++
 liboffloadmic/plugin/Makefile.am                 |    3 ++-
 liboffloadmic/plugin/Makefile.in                 |    3 ++-
 liboffloadmic/plugin/libgomp-plugin-intelmic.cpp |    3 ++-
 4 files changed, 13 insertions(+), 3 deletions(-)

diff --git liboffloadmic/ChangeLog.gomp liboffloadmic/ChangeLog.gomp
index 93d1e02..adb9e05 100644
--- liboffloadmic/ChangeLog.gomp
+++ liboffloadmic/ChangeLog.gomp
@@ -1,3 +1,10 @@
+2015-08-03  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* plugin/Makefile.am (include_src_dir): Set.
+	[PLUGIN_HOST] (libgomp_plugin_intelmic_la_CPPFLAGS): Use it.
+	* plugin/Makefile.in: Regenerate.
+	* plugin/libgomp-plugin-intelmic.cpp: Include "gomp-constants.h".
+
 2015-08-01  Nathan Sidwell  <nathan@codesourcery.com>
 
 	* plugin/libgomp-plugin-intelmic.cpp (GOMP_OFFLOAD_version): New.
diff --git liboffloadmic/plugin/Makefile.am liboffloadmic/plugin/Makefile.am
index 19d69ab..6ec444c 100644
--- liboffloadmic/plugin/Makefile.am
+++ liboffloadmic/plugin/Makefile.am
@@ -36,6 +36,7 @@ build_dir = $(top_builddir)
 source_dir = $(top_srcdir)
 coi_inc_dir = $(top_srcdir)/../include/coi
 myo_inc_dir = $(top_srcdir)/../include/myo
+include_src_dir = $(top_srcdir)/../../include
 libgomp_src_dir = $(top_srcdir)/../../libgomp
 libgomp_dir = $(build_dir)/../../libgomp
 liboffload_src_dir = $(top_srcdir)/../runtime
@@ -52,7 +53,7 @@ target_install_dir = $(accel_search_dir)/lib/gcc/$(accel_target)/$(gcc_version)$
 if PLUGIN_HOST
   toolexeclib_LTLIBRARIES = libgomp-plugin-intelmic.la
   libgomp_plugin_intelmic_la_SOURCES = libgomp-plugin-intelmic.cpp
-  libgomp_plugin_intelmic_la_CPPFLAGS = $(CPPFLAGS) -DLINUX -DCOI_LIBRARY_VERSION=2 -DMYO_SUPPORT -DOFFLOAD_DEBUG=1 -DSEP_SUPPORT -DTIMING_SUPPORT -DHOST_LIBRARY=1 -I$(coi_inc_dir) -I$(myo_inc_dir) -I$(liboffload_src_dir) -I$(libgomp_src_dir) -I$(libgomp_dir) -I$(target_prefix_dir)/include -I$(target_build_dir) -I$(target_install_dir)/include
+  libgomp_plugin_intelmic_la_CPPFLAGS = $(CPPFLAGS) -DLINUX -DCOI_LIBRARY_VERSION=2 -DMYO_SUPPORT -DOFFLOAD_DEBUG=1 -DSEP_SUPPORT -DTIMING_SUPPORT -DHOST_LIBRARY=1 -I$(coi_inc_dir) -I$(myo_inc_dir) -I$(liboffload_src_dir) -I$(libgomp_src_dir) -I$(libgomp_dir) -I$(include_src_dir) -I$(target_prefix_dir)/include -I$(target_build_dir) -I$(target_install_dir)/include
   libgomp_plugin_intelmic_la_LDFLAGS = -L$(liboffload_dir)/.libs -loffloadmic_host -version-info 1:0:0
 else # PLUGIN_TARGET
   plugin_includedir = $(libsubincludedir)
diff --git liboffloadmic/plugin/Makefile.in liboffloadmic/plugin/Makefile.in
index 19a1a96..458c9b2 100644
--- liboffloadmic/plugin/Makefile.in
+++ liboffloadmic/plugin/Makefile.in
@@ -306,6 +306,7 @@ build_dir = $(top_builddir)
 source_dir = $(top_srcdir)
 coi_inc_dir = $(top_srcdir)/../include/coi
 myo_inc_dir = $(top_srcdir)/../include/myo
+include_src_dir = $(top_srcdir)/../../include
 libgomp_src_dir = $(top_srcdir)/../../libgomp
 libgomp_dir = $(build_dir)/../../libgomp
 liboffload_src_dir = $(top_srcdir)/../runtime
@@ -320,7 +321,7 @@ target_build_dir = $(accel_search_dir)/$(accel_target)$(MULTISUBDIR)/liboffloadm
 target_install_dir = $(accel_search_dir)/lib/gcc/$(accel_target)/$(gcc_version)$(MULTISUBDIR)
 @PLUGIN_HOST_TRUE@toolexeclib_LTLIBRARIES = libgomp-plugin-intelmic.la
 @PLUGIN_HOST_TRUE@libgomp_plugin_intelmic_la_SOURCES = libgomp-plugin-intelmic.cpp
-@PLUGIN_HOST_TRUE@libgomp_plugin_intelmic_la_CPPFLAGS = $(CPPFLAGS) -DLINUX -DCOI_LIBRARY_VERSION=2 -DMYO_SUPPORT -DOFFLOAD_DEBUG=1 -DSEP_SUPPORT -DTIMING_SUPPORT -DHOST_LIBRARY=1 -I$(coi_inc_dir) -I$(myo_inc_dir) -I$(liboffload_src_dir) -I$(libgomp_src_dir) -I$(libgomp_dir) -I$(target_prefix_dir)/include -I$(target_build_dir) -I$(target_install_dir)/include
+@PLUGIN_HOST_TRUE@libgomp_plugin_intelmic_la_CPPFLAGS = $(CPPFLAGS) -DLINUX -DCOI_LIBRARY_VERSION=2 -DMYO_SUPPORT -DOFFLOAD_DEBUG=1 -DSEP_SUPPORT -DTIMING_SUPPORT -DHOST_LIBRARY=1 -I$(coi_inc_dir) -I$(myo_inc_dir) -I$(liboffload_src_dir) -I$(libgomp_src_dir) -I$(libgomp_dir) -I$(include_src_dir) -I$(target_prefix_dir)/include -I$(target_build_dir) -I$(target_install_dir)/include
 @PLUGIN_HOST_TRUE@libgomp_plugin_intelmic_la_LDFLAGS = -L$(liboffload_dir)/.libs -loffloadmic_host -version-info 1:0:0
 @PLUGIN_HOST_FALSE@plugin_includedir = $(libsubincludedir)
 @PLUGIN_HOST_FALSE@plugin_include_HEADERS = main_target_image.h
diff --git liboffloadmic/plugin/libgomp-plugin-intelmic.cpp liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
index b852b42..fde7d9e 100644
--- liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
+++ liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
@@ -1,6 +1,6 @@
 /* Plugin for offload execution on Intel MIC devices.
 
-   Copyright (C) 2014 Free Software Foundation, Inc.
+   Copyright (C) 2014-2015 Free Software Foundation, Inc.
 
    Contributed by Ilya Verbin <ilya.verbin@intel.com>.
 
@@ -38,6 +38,7 @@
 #include "libgomp-plugin.h"
 #include "compiler_if_host.h"
 #include "main_target_image.h"
+#include "gomp-constants.h"
 
 #define LD_LIBRARY_PATH_ENV	"LD_LIBRARY_PATH"
 #define MIC_LD_LIBRARY_PATH_ENV	"MIC_LD_LIBRARY_PATH"


Grüße,
 Thomas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-08-03 11:16                     ` Thomas Schwinge
@ 2015-08-03 13:26                       ` Nathan Sidwell
  0 siblings, 0 replies; 22+ messages in thread
From: Nathan Sidwell @ 2015-08-03 13:26 UTC (permalink / raw)
  To: Thomas Schwinge, GCC Patches; +Cc: ilya.verbin, Jakub Jelinek

On 08/03/15 07:15, Thomas Schwinge wrote:
> Hi!
>
> On Sat, 1 Aug 2015 20:20:49 -0400, Nathan Sidwell <nathan@acm.org> wrote:
>> this is the gomp4 committed version.
>
>> --- liboffloadmic/plugin/libgomp-plugin-intelmic.cpp	(revision 226462)
>> +++ liboffloadmic/plugin/libgomp-plugin-intelmic.cpp	(working copy)
>> @@ -327,12 +327,26 @@ offload_image (const void *target_image)
>>     free (image);

Please prepare a patch that can be applied to trunk.  We'll need that for the 
trunk version of the version patch I posted for review.  Are you able to check 
that patch builds for intelmic?

nathan

-- 
Nathan Sidwell

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-08-02  0:06                 ` Nathan Sidwell
  2015-08-02  0:20                   ` Nathan Sidwell
@ 2015-08-04 16:19                   ` Thomas Schwinge
  2015-08-04 16:20                     ` Nathan Sidwell
  2015-08-06 19:52                   ` Nathan Sidwell
  2 siblings, 1 reply; 22+ messages in thread
From: Thomas Schwinge @ 2015-08-04 16:19 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Nathan Sidwell, ilya.verbin, GCC Patches


[-- Attachment #1.1: Type: text/plain, Size: 1731 bytes --]

Hi!

Nathan's patch is waiting for trunk approval:

On Sat, 1 Aug 2015 20:06:39 -0400, Nathan Sidwell <nathan@acm.org> wrote:
> On 07/31/15 12:10, Jakub Jelinek wrote:
> 
> > This will hopefully be just GOMP_4.1 instead in the end, but it can
> > change when gomp-4_1-branch is merged to trunk, we don't guarantee
> > ABI stability at this point.
> 
> Sure.
> 
> > I'd prefer version to go after devicep argument rather than before.
> Fixed.
> 
> > And really don't have ver_func vs. unver_func, just a single
> > callback that will take the version argument too (again, if possible
> > after target_id).
> 
> Fixed (& elsewhere).  The patch should be checked for intelmic if possible 
> (Ilya?).  The  changes there are very mechanical so I'm not expecting a problem.
> 
> We don't need to  make the initial value of GOMP_VERSION non-zero, because the 
> absence of the GOMP_OFFLOAD_version func will distinguish out of date plugins at 
> this point.
> 
> 
> >> +
> >> +  if (DLSYM_OPT (version, version))
> >
> > I'd prefer requiring version always (i.e. DLSYM (version);
> > plus the v != GOMP_VERSION checking).
> 
> Fixed.
> 
> ok?

I'd found one "infrastructure-class" problem in the libgomp intelmic
plugin build, which I addressed on gomp-4_0-branch in r226497,
<http://news.gmane.org/find-root.php?message_id=%3C878u9soc60.fsf%40kepler.schwinge.homeip.net%3E>,
patch again attached here, for easy reference (applies to trunk as-is):
0001-Fix-intelmic-libgomp-plugin-build.patch.

Then, for convenience, I'm also again attaching Nathan's patch:
trunk-version-4.patch.

Nathan, for the trunk commit, I suggest you simply merge my patch into
yours.


Grüße,
 Thomas



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Fix-intelmic-libgomp-plugin-build.patch --]
[-- Type: text/x-diff, Size: 7534 bytes --]

From 4e0158f41a00d6c4d09ca8a48eb63832abdd2f84 Mon Sep 17 00:00:00 2001
From: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date: Mon, 3 Aug 2015 11:14:24 +0000
Subject: [PATCH] Fix intelmic libgomp plugin build

... which got broken in r226469:

    [...]/source-gcc/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp: In function 'unsigned int GOMP_OFFLOAD_version()':
    [...]/source-gcc/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp:336:10: error: 'GOMP_VERSION' was not declared in this scope
       return GOMP_VERSION;
              ^
    [...]/source-gcc/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp: In function 'int GOMP_OFFLOAD_load_image(int, unsigned int, void*, addr_pair**)':
    [...]/source-gcc/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp:345:32: error: 'GOMP_VERSION_DEV' was not declared in this scope
       if (GOMP_VERSION_DEV (version) > GOMP_VERSION_INTEL_MIC)
                                    ^
    [...]/source-gcc/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp:345:36: error: 'GOMP_VERSION_INTEL_MIC' was not declared in this scope
       if (GOMP_VERSION_DEV (version) > GOMP_VERSION_INTEL_MIC)
                                        ^
    [...]/source-gcc/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp: In function 'void GOMP_OFFLOAD_unload_image(int, unsigned int, const void*)':
    [...]/source-gcc/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp:373:32: error: 'GOMP_VERSION_DEV' was not declared in this scope
       if (GOMP_VERSION_DEV (version) > GOMP_VERSION_INTEL_MIC)
                                    ^
    [...]/source-gcc/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp:373:36: error: 'GOMP_VERSION_INTEL_MIC' was not declared in this scope
       if (GOMP_VERSION_DEV (version) > GOMP_VERSION_INTEL_MIC)
                                        ^
    make[6]: *** [libgomp_plugin_intelmic_la-libgomp-plugin-intelmic.lo] Error 1

	liboffloadmic/
	* plugin/Makefile.am (include_src_dir): Set.
	[PLUGIN_HOST] (libgomp_plugin_intelmic_la_CPPFLAGS): Use it.
	* plugin/Makefile.in: Regenerate.
	* plugin/libgomp-plugin-intelmic.cpp: Include "gomp-constants.h".

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@226497 138bc75d-0d04-0410-961f-82ee72b054a4
---
 liboffloadmic/ChangeLog.gomp                     | 7 +++++++
 liboffloadmic/plugin/Makefile.am                 | 3 ++-
 liboffloadmic/plugin/Makefile.in                 | 3 ++-
 liboffloadmic/plugin/libgomp-plugin-intelmic.cpp | 3 ++-
 4 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/liboffloadmic/ChangeLog.gomp b/liboffloadmic/ChangeLog.gomp
index 93d1e02..adb9e05 100644
--- a/liboffloadmic/ChangeLog.gomp
+++ b/liboffloadmic/ChangeLog.gomp
@@ -1,3 +1,10 @@
+2015-08-03  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* plugin/Makefile.am (include_src_dir): Set.
+	[PLUGIN_HOST] (libgomp_plugin_intelmic_la_CPPFLAGS): Use it.
+	* plugin/Makefile.in: Regenerate.
+	* plugin/libgomp-plugin-intelmic.cpp: Include "gomp-constants.h".
+
 2015-08-01  Nathan Sidwell  <nathan@codesourcery.com>
 
 	* plugin/libgomp-plugin-intelmic.cpp (GOMP_OFFLOAD_version): New.
diff --git a/liboffloadmic/plugin/Makefile.am b/liboffloadmic/plugin/Makefile.am
index 19d69ab..6ec444c 100644
--- a/liboffloadmic/plugin/Makefile.am
+++ b/liboffloadmic/plugin/Makefile.am
@@ -36,6 +36,7 @@ build_dir = $(top_builddir)
 source_dir = $(top_srcdir)
 coi_inc_dir = $(top_srcdir)/../include/coi
 myo_inc_dir = $(top_srcdir)/../include/myo
+include_src_dir = $(top_srcdir)/../../include
 libgomp_src_dir = $(top_srcdir)/../../libgomp
 libgomp_dir = $(build_dir)/../../libgomp
 liboffload_src_dir = $(top_srcdir)/../runtime
@@ -52,7 +53,7 @@ target_install_dir = $(accel_search_dir)/lib/gcc/$(accel_target)/$(gcc_version)$
 if PLUGIN_HOST
   toolexeclib_LTLIBRARIES = libgomp-plugin-intelmic.la
   libgomp_plugin_intelmic_la_SOURCES = libgomp-plugin-intelmic.cpp
-  libgomp_plugin_intelmic_la_CPPFLAGS = $(CPPFLAGS) -DLINUX -DCOI_LIBRARY_VERSION=2 -DMYO_SUPPORT -DOFFLOAD_DEBUG=1 -DSEP_SUPPORT -DTIMING_SUPPORT -DHOST_LIBRARY=1 -I$(coi_inc_dir) -I$(myo_inc_dir) -I$(liboffload_src_dir) -I$(libgomp_src_dir) -I$(libgomp_dir) -I$(target_prefix_dir)/include -I$(target_build_dir) -I$(target_install_dir)/include
+  libgomp_plugin_intelmic_la_CPPFLAGS = $(CPPFLAGS) -DLINUX -DCOI_LIBRARY_VERSION=2 -DMYO_SUPPORT -DOFFLOAD_DEBUG=1 -DSEP_SUPPORT -DTIMING_SUPPORT -DHOST_LIBRARY=1 -I$(coi_inc_dir) -I$(myo_inc_dir) -I$(liboffload_src_dir) -I$(libgomp_src_dir) -I$(libgomp_dir) -I$(include_src_dir) -I$(target_prefix_dir)/include -I$(target_build_dir) -I$(target_install_dir)/include
   libgomp_plugin_intelmic_la_LDFLAGS = -L$(liboffload_dir)/.libs -loffloadmic_host -version-info 1:0:0
 else # PLUGIN_TARGET
   plugin_includedir = $(libsubincludedir)
diff --git a/liboffloadmic/plugin/Makefile.in b/liboffloadmic/plugin/Makefile.in
index 19a1a96..458c9b2 100644
--- a/liboffloadmic/plugin/Makefile.in
+++ b/liboffloadmic/plugin/Makefile.in
@@ -306,6 +306,7 @@ build_dir = $(top_builddir)
 source_dir = $(top_srcdir)
 coi_inc_dir = $(top_srcdir)/../include/coi
 myo_inc_dir = $(top_srcdir)/../include/myo
+include_src_dir = $(top_srcdir)/../../include
 libgomp_src_dir = $(top_srcdir)/../../libgomp
 libgomp_dir = $(build_dir)/../../libgomp
 liboffload_src_dir = $(top_srcdir)/../runtime
@@ -320,7 +321,7 @@ target_build_dir = $(accel_search_dir)/$(accel_target)$(MULTISUBDIR)/liboffloadm
 target_install_dir = $(accel_search_dir)/lib/gcc/$(accel_target)/$(gcc_version)$(MULTISUBDIR)
 @PLUGIN_HOST_TRUE@toolexeclib_LTLIBRARIES = libgomp-plugin-intelmic.la
 @PLUGIN_HOST_TRUE@libgomp_plugin_intelmic_la_SOURCES = libgomp-plugin-intelmic.cpp
-@PLUGIN_HOST_TRUE@libgomp_plugin_intelmic_la_CPPFLAGS = $(CPPFLAGS) -DLINUX -DCOI_LIBRARY_VERSION=2 -DMYO_SUPPORT -DOFFLOAD_DEBUG=1 -DSEP_SUPPORT -DTIMING_SUPPORT -DHOST_LIBRARY=1 -I$(coi_inc_dir) -I$(myo_inc_dir) -I$(liboffload_src_dir) -I$(libgomp_src_dir) -I$(libgomp_dir) -I$(target_prefix_dir)/include -I$(target_build_dir) -I$(target_install_dir)/include
+@PLUGIN_HOST_TRUE@libgomp_plugin_intelmic_la_CPPFLAGS = $(CPPFLAGS) -DLINUX -DCOI_LIBRARY_VERSION=2 -DMYO_SUPPORT -DOFFLOAD_DEBUG=1 -DSEP_SUPPORT -DTIMING_SUPPORT -DHOST_LIBRARY=1 -I$(coi_inc_dir) -I$(myo_inc_dir) -I$(liboffload_src_dir) -I$(libgomp_src_dir) -I$(libgomp_dir) -I$(include_src_dir) -I$(target_prefix_dir)/include -I$(target_build_dir) -I$(target_install_dir)/include
 @PLUGIN_HOST_TRUE@libgomp_plugin_intelmic_la_LDFLAGS = -L$(liboffload_dir)/.libs -loffloadmic_host -version-info 1:0:0
 @PLUGIN_HOST_FALSE@plugin_includedir = $(libsubincludedir)
 @PLUGIN_HOST_FALSE@plugin_include_HEADERS = main_target_image.h
diff --git a/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp b/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
index b852b42..fde7d9e 100644
--- a/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
+++ b/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
@@ -1,6 +1,6 @@
 /* Plugin for offload execution on Intel MIC devices.
 
-   Copyright (C) 2014 Free Software Foundation, Inc.
+   Copyright (C) 2014-2015 Free Software Foundation, Inc.
 
    Contributed by Ilya Verbin <ilya.verbin@intel.com>.
 
@@ -38,6 +38,7 @@
 #include "libgomp-plugin.h"
 #include "compiler_if_host.h"
 #include "main_target_image.h"
+#include "gomp-constants.h"
 
 #define LD_LIBRARY_PATH_ENV	"LD_LIBRARY_PATH"
 #define MIC_LD_LIBRARY_PATH_ENV	"MIC_LD_LIBRARY_PATH"
-- 
2.1.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: trunk-version-4.patch --]
[-- Type: text/x-diff, Size: 23163 bytes --]

2015-08-01  Nathan Sidwell  <nathan@codesourcery.com>

	gcc/
	* config/nvptx/mkoffload.c (process): Replace
	GOMP_offload_{,un}register with GOMP_offload_{,un}register_ver.

	libgomp/
	* libgomp.map: Add 4.0.2 version.
	* target.c (offload_image_descr): Add version field.
	(gomp_load_image_to_device): Add version argument.  Adjust plugin
	call.  Improve load mismatch diagnostic.
	(gomp_unload_image_from_device): Add version argument.  Adjust plugin
	call.
	(GOMP_offload_regster): Make stub function, move bulk to ...
	(GOMP_offload_register_ver): ... here.  Process version argument.
	(GOMP_offload_unregister): Make stub function, move bulk to ...
	(GOMP_offload_unregister_ver): ... here.  Process version argument.
	(gomp_init_device): Process version field.
	(gomp_unload_device): Process version field.
	(gomp_load_plugin_for_device): Reimplement DLSYM & DLSYM_OPT
	macros.  Check plugin version.
	* libgomp.h (gomp_device_descr): Add version function field.  Adjust
	loader and unloader types.
	* oacc-host.c (host_dispatch): Adjust.
	* plugin/plugin-nvptx.c: Include gomp-constants.h.
	(GOMP_OFFLOAD_version): New.
	(GOMP_OFFLOAD_load_image): Add version arg and check it.
	(GOMP_OFFLOAD_unload_image): Likewise.
	* plugin/plugin-host.c: Include gomp-constants.h.
	(GOMP_OFFLOAD_version): New.
	(GOMP_OFFLOAD_load_image): Add version arg.
	(GOMP_OFFLOAD_unload_image): Likewise.
	* oacc-host.c (host_dispatch): Init version field.

	liboffloadmic/
	* plugin/libgomp-plugin-intelmic.cpp (GOMP_OFFLOAD_version): New.
	(GOMP_OFFLOAD_load_image): Add version arg and check it.
	(GOMP_OFFLOAD_unload_image): Likewise.

	include/
	* gomp-constants.h (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX,
	GOMP_VERSION_INTEL_MIC): New.
	(GOMP_VERSION_PACK, GOMP_VERSION_LIB, GOMP_VERSION_DEV): New.

Index: gcc/config/nvptx/mkoffload.c
===================================================================
--- gcc/config/nvptx/mkoffload.c	(revision 226462)
+++ gcc/config/nvptx/mkoffload.c	(working copy)
@@ -881,10 +881,10 @@ process (FILE *in, FILE *out)
 	   "extern \"C\" {\n"
 	   "#endif\n");
 
-  fprintf (out, "extern void GOMP_offload_register"
-	   " (const void *, int, const void *);\n");
-  fprintf (out, "extern void GOMP_offload_unregister"
-	   " (const void *, int, const void *);\n");
+  fprintf (out, "extern void GOMP_offload_register_ver"
+	   " (unsigned, const void *, int, const void *);\n");
+  fprintf (out, "extern void GOMP_offload_unregister_ver"
+	   " (unsigned, const void *, int, const void *);\n");
 
   fprintf (out, "#ifdef __cplusplus\n"
 	   "}\n"
@@ -894,15 +894,19 @@ process (FILE *in, FILE *out)
 
   fprintf (out, "static __attribute__((constructor)) void init (void)\n"
 	   "{\n"
-	   "  GOMP_offload_register (__OFFLOAD_TABLE__, %d/*NVIDIA_PTX*/,\n"
-	   "                         &target_data);\n"
-	   "};\n", GOMP_DEVICE_NVIDIA_PTX);
+	   "  GOMP_offload_register_ver (%#x, __OFFLOAD_TABLE__,"
+	   "%d/*NVIDIA_PTX*/, &target_data);\n"
+	   "};\n",
+	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
+	   GOMP_DEVICE_NVIDIA_PTX);
 
   fprintf (out, "static __attribute__((destructor)) void fini (void)\n"
 	   "{\n"
-	   "  GOMP_offload_unregister (__OFFLOAD_TABLE__, %d/*NVIDIA_PTX*/,\n"
-	   "                           &target_data);\n"
-	   "};\n", GOMP_DEVICE_NVIDIA_PTX);
+	   "  GOMP_offload_unregister_ver (%#x, __OFFLOAD_TABLE__,"
+	   "%d/*NVIDIA_PTX*/, &target_data);\n"
+	   "};\n",
+	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
+	   GOMP_DEVICE_NVIDIA_PTX);
 }
 
 static void
Index: libgomp/libgomp.map
===================================================================
--- libgomp/libgomp.map	(revision 226462)
+++ libgomp/libgomp.map	(working copy)
@@ -234,6 +234,12 @@ GOMP_4.0.1 {
 	GOMP_offload_unregister;
 } GOMP_4.0;
 
+GOMP_4.0.2 {
+  global:
+	GOMP_offload_register_ver;
+	GOMP_offload_unregister_ver;
+} GOMP_4.0.1;
+
 OACC_2.0 {
   global:
 	acc_get_num_devices;
Index: libgomp/target.c
===================================================================
--- libgomp/target.c	(revision 226462)
+++ libgomp/target.c	(working copy)
@@ -56,6 +56,7 @@ static gomp_mutex_t register_lock;
    It contains type of the target device, pointer to host table descriptor, and
    pointer to target data.  */
 struct offload_image_descr {
+  unsigned version;
   enum offload_target_type type;
   const void *host_table;
   const void *target_data;
@@ -642,7 +643,7 @@ gomp_update (struct gomp_device_descr *d
    emitting variable and functions in the same order.  */
 
 static void
-gomp_load_image_to_device (struct gomp_device_descr *devicep,
+gomp_load_image_to_device (struct gomp_device_descr *devicep, unsigned version,
 			   const void *host_table, const void *target_data,
 			   bool is_register_lock)
 {
@@ -658,16 +659,20 @@ gomp_load_image_to_device (struct gomp_d
 
   /* Load image to device and get target addresses for the image.  */
   struct addr_pair *target_table = NULL;
-  int i, num_target_entries
-    = devicep->load_image_func (devicep->target_id, target_data,
-				&target_table);
+  int i, num_target_entries;
+
+  num_target_entries
+    = devicep->load_image_func (devicep->target_id, version,
+				target_data, &target_table);
 
   if (num_target_entries != num_funcs + num_vars)
     {
       gomp_mutex_unlock (&devicep->lock);
       if (is_register_lock)
 	gomp_mutex_unlock (&register_lock);
-      gomp_fatal ("Can't map target functions or variables");
+      gomp_fatal ("Cannot map target functions or variables"
+		  " (expected %u, have %u)", num_funcs + num_vars,
+		  num_target_entries);
     }
 
   /* Insert host-target address mapping into splay tree.  */
@@ -732,6 +737,7 @@ gomp_load_image_to_device (struct gomp_d
 
 static void
 gomp_unload_image_from_device (struct gomp_device_descr *devicep,
+			       unsigned version,
 			       const void *host_table, const void *target_data)
 {
   void **host_func_table = ((void ***) host_table)[0];
@@ -756,8 +762,8 @@ gomp_unload_image_from_device (struct go
       k.host_end = k.host_start + 1;
       node = splay_tree_lookup (&devicep->mem_map, &k);
     }
-  
-  devicep->unload_image_func (devicep->target_id, target_data);
+
+  devicep->unload_image_func (devicep->target_id, version, target_data);
 
   /* Remove mappings from splay tree.  */
   for (j = 0; j < num_funcs; j++)
@@ -786,10 +792,15 @@ gomp_unload_image_from_device (struct go
    the target, and TARGET_DATA needed by target plugin.  */
 
 void
-GOMP_offload_register (const void *host_table, int target_type,
-		       const void *target_data)
+GOMP_offload_register_ver (unsigned version, const void *host_table,
+			   int target_type, const void *target_data)
 {
   int i;
+
+  if (GOMP_VERSION_LIB (version) > GOMP_VERSION)
+    gomp_fatal ("Library too old for offload (version %u < %u)",
+		GOMP_VERSION, GOMP_VERSION_LIB (version));
+  
   gomp_mutex_lock (&register_lock);
 
   /* Load image to all initialized devices.  */
@@ -798,7 +809,8 @@ GOMP_offload_register (const void *host_
       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, host_table, target_data, true);
+	gomp_load_image_to_device (devicep, version,
+				   host_table, target_data, true);
       gomp_mutex_unlock (&devicep->lock);
     }
 
@@ -807,6 +819,7 @@ GOMP_offload_register (const void *host_
     = gomp_realloc_unlock (offload_images,
 			   (num_offload_images + 1)
 			   * sizeof (struct offload_image_descr));
+  offload_images[num_offload_images].version = version;
   offload_images[num_offload_images].type = target_type;
   offload_images[num_offload_images].host_table = host_table;
   offload_images[num_offload_images].target_data = target_data;
@@ -815,13 +828,20 @@ GOMP_offload_register (const void *host_
   gomp_mutex_unlock (&register_lock);
 }
 
+void
+GOMP_offload_register (const void *host_table, int target_type,
+		       const void *target_data)
+{
+  GOMP_offload_register_ver (0, host_table, target_type, target_data);
+}
+
 /* This function should be called from every offload image while unloading.
    It gets the descriptor of the host func and var tables HOST_TABLE, TYPE of
    the target, and TARGET_DATA needed by target plugin.  */
 
 void
-GOMP_offload_unregister (const void *host_table, int target_type,
-			 const void *target_data)
+GOMP_offload_unregister_ver (unsigned version, const void *host_table,
+			     int target_type, const void *target_data)
 {
   int i;
 
@@ -833,7 +853,8 @@ GOMP_offload_unregister (const void *hos
       struct gomp_device_descr *devicep = &devices[i];
       gomp_mutex_lock (&devicep->lock);
       if (devicep->type == target_type && devicep->is_initialized)
-	gomp_unload_image_from_device (devicep, host_table, target_data);
+	gomp_unload_image_from_device (devicep, version,
+				       host_table, target_data);
       gomp_mutex_unlock (&devicep->lock);
     }
 
@@ -848,6 +869,13 @@ GOMP_offload_unregister (const void *hos
   gomp_mutex_unlock (&register_lock);
 }
 
+void
+GOMP_offload_unregister (const void *host_table, int target_type,
+			 const void *target_data)
+{
+  GOMP_offload_unregister_ver (0, host_table, target_type, target_data);
+}
+
 /* This function initializes the target device, specified by DEVICEP.  DEVICEP
    must be locked on entry, and remains locked on return.  */
 
@@ -862,8 +890,9 @@ gomp_init_device (struct gomp_device_des
     {
       struct offload_image_descr *image = &offload_images[i];
       if (image->type == devicep->type)
-	gomp_load_image_to_device (devicep, image->host_table,
-				   image->target_data, false);
+	gomp_load_image_to_device (devicep, image->version,
+				   image->host_table, image->target_data,
+				   false);
     }
 
   devicep->is_initialized = true;
@@ -881,7 +910,8 @@ gomp_unload_device (struct gomp_device_d
 	{
 	  struct offload_image_descr *image = &offload_images[i];
 	  if (image->type == devicep->type)
-	    gomp_unload_image_from_device (devicep, image->host_table,
+	    gomp_unload_image_from_device (devicep, image->version,
+					   image->host_table,
 					   image->target_data);
 	}
     }
@@ -1085,43 +1115,29 @@ gomp_load_plugin_for_device (struct gomp
 			     const char *plugin_name)
 {
   const char *err = NULL, *last_missing = NULL;
-  int optional_present, optional_total;
-
-  /* Clear any existing error.  */
-  dlerror ();
 
   void *plugin_handle = dlopen (plugin_name, RTLD_LAZY);
   if (!plugin_handle)
-    {
-      err = dlerror ();
-      goto out;
-    }
+    goto dl_fail;
 
   /* Check if all required functions are available in the plugin and store
-     their handlers.  */
+     their handlers.  None of the symbols can legitimately be NULL,
+     so we don't need to check dlerror all the time.  */
 #define DLSYM(f)							\
-  do									\
-    {									\
-      device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #f);	\
-      err = dlerror ();							\
-      if (err != NULL)							\
-	goto out;							\
-    }									\
-  while (0)
-  /* Similar, but missing functions are not an error.  */
-#define DLSYM_OPT(f, n)						\
-  do									\
-    {									\
-      const char *tmp_err;							\
-      device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #n);	\
-      tmp_err = dlerror ();						\
-      if (tmp_err == NULL)						\
-        optional_present++;						\
-      else								\
-        last_missing = #n;						\
-      optional_total++;							\
-    }									\
-  while (0)
+  if (!(device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #f)))	\
+    goto dl_fail
+  /* Similar, but missing functions are not an error.  Return false if
+     failed, true otherwise.  */
+#define DLSYM_OPT(f, n)							\
+  ((device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #n))	\
+   || (last_missing = #n, 0))
+
+  DLSYM (version);
+  if (device->version_func () != GOMP_VERSION)
+    {
+      err = "plugin version mismatch";
+      goto fail;
+    }
 
   DLSYM (get_name);
   DLSYM (get_caps);
@@ -1140,53 +1156,57 @@ gomp_load_plugin_for_device (struct gomp
     DLSYM (run);
   if (device->capabilities & GOMP_OFFLOAD_CAP_OPENACC_200)
     {
-      optional_present = optional_total = 0;
-      DLSYM_OPT (openacc.exec, openacc_parallel);
-      DLSYM_OPT (openacc.register_async_cleanup,
-		 openacc_register_async_cleanup);
-      DLSYM_OPT (openacc.async_test, openacc_async_test);
-      DLSYM_OPT (openacc.async_test_all, openacc_async_test_all);
-      DLSYM_OPT (openacc.async_wait, openacc_async_wait);
-      DLSYM_OPT (openacc.async_wait_async, openacc_async_wait_async);
-      DLSYM_OPT (openacc.async_wait_all, openacc_async_wait_all);
-      DLSYM_OPT (openacc.async_wait_all_async, openacc_async_wait_all_async);
-      DLSYM_OPT (openacc.async_set_async, openacc_async_set_async);
-      DLSYM_OPT (openacc.create_thread_data, openacc_create_thread_data);
-      DLSYM_OPT (openacc.destroy_thread_data, openacc_destroy_thread_data);
-      /* Require all the OpenACC handlers if we have
-	 GOMP_OFFLOAD_CAP_OPENACC_200.  */
-      if (optional_present != optional_total)
+      if (!DLSYM_OPT (openacc.exec, openacc_parallel)
+	  || !DLSYM_OPT (openacc.register_async_cleanup,
+			 openacc_register_async_cleanup)
+	  || !DLSYM_OPT (openacc.async_test, openacc_async_test)
+	  || !DLSYM_OPT (openacc.async_test_all, openacc_async_test_all)
+	  || !DLSYM_OPT (openacc.async_wait, openacc_async_wait)
+	  || !DLSYM_OPT (openacc.async_wait_async, openacc_async_wait_async)
+	  || !DLSYM_OPT (openacc.async_wait_all, openacc_async_wait_all)
+	  || !DLSYM_OPT (openacc.async_wait_all_async,
+			 openacc_async_wait_all_async)
+	  || !DLSYM_OPT (openacc.async_set_async, openacc_async_set_async)
+	  || !DLSYM_OPT (openacc.create_thread_data,
+			 openacc_create_thread_data)
+	  || !DLSYM_OPT (openacc.destroy_thread_data,
+			 openacc_destroy_thread_data))
 	{
+	  /* Require all the OpenACC handlers if we have
+	     GOMP_OFFLOAD_CAP_OPENACC_200.  */
 	  err = "plugin missing OpenACC handler function";
-	  goto out;
+	  goto fail;
 	}
-      optional_present = optional_total = 0;
-      DLSYM_OPT (openacc.cuda.get_current_device,
-		 openacc_get_current_cuda_device);
-      DLSYM_OPT (openacc.cuda.get_current_context,
-		 openacc_get_current_cuda_context);
-      DLSYM_OPT (openacc.cuda.get_stream, openacc_get_cuda_stream);
-      DLSYM_OPT (openacc.cuda.set_stream, openacc_set_cuda_stream);
-      /* Make sure all the CUDA functions are there if any of them are.  */
-      if (optional_present && optional_present != optional_total)
+
+      unsigned cuda = 0;
+      cuda += DLSYM_OPT (openacc.cuda.get_current_device,
+			 openacc_get_current_cuda_device);
+      cuda += DLSYM_OPT (openacc.cuda.get_current_context,
+			 openacc_get_current_cuda_context);
+      cuda += DLSYM_OPT (openacc.cuda.get_stream, openacc_get_cuda_stream);
+      cuda += DLSYM_OPT (openacc.cuda.set_stream, openacc_set_cuda_stream);
+      if (cuda && cuda != 4)
 	{
+	  /* Make sure all the CUDA functions are there if any of them are.  */
 	  err = "plugin missing OpenACC CUDA handler function";
-	  goto out;
+	  goto fail;
 	}
     }
 #undef DLSYM
 #undef DLSYM_OPT
 
- out:
-  if (err != NULL)
-    {
-      gomp_error ("while loading %s: %s", plugin_name, err);
-      if (last_missing)
-        gomp_error ("missing function was %s", last_missing);
-      if (plugin_handle)
-	dlclose (plugin_handle);
-    }
-  return err == NULL;
+  return 1;
+
+ dl_fail:
+  err = dlerror ();
+ fail:
+  gomp_error ("while loading %s: %s", plugin_name, err);
+  if (last_missing)
+    gomp_error ("missing function was %s", last_missing);
+  if (plugin_handle)
+    dlclose (plugin_handle);
+
+  return 0;
 }
 
 /* This function initializes the runtime needed for offloading.
Index: libgomp/libgomp.h
===================================================================
--- libgomp/libgomp.h	(revision 226462)
+++ libgomp/libgomp.h	(working copy)
@@ -748,8 +748,9 @@ struct gomp_device_descr
   int (*get_num_devices_func) (void);
   void (*init_device_func) (int);
   void (*fini_device_func) (int);
-  int (*load_image_func) (int, const void *, struct addr_pair **);
-  void (*unload_image_func) (int, const void *);
+  unsigned (*version_func) (void);
+  int (*load_image_func) (int, unsigned, const void *, struct addr_pair **);
+  void (*unload_image_func) (int, unsigned, const void *);
   void *(*alloc_func) (int, size_t);
   void (*free_func) (int, void *);
   void *(*dev2host_func) (int, void *, const void *, size_t);
Index: libgomp/plugin/plugin-nvptx.c
===================================================================
--- libgomp/plugin/plugin-nvptx.c	(revision 226462)
+++ libgomp/plugin/plugin-nvptx.c	(working copy)
@@ -36,6 +36,7 @@
 #include "libgomp-plugin.h"
 #include "oacc-ptx.h"
 #include "oacc-plugin.h"
+#include "gomp-constants.h"
 
 #include <pthread.h>
 #include <cuda.h>
@@ -1644,11 +1645,20 @@ typedef struct nvptx_tdata
   size_t fn_num;
 } nvptx_tdata_t;
 
+/* Return the libgomp version number we're compatible with.  There is
+   no requirement for cross-version compatibility.  */
+
+unsigned
+GOMP_OFFLOAD_version (void)
+{
+  return GOMP_VERSION;
+}
+
 /* Load the (partial) program described by TARGET_DATA to device
    number ORD.  Allocate and return TARGET_TABLE.  */
 
 int
-GOMP_OFFLOAD_load_image (int ord, const void *target_data,
+GOMP_OFFLOAD_load_image (int ord, unsigned version, const void *target_data,
 			 struct addr_pair **target_table)
 {
   CUmodule module;
@@ -1661,6 +1671,11 @@ GOMP_OFFLOAD_load_image (int ord, const
   struct ptx_image_data *new_image;
   struct ptx_device *dev;
 
+  if (GOMP_VERSION_DEV (version) > GOMP_VERSION_NVIDIA_PTX)
+    GOMP_PLUGIN_fatal ("Offload data incompatible with PTX plugin"
+		       " (expected %u, received %u)",
+		       GOMP_VERSION_NVIDIA_PTX, GOMP_VERSION_DEV (version));
+  
   GOMP_OFFLOAD_init_device (ord);
 
   dev = ptx_devices[ord];
@@ -1730,11 +1745,14 @@ GOMP_OFFLOAD_load_image (int ord, const
    function descriptors allocated by G_O_load_image.  */
 
 void
-GOMP_OFFLOAD_unload_image (int ord, const void *target_data)
+GOMP_OFFLOAD_unload_image (int ord, unsigned version, const void *target_data)
 {
   struct ptx_image_data *image, **prev_p;
   struct ptx_device *dev = ptx_devices[ord];
 
+  if (GOMP_VERSION_DEV (version) > GOMP_VERSION_NVIDIA_PTX)
+    return;
+  
   pthread_mutex_lock (&dev->image_lock);
   for (prev_p = &dev->images; (image = *prev_p) != 0; prev_p = &image->next)
     if (image->target_data == target_data)
Index: libgomp/plugin/plugin-host.c
===================================================================
--- libgomp/plugin/plugin-host.c	(revision 226462)
+++ libgomp/plugin/plugin-host.c	(working copy)
@@ -39,6 +39,7 @@
 #include "libgomp.h"
 #include "oacc-int.h"
 #endif
+#include "gomp-constants.h"
 
 #include <stdint.h>
 #include <stdlib.h>
@@ -109,8 +110,15 @@ GOMP_OFFLOAD_fini_device (int n __attrib
 {
 }
 
+STATIC unsigned
+GOMP_OFFLOAD_version (void)
+{
+  return GOMP_VERSION;
+}
+
 STATIC int
 GOMP_OFFLOAD_load_image (int n __attribute__ ((unused)),
+			 unsigned  v __attribute__ ((unused)),
 			 const void *t __attribute__ ((unused)),
 			 struct addr_pair **r __attribute__ ((unused)))
 {
@@ -119,6 +127,7 @@ GOMP_OFFLOAD_load_image (int n __attribu
 
 STATIC void
 GOMP_OFFLOAD_unload_image (int n __attribute__ ((unused)),
+			   unsigned  v __attribute__ ((unused)),
 			   const void *t __attribute__ ((unused)))
 {
 }
Index: libgomp/oacc-host.c
===================================================================
--- libgomp/oacc-host.c	(revision 226462)
+++ libgomp/oacc-host.c	(working copy)
@@ -45,6 +45,7 @@ static struct gomp_device_descr host_dis
     .get_num_devices_func = GOMP_OFFLOAD_get_num_devices,
     .init_device_func = GOMP_OFFLOAD_init_device,
     .fini_device_func = GOMP_OFFLOAD_fini_device,
+    .version_func = GOMP_OFFLOAD_version,
     .load_image_func = GOMP_OFFLOAD_load_image,
     .unload_image_func = GOMP_OFFLOAD_unload_image,
     .alloc_func = GOMP_OFFLOAD_alloc,
Index: liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
===================================================================
--- liboffloadmic/plugin/libgomp-plugin-intelmic.cpp	(revision 226462)
+++ liboffloadmic/plugin/libgomp-plugin-intelmic.cpp	(working copy)
@@ -327,12 +327,26 @@ offload_image (const void *target_image)
   free (image);
 }
 
+/* Return the libgomp version number we're compatible with.  There is
+   no requirement for cross-version compatibility.  */
+
+extern "C" unsigned
+GOMP_OFFLOAD_version (void)
+{
+  return GOMP_VERSION;
+}
+
 extern "C" int
-GOMP_OFFLOAD_load_image (int device, const void *target_image,
-			 addr_pair **result)
+GOMP_OFFLOAD_load_image (int device, const unsigned version,
+			 void *target_image, addr_pair **result)
 {
   TRACE ("(device = %d, target_image = %p)", device, target_image);
 
+  if (GOMP_VERSION_DEV (version) > GOMP_VERSION_INTEL_MIC)
+    GOMP_PLUGIN_fatal ("Offload data incompatible with intelmic plugin"
+		       " (expected %u, received %u)",
+		       GOMP_VERSION_INTEL_MIC, GOMP_VERSION_DEV (version));
+
   /* If target_image is already present in address_table, then there is no need
      to offload it.  */
   if (address_table->count (target_image) == 0)
@@ -353,8 +367,12 @@ GOMP_OFFLOAD_load_image (int device, con
 }
 
 extern "C" void
-GOMP_OFFLOAD_unload_image (int device, const void *target_image)
+GOMP_OFFLOAD_unload_image (int device, unsigned version,
+			   const void *target_image)
 {
+  if (GOMP_VERSION_DEV (version) > GOMP_VERSION_INTEL_MIC)
+    return;
+
   TRACE ("(device = %d, target_image = %p)", device, target_image);
 
   /* TODO: Currently liboffloadmic doesn't support __offload_unregister_image
Index: include/gomp-constants.h
===================================================================
--- include/gomp-constants.h	(revision 226462)
+++ include/gomp-constants.h	(working copy)
@@ -113,4 +113,13 @@ enum gomp_map_kind
 #define GOMP_DEVICE_ICV			-1
 #define GOMP_DEVICE_HOST_FALLBACK	-2
 
+/* Versions of libgomp and device-specific plugins.  */
+#define GOMP_VERSION	0
+#define GOMP_VERSION_NVIDIA_PTX 0
+#define GOMP_VERSION_INTEL_MIC 0
+
+#define GOMP_VERSION_PACK(LIB, DEV) (((LIB) << 16) | (DEV))
+#define GOMP_VERSION_LIB(PACK) (((PACK) >> 16) & 0xffff)
+#define GOMP_VERSION_DEV(PACK) ((PACK) & 0xffff)
+
 #endif

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-08-04 16:19                   ` Thomas Schwinge
@ 2015-08-04 16:20                     ` Nathan Sidwell
  0 siblings, 0 replies; 22+ messages in thread
From: Nathan Sidwell @ 2015-08-04 16:20 UTC (permalink / raw)
  To: Thomas Schwinge, Jakub Jelinek; +Cc: ilya.verbin, GCC Patches

On 08/04/15 12:17, Thomas Schwinge wrote:
> Hi!
>
> Nathan's patch is waiting for trunk approval:

> Then, for convenience, I'm also again attaching Nathan's patch:
> trunk-version-4.patch.
>
> Nathan, for the trunk commit, I suggest you simply merge my patch into
> yours.

Yes, I'll atomically commit them.

nathan

-- 
Nathan Sidwell

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-08-02  0:06                 ` Nathan Sidwell
  2015-08-02  0:20                   ` Nathan Sidwell
  2015-08-04 16:19                   ` Thomas Schwinge
@ 2015-08-06 19:52                   ` Nathan Sidwell
  2015-08-10 20:00                     ` Thomas Schwinge
  2015-08-24  8:45                     ` Jakub Jelinek
  2 siblings, 2 replies; 22+ messages in thread
From: Nathan Sidwell @ 2015-08-06 19:52 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: ilya.verbin, GCC Patches

Ping?

1) updated version patch https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00010.html

2) https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00204.html
An infrastructure piece from Thomas, who noticed liboffloadmic didn't have the 
include paths to #include  gomp-constants.h.

nathan

On 08/02/15 02:06, Nathan Sidwell wrote:
> On 07/31/15 12:10, Jakub Jelinek wrote:
>
>> This will hopefully be just GOMP_4.1 instead in the end, but it can
>> change when gomp-4_1-branch is merged to trunk, we don't guarantee
>> ABI stability at this point.
>
> Sure.
>
>> I'd prefer version to go after devicep argument rather than before.
> Fixed.
>
>> And really don't have ver_func vs. unver_func, just a single
>> callback that will take the version argument too (again, if possible
>> after target_id).
>
> Fixed (& elsewhere).  The patch should be checked for intelmic if possible
> (Ilya?).  The  changes there are very mechanical so I'm not expecting a problem.
>
> We don't need to  make the initial value of GOMP_VERSION non-zero, because the
> absence of the GOMP_OFFLOAD_version func will distinguish out of date plugins at
> this point.
>
>
>>> +
>>> +  if (DLSYM_OPT (version, version))
>>
>> I'd prefer requiring version always (i.e. DLSYM (version);
>> plus the v != GOMP_VERSION checking).
>
> Fixed.
>
> ok?  I'll commit a version of this to gomp4 branch shortly.
>
> nathan
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-08-06 19:52                   ` Nathan Sidwell
@ 2015-08-10 20:00                     ` Thomas Schwinge
  2015-08-24  8:45                     ` Jakub Jelinek
  1 sibling, 0 replies; 22+ messages in thread
From: Thomas Schwinge @ 2015-08-10 20:00 UTC (permalink / raw)
  To: Jakub Jelinek, GCC Patches; +Cc: ilya.verbin, Nathan Sidwell


[-- Attachment #1.1: Type: text/plain, Size: 570 bytes --]

Hi!

On Thu, 6 Aug 2015 21:52:43 +0200, Nathan Sidwell <nathan@acm.org> wrote:
> Ping?
> 
> 1) updated version patch https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00010.html
> 
> 2) https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00204.html
> An infrastructure piece from Thomas, who noticed liboffloadmic didn't have the 
> include paths to #include  gomp-constants.h.

(I merged these two, and) I'm again attaching the patch
0001-Offload-data-version-number.patch, which is modified to apply
cleanly after today's trunk changes.


Grüße,
 Thomas



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Offload-data-version-number.patch --]
[-- Type: text/x-diff, Size: 28294 bytes --]

From 1c33c5b77abedb429b590f4eead1bfb82745fe27 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Fri, 7 Aug 2015 09:49:50 +0200
Subject: [PATCH] Offload data version number

YYYY-MM-DD  Nathan Sidwell  <nathan@codesourcery.com>

	gcc/
	* config/nvptx/mkoffload.c (process): Replace
	GOMP_offload_{,un}register with GOMP_offload_{,un}register_ver.

	libgomp/
	* libgomp.map: Add 4.0.2 version.
	* target.c (offload_image_descr): Add version field.
	(gomp_load_image_to_device): Add version argument.  Adjust plugin
	call.  Improve load mismatch diagnostic.
	(gomp_unload_image_from_device): Add version argument.  Adjust plugin
	call.
	(GOMP_offload_regster): Make stub function, move bulk to ...
	(GOMP_offload_register_ver): ... here.  Process version argument.
	(GOMP_offload_unregister): Make stub function, move bulk to ...
	(GOMP_offload_unregister_ver): ... here.  Process version argument.
	(gomp_init_device): Process version field.
	(gomp_unload_device): Process version field.
	(gomp_load_plugin_for_device): Reimplement DLSYM & DLSYM_OPT
	macros.  Check plugin version.
	* libgomp.h (gomp_device_descr): Add version function field.  Adjust
	loader and unloader types.
	* oacc-host.c (host_dispatch): Adjust.
	* plugin/plugin-nvptx.c: Include gomp-constants.h.
	(GOMP_OFFLOAD_version): New.
	(GOMP_OFFLOAD_load_image): Add version arg and check it.
	(GOMP_OFFLOAD_unload_image): Likewise.
	* plugin/plugin-host.c: Include gomp-constants.h.
	(GOMP_OFFLOAD_version): New.
	(GOMP_OFFLOAD_load_image): Add version arg.
	(GOMP_OFFLOAD_unload_image): Likewise.
	* oacc-host.c (host_dispatch): Init version field.

	liboffloadmic/
	* plugin/libgomp-plugin-intelmic.cpp (GOMP_OFFLOAD_version): New.
	(GOMP_OFFLOAD_load_image): Add version arg and check it.
	(GOMP_OFFLOAD_unload_image): Likewise.

	include/
	* gomp-constants.h (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX)
	GOMP_VERSION_INTEL_MIC): New.
	(GOMP_VERSION_PACK, GOMP_VERSION_LIB, GOMP_VERSION_DEV): New.

YYYY-MM-DD  Thomas Schwinge  <thomas@codesourcery.com>

	liboffloadmic/
	* plugin/Makefile.am (include_src_dir): Set.
	[PLUGIN_HOST] (libgomp_plugin_intelmic_la_CPPFLAGS): Use it.
	* plugin/Makefile.in: Regenerate.
	* plugin/libgomp-plugin-intelmic.cpp: Include "gomp-constants.h".
---
 gcc/config/nvptx/mkoffload.c                     |   24 +--
 include/gomp-constants.h                         |    9 ++
 libgomp/libgomp.h                                |    5 +-
 libgomp/libgomp.map                              |    6 +
 libgomp/oacc-host.c                              |   10 ++
 libgomp/plugin/plugin-nvptx.c                    |   22 ++-
 libgomp/target.c                                 |  188 ++++++++++++----------
 liboffloadmic/plugin/Makefile.am                 |    3 +-
 liboffloadmic/plugin/Makefile.in                 |    3 +-
 liboffloadmic/plugin/libgomp-plugin-intelmic.cpp |   27 +++-
 10 files changed, 193 insertions(+), 104 deletions(-)

diff --git a/gcc/config/nvptx/mkoffload.c b/gcc/config/nvptx/mkoffload.c
index 1e154c8..ba0454e 100644
--- a/gcc/config/nvptx/mkoffload.c
+++ b/gcc/config/nvptx/mkoffload.c
@@ -881,10 +881,10 @@ process (FILE *in, FILE *out)
 	   "extern \"C\" {\n"
 	   "#endif\n");
 
-  fprintf (out, "extern void GOMP_offload_register"
-	   " (const void *, int, const void *);\n");
-  fprintf (out, "extern void GOMP_offload_unregister"
-	   " (const void *, int, const void *);\n");
+  fprintf (out, "extern void GOMP_offload_register_ver"
+	   " (unsigned, const void *, int, const void *);\n");
+  fprintf (out, "extern void GOMP_offload_unregister_ver"
+	   " (unsigned, const void *, int, const void *);\n");
 
   fprintf (out, "#ifdef __cplusplus\n"
 	   "}\n"
@@ -894,15 +894,19 @@ process (FILE *in, FILE *out)
 
   fprintf (out, "static __attribute__((constructor)) void init (void)\n"
 	   "{\n"
-	   "  GOMP_offload_register (__OFFLOAD_TABLE__, %d/*NVIDIA_PTX*/,\n"
-	   "                         &target_data);\n"
-	   "};\n", GOMP_DEVICE_NVIDIA_PTX);
+	   "  GOMP_offload_register_ver (%#x, __OFFLOAD_TABLE__,"
+	   "%d/*NVIDIA_PTX*/, &target_data);\n"
+	   "};\n",
+	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
+	   GOMP_DEVICE_NVIDIA_PTX);
 
   fprintf (out, "static __attribute__((destructor)) void fini (void)\n"
 	   "{\n"
-	   "  GOMP_offload_unregister (__OFFLOAD_TABLE__, %d/*NVIDIA_PTX*/,\n"
-	   "                           &target_data);\n"
-	   "};\n", GOMP_DEVICE_NVIDIA_PTX);
+	   "  GOMP_offload_unregister_ver (%#x, __OFFLOAD_TABLE__,"
+	   "%d/*NVIDIA_PTX*/, &target_data);\n"
+	   "};\n",
+	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
+	   GOMP_DEVICE_NVIDIA_PTX);
 }
 
 static void
diff --git a/include/gomp-constants.h b/include/gomp-constants.h
index 807e672..6e6a8b1 100644
--- a/include/gomp-constants.h
+++ b/include/gomp-constants.h
@@ -113,4 +113,13 @@ enum gomp_map_kind
 #define GOMP_DEVICE_ICV			-1
 #define GOMP_DEVICE_HOST_FALLBACK	-2
 
+/* Versions of libgomp and device-specific plugins.  */
+#define GOMP_VERSION	0
+#define GOMP_VERSION_NVIDIA_PTX 0
+#define GOMP_VERSION_INTEL_MIC 0
+
+#define GOMP_VERSION_PACK(LIB, DEV) (((LIB) << 16) | (DEV))
+#define GOMP_VERSION_LIB(PACK) (((PACK) >> 16) & 0xffff)
+#define GOMP_VERSION_DEV(PACK) ((PACK) & 0xffff)
+
 #endif
diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index ac40e2a..62454b8 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -748,8 +748,9 @@ struct gomp_device_descr
   int (*get_num_devices_func) (void);
   void (*init_device_func) (int);
   void (*fini_device_func) (int);
-  int (*load_image_func) (int, const void *, struct addr_pair **);
-  void (*unload_image_func) (int, const void *);
+  unsigned (*version_func) (void);
+  int (*load_image_func) (int, unsigned, const void *, struct addr_pair **);
+  void (*unload_image_func) (int, unsigned, const void *);
   void *(*alloc_func) (int, size_t);
   void (*free_func) (int, void *);
   void *(*dev2host_func) (int, void *, const void *, size_t);
diff --git a/libgomp/libgomp.map b/libgomp/libgomp.map
index 2b2b953..ec3c3c1 100644
--- a/libgomp/libgomp.map
+++ b/libgomp/libgomp.map
@@ -234,6 +234,12 @@ GOMP_4.0.1 {
 	GOMP_offload_unregister;
 } GOMP_4.0;
 
+GOMP_4.0.2 {
+  global:
+	GOMP_offload_register_ver;
+	GOMP_offload_unregister_ver;
+} GOMP_4.0.1;
+
 OACC_2.0 {
   global:
 	acc_get_num_devices;
diff --git a/libgomp/oacc-host.c b/libgomp/oacc-host.c
index 17a5102..e49122b 100644
--- a/libgomp/oacc-host.c
+++ b/libgomp/oacc-host.c
@@ -28,6 +28,7 @@
 
 #include "libgomp.h"
 #include "oacc-int.h"
+#include "gomp-constants.h"
 
 #include <stdbool.h>
 #include <stddef.h>
@@ -69,8 +70,15 @@ host_fini_device (int n __attribute__ ((unused)))
 {
 }
 
+static unsigned
+host_version (void)
+{
+  return GOMP_VERSION;
+}
+
 static int
 host_load_image (int n __attribute__ ((unused)),
+		 unsigned  v __attribute__ ((unused)),
 		 const void *t __attribute__ ((unused)),
 		 struct addr_pair **r __attribute__ ((unused)))
 {
@@ -79,6 +87,7 @@ host_load_image (int n __attribute__ ((unused)),
 
 static void
 host_unload_image (int n __attribute__ ((unused)),
+		   unsigned  v __attribute__ ((unused)),
 		   const void *t __attribute__ ((unused)))
 {
 }
@@ -206,6 +215,7 @@ static struct gomp_device_descr host_dispatch =
     .get_num_devices_func = host_get_num_devices,
     .init_device_func = host_init_device,
     .fini_device_func = host_fini_device,
+    .version_func = host_version,
     .load_image_func = host_load_image,
     .unload_image_func = host_unload_image,
     .alloc_func = host_alloc,
diff --git a/libgomp/plugin/plugin-nvptx.c b/libgomp/plugin/plugin-nvptx.c
index fc29632..b969c72 100644
--- a/libgomp/plugin/plugin-nvptx.c
+++ b/libgomp/plugin/plugin-nvptx.c
@@ -36,6 +36,7 @@
 #include "libgomp-plugin.h"
 #include "oacc-ptx.h"
 #include "oacc-plugin.h"
+#include "gomp-constants.h"
 
 #include <pthread.h>
 #include <cuda.h>
@@ -1644,11 +1645,20 @@ typedef struct nvptx_tdata
   size_t fn_num;
 } nvptx_tdata_t;
 
+/* Return the libgomp version number we're compatible with.  There is
+   no requirement for cross-version compatibility.  */
+
+unsigned
+GOMP_OFFLOAD_version (void)
+{
+  return GOMP_VERSION;
+}
+
 /* Load the (partial) program described by TARGET_DATA to device
    number ORD.  Allocate and return TARGET_TABLE.  */
 
 int
-GOMP_OFFLOAD_load_image (int ord, const void *target_data,
+GOMP_OFFLOAD_load_image (int ord, unsigned version, const void *target_data,
 			 struct addr_pair **target_table)
 {
   CUmodule module;
@@ -1661,6 +1671,11 @@ GOMP_OFFLOAD_load_image (int ord, const void *target_data,
   struct ptx_image_data *new_image;
   struct ptx_device *dev;
 
+  if (GOMP_VERSION_DEV (version) > GOMP_VERSION_NVIDIA_PTX)
+    GOMP_PLUGIN_fatal ("Offload data incompatible with PTX plugin"
+		       " (expected %u, received %u)",
+		       GOMP_VERSION_NVIDIA_PTX, GOMP_VERSION_DEV (version));
+  
   GOMP_OFFLOAD_init_device (ord);
 
   dev = ptx_devices[ord];
@@ -1730,11 +1745,14 @@ GOMP_OFFLOAD_load_image (int ord, const void *target_data,
    function descriptors allocated by G_O_load_image.  */
 
 void
-GOMP_OFFLOAD_unload_image (int ord, const void *target_data)
+GOMP_OFFLOAD_unload_image (int ord, unsigned version, const void *target_data)
 {
   struct ptx_image_data *image, **prev_p;
   struct ptx_device *dev = ptx_devices[ord];
 
+  if (GOMP_VERSION_DEV (version) > GOMP_VERSION_NVIDIA_PTX)
+    return;
+  
   pthread_mutex_lock (&dev->image_lock);
   for (prev_p = &dev->images; (image = *prev_p) != 0; prev_p = &image->next)
     if (image->target_data == target_data)
diff --git a/libgomp/target.c b/libgomp/target.c
index 7b3d0f9..758ece5 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -56,6 +56,7 @@ static gomp_mutex_t register_lock;
    It contains type of the target device, pointer to host table descriptor, and
    pointer to target data.  */
 struct offload_image_descr {
+  unsigned version;
   enum offload_target_type type;
   const void *host_table;
   const void *target_data;
@@ -642,7 +643,7 @@ gomp_update (struct gomp_device_descr *devicep, size_t mapnum, void **hostaddrs,
    emitting variable and functions in the same order.  */
 
 static void
-gomp_load_image_to_device (struct gomp_device_descr *devicep,
+gomp_load_image_to_device (struct gomp_device_descr *devicep, unsigned version,
 			   const void *host_table, const void *target_data,
 			   bool is_register_lock)
 {
@@ -658,16 +659,20 @@ gomp_load_image_to_device (struct gomp_device_descr *devicep,
 
   /* Load image to device and get target addresses for the image.  */
   struct addr_pair *target_table = NULL;
-  int i, num_target_entries
-    = devicep->load_image_func (devicep->target_id, target_data,
-				&target_table);
+  int i, num_target_entries;
+
+  num_target_entries
+    = devicep->load_image_func (devicep->target_id, version,
+				target_data, &target_table);
 
   if (num_target_entries != num_funcs + num_vars)
     {
       gomp_mutex_unlock (&devicep->lock);
       if (is_register_lock)
 	gomp_mutex_unlock (&register_lock);
-      gomp_fatal ("Can't map target functions or variables");
+      gomp_fatal ("Cannot map target functions or variables"
+		  " (expected %u, have %u)", num_funcs + num_vars,
+		  num_target_entries);
     }
 
   /* Insert host-target address mapping into splay tree.  */
@@ -732,6 +737,7 @@ gomp_load_image_to_device (struct gomp_device_descr *devicep,
 
 static void
 gomp_unload_image_from_device (struct gomp_device_descr *devicep,
+			       unsigned version,
 			       const void *host_table, const void *target_data)
 {
   void **host_func_table = ((void ***) host_table)[0];
@@ -756,8 +762,8 @@ gomp_unload_image_from_device (struct gomp_device_descr *devicep,
       k.host_end = k.host_start + 1;
       node = splay_tree_lookup (&devicep->mem_map, &k);
     }
-  
-  devicep->unload_image_func (devicep->target_id, target_data);
+
+  devicep->unload_image_func (devicep->target_id, version, target_data);
 
   /* Remove mappings from splay tree.  */
   for (j = 0; j < num_funcs; j++)
@@ -786,10 +792,15 @@ gomp_unload_image_from_device (struct gomp_device_descr *devicep,
    the target, and TARGET_DATA needed by target plugin.  */
 
 void
-GOMP_offload_register (const void *host_table, int target_type,
-		       const void *target_data)
+GOMP_offload_register_ver (unsigned version, const void *host_table,
+			   int target_type, const void *target_data)
 {
   int i;
+
+  if (GOMP_VERSION_LIB (version) > GOMP_VERSION)
+    gomp_fatal ("Library too old for offload (version %u < %u)",
+		GOMP_VERSION, GOMP_VERSION_LIB (version));
+  
   gomp_mutex_lock (&register_lock);
 
   /* Load image to all initialized devices.  */
@@ -798,7 +809,8 @@ GOMP_offload_register (const void *host_table, int target_type,
       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, host_table, target_data, true);
+	gomp_load_image_to_device (devicep, version,
+				   host_table, target_data, true);
       gomp_mutex_unlock (&devicep->lock);
     }
 
@@ -807,6 +819,7 @@ GOMP_offload_register (const void *host_table, int target_type,
     = gomp_realloc_unlock (offload_images,
 			   (num_offload_images + 1)
 			   * sizeof (struct offload_image_descr));
+  offload_images[num_offload_images].version = version;
   offload_images[num_offload_images].type = target_type;
   offload_images[num_offload_images].host_table = host_table;
   offload_images[num_offload_images].target_data = target_data;
@@ -815,13 +828,20 @@ GOMP_offload_register (const void *host_table, int target_type,
   gomp_mutex_unlock (&register_lock);
 }
 
+void
+GOMP_offload_register (const void *host_table, int target_type,
+		       const void *target_data)
+{
+  GOMP_offload_register_ver (0, host_table, target_type, target_data);
+}
+
 /* This function should be called from every offload image while unloading.
    It gets the descriptor of the host func and var tables HOST_TABLE, TYPE of
    the target, and TARGET_DATA needed by target plugin.  */
 
 void
-GOMP_offload_unregister (const void *host_table, int target_type,
-			 const void *target_data)
+GOMP_offload_unregister_ver (unsigned version, const void *host_table,
+			     int target_type, const void *target_data)
 {
   int i;
 
@@ -833,7 +853,8 @@ GOMP_offload_unregister (const void *host_table, int target_type,
       struct gomp_device_descr *devicep = &devices[i];
       gomp_mutex_lock (&devicep->lock);
       if (devicep->type == target_type && devicep->is_initialized)
-	gomp_unload_image_from_device (devicep, host_table, target_data);
+	gomp_unload_image_from_device (devicep, version,
+				       host_table, target_data);
       gomp_mutex_unlock (&devicep->lock);
     }
 
@@ -848,6 +869,13 @@ GOMP_offload_unregister (const void *host_table, int target_type,
   gomp_mutex_unlock (&register_lock);
 }
 
+void
+GOMP_offload_unregister (const void *host_table, int target_type,
+			 const void *target_data)
+{
+  GOMP_offload_unregister_ver (0, host_table, target_type, target_data);
+}
+
 /* This function initializes the target device, specified by DEVICEP.  DEVICEP
    must be locked on entry, and remains locked on return.  */
 
@@ -862,8 +890,9 @@ gomp_init_device (struct gomp_device_descr *devicep)
     {
       struct offload_image_descr *image = &offload_images[i];
       if (image->type == devicep->type)
-	gomp_load_image_to_device (devicep, image->host_table,
-				   image->target_data, false);
+	gomp_load_image_to_device (devicep, image->version,
+				   image->host_table, image->target_data,
+				   false);
     }
 
   devicep->is_initialized = true;
@@ -881,7 +910,8 @@ gomp_unload_device (struct gomp_device_descr *devicep)
 	{
 	  struct offload_image_descr *image = &offload_images[i];
 	  if (image->type == devicep->type)
-	    gomp_unload_image_from_device (devicep, image->host_table,
+	    gomp_unload_image_from_device (devicep, image->version,
+					   image->host_table,
 					   image->target_data);
 	}
     }
@@ -1085,43 +1115,29 @@ gomp_load_plugin_for_device (struct gomp_device_descr *device,
 			     const char *plugin_name)
 {
   const char *err = NULL, *last_missing = NULL;
-  int optional_present, optional_total;
-
-  /* Clear any existing error.  */
-  dlerror ();
 
   void *plugin_handle = dlopen (plugin_name, RTLD_LAZY);
   if (!plugin_handle)
-    {
-      err = dlerror ();
-      goto out;
-    }
+    goto dl_fail;
 
   /* Check if all required functions are available in the plugin and store
-     their handlers.  */
+     their handlers.  None of the symbols can legitimately be NULL,
+     so we don't need to check dlerror all the time.  */
 #define DLSYM(f)							\
-  do									\
-    {									\
-      device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #f);	\
-      err = dlerror ();							\
-      if (err != NULL)							\
-	goto out;							\
-    }									\
-  while (0)
-  /* Similar, but missing functions are not an error.  */
-#define DLSYM_OPT(f, n)						\
-  do									\
-    {									\
-      const char *tmp_err;							\
-      device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #n);	\
-      tmp_err = dlerror ();						\
-      if (tmp_err == NULL)						\
-        optional_present++;						\
-      else								\
-        last_missing = #n;						\
-      optional_total++;							\
-    }									\
-  while (0)
+  if (!(device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #f)))	\
+    goto dl_fail
+  /* Similar, but missing functions are not an error.  Return false if
+     failed, true otherwise.  */
+#define DLSYM_OPT(f, n)							\
+  ((device->f##_func = dlsym (plugin_handle, "GOMP_OFFLOAD_" #n))	\
+   || (last_missing = #n, 0))
+
+  DLSYM (version);
+  if (device->version_func () != GOMP_VERSION)
+    {
+      err = "plugin version mismatch";
+      goto fail;
+    }
 
   DLSYM (get_name);
   DLSYM (get_caps);
@@ -1140,53 +1156,57 @@ gomp_load_plugin_for_device (struct gomp_device_descr *device,
     DLSYM (run);
   if (device->capabilities & GOMP_OFFLOAD_CAP_OPENACC_200)
     {
-      optional_present = optional_total = 0;
-      DLSYM_OPT (openacc.exec, openacc_parallel);
-      DLSYM_OPT (openacc.register_async_cleanup,
-		 openacc_register_async_cleanup);
-      DLSYM_OPT (openacc.async_test, openacc_async_test);
-      DLSYM_OPT (openacc.async_test_all, openacc_async_test_all);
-      DLSYM_OPT (openacc.async_wait, openacc_async_wait);
-      DLSYM_OPT (openacc.async_wait_async, openacc_async_wait_async);
-      DLSYM_OPT (openacc.async_wait_all, openacc_async_wait_all);
-      DLSYM_OPT (openacc.async_wait_all_async, openacc_async_wait_all_async);
-      DLSYM_OPT (openacc.async_set_async, openacc_async_set_async);
-      DLSYM_OPT (openacc.create_thread_data, openacc_create_thread_data);
-      DLSYM_OPT (openacc.destroy_thread_data, openacc_destroy_thread_data);
-      /* Require all the OpenACC handlers if we have
-	 GOMP_OFFLOAD_CAP_OPENACC_200.  */
-      if (optional_present != optional_total)
+      if (!DLSYM_OPT (openacc.exec, openacc_parallel)
+	  || !DLSYM_OPT (openacc.register_async_cleanup,
+			 openacc_register_async_cleanup)
+	  || !DLSYM_OPT (openacc.async_test, openacc_async_test)
+	  || !DLSYM_OPT (openacc.async_test_all, openacc_async_test_all)
+	  || !DLSYM_OPT (openacc.async_wait, openacc_async_wait)
+	  || !DLSYM_OPT (openacc.async_wait_async, openacc_async_wait_async)
+	  || !DLSYM_OPT (openacc.async_wait_all, openacc_async_wait_all)
+	  || !DLSYM_OPT (openacc.async_wait_all_async,
+			 openacc_async_wait_all_async)
+	  || !DLSYM_OPT (openacc.async_set_async, openacc_async_set_async)
+	  || !DLSYM_OPT (openacc.create_thread_data,
+			 openacc_create_thread_data)
+	  || !DLSYM_OPT (openacc.destroy_thread_data,
+			 openacc_destroy_thread_data))
 	{
+	  /* Require all the OpenACC handlers if we have
+	     GOMP_OFFLOAD_CAP_OPENACC_200.  */
 	  err = "plugin missing OpenACC handler function";
-	  goto out;
+	  goto fail;
 	}
-      optional_present = optional_total = 0;
-      DLSYM_OPT (openacc.cuda.get_current_device,
-		 openacc_get_current_cuda_device);
-      DLSYM_OPT (openacc.cuda.get_current_context,
-		 openacc_get_current_cuda_context);
-      DLSYM_OPT (openacc.cuda.get_stream, openacc_get_cuda_stream);
-      DLSYM_OPT (openacc.cuda.set_stream, openacc_set_cuda_stream);
-      /* Make sure all the CUDA functions are there if any of them are.  */
-      if (optional_present && optional_present != optional_total)
+
+      unsigned cuda = 0;
+      cuda += DLSYM_OPT (openacc.cuda.get_current_device,
+			 openacc_get_current_cuda_device);
+      cuda += DLSYM_OPT (openacc.cuda.get_current_context,
+			 openacc_get_current_cuda_context);
+      cuda += DLSYM_OPT (openacc.cuda.get_stream, openacc_get_cuda_stream);
+      cuda += DLSYM_OPT (openacc.cuda.set_stream, openacc_set_cuda_stream);
+      if (cuda && cuda != 4)
 	{
+	  /* Make sure all the CUDA functions are there if any of them are.  */
 	  err = "plugin missing OpenACC CUDA handler function";
-	  goto out;
+	  goto fail;
 	}
     }
 #undef DLSYM
 #undef DLSYM_OPT
 
- out:
-  if (err != NULL)
-    {
-      gomp_error ("while loading %s: %s", plugin_name, err);
-      if (last_missing)
-        gomp_error ("missing function was %s", last_missing);
-      if (plugin_handle)
-	dlclose (plugin_handle);
-    }
-  return err == NULL;
+  return 1;
+
+ dl_fail:
+  err = dlerror ();
+ fail:
+  gomp_error ("while loading %s: %s", plugin_name, err);
+  if (last_missing)
+    gomp_error ("missing function was %s", last_missing);
+  if (plugin_handle)
+    dlclose (plugin_handle);
+
+  return 0;
 }
 
 /* This function initializes the runtime needed for offloading.
diff --git a/liboffloadmic/plugin/Makefile.am b/liboffloadmic/plugin/Makefile.am
index 19d69ab..6ec444c 100644
--- a/liboffloadmic/plugin/Makefile.am
+++ b/liboffloadmic/plugin/Makefile.am
@@ -36,6 +36,7 @@ build_dir = $(top_builddir)
 source_dir = $(top_srcdir)
 coi_inc_dir = $(top_srcdir)/../include/coi
 myo_inc_dir = $(top_srcdir)/../include/myo
+include_src_dir = $(top_srcdir)/../../include
 libgomp_src_dir = $(top_srcdir)/../../libgomp
 libgomp_dir = $(build_dir)/../../libgomp
 liboffload_src_dir = $(top_srcdir)/../runtime
@@ -52,7 +53,7 @@ target_install_dir = $(accel_search_dir)/lib/gcc/$(accel_target)/$(gcc_version)$
 if PLUGIN_HOST
   toolexeclib_LTLIBRARIES = libgomp-plugin-intelmic.la
   libgomp_plugin_intelmic_la_SOURCES = libgomp-plugin-intelmic.cpp
-  libgomp_plugin_intelmic_la_CPPFLAGS = $(CPPFLAGS) -DLINUX -DCOI_LIBRARY_VERSION=2 -DMYO_SUPPORT -DOFFLOAD_DEBUG=1 -DSEP_SUPPORT -DTIMING_SUPPORT -DHOST_LIBRARY=1 -I$(coi_inc_dir) -I$(myo_inc_dir) -I$(liboffload_src_dir) -I$(libgomp_src_dir) -I$(libgomp_dir) -I$(target_prefix_dir)/include -I$(target_build_dir) -I$(target_install_dir)/include
+  libgomp_plugin_intelmic_la_CPPFLAGS = $(CPPFLAGS) -DLINUX -DCOI_LIBRARY_VERSION=2 -DMYO_SUPPORT -DOFFLOAD_DEBUG=1 -DSEP_SUPPORT -DTIMING_SUPPORT -DHOST_LIBRARY=1 -I$(coi_inc_dir) -I$(myo_inc_dir) -I$(liboffload_src_dir) -I$(libgomp_src_dir) -I$(libgomp_dir) -I$(include_src_dir) -I$(target_prefix_dir)/include -I$(target_build_dir) -I$(target_install_dir)/include
   libgomp_plugin_intelmic_la_LDFLAGS = -L$(liboffload_dir)/.libs -loffloadmic_host -version-info 1:0:0
 else # PLUGIN_TARGET
   plugin_includedir = $(libsubincludedir)
diff --git a/liboffloadmic/plugin/Makefile.in b/liboffloadmic/plugin/Makefile.in
index 19a1a96..458c9b2 100644
--- a/liboffloadmic/plugin/Makefile.in
+++ b/liboffloadmic/plugin/Makefile.in
@@ -306,6 +306,7 @@ build_dir = $(top_builddir)
 source_dir = $(top_srcdir)
 coi_inc_dir = $(top_srcdir)/../include/coi
 myo_inc_dir = $(top_srcdir)/../include/myo
+include_src_dir = $(top_srcdir)/../../include
 libgomp_src_dir = $(top_srcdir)/../../libgomp
 libgomp_dir = $(build_dir)/../../libgomp
 liboffload_src_dir = $(top_srcdir)/../runtime
@@ -320,7 +321,7 @@ target_build_dir = $(accel_search_dir)/$(accel_target)$(MULTISUBDIR)/liboffloadm
 target_install_dir = $(accel_search_dir)/lib/gcc/$(accel_target)/$(gcc_version)$(MULTISUBDIR)
 @PLUGIN_HOST_TRUE@toolexeclib_LTLIBRARIES = libgomp-plugin-intelmic.la
 @PLUGIN_HOST_TRUE@libgomp_plugin_intelmic_la_SOURCES = libgomp-plugin-intelmic.cpp
-@PLUGIN_HOST_TRUE@libgomp_plugin_intelmic_la_CPPFLAGS = $(CPPFLAGS) -DLINUX -DCOI_LIBRARY_VERSION=2 -DMYO_SUPPORT -DOFFLOAD_DEBUG=1 -DSEP_SUPPORT -DTIMING_SUPPORT -DHOST_LIBRARY=1 -I$(coi_inc_dir) -I$(myo_inc_dir) -I$(liboffload_src_dir) -I$(libgomp_src_dir) -I$(libgomp_dir) -I$(target_prefix_dir)/include -I$(target_build_dir) -I$(target_install_dir)/include
+@PLUGIN_HOST_TRUE@libgomp_plugin_intelmic_la_CPPFLAGS = $(CPPFLAGS) -DLINUX -DCOI_LIBRARY_VERSION=2 -DMYO_SUPPORT -DOFFLOAD_DEBUG=1 -DSEP_SUPPORT -DTIMING_SUPPORT -DHOST_LIBRARY=1 -I$(coi_inc_dir) -I$(myo_inc_dir) -I$(liboffload_src_dir) -I$(libgomp_src_dir) -I$(libgomp_dir) -I$(include_src_dir) -I$(target_prefix_dir)/include -I$(target_build_dir) -I$(target_install_dir)/include
 @PLUGIN_HOST_TRUE@libgomp_plugin_intelmic_la_LDFLAGS = -L$(liboffload_dir)/.libs -loffloadmic_host -version-info 1:0:0
 @PLUGIN_HOST_FALSE@plugin_includedir = $(libsubincludedir)
 @PLUGIN_HOST_FALSE@plugin_include_HEADERS = main_target_image.h
diff --git a/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp b/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
index a6e5e1c..fde7d9e 100644
--- a/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
+++ b/liboffloadmic/plugin/libgomp-plugin-intelmic.cpp
@@ -1,6 +1,6 @@
 /* Plugin for offload execution on Intel MIC devices.
 
-   Copyright (C) 2014 Free Software Foundation, Inc.
+   Copyright (C) 2014-2015 Free Software Foundation, Inc.
 
    Contributed by Ilya Verbin <ilya.verbin@intel.com>.
 
@@ -38,6 +38,7 @@
 #include "libgomp-plugin.h"
 #include "compiler_if_host.h"
 #include "main_target_image.h"
+#include "gomp-constants.h"
 
 #define LD_LIBRARY_PATH_ENV	"LD_LIBRARY_PATH"
 #define MIC_LD_LIBRARY_PATH_ENV	"MIC_LD_LIBRARY_PATH"
@@ -327,12 +328,26 @@ offload_image (const void *target_image)
   free (image);
 }
 
+/* Return the libgomp version number we're compatible with.  There is
+   no requirement for cross-version compatibility.  */
+
+extern "C" unsigned
+GOMP_OFFLOAD_version (void)
+{
+  return GOMP_VERSION;
+}
+
 extern "C" int
-GOMP_OFFLOAD_load_image (int device, const void *target_image,
-			 addr_pair **result)
+GOMP_OFFLOAD_load_image (int device, const unsigned version,
+			 void *target_image, addr_pair **result)
 {
   TRACE ("(device = %d, target_image = %p)", device, target_image);
 
+  if (GOMP_VERSION_DEV (version) > GOMP_VERSION_INTEL_MIC)
+    GOMP_PLUGIN_fatal ("Offload data incompatible with intelmic plugin"
+		       " (expected %u, received %u)",
+		       GOMP_VERSION_INTEL_MIC, GOMP_VERSION_DEV (version));
+
   /* If target_image is already present in address_table, then there is no need
      to offload it.  */
   if (address_table->count (target_image) == 0)
@@ -353,8 +368,12 @@ GOMP_OFFLOAD_load_image (int device, const void *target_image,
 }
 
 extern "C" void
-GOMP_OFFLOAD_unload_image (int device, const void *target_image)
+GOMP_OFFLOAD_unload_image (int device, unsigned version,
+			   const void *target_image)
 {
+  if (GOMP_VERSION_DEV (version) > GOMP_VERSION_INTEL_MIC)
+    return;
+
   TRACE ("(device = %d, target_image = %p)", device, target_image);
 
   /* TODO: Currently liboffloadmic doesn't support __offload_unregister_image
-- 
1.7.10.4


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: offload data version number
  2015-08-06 19:52                   ` Nathan Sidwell
  2015-08-10 20:00                     ` Thomas Schwinge
@ 2015-08-24  8:45                     ` Jakub Jelinek
  1 sibling, 0 replies; 22+ messages in thread
From: Jakub Jelinek @ 2015-08-24  8:45 UTC (permalink / raw)
  To: Nathan Sidwell; +Cc: ilya.verbin, GCC Patches

On Thu, Aug 06, 2015 at 09:52:43PM +0200, Nathan Sidwell wrote:
> Ping?
> 
> 1) updated version patch https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00010.html
> 
> 2) https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00204.html
> An infrastructure piece from Thomas, who noticed liboffloadmic didn't have
> the include paths to #include  gomp-constants.h.

Ok.

	Jakub

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2015-08-24  8:37 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-21 13:28 offload data version number Nathan Sidwell
2015-07-21 16:02 ` Nathan Sidwell
2015-07-24 14:21   ` Nathan Sidwell
2015-07-24 16:35     ` Jakub Jelinek
2015-07-24 16:46       ` Ilya Verbin
2015-07-24 17:22       ` Nathan Sidwell
2015-07-24 19:51         ` Nathan Sidwell
2015-07-24 20:02           ` Ilya Verbin
2015-07-24 20:05             ` Nathan Sidwell
2015-07-31 12:30           ` Nathan Sidwell
2015-07-31 14:08             ` Nathan Sidwell
2015-07-31 16:20               ` Jakub Jelinek
2015-08-02  0:06                 ` Nathan Sidwell
2015-08-02  0:20                   ` Nathan Sidwell
2015-08-03 11:16                     ` Thomas Schwinge
2015-08-03 13:26                       ` Nathan Sidwell
2015-08-04 16:19                   ` Thomas Schwinge
2015-08-04 16:20                     ` Nathan Sidwell
2015-08-06 19:52                   ` Nathan Sidwell
2015-08-10 20:00                     ` Thomas Schwinge
2015-08-24  8:45                     ` Jakub Jelinek
2015-07-27 15:21     ` Nathan Sidwell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).