Re: Fix libgomp crash without TLS (PR42616)

2014-12-09 Thread Varvara Rainchik 
Can we instead of adding new macroses in config/tls.m4 use something
like that in libgomp:

#if defined (HAVE_TLS)  defined (USE_EMUTLS)

(with GCC_CHECK_EMUTLS in libgomp/configure.ac)?

2014-12-08 19:28 GMT+03:00 Jakub Jelinek ja...@redhat.com:
 On Mon, Dec 08, 2014 at 07:01:09PM +0300, Varvara Rainchik wrote:
 Is it ok to add GCC_CHECK_EMUTLS test in libgomp/configure.ac or
 should I add in tls.m3 a similair test that would be used only in
 libgomp?

 I think config/tls.m4 would be a better place, but doing it in some way
 where the users of config/tls.m4 could actually by using different macros
 from there choose what they want (either check solely for real TLS, or
 only for emutls, or for both).  And libgomp/configure.ac would then choose
 it is happy with both.

 Jakub

Re: Fix libgomp crash without TLS (PR42616)

2014-12-09 Thread Jakub Jelinek 
On Tue, Dec 09, 2014 at 02:49:44PM +0300, Varvara Rainchik wrote:
 Can we instead of adding new macroses in config/tls.m4 use something
 like that in libgomp:
 
 #if defined (HAVE_TLS)  defined (USE_EMUTLS)
 
 (with GCC_CHECK_EMUTLS in libgomp/configure.ac)?

That would be fine too.

Jakub

Re: Fix libgomp crash without TLS (PR42616)

2014-12-09 Thread Varvara Rainchik 
Ok, the following patch works for Android:

2014-12-09  Varvara Rainchik  varvara.rainc...@intel.com

* config.h.in: Regenerate.
* configure: Regenerate.
* configure.ac: Add GCC_CHECK_EMUTLS.
* libgomp.h: Add check for USE_EMUTLS: this case
is equal to HAVE_TLS.
* team.c: Likewise.

--

diff --git a/libgomp/configure.ac b/libgomp/configure.ac
index cea6366..16ec158 100644
--- a/libgomp/configure.ac
+++ b/libgomp/configure.ac
@@ -245,6 +245,9 @@ fi
 # See if we support thread-local storage.
 GCC_CHECK_TLS

+# See if we have emulated thread-local storage.
+GCC_CHECK_EMUTLS
+
 # See what sort of export controls are available.
 LIBGOMP_CHECK_ATTRIBUTE_VISIBILITY
 LIBGOMP_CHECK_ATTRIBUTE_DLLEXPORT
diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index a1482cc..b694356 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -471,7 +471,7 @@ enum gomp_cancel_kind

 /* ... and here is that TLS data.  */

