This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: libgomp: Compile-time error for non-portable gomp_mutex_t initialization
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Ilya Verbin <iverbin at gmail dot com>
- Cc: Thomas Schwinge <thomas at codesourcery dot com>, Julian Brown <julian at codesourcery dot com>, gcc-patches at gcc dot gnu dot org, Kirill Yukhin <kirill dot yukhin at gmail dot com>
- Date: Thu, 19 Nov 2015 13:31:08 +0100
- Subject: Re: libgomp: Compile-time error for non-portable gomp_mutex_t initialization
- Authentication-results: sourceware.org; auth=none
- References: <87a902ib93 dot fsf at schwinge dot name> <20150226172511 dot GA49293 at msticlxl57 dot ims dot intel dot com> <20150306140113 dot GB26588 at msticlxl57 dot ims dot intel dot com> <20150309144555 dot 3a078f48 at octopus> <20150323194439 dot GA12972 at msticlxl57 dot ims dot intel dot com> <20150326120919 dot GZ1746 at tucnak dot redhat dot com> <20150326204130 dot GA65474 at msticlxl57 dot ims dot intel dot com> <87io6ywne8 dot fsf at kepler dot schwinge dot homeip dot net> <20150925152825 dot GQ1847 at tucnak dot redhat dot com> <20151118151929 dot GA4267 at msticlxl57 dot ims dot intel dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
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 <iverbin@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