On Wed, Nov 18, 2015 at 06:19:29PM +0300, Ilya Verbin wrote:
> 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...
Thomas has been proposing to add an #error when !GOMP_MUTEX_INIT_0 into
target.c, so that means break build of libgomp on all targets where
config/posix/mutex.h is used. That includes --disable-linux-futex on Linux,
and various other 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*.
>
> So, libgomp should contain a constructor, which will call gomp_mutex_init
> (®ister_lock) before any call to GOMP_offload_register*, right?
No, I think the GOMP_MUTEX_INITIALIZER case is better.
All pthread targets need to support PTHREAD_MUTEX_INITIALIZER, and the other
config/*/bar.h implementations are GOMP_MUTEX_INIT_0 1.
So, config/posix/bar.h would
#define GOMP_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER
config/linux/bar.h would
#define GOMP_MUTEX_INITIALIZER { 0 }
and
config/rtems/bar.h would
#define GOMP_MUTEX_INITIALIZER {}
// or something similar.
and then just initialize the file scope locks with that static initializer.
Jakub