-#ifdef HAVE_TLS
+#if defined HAVE_TLS || defined USE_EMUTLS
 extern __thread struct gomp_thread gomp_tls_data;
 static inline struct gomp_thread *gomp_thread (void)
 {
diff --git a/libgomp/team.c b/libgomp/team.c
index e6a6d8f..594127c 100644
--- a/libgomp/team.c
+++ b/libgomp/team.c
@@ -37,7 +37,7 @@ pthread_key_t gomp_thread_destructor;


 /* This is the libgomp per-thread data structure.  */
-#ifdef HAVE_TLS
+#if defined HAVE_TLS || defined USE_EMUTLS
 __thread struct gomp_thread gomp_tls_data;
 #else
 pthread_key_t gomp_tls_key;
@@ -70,7 +70,7 @@ gomp_thread_start (void *xdata)
   void (*local_fn) (void *);
   void *local_data;

-#ifdef HAVE_TLS
+#if defined HAVE_TLS || defined USE_EMUTLS
   thr = gomp_tls_data;
 #else
   struct gomp_thread local_thr;
@@ -916,7 +916,7 @@ gomp_team_end (void)
 static void __attribute__((constructor))
 initialize_team (void)
 {
-#ifndef HAVE_TLS
+#if !defined HAVE_TLS  !defined USE_EMUTLS
   static struct gomp_thread initial_thread_tls_data;

   pthread_key_create (gomp_tls_key, NULL);

Changes are bootstrapped and regtested on linux, all make check tests
now also pass with --disable-tls.
Is it ok for upstream?

2014-12-09 15:12 GMT+03:00 Jakub Jelinek ja...@redhat.com:
 On Tue, Dec 09, 2014 at 02:49:44PM +0300, Varvara Rainchik wrote:
 Can we instead of adding new macroses in config/tls.m4 use something
 like that in libgomp:

 #if defined (HAVE_TLS)  defined (USE_EMUTLS)

 (with GCC_CHECK_EMUTLS in libgomp/configure.ac)?

 That would be fine too.

 Jakub

Re: Fix libgomp crash without TLS (PR42616)

2014-12-09 Thread Richard Henderson 
On 12/09/2014 06:53 AM, Varvara Rainchik wrote:
 2014-12-09  Varvara Rainchik  varvara.rainc...@intel.com
 
 * config.h.in: Regenerate.
 * configure: Regenerate.
 * configure.ac: Add GCC_CHECK_EMUTLS.
 * libgomp.h: Add check for USE_EMUTLS: this case
 is equal to HAVE_TLS.
 * team.c: Likewise.

Ok.


r~

Re: Fix libgomp crash without TLS (PR42616)

2014-12-08 Thread Varvara Rainchik 
Hi guys,

Could you please take a look at this issue? This fix is still urgent
for Android.

2014-12-01 18:25 GMT+03:00 Varvara Rainchik varvara.s.rainc...@gmail.com:
 Hi Jakub,

 Do you think this patch is ok for upstream:

 2014-12-01  Varvara Rainchik  varvara.rainc...@intel.com

 * libgomp/libgomp.h: Eliminate case when HAVE_TLS is not defined:
 always use tls emulation.
 * libgomp/team.c: Likewise.

 --
 diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
 index a1482cc..a659ebc 100644
 --- a/libgomp/libgomp.h
 +++ b/libgomp/libgomp.h
 @@ -471,19 +471,11 @@ enum gomp_cancel_kind

  /* ... and here is that TLS data.  */

 -#ifdef HAVE_TLS
  extern __thread struct gomp_thread gomp_tls_data;
  static inline struct gomp_thread *gomp_thread (void)
  {
return gomp_tls_data;
  }
 -#else
 -extern pthread_key_t gomp_tls_key;
 -static inline struct gomp_thread *gomp_thread (void)
 -{
 -  return pthread_getspecific (gomp_tls_key);
 -}
 -#endif

  extern struct gomp_task_icv *gomp_new_icv (void);

 diff --git a/libgomp/team.c b/libgomp/team.c
 index e6a6d8f..2e8dc47 100644
 --- a/libgomp/team.c
 +++ b/libgomp/team.c
 @@ -37,12 +37,7 @@ pthread_key_t gomp_thread_destructor;


  /* This is the libgomp per-thread data structure.  */
 -#ifdef HAVE_TLS
  __thread struct gomp_thread gomp_tls_data;
 -#else
 -pthread_key_t gomp_tls_key;
 -#endif
 -

  /* This structure is used to communicate across pthread_create.  */

 @@ -70,13 +65,7 @@ gomp_thread_start (void *xdata)
void (*local_fn) (void *);
void *local_data;

 -#ifdef HAVE_TLS
thr = gomp_tls_data;
 -#else
 -  struct gomp_thread local_thr;
 -  thr = local_thr;
 -  pthread_setspecific (gomp_tls_key, thr);
 -#endif
gomp_sem_init (thr-release, 0);

/* Extract what we need from data.  */
 @@ -916,13 +905,6 @@ gomp_team_end (void)
  static void __attribute__((constructor))
  initialize_team (void)
  {
 -#ifndef HAVE_TLS
 -  static struct gomp_thread initial_thread_tls_data;
 -
 -  pthread_key_create (gomp_tls_key, NULL);
 -  pthread_setspecific (gomp_tls_key, initial_thread_tls_data);
 -#endif
 -
if (pthread_key_create (gomp_thread_destructor, gomp_free_thread) != 0)
  gomp_fatal (could not create thread pool destructor.);
  }

 Or should I add some extra checks in config/tls.m4? As far as I
 understand you have mentioned case when both native tls and tls
 emulation are not supported. So, is it sufficient to check that
 HAVE_TLS and USE_EMUTLS are not defined to detect this case?

 2014-11-10 16:15 GMT+03:00 Varvara Rainchik varvara.s.rainc...@gmail.com:
 *Ping*

 2014-10-13 14:48 GMT+04:00 Varvara Rainchik varvara.s.rainc...@gmail.com:
 Now, I wonder on which OS and why does config/tls.m4 CHECK_GCC_TLS
 actually fail?  Can you figure that out?


 On Android check passes with --disable-tls (standard while building
 gcc for Android as TLS is not supported in bionic) and fails with
 --enable-tls (i686-linux-android/libgomp/conftest.c:32: undefined
 reference to `___tls_get_addr'). So, HAVE_TLS is not defined in both
 cases.

 If we get rid of HAVE_TLS code altogether, we might lose support of
 some very old OSes, e.g. some Linux distros with a recent gcc and binutils
 (so that emutls isn't used), but very old glibc (that doesn't support
 TLS or supports it incorrectly, think of pre-2002 glibc).  So, if we get
 rid of !HAVE_TLS code in libgomp, it would be nice if config/tls.m4 
 detected
 it properly and we'd just fail at configure time.

 How can we check this in config/tls.m4? Can we just combine tests on
 TLS and emutls? E.g. check whether HAVE_TLS and USE_EMUTLS are both
 defined.

 And if we don't, just make sure that on Android, Darwin and/or M$Win (or
 whatever other OS you had in mind which does support pthreads, but doesn't
 support native TLS) find out why HAVE_AS_TLS is not defined (guess
 config.log should explain that).

 HAVE_AS_TLS is also not defined for Android as it depends on --enable-tls.

Re: Fix libgomp crash without TLS (PR42616)

2014-12-08 Thread Jakub Jelinek 
On Mon, Dec 08, 2014 at 04:30:46PM +0300, Varvara Rainchik wrote:
 Hi guys,
 
 Could you please take a look at this issue? This fix is still urgent
 for Android.

I'm afraid it will break those targets where emutls is not on, but the C
library doesn't support TLS.  I think it is acceptable not to care about
#pragma omp from different pthread_create created threads in that case, but
stopping support completely might be too early.
So, can you instead arrange for HAVE_TLS to be defined if emutls is
supported (check for that during configure)?

Jakub

Re: Fix libgomp crash without TLS (PR42616)

2014-12-08 Thread Varvara Rainchik 
Is it ok to add GCC_CHECK_EMUTLS test in libgomp/configure.ac or
should I add in tls.m3 a similair test that would be used only in
libgomp?

2014-12-08 17:03 GMT+03:00 Jakub Jelinek ja...@redhat.com:
 On Mon, Dec 08, 2014 at 04:30:46PM +0300, Varvara Rainchik wrote:
 Hi guys,

 Could you please take a look at this issue? This fix is still urgent
 for Android.

 I'm afraid it will break those targets where emutls is not on, but the C
 library doesn't support TLS.  I think it is acceptable not to care about
 #pragma omp from different pthread_create created threads in that case, but
 stopping support completely might be too early.
 So, can you instead arrange for HAVE_TLS to be defined if emutls is
 supported (check for that during configure)?

 Jakub

Re: Fix libgomp crash without TLS (PR42616)

2014-12-08 Thread Jakub Jelinek 
On Mon, Dec 08, 2014 at 07:01:09PM +0300, Varvara Rainchik wrote:
 Is it ok to add GCC_CHECK_EMUTLS test in libgomp/configure.ac or
 should I add in tls.m3 a similair test that would be used only in
 libgomp?

I think config/tls.m4 would be a better place, but doing it in some way
where the users of config/tls.m4 could actually by using different macros
from there choose what they want (either check solely for real TLS, or
only for emutls, or for both).  And libgomp/configure.ac would then choose
it is happy with both.

Jakub

Re: Fix libgomp crash without TLS (PR42616)

2014-12-01 Thread Varvara Rainchik 
Hi Jakub,

Do you think this patch is ok for upstream:

2014-12-01  Varvara Rainchik  varvara.rainc...@intel.com

* libgomp/libgomp.h: Eliminate case when HAVE_TLS is not defined:
always use tls emulation.
* libgomp/team.c: Likewise.

--
diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index a1482cc..a659ebc 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -471,19 +471,11 @@ enum gomp_cancel_kind

 /* ... and here is that TLS data.  */

-#ifdef HAVE_TLS
 extern __thread struct gomp_thread gomp_tls_data;
 static inline struct gomp_thread *gomp_thread (void)
 {
   return gomp_tls_data;
 }
-#else
-extern pthread_key_t gomp_tls_key;
-static inline struct gomp_thread *gomp_thread (void)
-{
-  return pthread_getspecific (gomp_tls_key);
-}
-#endif

 extern struct gomp_task_icv *gomp_new_icv (void);

diff --git a/libgomp/team.c b/libgomp/team.c
index e6a6d8f..2e8dc47 100644
--- a/libgomp/team.c
+++ b/libgomp/team.c
@@ -37,12 +37,7 @@ pthread_key_t gomp_thread_destructor;


 /* This is the libgomp per-thread data structure.  */
-#ifdef HAVE_TLS
 __thread struct gomp_thread gomp_tls_data;
-#else
-pthread_key_t gomp_tls_key;
-#endif
-

 /* This structure is used to communicate across pthread_create.  */

@@ -70,13 +65,7 @@ gomp_thread_start (void *xdata)
   void (*local_fn) (void *);
   void *local_data;

-#ifdef HAVE_TLS
   thr = gomp_tls_data;
-#else
-  struct gomp_thread local_thr;
-  thr = local_thr;
-  pthread_setspecific (gomp_tls_key, thr);
-#endif
   gomp_sem_init (thr-release, 0);

   /* Extract what we need from data.  */
@@ -916,13 +905,6 @@ gomp_team_end (void)
 static void __attribute__((constructor))
 initialize_team (void)
 {
-#ifndef HAVE_TLS
-  static struct gomp_thread initial_thread_tls_data;
-
-  pthread_key_create (gomp_tls_key, NULL);
-  pthread_setspecific (gomp_tls_key, initial_thread_tls_data);
-#endif
-
   if (pthread_key_create (gomp_thread_destructor, gomp_free_thread) != 0)
 gomp_fatal (could not create thread pool destructor.);
 }

Or should I add some extra checks in config/tls.m4? As far as I
understand you have mentioned case when both native tls and tls
emulation are not supported. So, is it sufficient to check that
HAVE_TLS and USE_EMUTLS are not defined to detect this case?

2014-11-10 16:15 GMT+03:00 Varvara Rainchik varvara.s.rainc...@gmail.com:
 *Ping*

 2014-10-13 14:48 GMT+04:00 Varvara Rainchik varvara.s.rainc...@gmail.com:
 Now, I wonder on which OS and why does config/tls.m4 CHECK_GCC_TLS
 actually fail?  Can you figure that out?


 On Android check passes with --disable-tls (standard while building
 gcc for Android as TLS is not supported in bionic) and fails with
 --enable-tls (i686-linux-android/libgomp/conftest.c:32: undefined
 reference to `___tls_get_addr'). So, HAVE_TLS is not defined in both
 cases.

 If we get rid of HAVE_TLS code altogether, we might lose support of
 some very old OSes, e.g. some Linux distros with a recent gcc and binutils
 (so that emutls isn't used), but very old glibc (that doesn't support
 TLS or supports it incorrectly, think of pre-2002 glibc).  So, if we get
 rid of !HAVE_TLS code in libgomp, it would be nice if config/tls.m4 detected
 it properly and we'd just fail at configure time.

 How can we check this in config/tls.m4? Can we just combine tests on
 TLS and emutls? E.g. check whether HAVE_TLS and USE_EMUTLS are both
 defined.

 And if we don't, just make sure that on Android, Darwin and/or M$Win (or
 whatever other OS you had in mind which does support pthreads, but doesn't
 support native TLS) find out why HAVE_AS_TLS is not defined (guess
 config.log should explain that).

 HAVE_AS_TLS is also not defined for Android as it depends on --enable-tls.

Re: Fix libgomp crash without TLS (PR42616)

2014-11-10 Thread Varvara Rainchik 
*Ping*

2014-10-13 14:48 GMT+04:00 Varvara Rainchik varvara.s.rainc...@gmail.com:
 Now, I wonder on which OS and why does config/tls.m4 CHECK_GCC_TLS
 actually fail?  Can you figure that out?


 On Android check passes with --disable-tls (standard while building
 gcc for Android as TLS is not supported in bionic) and fails with
 --enable-tls (i686-linux-android/libgomp/conftest.c:32: undefined
 reference to `___tls_get_addr'). So, HAVE_TLS is not defined in both
 cases.

 If we get rid of HAVE_TLS code altogether, we might lose support of
 some very old OSes, e.g. some Linux distros with a recent gcc and binutils
 (so that emutls isn't used), but very old glibc (that doesn't support
 TLS or supports it incorrectly, think of pre-2002 glibc).  So, if we get
 rid of !HAVE_TLS code in libgomp, it would be nice if config/tls.m4 detected
 it properly and we'd just fail at configure time.

 How can we check this in config/tls.m4? Can we just combine tests on
 TLS and emutls? E.g. check whether HAVE_TLS and USE_EMUTLS are both
 defined.

 And if we don't, just make sure that on Android, Darwin and/or M$Win (or
 whatever other OS you had in mind which does support pthreads, but doesn't
 support native TLS) find out why HAVE_AS_TLS is not defined (guess
 config.log should explain that).

 HAVE_AS_TLS is also not defined for Android as it depends on --enable-tls.

Re: Fix libgomp crash without TLS (PR42616)

2014-10-13 Thread Varvara Rainchik 
 Now, I wonder on which OS and why does config/tls.m4 CHECK_GCC_TLS
 actually fail?  Can you figure that out?


On Android check passes with --disable-tls (standard while building
gcc for Android as TLS is not supported in bionic) and fails with
--enable-tls (i686-linux-android/libgomp/conftest.c:32: undefined
reference to `___tls_get_addr'). So, HAVE_TLS is not defined in both
cases.

 If we get rid of HAVE_TLS code altogether, we might lose support of
 some very old OSes, e.g. some Linux distros with a recent gcc and binutils
 (so that emutls isn't used), but very old glibc (that doesn't support
 TLS or supports it incorrectly, think of pre-2002 glibc).  So, if we get
 rid of !HAVE_TLS code in libgomp, it would be nice if config/tls.m4 detected
 it properly and we'd just fail at configure time.

How can we check this in config/tls.m4? Can we just combine tests on
TLS and emutls? E.g. check whether HAVE_TLS and USE_EMUTLS are both
defined.

 And if we don't, just make sure that on Android, Darwin and/or M$Win (or
 whatever other OS you had in mind which does support pthreads, but doesn't
 support native TLS) find out why HAVE_AS_TLS is not defined (guess
 config.log should explain that).

HAVE_AS_TLS is also not defined for Android as it depends on --enable-tls.

Re: Fix libgomp crash without TLS (PR42616)

2014-10-07 Thread Jakub Jelinek 
On Wed, Oct 01, 2014 at 08:44:59PM +0400, Varvara Rainchik wrote:
 Ok, then here it is a new patch (tested and bootstrapped on linux).
 
 On linux with --disable-tls now all libgomp make check tests pass; for
 Android I've patched toolchain and tried test from one of the
 mentioned bugs, test passes too.

 Is there some benchmark to check performance?

There is SPEC OMP,
http://www.spec.org/hpg/omp2001/
EPCC,
http://www2.epcc.ed.ac.uk/computing/research_activities/openmpbench/openmp_index.html
NAS,
http://www.nas.nasa.gov/publications/npb.html
http://phase.hpcc.jp/Omni/benchmarks/NPB/
Rodinia,
https://www.cs.virginia.edu/~skadron/wiki/rodinia/index.php/Main_Page

Now, I wonder on which OS and why does config/tls.m4 CHECK_GCC_TLS
actually fail?  Can you figure that out?

If we get rid of HAVE_TLS code altogether, we might lose support of
some very old OSes, e.g. some Linux distros with a recent gcc and binutils
(so that emutls isn't used), but very old glibc (that doesn't support
TLS or supports it incorrectly, think of pre-2002 glibc).  So, if we get
rid of !HAVE_TLS code in libgomp, it would be nice if config/tls.m4 detected
it properly and we'd just fail at configure time.
And if we don't, just make sure that on Android, Darwin and/or M$Win (or
whatever other OS you had in mind which does support pthreads, but doesn't
support native TLS) find out why HAVE_AS_TLS is not defined (guess
config.log should explain that).

 2014-10-01  Varvara Rainchik  varvara.rainc...@intel.com
 
 * libgomp.h (HAVE_TLS): Set to 1.

Jakub

Re: Fix libgomp crash without TLS (PR42616)

2014-10-01 Thread Varvara Rainchik 
Ok, then here it is a new patch (tested and bootstrapped on linux).

On linux with --disable-tls now all libgomp make check tests pass; for
Android I've patched toolchain and tried test from one of the
mentioned bugs, test passes too.

Is there some benchmark to check performance?


2014-10-01  Varvara Rainchik  varvara.rainc...@intel.com

* libgomp.h (HAVE_TLS): Set to 1.

--
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -45,6 +45,8 @@
 # pragma GCC visibility push(hidden)
 #endif

+#define HAVE_TLS 1
+
 /* If we were a C++ library, we'd get this from std/atomic.  */
 enum memmodel
 {

2014-09-30 18:40 GMT+04:00 Richard Henderson r...@redhat.com:
 On 09/30/2014 02:52 AM, Jakub Jelinek wrote:
 On Tue, Sep 30, 2014 at 11:03:47AM +0400, Varvara Rainchik wrote:
 Corrected patch: call pthread_setspecific (gomp_tls_key, NULL) in
 gomp_thread_start if HAVE_TLS is not defined.

 2014-09-19  Varvara Rainchik  varvara.rainc...@intel.com

 * libgomp.h (gomp_thread): For non TLS case create thread data.
 * team.c (non_tls_thread_data_destructor,
 create_non_tls_thread_data): New functions.

 I actually wonder when we have emutls support in libgcc if it wouldn't
 be better to just define HAVE_TLS always to 1 (i.e. remove all the
 conditionals on it), then you wouldn't need to bother with this at all.

 I don't have an OS which doesn't support native TLS though, so somebody with
 such a system would need to test it and benchmark if it doesn't make things
 slower.

 Richard, thoughts on this?

 I like that idea better as well.


 r~

Re: Fix libgomp crash without TLS (PR42616)

2014-09-30 Thread Varvara Rainchik 
Corrected patch: call pthread_setspecific (gomp_tls_key, NULL) in
gomp_thread_start if HAVE_TLS is not defined.

2014-09-19  Varvara Rainchik  varvara.rainc...@intel.com

* libgomp.h (gomp_thread): For non TLS case create thread data.
* team.c (non_tls_thread_data_destructor,
create_non_tls_thread_data): New functions.


---
diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index bcd5b34..2f33d99 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -467,9 +467,15 @@ static inline struct gomp_thread *gomp_thread (void)
 }
 #else
 extern pthread_key_t gomp_tls_key;
-static inline struct gomp_thread *gomp_thread (void)
+extern struct gomp_thread *create_non_tls_thread_data (void);
+static struct gomp_thread *gomp_thread (void)
 {
-  return pthread_getspecific (gomp_tls_key);
+  struct gomp_thread *thr = pthread_getspecific (gomp_tls_key);
+  if (thr == NULL)
+  {
+thr = create_non_tls_thread_data ();
+  }
+  return thr;
 }
 #endif

diff --git a/libgomp/team.c b/libgomp/team.c
index e6a6d8f..1854d8a 100644
--- a/libgomp/team.c
+++ b/libgomp/team.c
@@ -41,6 +41,7 @@ pthread_key_t gomp_thread_destructor;
 __thread struct gomp_thread gomp_tls_data;
 #else
 pthread_key_t gomp_tls_key;
+struct gomp_thread initial_thread_tls_data;
 #endif


@@ -130,6 +131,9 @@ gomp_thread_start (void *xdata)
   gomp_sem_destroy (thr-release);
   thr-thread_pool = NULL;
   thr-task = NULL;
+#ifndef HAVE_TLS
+  pthread_setspecific (gomp_tls_key, NULL);
+#endif
   return NULL;
 }

