On Fri, Sep 25, 2015 at 17:28:25 +0200, Jakub Jelinek wrote:
> On Fri, Sep 25, 2015 at 05:04:47PM +0200, Thomas Schwinge wrote:
> > On Thu, 26 Mar 2015 23:41:30 +0300, Ilya Verbin <iver...@gmail.com> wrote:
> > > On Thu, Mar 26, 2015 at 13:09:19 +0100, Jakub Jelinek wrote:
> > > > the current code is majorly broken. As I've said earlier, e.g. the lack
> > > > of mutex guarding gomp_target_init (which is using pthread_once
> > > > guaranteed
> > > > to be run just once) vs. concurrent GOMP_offload_register calls
> > > > (if those are run from ctors, then I guess something like dl_load_lock
> > > > ensures at least on glibc that multiple GOMP_offload_register calls
> > > > aren't
> > > > performed at the same time) in accessing/reallocating offload_images
> > > > and num_offload_images and the lack of support to register further
> > > > images after the gomp_target_init call (if you dlopen further shared
> > > > libraries) is really bad. And it would be really nice to support the
> > > > unloading.
> >
> > > Here is the latest patch for libgomp and mic plugin.
> >
> > > libgomp/
> >
> > > * target.c (register_lock): New mutex for offload image registration.
> >
> > > (GOMP_offload_register): Add mutex lock.
>
> That is definitely wrong. You'd totally break --disable-linux-futex support
> on linux and bootstrap on e.g. Solaris and various other pthread targets.
I don't quite understand, do you mean that gcc 5 and trunk are broken, because
register_lock doesn't have initialization? But it seems that bootstrap on
Solaris and other targets works fine...
> At least for ELF and dynamic linking, shared libraries that contain
> constructors that call GOMP_offload_register* should have DT_NEEDED libgomp
> and thus libgomp's constructors should be run before the constructors of
> the libraries that call GOMP_offload_register*.
So, libgomp should contain a constructor, which will call gomp_mutex_init
(®ister_lock) before any call to GOMP_offload_register*, right?
> For the targets without known zero initializer for gomp_mutex_lock, either
> there is an option to use pthread_once to make sure it is initialized once,
> or there is an option to define a macro like GOMP_MUTEX_INITIALIZER,
> defined to PTHREAD_MUTEX_INITIALIZER in config/posix/mutex.h and to
> { 0 } in config/linux/mutex.h and something like {} or whatever in
> config/rtems/mutex.h. Then for the non-automatic non-heap
> gomp_mutex_t's you could just initialize them in their initializers
> with GOMP_MUTEX_INITIALIZER.
-- Ilya
