On 2018/12/18 8:36 PM, Jakub Jelinek wrote:
On Fri, Dec 14, 2018 at 06:52:20PM +0100, Thomas Schwinge wrote:
--- a/include/gomp-constants.h
+++ b/include/gomp-constants.h
@@ -199,7 +200,7 @@ enum gomp_map_kind
/* Versions of libgomp and device-specific plugins. GOMP_VERSION
should be incremented whenever an ABI-incompatible change is introduced
to the plugin interface defined in libgomp/libgomp.h. */
-#define GOMP_VERSION 1
+#define GOMP_VERSION 2
#define GOMP_VERSION_NVIDIA_PTX 1
#define GOMP_VERSION_INTEL_MIC 0
#define GOMP_VERSION_HSA 0
OK, I think -- but I'm never quite sure whether we do need to increment
"GOMP_VERSION" when only doing libgomp-internal libgomp-plugin changes,
which don't affect the user/GCC side?
GCC encodes "GOMP_VERSION" in "GOMP_offload_register_ver" calls
synthesized by "mkoffload": "GOMP_VERSION_PACK (/* LIB */ GOMP_VERSION,
/* DEV */ GOMP_VERSION_NVIDIA_PTX)", and then at run time libgomp checks
in "GOMP_offload_register_ver", so that we don't try to load offloading
code with an _old_ libgomp that has been compiled with/for the _new_
version. (Right?)
To me it looks wrong to tie two different things in the same version number.
Just because we are changing something in the libgomp vs. the corresponding
plugin APIs doesn't mean we need to rebuild all binaries and libraries that
have offloading code in it.
The GOMP_offload_register_ver test is for "> GOMP_VERSION", so a wrt
GOMP_VERSION's value
a libgomp can be too old, but never too new. It should not require a rebuild of
programs
with offloading just because of this.
So, IMHO GOMP_VERSION should be bumped only if we do a change that requires
the offloading data to be changed, and either have an additional internal
version to make sure that the plugin are kept in sync with libgomp, or just
figure that out because dlsym will fail on some of the new symbols in the
plugin.
We can of course create a new symbol version number specifically for the
libgomp/plugin
interface.
I'll update this.
--- a/libgomp/libgomp.map
+++ b/libgomp/libgomp.map
@@ -458,7 +462,6 @@ GOMP_PLUGIN_1.0 {
GOMP_PLUGIN_debug;
GOMP_PLUGIN_error;
GOMP_PLUGIN_fatal;
- GOMP_PLUGIN_async_unmap_vars;
GOMP_PLUGIN_acc_thread;
};
I think that's fine, but highlighting this again for Jakub, in case
there's an issue with removing a symbol from the libgomp-plugin
interface.
I'd prefer not to remove symbols from libgomp.so.*. You can
do a gomp_fatal in it.
Okay, then.
--- a/libgomp/libgomp-plugin.h
+++ b/libgomp/libgomp-plugin.h
+/* Opaque type to represent plugin-dependent implementation of an
+ OpenACC asynchronous queue. */
+struct goacc_asyncqueue;
+
+/* Used to keep a list of active asynchronous queues. */
+struct goacc_asyncqueue_list
+{
+ struct goacc_asyncqueue *aq;
+ struct goacc_asyncqueue_list *next;
+};
+
+typedef struct goacc_asyncqueue *goacc_aq;
+typedef struct goacc_asyncqueue_list *goacc_aq_list;
I'm not too fond of such "syntactic sugar" typedefs, but if that's fine
for Jakub to have in libgomp, then I won't object.
If it helps with making sure the formatting of the code isn't too ugly,
yes, otherwise no.
Thanks, formatting was exactly my intention.
Chung-Lin
I'd be in favor then of "typedef struct N *N" or "typedef struct N *N_t"
Please avoid *_t, that is reserved in POSIX.
Jakub