@@ -222,8 +226,16 @@ gomp_free_pool_helper (void *thread_pool)
 void
 gomp_free_thread (void *arg __attribute__((unused)))
 {
-  struct gomp_thread *thr = gomp_thread ();
-  struct gomp_thread_pool *pool = thr-thread_pool;
+  struct gomp_thread *thr;
+  struct gomp_thread_pool *pool;
+#ifdef HAVE_TLS
+  thr = gomp_thread ();
+#else
+  thr = pthread_getspecific (gomp_tls_key);
+  if (thr == NULL)
+return;
+#endif
+  pool = thr-thread_pool;
   if (pool)
 {
   if (pool-threads_used  0)
@@ -910,6 +922,21 @@ gomp_team_end (void)
 }
 }

+/* Destructor for data created in create_non_tls_thread_data.  */
+
+#ifndef HAVE_TLS
+void
+non_tls_thread_data_destructor (void *arg __attribute__((unused)))
+{
+  struct gomp_thread *thr = pthread_getspecific (gomp_tls_key);
+  if (thr != NULL  thr != initial_thread_tls_data)
+  {
+gomp_free_thread (arg);
+free (thr);
+pthread_setspecific (gomp_tls_key, NULL);
+  }
+}
+#endif

 /* Constructors for this file.  */

