On Thu, Nov 12, 2015 at 02:21:56PM +0100, Thomas Schwinge wrote: > > > --- a/libgomp/libgomp.h > > > +++ b/libgomp/libgomp.h > > > @@ -876,7 +876,8 @@ struct gomp_device_descr > > > void *(*dev2host_func) (int, void *, const void *, size_t); > > > void *(*host2dev_func) (int, void *, const void *, size_t); > > > void *(*dev2dev_func) (int, void *, const void *, size_t); > > > - void (*run_func) (int, void *, void *); > > > + void (*run_func) (int, void *, void *, const void *); > > > > Adding arguments to existing plugin methods is a plugin ABI incompatible > > change. We now have: > > DLSYM (version); > > if (device->version_func () != GOMP_VERSION) > > { > > err = "plugin version mismatch"; > > goto fail; > > } > > so there is a way to deal with it, but you need to adjust all plugins. > > I'm confused -- didn't we agree that we don't need to maintain backwards > compatibility in the libgomp <-> plugins interface? (Nathan?) As far as > I remember, the argument was that libgomp and all its plugins will always > be built from the same source tree, so will be compatible with each > other, "by definition"? > > (We do need, and have, versioning between GCC proper and libgomp > interfaces.)
I've mentioned the GOMP_VERSION check in there, and that all the plugins would need to be modified, which was I think not shown in the patch. > > But this shows a worse problem, > > if you have GCC 5 compiled OpenMP code, of course there won't be HSA > > offloaded copy, but if you try to run it on a box with HSA offloading > > enabled, you can run into this assertion failure. > > That's one of the issues that I'm working on resolving with my > "Forwarding -foffload=[...] from the driver (compile-time) to libgomp > (run-time)" patch, > <http://news.gmane.org/find-root.php?message_id=%3C87mvve95af.fsf%40schwinge.name%3E>. > In such a case (no GOMP_offload_register_ver call for HSA), HSA > offloading would not be considered (not "enabled") in libgomp. (It'll be > two more weeks before I can make progress with that patch; will be > attending SuperComputing 2015 next week -- anyone else will be there, > too?) You are aware of my objections to that and what does it do with later dlopened libraries. > > GOMP_target_ext has different arguments, you get the num_teams and > > thread_limit clauses values in there already (if known at compile time or > > before entering target region; 0 stands for implementation defined choice, > > -1 for unknown before GOMP_target_ext). > > Plus I must say I really don't like the addition of HSA specific argument > > to the API, it is unclean and really doesn't scale, when somebody adds > > support for another offloading target, would we add again another argument? > > Can't use the same one, because one could have configured both HSA and that > > other kind offloading at the same time and which one is picked would be only > > a runtime decision, based on env vars of omp_set_default_device etc. > > num_teams/thread_limit, as runtime arguments, you already get on the trunk. > > For compile time decided values, those should go into some data section > > and be somehow attached to what fn is translated into in the AVL tree (which > > you really don't need to use for variables on GOMP_OFFLOAD_CAP_SHARED_MEM > > obviously, but can still use for the kernels, and populate during > > registration of the offloading region). > > What about adopting the "tagging" scheme that we added for > libgomp/oacc-parallel.c:GOACC_parallel_keyed? With support for other > offloading schemes being added one by one, isn't it quite likely that the > interface will need to be adjusted for each of them, because > more/different data will have to be transmitted from GCC proper to > libgomp? Perhaps something similar, but certainly not as varargs, that is a really bad idea. Instead of the long * array I've talked about perhaps use void **, and put in unkeyed num_teams and thread_limit as the first two arguments thereof, then add keyed arguments in there, terminate by 0. Jakub