Hi!

On 2022-07-01T15:06:05+0200, Tobias Burnus <tob...@codesourcery.com> wrote:
> Attached is the updated patch. Main changes: [...]

This is now a great implementation of cross-component communication
(host/offloading compilers, runtime), thanks!  I'm sure this will be
usable (or at least instructing) for further purposes, too.

> - Uses GOMP_register_var to pass the mask to libgomp

Like 'GOMP_offload_register_ver', also 'GOMP_offload_unregister_ver'
needs to be adjusted correspondingly.  OK to push the attached
"Restore 'GOMP_offload_unregister_ver' functionality"?  (Currently
testing.)

> (and no longer a weak variable)

... which actually removed my "contribution" (hack!) to this patch.  ;-)


Grüße
 Thomas


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
>From 9a49a3e1e4d3def7b48beccdde6fa9f218719244 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tho...@codesourcery.com>
Date: Tue, 5 Jul 2022 18:23:15 +0200
Subject: [PATCH] Restore 'GOMP_offload_unregister_ver' functionality

The recent commit 683f11843974f0bdf42f79cdcbb0c2b43c7b81b0
"OpenMP: Move omp requires checks to libgomp" changed the
'GOMP_offload_register_ver' interface but didn't change
'GOMP_offload_unregister_ver' accordingly, so we're no longer
actually unregistering.

	gcc/
	* config/gcn/mkoffload.cc (process_obj): Clarify 'target_data' ->
	'[...]_data'.
	* config/nvptx/mkoffload.cc (process): Likewise.
	libgomp/
	* target.c (GOMP_offload_register_ver): Clarify 'target_data' ->
	'data'.
	(GOMP_offload_unregister_ver): Likewise.  Fix up 'target_data',
	and add 'assert'.
---
 gcc/config/gcn/mkoffload.cc   |  8 ++++----
 gcc/config/nvptx/mkoffload.cc |  8 ++++----
 libgomp/target.c              | 33 ++++++++++++++++++++++++++-------
 3 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/gcc/config/gcn/mkoffload.cc b/gcc/config/gcn/mkoffload.cc
index b8b3fecfcb4..d2464332275 100644
--- a/gcc/config/gcn/mkoffload.cc
+++ b/gcc/config/gcn/mkoffload.cc
@@ -692,13 +692,13 @@ process_obj (FILE *in, FILE *cfile, uint32_t omp_requires)
 	   len);
 
   fprintf (cfile,
-	   "static const struct gcn_image_desc {\n"
+	   "static const struct gcn_data {\n"
 	   "  uintptr_t omp_requires_mask;\n"
 	   "  const struct gcn_image *gcn_image;\n"
 	   "  unsigned kernel_count;\n"
 	   "  const struct hsa_kernel_description *kernel_infos;\n"
 	   "  unsigned global_variable_count;\n"
-	   "} target_data = {\n"
+	   "} gcn_data = {\n"
 	   "  %d,\n"
 	   "  &gcn_image,\n"
 	   "  sizeof (gcn_kernels) / sizeof (gcn_kernels[0]),\n"
@@ -723,7 +723,7 @@ process_obj (FILE *in, FILE *cfile, uint32_t omp_requires)
   fprintf (cfile, "static __attribute__((constructor)) void init (void)\n"
 	   "{\n"
 	   "  GOMP_offload_register_ver (%#x, __OFFLOAD_TABLE__,"
-	   " %d/*GCN*/, &target_data);\n"
+	   " %d/*GCN*/, &gcn_data);\n"
 	   "};\n",
 	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_GCN),
 	   GOMP_DEVICE_GCN);
@@ -731,7 +731,7 @@ process_obj (FILE *in, FILE *cfile, uint32_t omp_requires)
   fprintf (cfile, "static __attribute__((destructor)) void fini (void)\n"
 	   "{\n"
 	   "  GOMP_offload_unregister_ver (%#x, __OFFLOAD_TABLE__,"
-	   " %d/*GCN*/, &target_data);\n"
+	   " %d/*GCN*/, &gcn_data);\n"
 	   "};\n",
 	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_GCN),
 	   GOMP_DEVICE_GCN);