@@ -917,9 +944,7 @@ static void __attribute__((constructor))
 initialize_team (void)
 {
 #ifndef HAVE_TLS
-  static struct gomp_thread initial_thread_tls_data;
-
-  pthread_key_create (gomp_tls_key, NULL);
+  pthread_key_create (gomp_tls_key, non_tls_thread_data_destructor);
   pthread_setspecific (gomp_tls_key, initial_thread_tls_data);
 #endif

@@ -927,6 +952,19 @@ initialize_team (void)
 gomp_fatal (could not create thread pool destructor.);
 }

+/* Create data for thread created by pthread_create.  */
+
+#ifndef HAVE_TLS
+struct gomp_thread *create_non_tls_thread_data (void)
+{
+  struct gomp_thread *thr = gomp_malloc_cleared (sizeof (struct gomp_thread));
+  pthread_setspecific (gomp_tls_key, thr);
+  gomp_sem_init (thr-release, 0);
+
+  return thr;
+}
+#endif
+
 static void __attribute__((destructor))
 team_destructor (void)
 {




2014-09-24 14:19 GMT+04:00 Varvara Rainchik varvara.s.rainc...@gmail.com:
 *Ping*

 2014-09-19 15:41 GMT+04:00 Varvara Rainchik varvara.s.rainc...@gmail.com:
 I've corrected my patch accordingly to what you said. To diffirentiate
 second case in destructor I've added pthread_setspecific
 (gomp_tls_key, NULL) at the end of gomp_thread_start. So, destructor
 can simply skip the case when pthread_getspecific (gomp_tls_key)
 returns 0. I also think that it's better to set 0 in gomp_thread_start
 explicitly as thread data is initialized by a local variable in this
 function.

 But, I see that pthread_getspecific always returns 0 in destrucor
 because data pointer is implicitly set to 0 before destructor call in
 glibc:

 (pthread_create.c):

 /* Always clear the data. */
 level2[inner].data = NULL;

 /* Make sure the data corresponds to a valid
 key. This test fails if the key was
 deallocated and also if it was
 re-allocated. It is the user's
 responsibility to free the memory in this
 case. */
 if (level2[inner].seq
== __pthread_keys[idx].seq
/* It is not necessary to register a destructor
   function. */
   __pthread_keys[idx].destr != NULL)
 /* Call the user-provided destructor. */
 __pthread_keys[idx].destr (data);

 I suppose it's not necessary if everything is cleaned up in
 gomp_thread_start  and destructor. What do you think?


 Changes are bootstrapped and regtested on x86_64-linux.

 2014-09-19  Varvara Rainchik  varvara.rainc...@intel.com

 * libgomp.h (gomp_thread): For non TLS case create thread data.
 * team.c (non_tls_thread_data_destructor,
 create_non_tls_thread_data): New functions.

Re: Fix libgomp crash without TLS (PR42616)

2014-09-30 Thread Jakub Jelinek 
On Tue, Sep 30, 2014 at 11:03:47AM +0400, Varvara Rainchik wrote:
 Corrected patch: call pthread_setspecific (gomp_tls_key, NULL) in
 gomp_thread_start if HAVE_TLS is not defined.
 
 2014-09-19  Varvara Rainchik  varvara.rainc...@intel.com
 
 * libgomp.h (gomp_thread): For non TLS case create thread data.
 * team.c (non_tls_thread_data_destructor,
 create_non_tls_thread_data): New functions.

I actually wonder when we have emutls support in libgcc if it wouldn't
be better to just define HAVE_TLS always to 1 (i.e. remove all the
conditionals on it), then you wouldn't need to bother with this at all.

I don't have an OS which doesn't support native TLS though, so somebody with
such a system would need to test it and benchmark if it doesn't make things
slower.

Richard, thoughts on this?

Jakub

Re: Fix libgomp crash without TLS (PR42616)

2014-09-30 Thread Richard Henderson 
On 09/30/2014 02:52 AM, Jakub Jelinek wrote:
 On Tue, Sep 30, 2014 at 11:03:47AM +0400, Varvara Rainchik wrote:
 Corrected patch: call pthread_setspecific (gomp_tls_key, NULL) in
 gomp_thread_start if HAVE_TLS is not defined.

 2014-09-19  Varvara Rainchik  varvara.rainc...@intel.com

 * libgomp.h (gomp_thread): For non TLS case create thread data.
 * team.c (non_tls_thread_data_destructor,
 create_non_tls_thread_data): New functions.
 
 I actually wonder when we have emutls support in libgcc if it wouldn't
 be better to just define HAVE_TLS always to 1 (i.e. remove all the
 conditionals on it), then you wouldn't need to bother with this at all.
 
 I don't have an OS which doesn't support native TLS though, so somebody with
 such a system would need to test it and benchmark if it doesn't make things
 slower.
 
 Richard, thoughts on this?

I like that idea better as well.


r~

Re: Fix libgomp crash without TLS (PR42616)

2014-09-24 Thread Varvara Rainchik 
*Ping*

2014-09-19 15:41 GMT+04:00 Varvara Rainchik varvara.s.rainc...@gmail.com:
 I've corrected my patch accordingly to what you said. To diffirentiate
 second case in destructor I've added pthread_setspecific
 (gomp_tls_key, NULL) at the end of gomp_thread_start. So, destructor
 can simply skip the case when pthread_getspecific (gomp_tls_key)
 returns 0. I also think that it's better to set 0 in gomp_thread_start
 explicitly as thread data is initialized by a local variable in this
 function.

 But, I see that pthread_getspecific always returns 0 in destrucor
 because data pointer is implicitly set to 0 before destructor call in
 glibc:

 (pthread_create.c):

 /* Always clear the data. */
 level2[inner].data = NULL;

 /* Make sure the data corresponds to a valid
 key. This test fails if the key was
 deallocated and also if it was
 re-allocated. It is the user's
 responsibility to free the memory in this
 case. */
 if (level2[inner].seq
== __pthread_keys[idx].seq
/* It is not necessary to register a destructor
   function. */
   __pthread_keys[idx].destr != NULL)
 /* Call the user-provided destructor. */
 __pthread_keys[idx].destr (data);

 I suppose it's not necessary if everything is cleaned up in
 gomp_thread_start  and destructor. What do you think?


 Changes are bootstrapped and regtested on x86_64-linux.

 2014-09-19  Varvara Rainchik  varvara.rainc...@intel.com

 * libgomp.h (gomp_thread): For non TLS case create thread data.
 * team.c (non_tls_thread_data_destructor,
 create_non_tls_thread_data): New functions.


 ---
 diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
 index bcd5b34..2f33d99 100644
 --- a/libgomp/libgomp.h
 +++ b/libgomp/libgomp.h
 @@ -467,9 +467,15 @@ static inline struct gomp_thread *gomp_thread (void)
  }
  #else
  extern pthread_key_t gomp_tls_key;
 -static inline struct gomp_thread *gomp_thread (void)
 +extern struct gomp_thread *create_non_tls_thread_data (void);
 +static struct gomp_thread *gomp_thread (void)
  {
 -  return pthread_getspecific (gomp_tls_key);
 +  struct gomp_thread *thr = pthread_getspecific (gomp_tls_key);
 +  if (thr == NULL)
 +  {
 +thr = create_non_tls_thread_data ();
 +  }
 +  return thr;
  }
  #endif

 diff --git a/libgomp/team.c b/libgomp/team.c
 index e6a6d8f..a692df8 100644
 --- a/libgomp/team.c
 +++ b/libgomp/team.c
 @@ -41,6 +41,7 @@ pthread_key_t gomp_thread_destructor;
  __thread struct gomp_thread gomp_tls_data;
  #else
  pthread_key_t gomp_tls_key;
 +struct gomp_thread initial_thread_tls_data;
  #endif


 @@ -130,6 +131,7 @@ gomp_thread_start (void *xdata)
