public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* C++ PATCH for target/54908 (thread_local vs emutls)
@ 2012-10-15  7:38 Jason Merrill
  2013-01-18 22:29 ` Jason Merrill
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2012-10-15  7:38 UTC (permalink / raw)
  To: gcc-patches List; +Cc: Jack Howarth, Dominique Dhumieres

[-- Attachment #1: Type: text/plain, Size: 379 bytes --]

This patch completely rewrites atexit_thread.cc to use 
__gthread_getspecific/setspecific rather than a thread_local variable to 
store the cleanup list, so that the list won't vanish if emutls_destroy 
runs first.  With this patch all the TLS tests pass on i686-pc-linux-gnu 
configured with --disable-tls to force use of emutls.

Tested x86_64-pc-linux-gnu, applying to trunk.

[-- Attachment #2: atexit-setspecific.patch --]
[-- Type: text/x-patch, Size: 5351 bytes --]

commit 8735583f399914e92f126d98c120aba317f6626a
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Oct 8 10:50:20 2012 -0400

    	PR target/54908
    	* libsupc++/atexit_thread.cc: Rewrite to keep the cleanup list
    	with get/setspecific.  Destroy the key on dlclose.

diff --git a/gcc/testsuite/g++.dg/tls/thread_local7g.C b/gcc/testsuite/g++.dg/tls/thread_local7g.C
index 6960598..3479aeb 100644
--- a/gcc/testsuite/g++.dg/tls/thread_local7g.C
+++ b/gcc/testsuite/g++.dg/tls/thread_local7g.C
@@ -3,7 +3,7 @@
 // { dg-require-alias }
 
 // The reference temp should be TLS, not normal data.
-// { dg-final { scan-assembler-not "\\.data" } }
+// { dg-final { scan-assembler-not "\\.data" { target tls_native } } }
 
 thread_local int&& ir = 42;
 
diff --git a/libstdc++-v3/libsupc++/atexit_thread.cc b/libstdc++-v3/libsupc++/atexit_thread.cc
index 5e47708..95bdcf0 100644
--- a/libstdc++-v3/libsupc++/atexit_thread.cc
+++ b/libstdc++-v3/libsupc++/atexit_thread.cc
@@ -27,109 +27,92 @@
 #include "bits/gthr.h"
 
 namespace {
-  // Data structure for the list of destructors: Singly-linked list
-  // of arrays.
-  class list
+  // One element in a singly-linked stack of cleanups.
+  struct elt
   {
-    struct elt
-    {
-      void *object;
-      void (*destructor)(void *);
-    };
-
-    static const int max_nelts = 32;
-
-    list *next;
-    int nelts;
-    elt array[max_nelts];
-
-    elt *allocate_elt();
-  public:
-    void run();
-    static void run(void *p);
-    int add_elt(void (*)(void *), void *);
+    void (*destructor)(void *);
+    void *object;
+    elt *next;
   };
 
-  // Return the address of an open slot.
-  list::elt *
-  list::allocate_elt()
-  {
-    if (nelts < max_nelts)
-      return &array[nelts++];
-    if (!next)
-      next = new (std::nothrow) list();
-    if (!next)
-      return 0;
-    return next->allocate_elt();
-  }
-
-  // Run all the cleanups in the list.
-  void
-  list::run()
-  {
-    for (int i = nelts - 1; i >= 0; --i)
-      array[i].destructor (array[i].object);
-    if (next)
-      next->run();
-  }
-
-  // Static version to use as a callback to __gthread_key_create.
-  void
-  list::run(void *p)
-  {
-    static_cast<list *>(p)->run();
-  }
-
-  // The list of cleanups is per-thread.
-  thread_local list first;
-
-  // The pthread data structures for actually running the destructors at
-  // thread exit are shared.  The constructor of the thread-local sentinel
-  // object in add_elt performs the initialization.
+  // Keep a per-thread list of cleanups in gthread_key storage.
   __gthread_key_t key;
-  __gthread_once_t once = __GTHREAD_ONCE_INIT;
-  void run_current () { first.run(); }
+  // But also support non-threaded mode.
+  elt *single_thread;
+
+  // Run the specified stack of cleanups.
+  void run (void *p)
+  {
+    elt *e = static_cast<elt*>(p);
+    for (; e; e = e->next)
+      e->destructor (e->object);
+  }
+
+  // Run the stack of cleanups for the current thread.
+  void run ()
+  {
+    void *e;
+    if (__gthread_active_p ())
+      e = __gthread_getspecific (key);
+    else
+      e = single_thread;
+    run (e);
+  }
+
+  // Initialize the key for the cleanup stack.  We use a static local for
+  // key init/delete rather than atexit so that delete is run on dlclose.
   void key_init() {
-    __gthread_key_create (&key, list::run);
+    struct key_s {
+      key_s() { __gthread_key_create (&key, run); }
+      ~key_s() { __gthread_key_delete (key); }
+    };
+    static key_s ks;
     // Also make sure the destructors are run by std::exit.
     // FIXME TLS cleanups should run before static cleanups and atexit
     // cleanups.
-    std::atexit (run_current);
+    std::atexit (run);
   }
-  struct sentinel
-  {
-    sentinel()
+}
+
+extern "C" int
+__cxxabiv1::__cxa_thread_atexit (void (*dtor)(void *), void *obj, void */*dso_handle*/)
+  _GLIBCXX_NOTHROW
+{
+  // Do this initialization once.
+  if (__gthread_active_p ())
     {
-      if (__gthread_active_p ())
+      // When threads are active use __gthread_once.
+      static __gthread_once_t once = __GTHREAD_ONCE_INIT;
+      __gthread_once (&once, key_init);
+    }
+  else
+    {
+      // And when threads aren't active use a static local guard.
+      static bool queued;
+      if (!queued)
 	{
-	  __gthread_once (&once, key_init);
-	  __gthread_setspecific (key, &first);
+	  queued = true;
+	  std::atexit (run);
 	}
-      else
-	std::atexit (run_current);
     }
-  };
 
-  // Actually insert an element.
-  int
-  list::add_elt(void (*dtor)(void *), void *obj)
-  {
-    thread_local sentinel s;
-    elt *e = allocate_elt ();
-    if (!e)
-      return -1;
-    e->object = obj;
-    e->destructor = dtor;
-    return 0;
-  }
-}
+  elt *first;
+  if (__gthread_active_p ())
+    first = static_cast<elt*>(__gthread_getspecific (key));
+  else
+    first = single_thread;
+
+  elt *new_elt = new (std::nothrow) elt;
+  if (!new_elt)
+    return -1;
+  new_elt->destructor = dtor;
+  new_elt->object = obj;
+  new_elt->next = first;
+
+  if (__gthread_active_p ())
+    __gthread_setspecific (key, new_elt);
+  else
+    single_thread = new_elt;
 
-namespace __cxxabiv1
-{
-  extern "C" int
-  __cxa_thread_atexit (void (*dtor)(void *), void *obj, void */*dso_handle*/)
-    _GLIBCXX_NOTHROW
-  {
-    return first.add_elt (dtor, obj);
-  }
+  return 0;
 }

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: C++ PATCH for target/54908 (thread_local vs emutls)
@ 2013-01-19 13:25 Dominique Dhumieres
  2013-01-19 15:35 ` Jason Merrill
  0 siblings, 1 reply; 6+ messages in thread
From: Dominique Dhumieres @ 2013-01-19 13:25 UTC (permalink / raw)
  To: gcc-patches; +Cc: howarth, jason

> > FAIL: g++.dg/tls/thread_local-wrap3.C scan-assembler _ZTH1i
> > FAIL: g++.dg/gomp/tls-wrap3.C -std=c++98  scan-assembler _ZTH1i
> > FAIL: g++.dg/gomp/tls-wrap3.C -std=c++11  scan-assembler _ZTH1i
>
> Ah, yes; those are specifically testing for the aliases that we can't 
> generate on darwin. I'll add dg-require-alias to those tests. ...

These tests still fail on x86_64-apple-darwin10 after bootstrapping 
a clean revision 195310.

Dominique

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-01-19 15:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-15  7:38 C++ PATCH for target/54908 (thread_local vs emutls) Jason Merrill
2013-01-18 22:29 ` Jason Merrill
2013-01-19  2:20   ` Jack Howarth
2013-01-19  5:16     ` Jason Merrill
2013-01-19 13:25 Dominique Dhumieres
2013-01-19 15:35 ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).