On Fri, Sep 25, 2015 at 05:04:47PM +0200, Thomas Schwinge wrote:
> Hi!
>
> It's Friday afternoon -- but anyway, is the following analysis correct?
>
> 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.
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*.
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.
Jakub