gomp_sem_destroy (thr-release);
thr-thread_pool = NULL;
thr-task = NULL;
 +  pthread_setspecific (gomp_tls_key, NULL);
return NULL;
  }

 @@ -222,8 +224,16 @@ gomp_free_pool_helper (void *thread_pool)
  void
  gomp_free_thread (void *arg __attribute__((unused)))
  {
 -  struct gomp_thread *thr = gomp_thread ();
 -  struct gomp_thread_pool *pool = thr-thread_pool;
 +  struct gomp_thread *thr;
 +  struct gomp_thread_pool *pool;
 +#ifdef HAVE_TLS
 +  thr = gomp_thread ();
 +#else
 +  thr = pthread_getspecific (gomp_tls_key);
 +  if (thr == NULL)
 +return;
 +#endif
 +  pool = thr-thread_pool;
if (pool)
  {
if (pool-threads_used  0)
 @@ -910,6 +920,21 @@ gomp_team_end (void)
  }
  }

 +/* Destructor for data created in create_non_tls_thread_data.  */
 +
 +#ifndef HAVE_TLS
 +void
 +non_tls_thread_data_destructor (void *arg __attribute__((unused)))
 +{
 +  struct gomp_thread *thr = pthread_getspecific (gomp_tls_key);
 +  if (thr != NULL  thr != initial_thread_tls_data)
 +  {
 +gomp_free_thread (arg);
 +free (thr);
 +pthread_setspecific (gomp_tls_key, NULL);
 +  }
 +}
 +#endif

  /* Constructors for this file.  */

 @@ -917,9 +942,7 @@ static void __attribute__((constructor))
  initialize_team (void)
  {
  #ifndef HAVE_TLS
 -  static struct gomp_thread initial_thread_tls_data;
 -
 -  pthread_key_create (gomp_tls_key, NULL);
 +  pthread_key_create (gomp_tls_key, non_tls_thread_data_destructor);
pthread_setspecific (gomp_tls_key, initial_thread_tls_data);
  #endif

 @@ -927,6 +950,19 @@ initialize_team (void)
  gomp_fatal (could not create thread pool destructor.);
  }

 +/* Create data for thread created by pthread_create.  */
 +
 +#ifndef HAVE_TLS
 +struct gomp_thread *create_non_tls_thread_data (void)
 +{
 +  struct gomp_thread *thr = gomp_malloc_cleared (sizeof (struct 
 gomp_thread));
 +  pthread_setspecific (gomp_tls_key, thr);
 +  gomp_sem_init (thr-release, 0);
 +
 +  return thr;
 +}
 +#endif
 +
  static void __attribute__((destructor))
  team_destructor (void)
 {

 2014-09-02 14:36 GMT+04:00 Varvara Rainchik varvara.s.rainc...@gmail.com:
 May I use gomp_free_thread as a destructor for pthread_key_create?
 Then I'll make initial_thread_tls_data global for the first case, but
 how can I differentiate thread created by gomp_thread_start (second
 case)?

 2014-09-01 14:51 GMT+04:00 Jakub Jelinek

Re: Fix libgomp crash without TLS (PR42616)

2014-09-19 Thread Varvara Rainchik 
I've corrected my patch accordingly to what you said. To diffirentiate
second case in destructor I've added pthread_setspecific
(gomp_tls_key, NULL) at the end of gomp_thread_start. So, destructor
can simply skip the case when pthread_getspecific (gomp_tls_key)
returns 0. I also think that it's better to set 0 in gomp_thread_start
explicitly as thread data is initialized by a local variable in this
function.

But, I see that pthread_getspecific always returns 0 in destrucor
because data pointer is implicitly set to 0 before destructor call in
glibc:

(pthread_create.c):

/* Always clear the data. */
level2[inner].data = NULL;

/* Make sure the data corresponds to a valid
key. This test fails if the key was
deallocated and also if it was
re-allocated. It is the user's
responsibility to free the memory in this
case. */
if (level2[inner].seq
   == __pthread_keys[idx].seq
   /* It is not necessary to register a destructor
  function. */
  __pthread_keys[idx].destr != NULL)
/* Call the user-provided destructor. */
__pthread_keys[idx].destr (data);

I suppose it's not necessary if everything is cleaned up in
gomp_thread_start  and destructor. What do you think?


Changes are bootstrapped and regtested on x86_64-linux.

2014-09-19  Varvara Rainchik  varvara.rainc...@intel.com

* libgomp.h (gomp_thread): For non TLS case create thread data.
* team.c (non_tls_thread_data_destructor,
create_non_tls_thread_data): New functions.


---
diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index bcd5b34..2f33d99 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -467,9 +467,15 @@ static inline struct gomp_thread *gomp_thread (void)
 }
 #else
 extern pthread_key_t gomp_tls_key;