diff --git a/gcc/config/nvptx/mkoffload.cc b/gcc/config/nvptx/mkoffload.cc
index d8c81eb0547..0fa5f4423bf 100644
--- a/gcc/config/nvptx/mkoffload.cc
+++ b/gcc/config/nvptx/mkoffload.cc
@@ -310,7 +310,7 @@ process (FILE *in, FILE *out, uint32_t omp_requires)
   fprintf (out, "\n};\n\n");
 
   fprintf (out,
-	   "static const struct nvptx_tdata {\n"
+	   "static const struct nvptx_data {\n"
 	   "  uintptr_t omp_requires_mask;\n"
 	   "  const struct ptx_obj *ptx_objs;\n"
 	   "  unsigned ptx_num;\n"
@@ -318,7 +318,7 @@ process (FILE *in, FILE *out, uint32_t omp_requires)
 	   "  unsigned var_num;\n"
 	   "  const struct nvptx_fn *fn_names;\n"
 	   "  unsigned fn_num;\n"
-	   "} target_data = {\n"
+	   "} nvptx_data = {\n"
 	   "  %d, ptx_objs, sizeof (ptx_objs) / sizeof (ptx_objs[0]),\n"
 	   "  var_mappings,"
 	   "  sizeof (var_mappings) / sizeof (var_mappings[0]),\n"
@@ -344,7 +344,7 @@ process (FILE *in, FILE *out, uint32_t omp_requires)
   fprintf (out, "static __attribute__((constructor)) void init (void)\n"
 	   "{\n"
 	   "  GOMP_offload_register_ver (%#x, __OFFLOAD_TABLE__,"
-	   " %d/*NVIDIA_PTX*/, &target_data);\n"
+	   " %d/*NVIDIA_PTX*/, &nvptx_data);\n"
 	   "};\n",
 	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
 	   GOMP_DEVICE_NVIDIA_PTX);
@@ -352,7 +352,7 @@ process (FILE *in, FILE *out, uint32_t omp_requires)
   fprintf (out, "static __attribute__((destructor)) void fini (void)\n"
 	   "{\n"
 	   "  GOMP_offload_unregister_ver (%#x, __OFFLOAD_TABLE__,"
-	   " %d/*NVIDIA_PTX*/, &target_data);\n"
+	   " %d/*NVIDIA_PTX*/, &nvptx_data);\n"
 	   "};\n",
 	   GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_NVIDIA_PTX),
 	   GOMP_DEVICE_NVIDIA_PTX);
diff --git a/libgomp/target.c b/libgomp/target.c
index 4dac81862d7..288b748b9e8 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -2334,23 +2334,29 @@ gomp_requires_to_name (char *buf, size_t size, int requires_mask)
 
 /* This function should be called from every offload image while loading.
    It gets the descriptor of the host func and var tables HOST_TABLE, TYPE of
-   the target, and TARGET_DATA needed by target plugin.  */
+   the target, and DATA.  */
 
 void
 GOMP_offload_register_ver (unsigned version, const void *host_table,
-			   int target_type, const void *target_data)
+			   int target_type, const void *data)
 {
   int i;
-  int omp_req = 0;
 
   if (GOMP_VERSION_LIB (version) > GOMP_VERSION)
     gomp_fatal ("Library too old for offload (version %u < %u)",
 		GOMP_VERSION, GOMP_VERSION_LIB (version));
 
+  int omp_req;
+  const void *target_data;
   if (GOMP_VERSION_LIB (version) > 1)
     {
-      omp_req = (int) (size_t) ((void **) target_data)[0];
-      target_data = &((void **) target_data)[1];
+      omp_req = (int) (size_t) ((void **) data)[0];
+      target_data = &((void **) data)[1];
+    }
+  else
+    {
+      omp_req = 0;
+      target_data = data;
     }
 
   gomp_mutex_lock (&register_lock);
@@ -2413,14 +2419,24 @@ GOMP_offload_register (const void *host_table, int target_type,
 
 /* 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.  */
+   the target, and DATA.  */
 
 void
 GOMP_offload_unregister_ver (unsigned version, const void *host_table,
-			     int target_type, const void *target_data)
+			     int target_type, const void *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));
+
+  const void *target_data;
+  if (GOMP_VERSION_LIB (version) > 1)
+    target_data = &((void **) data)[1];
+  else
+    target_data = data;
+
   gomp_mutex_lock (&register_lock);
 
   /* Unload image from all initialized devices.  */
@@ -2436,12 +2452,15 @@ GOMP_offload_unregister_ver (unsigned version, const void *host_table,
     }
 
   /* Remove image from array of pending images.  */
+  bool found = false;
   for (i = 0; i < num_offload_images; i++)
     if (offload_images[i].target_data == target_data)
       {
 	offload_images[i] = offload_images[--num_offload_images];
+	found = true;
 	break;
       }
+  assert (found);
 
   gomp_mutex_unlock (&register_lock);
 }
-- 
2.35.1

Reply via email to