-static inline struct gomp_thread *gomp_thread (void)
+extern struct gomp_thread *create_non_tls_thread_data (void);
+static struct gomp_thread *gomp_thread (void)
 {
-  return pthread_getspecific (gomp_tls_key);
+  struct gomp_thread *thr = pthread_getspecific (gomp_tls_key);
+  if (thr == NULL)
+  {
+thr = create_non_tls_thread_data ();
+  }
+  return thr;
 }
 #endif

diff --git a/libgomp/team.c b/libgomp/team.c
index e6a6d8f..a692df8 100644
--- a/libgomp/team.c
+++ b/libgomp/team.c
@@ -41,6 +41,7 @@ pthread_key_t gomp_thread_destructor;
 __thread struct gomp_thread gomp_tls_data;
 #else
 pthread_key_t gomp_tls_key;
+struct gomp_thread initial_thread_tls_data;
 #endif


@@ -130,6 +131,7 @@ gomp_thread_start (void *xdata)
   gomp_sem_destroy (thr-release);
   thr-thread_pool = NULL;
   thr-task = NULL;
+  pthread_setspecific (gomp_tls_key, NULL);
   return NULL;
 }

@@ -222,8 +224,16 @@ gomp_free_pool_helper (void *thread_pool)
 void
 gomp_free_thread (void *arg __attribute__((unused)))
 {
-  struct gomp_thread *thr = gomp_thread ();
-  struct gomp_thread_pool *pool = thr-thread_pool;
+  struct gomp_thread *thr;
+  struct gomp_thread_pool *pool;
+#ifdef HAVE_TLS
+  thr = gomp_thread ();
+#else
+  thr = pthread_getspecific (gomp_tls_key);
+  if (thr == NULL)
+return;
+#endif
+  pool = thr-thread_pool;
   if (pool)
 {
   if (pool-threads_used  0)
@@ -910,6 +920,21 @@ gomp_team_end (void)
 }
 }

+/* Destructor for data created in create_non_tls_thread_data.  */
+
+#ifndef HAVE_TLS
+void
+non_tls_thread_data_destructor (void *arg __attribute__((unused)))
+{
+  struct gomp_thread *thr = pthread_getspecific (gomp_tls_key);
+  if (thr != NULL  thr != initial_thread_tls_data)
+  {
+gomp_free_thread (arg);
+free (thr);
+pthread_setspecific (gomp_tls_key, NULL);
+  }
+}
+#endif

 /* Constructors for this file.  */

@@ -917,9 +942,7 @@ static void __attribute__((constructor))
 initialize_team (void)
 {
 #ifndef HAVE_TLS
-  static struct gomp_thread initial_thread_tls_data;
-
-  pthread_key_create (gomp_tls_key, NULL);
+  pthread_key_create (gomp_tls_key, non_tls_thread_data_destructor);
   pthread_setspecific (gomp_tls_key, initial_thread_tls_data);
 #endif

@@ -927,6 +950,19 @@ initialize_team (void)
 gomp_fatal (could not create thread pool destructor.);
 }

+/* Create data for thread created by pthread_create.  */
+
+#ifndef HAVE_TLS
+struct gomp_thread *create_non_tls_thread_data (void)
+{
+  struct gomp_thread *thr = gomp_malloc_cleared (sizeof (struct gomp_thread));
+  pthread_setspecific (gomp_tls_key, thr);
+  gomp_sem_init (thr-release, 0);
+
+  return thr;
+}
+#endif
+
 static void __attribute__((destructor))
 team_destructor (void)
{

2014-09-02 14:36 GMT+04:00 Varvara Rainchik varvara.s.rainc...@gmail.com:
 May I use gomp_free_thread as a destructor for pthread_key_create?
 Then I'll make initial_thread_tls_data global for the first case, but
 how can I differentiate thread created by gomp_thread_start (second
 case)?

 2014-09-01 14:51 GMT+04:00 Jakub Jelinek ja...@redhat.com:
 On Fri, Aug 29, 2014 at 10:40:57AM -0700, Richard Henderson wrote:
 On 08/06/2014 03:05 AM, Varvara Rainchik wrote:
  * libgomp.h (gomp_thread): For non TLS case create thread data.
  * team.c

Re: Fix libgomp crash without TLS (PR42616)

2014-09-02 Thread Varvara Rainchik 
May I use gomp_free_thread as a destructor for pthread_key_create?
Then I'll make initial_thread_tls_data global for the first case, but
how can I differentiate thread created by gomp_thread_start (second
case)?

2014-09-01 14:51 GMT+04:00 Jakub Jelinek ja...@redhat.com:
 On Fri, Aug 29, 2014 at 10:40:57AM -0700, Richard Henderson wrote:
 On 08/06/2014 03:05 AM, Varvara Rainchik wrote:
  * libgomp.h (gomp_thread): For non TLS case create thread data.
  * team.c (create_non_tls_thread_data): New function.
 
 
  ---
  diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
  index a1482cc..cf3ec8f 100644
  --- a/libgomp/libgomp.h
  +++ b/libgomp/libgomp.h
  @@ -479,9 +479,15 @@ static inline struct gomp_thread *gomp_thread (void)
  }
  #else
  extern pthread_key_t gomp_tls_key;
  +extern struct gomp_thread *create_non_tls_thread_data (void);
  static inline struct gomp_thread *gomp_thread (void)
  {
  -  return pthread_getspecific (gomp_tls_key);
  +  struct gomp_thread *thr = pthread_getspecific (gomp_tls_key);
  +  if (thr == NULL)
  +  {
  +thr = create_non_tls_thread_data ();
  +  }
  +  return thr;
  }

 This should never happen.

 I guess it can happen if you mix up explicit pthread_create and libgomp APIs.
 initialize_team will only initialize it in the initial thread, while if you
 use #pragma omp ... or omp_* calls from a thread created with
 pthread_create, in the !HAVE_TLS case pthread_getspecific will return NULL.

 Now, the patch doesn't handle that case completely though (and is badly
 formatted), the problem is that if we allocate in the !HAVE_TLS case
 in non-initial thread the TLS data, we want to free them again, so that
 would mean pthread_key_create with non-NULL destructor, and then we need to
 differentiate in between the 3 cases - key equal to initial_thread_tls_data
 (would need to move out of the block context), no freeing needed, thread
 created by gomp_thread_start, no freeing needed, otherwise free.

 The thread-specific data is set in gomp_thread_start and initialize_team.

 Where are you getting a call to gomp_thread that hasn't been through one of
 those functions?

 Jakub

Re: Fix libgomp crash without TLS (PR42616)

2014-09-01 Thread Varvara Rainchik 
I've checked several tests, I see that for all tests failure occurs in
function gomp_icv (). E.g.:

icv-2:
#0  gomp_icv (write=true) at ../../../libgomp/libgomp.h:494
#1  omp_set_num_threads (n=6) at ../../../libgomp/env.c:1282
#2  0x00404014 in tf ()
#3  0x0040d063 in start_thread ()
#4  0x00450139 in clone ()

lock-3:
#0  gomp_icv (write=true) at ../../../libgomp/libgomp.h:494
#1  omp_test_nest_lock (lock=0x6dd580 lock) at
../../../libgomp/config/linux/lock.c:109
#2  0x00403fbc in tf ()
#3  0x0040ccd3 in start_thread ()
#4  0x0044fda9 in clone ()

2014-08-29 21:40 GMT+04:00 Richard Henderson r...@redhat.com:
 On 08/06/2014 03:05 AM, Varvara Rainchik wrote:
 * libgomp.h (gomp_thread): For non TLS case create thread data.
 * team.c (create_non_tls_thread_data): New function.


 ---
 diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
 index a1482cc..cf3ec8f 100644
 --- a/libgomp/libgomp.h
 +++ b/libgomp/libgomp.h
 @@ -479,9 +479,15 @@ static inline struct gomp_thread *gomp_thread (void)
 }
 #else
 extern pthread_key_t gomp_tls_key;
 +extern struct gomp_thread *create_non_tls_thread_data (void);
 static inline struct gomp_thread *gomp_thread (void)
 {
 -  return pthread_getspecific (gomp_tls_key);
 +  struct gomp_thread *thr = pthread_getspecific (gomp_tls_key);
 +  if (thr == NULL)
 +  {
 +thr = create_non_tls_thread_data ();
 +  }
 +  return thr;
 }

 This should never happen.

 The thread-specific data is set in gomp_thread_start and initialize_team.

 Where are you getting a call to gomp_thread that hasn't been through one of
 those functions?


 r~

Re: Fix libgomp crash without TLS (PR42616)

2014-09-01 Thread Jakub Jelinek 
On Fri, Aug 29, 2014 at 10:40:57AM -0700, Richard Henderson wrote:
 On 08/06/2014 03:05 AM, Varvara Rainchik wrote:
  * libgomp.h (gomp_thread): For non TLS case create thread data.
  * team.c (create_non_tls_thread_data): New function.
  
  
  ---
  diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
  index a1482cc..cf3ec8f 100644
  --- a/libgomp/libgomp.h
  +++ b/libgomp/libgomp.h
  @@ -479,9 +479,15 @@ static inline struct gomp_thread *gomp_thread (void)
  }
  #else
  extern pthread_key_t gomp_tls_key;
  +extern struct gomp_thread *create_non_tls_thread_data (void);
  static inline struct gomp_thread *gomp_thread (void)
  {
  -  return pthread_getspecific (gomp_tls_key);
  +  struct gomp_thread *thr = pthread_getspecific (gomp_tls_key);
  +  if (thr == NULL)
  +  {
  +thr = create_non_tls_thread_data ();
  +  }
  +  return thr;
  }
 
 This should never happen.

I guess it can happen if you mix up explicit pthread_create and libgomp APIs.
initialize_team will only initialize it in the initial thread, while if you
use #pragma omp ... or omp_* calls from a thread created with
pthread_create, in the !HAVE_TLS case pthread_getspecific will return NULL.

Now, the patch doesn't handle that case completely though (and is badly
formatted), the problem is that if we allocate in the !HAVE_TLS case
in non-initial thread the TLS data, we want to free them again, so that
would mean pthread_key_create with non-NULL destructor, and then we need to
differentiate in between the 3 cases - key equal to initial_thread_tls_data
(would need to move out of the block context), no freeing needed, thread
created by gomp_thread_start, no freeing needed, otherwise free.

 The thread-specific data is set in gomp_thread_start and initialize_team.
 
 Where are you getting a call to gomp_thread that hasn't been through one of
 those functions?

Jakub

Re: Fix libgomp crash without TLS (PR42616)

2014-08-29 Thread Varvara Rainchik 
Hi again! I want to remind that issue is urgent for Android.

2014-08-13 12:13 GMT+04:00 Varvara Rainchik varvara.s.rainc...@gmail.com:
 *Ping*

 Thanks,
 Varvara

 2014-08-06 14:05 GMT+04:00 Varvara Rainchik varvara.s.rainc...@gmail.com:
 Hi,

 The issue was firstly observed on NDK gcc since TLS is not supported
 in Android bionic. I also see the same failure on gcc configured for
 linux with –disable-tls, libgomp make check log:

 FAIL: libgomp.c/affinity-1.c execution test
 FAIL: libgomp.c/icv-2.c execution test
 FAIL: libgomp.c/lock-3.c execution test
 FAIL: libgomp.c/target-6.c execution test

 These tests except affinity-1.c fail because gomp_thread () function
 returns null pointer. I’ve found 2 bugs, first one addresses this
 problem on Windows:
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42616;
 second one addresses original problem (for both cases, with and without TLS):
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36242.
 Tests from both bugs fail with –disable-tls. So, it seems that non TLS
 case was fixed just partially. The following patch solves the problem.
 With this patch 3 tests from make check pass, affinity-1.c fails, but
 I think it’s other non TLS problem.
 Changes are bootstrapped and regtested on x86_64-linux.


 2014-08-06  Varvara Rainchik  varvara.rainc...@intel.com

 * libgomp.h (gomp_thread): For non TLS case create thread data.
 * team.c (create_non_tls_thread_data): New function.


 ---
 diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
 index a1482cc..cf3ec8f 100644
 --- a/libgomp/libgomp.h
 +++ b/libgomp/libgomp.h
 @@ -479,9 +479,15 @@ static inline struct gomp_thread *gomp_thread (void)
 }
 #else
 extern pthread_key_t gomp_tls_key;
 +extern struct gomp_thread *create_non_tls_thread_data (void);
 static inline struct gomp_thread *gomp_thread (void)
 {
 -  return pthread_getspecific (gomp_tls_key);
 +  struct gomp_thread *thr = pthread_getspecific (gomp_tls_key);
 +  if (thr == NULL)
 +  {
 +thr = create_non_tls_thread_data ();
 +  }
 +  return thr;
 }
 #endif

 diff --git a/libgomp/team.c b/libgomp/team.c
 index e6a6d8f..bf8bd4b 100644
 --- a/libgomp/team.c
 +++ b/libgomp/team.c
 @@ -927,6 +927,17 @@ initialize_team (void)
  gomp_fatal (could not create thread pool destructor.);
 }

 +#ifndef HAVE_TLS
 +struct gomp_thread *create_non_tls_thread_data (void)
 +{
 +  struct gomp_thread *thr = gomp_malloc (sizeof (struct gomp_thread));
 +  pthread_setspecific (gomp_tls_key, thr);
 +  gomp_sem_init (thr-release, 0);
 +
 +  return thr;
 +}
 +#endif
 +
 static void __attribute__((destructor))
 team_destructor (void)
 {
 ---


 Is it ok?


 Best regards,

 Varvara

Re: Fix libgomp crash without TLS (PR42616)

2014-08-29 Thread Richard Henderson 
On 08/06/2014 03:05 AM, Varvara Rainchik wrote:
 * libgomp.h (gomp_thread): For non TLS case create thread data.
 * team.c (create_non_tls_thread_data): New function.
 
 
 ---
 diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
 index a1482cc..cf3ec8f 100644
 --- a/libgomp/libgomp.h
 +++ b/libgomp/libgomp.h
 @@ -479,9 +479,15 @@ static inline struct gomp_thread *gomp_thread (void)
 }
 #else
 extern pthread_key_t gomp_tls_key;
 +extern struct gomp_thread *create_non_tls_thread_data (void);
 static inline struct gomp_thread *gomp_thread (void)
 {
 -  return pthread_getspecific (gomp_tls_key);
 +  struct gomp_thread *thr = pthread_getspecific (gomp_tls_key);
 +  if (thr == NULL)
 +  {
 +thr = create_non_tls_thread_data ();
 +  }
 +  return thr;
 }

This should never happen.

The thread-specific data is set in gomp_thread_start and initialize_team.

Where are you getting a call to gomp_thread that hasn't been through one of
those functions?


r~

Re: Fix libgomp crash without TLS (PR42616)

2014-08-13 Thread Varvara Rainchik 
*Ping*

Thanks,
Varvara

2014-08-06 14:05 GMT+04:00 Varvara Rainchik varvara.s.rainc...@gmail.com:
 Hi,

 The issue was firstly observed on NDK gcc since TLS is not supported
 in Android bionic. I also see the same failure on gcc configured for
 linux with –disable-tls, libgomp make check log:

 FAIL: libgomp.c/affinity-1.c execution test
 FAIL: libgomp.c/icv-2.c execution test
 FAIL: libgomp.c/lock-3.c execution test
 FAIL: libgomp.c/target-6.c execution test

 These tests except affinity-1.c fail because gomp_thread () function
 returns null pointer. I’ve found 2 bugs, first one addresses this
 problem on Windows:
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42616;
 second one addresses original problem (for both cases, with and without TLS):
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36242.
 Tests from both bugs fail with –disable-tls. So, it seems that non TLS
 case was fixed just partially. The following patch solves the problem.
 With this patch 3 tests from make check pass, affinity-1.c fails, but
 I think it’s other non TLS problem.
 Changes are bootstrapped and regtested on x86_64-linux.


 2014-08-06  Varvara Rainchik  varvara.rainc...@intel.com

 * libgomp.h (gomp_thread): For non TLS case create thread data.
 * team.c (create_non_tls_thread_data): New function.


 ---
 diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
 index a1482cc..cf3ec8f 100644
 --- a/libgomp/libgomp.h
 +++ b/libgomp/libgomp.h
 @@ -479,9 +479,15 @@ static inline struct gomp_thread *gomp_thread (void)
 }
 #else
 extern pthread_key_t gomp_tls_key;
 +extern struct gomp_thread *create_non_tls_thread_data (void);
 static inline struct gomp_thread *gomp_thread (void)
 {
 -  return pthread_getspecific (gomp_tls_key);
 +  struct gomp_thread *thr = pthread_getspecific (gomp_tls_key);
 +  if (thr == NULL)
 +  {
 +thr = create_non_tls_thread_data ();
 +  }
 +  return thr;
 }
 #endif

 diff --git a/libgomp/team.c b/libgomp/team.c
 index e6a6d8f..bf8bd4b 100644
 --- a/libgomp/team.c
 +++ b/libgomp/team.c
 @@ -927,6 +927,17 @@ initialize_team (void)
  gomp_fatal (could not create thread pool destructor.);
 }

 +#ifndef HAVE_TLS
 +struct gomp_thread *create_non_tls_thread_data (void)
 +{
 +  struct gomp_thread *thr = gomp_malloc (sizeof (struct gomp_thread));
 +  pthread_setspecific (gomp_tls_key, thr);
 +  gomp_sem_init (thr-release, 0);
 +
 +  return thr;
 +}
 +#endif
 +
 static void __attribute__((destructor))
 team_destructor (void)
 {
 ---


 Is it ok?


 Best regards,

 Varvara

Fix libgomp crash without TLS (PR42616)

2014-08-06 Thread Varvara Rainchik 
Hi,

The issue was firstly observed on NDK gcc since TLS is not supported
in Android bionic. I also see the same failure on gcc configured for
linux with –disable-tls, libgomp make check log:

FAIL: libgomp.c/affinity-1.c execution test
FAIL: libgomp.c/icv-2.c execution test
FAIL: libgomp.c/lock-3.c execution test
FAIL: libgomp.c/target-6.c execution test

These tests except affinity-1.c fail because gomp_thread () function
returns null pointer. I’ve found 2 bugs, first one addresses this
problem on Windows:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42616;
second one addresses original problem (for both cases, with and without TLS):
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36242.
Tests from both bugs fail with –disable-tls. So, it seems that non TLS
case was fixed just partially. The following patch solves the problem.
With this patch 3 tests from make check pass, affinity-1.c fails, but
I think it’s other non TLS problem.
Changes are bootstrapped and regtested on x86_64-linux.


2014-08-06  Varvara Rainchik  varvara.rainc...@intel.com

* libgomp.h (gomp_thread): For non TLS case create thread data.
* team.c (create_non_tls_thread_data): New function.


---
diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index a1482cc..cf3ec8f 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -479,9 +479,15 @@ static inline struct gomp_thread *gomp_thread (void)
}
#else
extern pthread_key_t gomp_tls_key;
+extern struct gomp_thread *create_non_tls_thread_data (void);
static inline struct gomp_thread *gomp_thread (void)
{
-  return pthread_getspecific (gomp_tls_key);
+  struct gomp_thread *thr = pthread_getspecific (gomp_tls_key);
+  if (thr == NULL)
+  {
+thr = create_non_tls_thread_data ();
+  }
+  return thr;
}
#endif

diff --git a/libgomp/team.c b/libgomp/team.c
index e6a6d8f..bf8bd4b 100644
--- a/libgomp/team.c
+++ b/libgomp/team.c
@@ -927,6 +927,17 @@ initialize_team (void)
 gomp_fatal (could not create thread pool destructor.);
}

+#ifndef HAVE_TLS
+struct gomp_thread *create_non_tls_thread_data (void)
+{
+  struct gomp_thread *thr = gomp_malloc (sizeof (struct gomp_thread));
+  pthread_setspecific (gomp_tls_key, thr);
+  gomp_sem_init (thr-release, 0);
+
+  return thr;
+}
+#endif
+
static void __attribute__((destructor))
team_destructor (void)
{
---


Is it ok?


Best regards,

Varvara