public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [trans-mem] Beginning of refactoring
@ 2011-05-25 22:45 Torvald Riegel
  2011-06-29 21:22 ` Torvald Riegel
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Torvald Riegel @ 2011-05-25 22:45 UTC (permalink / raw)
  To: GCC Patches

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

Here's the beginning of a refactoring aimed at being able to merge more
TM algorithms later on.

Patch 1: Just a straightfoward rename to make it clear that we're
dispatching on the level of ABI calls, not internals.

Patch 2: _ITM_dropReferences is not sufficiently defined in the ABI. It
seems to target some form of open nesting for txnal wrappers, but the
prose in the ABI specification is unclear. Thus, disable this for now
(aka fatal runtime error), and expect the related tests to fail. Pick it
up again once that the ABI has been improved and the use cases are
clear.

Patch 3: The actual change in how ABI calls are dispatched. Also,
removed method-readonly (broken, will in a similar form reappear in the
family of globallock-based algorithms), and disabled method-wbetl (needs
larger refactoring, will be revived/remerged later).

Ok for branch?


Coarse roadmap of planned future changes:

1) Improve serial-mode performance, which also gives a base for the
implementation of privatization.
2) The globallock family of algorithms (single versioned lock, with undo
or redo logging), and eager or lazy acquisition of the lock on writes.
3) Privatization safety for this globallock (includes user actions and
malloc/free commits).
4) The multilock family of algorithms (variants of LSA).

Other things: More cleanup in the ABI. Documentation.


Torvald

[-- Attachment #2: patch1 --]
[-- Type: text/plain, Size: 17494 bytes --]

commit 9f1ff3be017e7d2e4ad57669de95a9e61c110146
Author: Torvald Riegel <triegel@redhat.com>
Date:   Mon May 23 12:40:34 2011 +0200

    rename gtm_disp to abi_disp

diff --git a/libitm/alloc_c.cc b/libitm/alloc_c.cc
index 4160000..66d1109 100644
--- a/libitm/alloc_c.cc
+++ b/libitm/alloc_c.cc
@@ -64,7 +64,7 @@ void ITM_REGPARM
 _ITM_dropReferences (void *ptr, size_t len)
 {
   gtm_transaction *tx = gtm_tx();
-  if (!gtm_disp()->trydropreference (ptr, len))
+  if (!abi_disp()->trydropreference (ptr, len))
     tx->restart (RESTART_VALIDATE_READ);
   tx->drop_references_local (ptr, len);
   tx->drop_references_allocations (ptr);
diff --git a/libitm/barrier.tpl b/libitm/barrier.tpl
index 50bd89b..f74fcbc 100644
--- a/libitm/barrier.tpl
+++ b/libitm/barrier.tpl
@@ -30,12 +30,12 @@ namespace {
 using namespace GTM;
 
 template<typename T>
-T do_read (const T *ptr, gtm_dispatch::lock_type lock)
+T do_read (const T *ptr, abi_dispatch::lock_type lock)
 {
   //
   // Find the cacheline that holds the current value of *PTR.
   //
-  gtm_dispatch *disp = gtm_disp();
+  abi_dispatch *disp = abi_disp();
   uintptr_t iptr = reinterpret_cast<uintptr_t>(ptr);
   // Normalize PTR by chopping off the bottom bits so we can search
   // for PTR in the cacheline hash.
@@ -95,19 +95,19 @@ T do_read (const T *ptr, gtm_dispatch::lock_type lock)
 }
 
 template<typename T>
-void do_write (T *ptr, T val, gtm_dispatch::lock_type lock)
+void do_write (T *ptr, T val, abi_dispatch::lock_type lock)
 {
   // Note: See comments for do_read() above for hints on this
   // function.  Ideally we should abstract out a lot out of these two
   // functions, and avoid all this duplication.
 
-  gtm_dispatch *disp = gtm_disp();
+  abi_dispatch *disp = abi_disp();
   uintptr_t iptr = reinterpret_cast<uintptr_t>(ptr);
   uintptr_t iline = iptr & -CACHELINE_SIZE;
   uintptr_t iofs = iptr & (CACHELINE_SIZE - 1);
   gtm_cacheline *pline = reinterpret_cast<gtm_cacheline *>(iline);
   gtm_cacheline_mask m = ((gtm_cacheline_mask)2 << (sizeof(T) - 1)) - 1;
-  gtm_dispatch::mask_pair pair = disp->write_lock(pline, lock);
+  abi_dispatch::mask_pair pair = disp->write_lock(pline, lock);
 
   ptr = reinterpret_cast<T *>(&pair.line->b[iofs]);
 
@@ -129,7 +129,7 @@ void do_write (T *ptr, T val, gtm_dispatch::lock_type lock)
   else
     {
       *pair.mask |= m << iofs;
-      gtm_dispatch::mask_pair pair2 = disp->write_lock(pline + 1, lock);
+      abi_dispatch::mask_pair pair2 = disp->write_lock(pline + 1, lock);
 
       uintptr_t ileft = CACHELINE_SIZE - iofs;
       *pair2.mask |= m >> ileft;
@@ -151,13 +151,13 @@ void do_write (T *ptr, T val, gtm_dispatch::lock_type lock)
 #define ITM_READ(T, LOCK)						\
   _ITM_TYPE_##T ITM_REGPARM _ITM_##LOCK##T (const _ITM_TYPE_##T *ptr)	\
   {									\
-    return do_read (ptr, gtm_dispatch::LOCK);				\
+    return do_read (ptr, abi_dispatch::LOCK);				\
   }
 
 #define ITM_WRITE(T, LOCK)						\
   void ITM_REGPARM _ITM_##LOCK##T (_ITM_TYPE_##T *ptr, _ITM_TYPE_##T val) \
   {									\
-    do_write (ptr, val, gtm_dispatch::LOCK);				\
+    do_write (ptr, val, abi_dispatch::LOCK);				\
   }
 
 #define ITM_BARRIERS(T)		\
diff --git a/libitm/beginend.cc b/libitm/beginend.cc
index f9ad09a..37e23e2 100644
--- a/libitm/beginend.cc
+++ b/libitm/beginend.cc
@@ -94,7 +94,7 @@ GTM::gtm_transaction::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb)
   static const _ITM_transactionId_t tid_block_size = 1 << 16;
 
   gtm_transaction *tx;
-  gtm_dispatch *disp;
+  abi_dispatch *disp;
   uint32_t ret;
 
   gtm_thread *thr = setup_gtm_thr ();
@@ -154,7 +154,7 @@ GTM::gtm_transaction::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb)
       ret = a_runInstrumentedCode | a_saveLiveVariables;
     }
 
-  set_gtm_disp (disp);
+  set_abi_disp (disp);
 
   return ret;
 }
@@ -162,7 +162,7 @@ GTM::gtm_transaction::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb)
 void
 GTM::gtm_transaction::rollback ()
 {
-  gtm_disp()->rollback ();
+  abi_disp()->rollback ();
   rollback_local ();
 
   free_actions (&this->commit_actions);
@@ -202,7 +202,7 @@ _ITM_abortTransaction (_ITM_abortReason reason)
     abort ();
 
   tx->rollback ();
-  gtm_disp()->fini ();
+  abi_disp()->fini ();
 
   if (tx->state & gtm_transaction::STATE_SERIAL)
     gtm_transaction::serial_lock.write_unlock ();
@@ -218,7 +218,7 @@ _ITM_abortTransaction (_ITM_abortReason reason)
 bool
 GTM::gtm_transaction::trycommit ()
 {
-  if (gtm_disp()->trycommit ())
+  if (abi_disp()->trycommit ())
     {
       commit_local ();
       free_actions (&this->undo_actions);
@@ -234,7 +234,7 @@ GTM::gtm_transaction::trycommit_and_finalize ()
 {
   if ((this->state & gtm_transaction::STATE_ABORTING) || trycommit ())
     {
-      gtm_disp()->fini ();
+      abi_disp()->fini ();
       set_gtm_tx (this->prev);
       delete this;
       if (this->state & gtm_transaction::STATE_SERIAL)
diff --git a/libitm/config/generic/tls.h b/libitm/config/generic/tls.h
index 145375f..82d7c66 100644
--- a/libitm/config/generic/tls.h
+++ b/libitm/config/generic/tls.h
@@ -38,7 +38,7 @@ struct gtm_thread
 #ifndef HAVE_ARCH_GTM_THREAD_DISP
   // The dispatch table for the STM implementation currently in use.  Elided
   // if the target provides some efficient mechanism for storing this.
-  gtm_dispatch *disp;
+  abi_dispatch *disp;
 #endif
 
   // The maximum number of free gtm_transaction structs to be kept.
@@ -87,8 +87,8 @@ static inline void set_gtm_tx(gtm_transaction *x) { gtm_thr()->tx = x; }
 #ifndef HAVE_ARCH_GTM_THREAD_DISP
 // If the target does not provide optimized access to the currently
 // active dispatch table, simply access via GTM_THR.
-static inline gtm_dispatch * gtm_disp() { return gtm_thr()->disp; }
-static inline void set_gtm_disp(gtm_dispatch *x) { gtm_thr()->disp = x; }
+static inline abi_dispatch * abi_disp() { return gtm_thr()->disp; }
+static inline void set_abi_disp(abi_dispatch *x) { gtm_thr()->disp = x; }
 #endif
 
 } // namespace GTM
diff --git a/libitm/config/x86/tls.h b/libitm/config/x86/tls.h
index ef51d33..712b1c2 100644
--- a/libitm/config/x86/tls.h
+++ b/libitm/config/x86/tls.h
@@ -88,14 +88,14 @@ static inline void set_gtm_tx(struct gtm_transaction *x)
   asm volatile (SEG_WRITE(11) : : "r"(x));
 }
 
-static inline struct gtm_dispatch *gtm_disp(void)
+static inline struct abi_dispatch *abi_disp(void)
 {
-  struct gtm_dispatch *r;
+  struct abi_dispatch *r;
   asm (SEG_DECODE_READ(12) : "=r"(r));
   return r;
 }
 
-static inline void set_gtm_disp(struct gtm_dispatch *x)
+static inline void set_abi_disp(struct abi_dispatch *x)
 {
   void *scratch;
   asm volatile (SEG_ENCODE_WRITE(12) : "=r"(scratch) : "0"(x));
diff --git a/libitm/libitm_i.h b/libitm/libitm_i.h
index 57f18c0..3291f1b 100644
--- a/libitm/libitm_i.h
+++ b/libitm/libitm_i.h
@@ -86,7 +86,7 @@ extern void * xrealloc (void *p, size_t s) __attribute__((malloc, nothrow));
 namespace GTM HIDDEN {
 
 // A dispatch table parameterizes the implementation of the STM.
-struct gtm_dispatch
+struct abi_dispatch
 {
  public:
   enum lock_type { NOLOCK, R, RaR, RaW, RfW, W, WaR, WaW };
@@ -102,8 +102,8 @@ struct gtm_dispatch
 
  private:
   // Disallow copies
-  gtm_dispatch(const gtm_dispatch &) = delete;
-  gtm_dispatch& operator=(const gtm_dispatch &) = delete;
+  abi_dispatch(const abi_dispatch &) = delete;
+  abi_dispatch& operator=(const abi_dispatch &) = delete;
 
  public:
   // The default version of these is pass-through.  This merely gives the
@@ -129,7 +129,7 @@ struct gtm_dispatch
  protected:
   const bool m_read_only;
   const bool m_write_through;
-  gtm_dispatch(bool ro, bool wt) : m_read_only(ro), m_write_through(wt) { }
+  abi_dispatch(bool ro, bool wt) : m_read_only(ro), m_write_through(wt) { }
 
   static gtm_cacheline_mask mask_sink;
 };
@@ -287,9 +287,9 @@ extern void GTM_error (const char *fmt, ...)
 extern void GTM_fatal (const char *fmt, ...)
 	__attribute__((noreturn, format (printf, 1, 2)));
 
-extern gtm_dispatch *dispatch_wbetl();
-extern gtm_dispatch *dispatch_readonly();
-extern gtm_dispatch *dispatch_serial();
+extern abi_dispatch *dispatch_wbetl();
+extern abi_dispatch *dispatch_readonly();
+extern abi_dispatch *dispatch_serial();
 
 extern gtm_cacheline_mask gtm_mask_stack(gtm_cacheline *, gtm_cacheline_mask);
 
diff --git a/libitm/memcpy.cc b/libitm/memcpy.cc
index bbba938..f139df5 100644
--- a/libitm/memcpy.cc
+++ b/libitm/memcpy.cc
@@ -28,9 +28,9 @@ using namespace GTM;
 
 static void
 do_memcpy (uintptr_t idst, uintptr_t isrc, size_t size,
-	   gtm_dispatch::lock_type W, gtm_dispatch::lock_type R)
+	   abi_dispatch::lock_type W, abi_dispatch::lock_type R)
 {
-  gtm_dispatch *disp = gtm_disp();
+  abi_dispatch *disp = abi_disp();
   // The position in the destination cacheline where *IDST starts.
   uintptr_t dofs = idst & (CACHELINE_SIZE - 1);
   // The position in the source cacheline where *ISRC starts.
@@ -40,7 +40,7 @@ do_memcpy (uintptr_t idst, uintptr_t isrc, size_t size,
   gtm_cacheline *dst
     = reinterpret_cast<gtm_cacheline *>(idst & -CACHELINE_SIZE);
   const gtm_cacheline *sline;
-  gtm_dispatch::mask_pair dpair;
+  abi_dispatch::mask_pair dpair;
 
   if (size == 0)
     return;
@@ -177,12 +177,12 @@ do_memcpy (uintptr_t idst, uintptr_t isrc, size_t size,
 
 static void
 do_memmove (uintptr_t idst, uintptr_t isrc, size_t size,
-	    gtm_dispatch::lock_type W, gtm_dispatch::lock_type R)
+	    abi_dispatch::lock_type W, abi_dispatch::lock_type R)
 {
-  gtm_dispatch *disp = gtm_disp();
+  abi_dispatch *disp = abi_disp();
   uintptr_t dleft, sleft, sofs, dofs;
   const gtm_cacheline *sline;
-  gtm_dispatch::mask_pair dpair;
+  abi_dispatch::mask_pair dpair;
   
   if (size == 0)
     return;
@@ -194,13 +194,13 @@ do_memmove (uintptr_t idst, uintptr_t isrc, size_t size,
   if (__builtin_expect (idst == isrc, 0))
     {
       /* If the write lock is already acquired, nothing to do.  */
-      if (W == gtm_dispatch::WaW)
+      if (W == abi_dispatch::WaW)
 	return;
       /* If the destination is protected, acquire a write lock.  */
-      if (W != gtm_dispatch::NOLOCK)
-	R = gtm_dispatch::RfW;
+      if (W != abi_dispatch::NOLOCK)
+	R = abi_dispatch::RfW;
       /* Notice serial mode, where we don't acquire locks at all.  */
-      if (R == gtm_dispatch::NOLOCK)
+      if (R == abi_dispatch::NOLOCK)
 	return;
 
       idst = isrc + size;
@@ -337,12 +337,12 @@ do_memmove (uintptr_t idst, uintptr_t isrc, size_t size,
 void ITM_REGPARM _ITM_memcpy##NAME(void *dst, const void *src, size_t size)  \
 {									     \
   do_memcpy ((uintptr_t)dst, (uintptr_t)src, size,			     \
-	     gtm_dispatch::WRITE, gtm_dispatch::READ);			     \
+	     abi_dispatch::WRITE, abi_dispatch::READ);			     \
 }									     \
 void ITM_REGPARM _ITM_memmove##NAME(void *dst, const void *src, size_t size) \
 {									     \
   do_memmove ((uintptr_t)dst, (uintptr_t)src, size,			     \
-	      gtm_dispatch::WRITE, gtm_dispatch::READ);			     \
+	      abi_dispatch::WRITE, abi_dispatch::READ);			     \
 }
 
 ITM_MEM_DEF(RnWt,	NOLOCK,		W)
diff --git a/libitm/memset.cc b/libitm/memset.cc
index 39ee72b..40cab1a 100644
--- a/libitm/memset.cc
+++ b/libitm/memset.cc
@@ -27,11 +27,11 @@
 using namespace GTM;
 
 static void
-do_memset(uintptr_t idst, int c, size_t size, gtm_dispatch::lock_type W)
+do_memset(uintptr_t idst, int c, size_t size, abi_dispatch::lock_type W)
 {
-  gtm_dispatch *disp = gtm_disp();
+  abi_dispatch *disp = abi_disp();
   uintptr_t dofs = idst & (CACHELINE_SIZE - 1);
-  gtm_dispatch::mask_pair dpair;
+  abi_dispatch::mask_pair dpair;
   gtm_cacheline *dst
     = reinterpret_cast<gtm_cacheline *>(idst & -CACHELINE_SIZE);
 
@@ -70,7 +70,7 @@ do_memset(uintptr_t idst, int c, size_t size, gtm_dispatch::lock_type W)
 #define ITM_MEM_DEF(WRITE) \
 void ITM_REGPARM _ITM_memset##WRITE(void *dst, int c, size_t size)	\
 {									\
-  do_memset ((uintptr_t)dst, c, size, gtm_dispatch::WRITE);		\
+  do_memset ((uintptr_t)dst, c, size, abi_dispatch::WRITE);		\
 }
 
 ITM_MEM_DEF(W)
diff --git a/libitm/method-readonly.cc b/libitm/method-readonly.cc
index 5f33305..ed573e2 100644
--- a/libitm/method-readonly.cc
+++ b/libitm/method-readonly.cc
@@ -28,7 +28,7 @@ namespace {
 
 using namespace GTM;
 
-class readonly_dispatch : public gtm_dispatch
+class readonly_dispatch : public abi_dispatch
 {
  private:
   gtm_version m_start;
@@ -47,7 +47,7 @@ class readonly_dispatch : public gtm_dispatch
 
 inline
 readonly_dispatch::readonly_dispatch()
-  : gtm_dispatch(true, true), m_start(gtm_get_clock ())
+  : abi_dispatch(true, true), m_start(gtm_get_clock ())
 { }
 
 
@@ -70,14 +70,14 @@ readonly_dispatch::read_lock(const gtm_cacheline *line, lock_type lock)
     }
 }
 
-gtm_dispatch::mask_pair
+abi_dispatch::mask_pair
 readonly_dispatch::write_lock(gtm_cacheline *line, lock_type lock)
 {
   switch (lock)
     {
     case NOLOCK:
       {
-	gtm_dispatch::mask_pair pair;
+	abi_dispatch::mask_pair pair;
 	pair.line = line;
 	pair.mask = &mask_sink;
 	return pair;
@@ -117,7 +117,7 @@ readonly_dispatch::fini ()
 
 } // anon namespace
 
-gtm_dispatch *
+abi_dispatch *
 GTM::dispatch_readonly ()
 {
   return new readonly_dispatch();
diff --git a/libitm/method-serial.cc b/libitm/method-serial.cc
index cdeccc6..b46de4d 100644
--- a/libitm/method-serial.cc
+++ b/libitm/method-serial.cc
@@ -26,23 +26,23 @@
 
 namespace GTM HIDDEN {
 
-gtm_cacheline_mask gtm_dispatch::mask_sink;
+gtm_cacheline_mask abi_dispatch::mask_sink;
 
 const gtm_cacheline *
-gtm_dispatch::read_lock(const gtm_cacheline *addr, lock_type)
+abi_dispatch::read_lock(const gtm_cacheline *addr, lock_type)
 {
   return addr;
 }
 
-gtm_dispatch::mask_pair
-gtm_dispatch::write_lock(gtm_cacheline *addr, lock_type)
+abi_dispatch::mask_pair
+abi_dispatch::write_lock(gtm_cacheline *addr, lock_type)
 {
   return mask_pair (addr, &mask_sink);
 }
 
 } // namespace GTM
 
-// Avoid a dependency on libstdc++ for the pure virtuals in gtm_dispatch.
+// Avoid a dependency on libstdc++ for the pure virtuals in abi_dispatch.
 extern "C" void HIDDEN
 __cxa_pure_virtual ()
 {
@@ -53,10 +53,10 @@ using namespace GTM;
 
 namespace {
 
-class serial_dispatch : public gtm_dispatch
+class serial_dispatch : public abi_dispatch
 {
  public:
-  serial_dispatch() : gtm_dispatch(false, true) { }
+  serial_dispatch() : abi_dispatch(false, true) { }
 
   // The read_lock and write_lock methods are implented by the base class.
 
@@ -71,7 +71,7 @@ class serial_dispatch : public gtm_dispatch
 
 static const serial_dispatch o_serial_dispatch;
 
-gtm_dispatch *
+abi_dispatch *
 GTM::dispatch_serial ()
 {
   return const_cast<serial_dispatch *>(&o_serial_dispatch);
@@ -82,7 +82,7 @@ GTM::dispatch_serial ()
 void
 GTM::gtm_transaction::serialirr_mode ()
 {
-  struct gtm_dispatch *disp = gtm_disp ();
+  struct abi_dispatch *disp = abi_disp ();
   bool need_restart = true;
 
   if (this->state & STATE_SERIAL)
@@ -111,7 +111,7 @@ GTM::gtm_transaction::serialirr_mode ()
   else
     {
       this->state |= (STATE_SERIAL | STATE_IRREVOCABLE);
-      set_gtm_disp (dispatch_serial ());
+      set_abi_disp (dispatch_serial ());
     }
 }
 
diff --git a/libitm/method-wbetl.cc b/libitm/method-wbetl.cc
index a0359a3..7ce317e 100644
--- a/libitm/method-wbetl.cc
+++ b/libitm/method-wbetl.cc
@@ -28,7 +28,7 @@ namespace {
 
 using namespace GTM;
 
-class wbetl_dispatch : public gtm_dispatch
+class wbetl_dispatch : public abi_dispatch
 {
  private:
   static const size_t RW_SET_SIZE = 4096;
@@ -394,7 +394,7 @@ wbetl_dispatch::read_lock (const gtm_cacheline *addr, lock_type ltype)
     }
 }
 
-gtm_dispatch::mask_pair
+abi_dispatch::mask_pair
 wbetl_dispatch::write_lock (gtm_cacheline *addr, lock_type ltype)
 {
   gtm_cacheline *line;
@@ -547,7 +547,7 @@ wbetl_dispatch::trydropreference (void *ptr, size_t size)
   gtm_cacheline *src
     = reinterpret_cast<gtm_cacheline *>(isrc & -CACHELINE_SIZE);
   unsigned char *dst = (unsigned char *)ptr;
-  gtm_dispatch::mask_pair pair;
+  abi_dispatch::mask_pair pair;
 
   // If we're trying to drop a reference, we should already have a
   // write lock on it.  If we don't have one, there's no work to do.
@@ -602,7 +602,7 @@ wbetl_dispatch::trydropreference (void *ptr, size_t size)
 
 
 wbetl_dispatch::wbetl_dispatch ()
-  : gtm_dispatch (false, false)
+  : abi_dispatch (false, false)
 {
   m_rset_entries = (r_entry *) xmalloc (RW_SET_SIZE * sizeof(r_entry));
   m_rset_nb_entries = 0;
@@ -621,7 +621,7 @@ wbetl_dispatch::wbetl_dispatch ()
 
 } // anon namespace
 
-gtm_dispatch *
+abi_dispatch *
 GTM::dispatch_wbetl ()
 {
   return new wbetl_dispatch ();
diff --git a/libitm/retry.cc b/libitm/retry.cc
index f602f54..a25b282 100644
--- a/libitm/retry.cc
+++ b/libitm/retry.cc
@@ -27,7 +27,7 @@
 void
 GTM::gtm_transaction::decide_retry_strategy (gtm_restart_reason r)
 {
-  struct gtm_dispatch *disp = gtm_disp ();
+  struct abi_dispatch *disp = abi_disp ();
 
   this->restart_reason[r]++;
   this->restart_total++;
@@ -61,7 +61,7 @@ GTM::gtm_transaction::decide_retry_strategy (gtm_restart_reason r)
       this->state = (STATE_SERIAL | STATE_IRREVOCABLE);
       disp->fini ();
       disp = dispatch_serial ();
-      set_gtm_disp (disp);
+      set_abi_disp (disp);
     }
   else
     {
@@ -72,7 +72,7 @@ GTM::gtm_transaction::decide_retry_strategy (gtm_restart_reason r)
 	    {
 	      disp->fini ();
 	      disp = dispatch_wbetl ();
-	      set_gtm_disp (disp);
+	      set_abi_disp (disp);
 	      return;
 	    }
 	}

[-- Attachment #3: patch2 --]
[-- Type: text/plain, Size: 2567 bytes --]

commit eaf0b413200a535cff13b0126ea51e0991f12ae1
Author: Torvald Riegel <triegel@redhat.com>
Date:   Tue May 24 18:30:28 2011 +0200

    Do not support _ITM_dropReferences anymore
    
    	* alloc_c.cc (_ITM_dropReferences): Don't support it anymore.
    	* testsuite/libitm.c++/dropref.C: _ITM_dropReferences is expected to fail.
    	* testsuite/libitm.c/dropref-2.c: Same.
    	* testsuite/libitm.c/dropref.c: Same.

diff --git a/libitm/alloc_c.cc b/libitm/alloc_c.cc
index 66d1109..549335c 100644
--- a/libitm/alloc_c.cc
+++ b/libitm/alloc_c.cc
@@ -1,4 +1,4 @@
-/* Copyright (C) 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU Transactional Memory Library (libitm).
@@ -63,11 +63,15 @@ __attribute__((transaction_pure))
 void ITM_REGPARM
 _ITM_dropReferences (void *ptr, size_t len)
 {
-  gtm_transaction *tx = gtm_tx();
-  if (!abi_disp()->trydropreference (ptr, len))
-    tx->restart (RESTART_VALIDATE_READ);
-  tx->drop_references_local (ptr, len);
-  tx->drop_references_allocations (ptr);
+  // The semantics of _ITM_dropReferences are not sufficiently defined in the
+  // ABI specification, so it does not make sense to support it right now. See
+  // the libitm documentation for details.
+  GTM_fatal("_ITM_dropReferences is not supported");
+//  gtm_transaction *tx = gtm_tx();
+//  if (!abi_disp()->trydropreference (ptr, len))
+//    tx->restart (RESTART_VALIDATE_READ);
+//  tx->drop_references_local (ptr, len);
+//  tx->drop_references_allocations (ptr);
 }
 
 } // extern "C"
diff --git a/libitm/testsuite/libitm.c++/dropref.C b/libitm/testsuite/libitm.c++/dropref.C
index 92c8812..ee4f1bb 100644
--- a/libitm/testsuite/libitm.c++/dropref.C
+++ b/libitm/testsuite/libitm.c++/dropref.C
@@ -1,3 +1,4 @@
+/* { dg-xfail-run-if "unsupported" { *-*-* } } */
 #include <libitm.h>
 
 char *pp;
diff --git a/libitm/testsuite/libitm.c/dropref-2.c b/libitm/testsuite/libitm.c/dropref-2.c
index ddd4eae..2386b18 100644
--- a/libitm/testsuite/libitm.c/dropref-2.c
+++ b/libitm/testsuite/libitm.c/dropref-2.c
@@ -1,3 +1,4 @@
+/* { dg-xfail-run-if "unsupported" { *-*-* } } */
 #include <stdlib.h>
 #include <libitm.h>
 
diff --git a/libitm/testsuite/libitm.c/dropref.c b/libitm/testsuite/libitm.c/dropref.c
index 92c8812..ee4f1bb 100644
--- a/libitm/testsuite/libitm.c/dropref.c
+++ b/libitm/testsuite/libitm.c/dropref.c
@@ -1,3 +1,4 @@
+/* { dg-xfail-run-if "unsupported" { *-*-* } } */
 #include <libitm.h>
 
 char *pp;

[-- Attachment #4: patch3 --]
[-- Type: text/plain, Size: 37188 bytes --]

commit e3454b7e33666b20866ea5410d791cbf79c297f0
Author: Torvald Riegel <triegel@redhat.com>
Date:   Wed May 25 17:21:53 2011 +0200

    New dispatch class/functions. Remove/disable old methods (readonly, wbetl).
    
    	* libitm_i.h: Move parts to common.h and dispatch.h.
    	* common.h: New file.
    	* dispatch.h: New file, new dispatch class.
    	Rename GTM::abi_dispatch::lock_type to ls_modifier.
    	RenameGTM::abi_dispatch::NOLOCK to NONTXNAL.
    	* beginend.cc (GTM::gtm_transaction::begin_transaction): Delegate mode
    	decision to retry.cc.
    	* retry.cc (GTM::gtm_transaction::decide_retry_strategy): Use serial mode
    	only.
    	(GTM::gtm_transaction::decide_begin_dispatch): Same.
    	* method-serial.cc: Adapt to new dispatch. Add serial mode with undo
    	logging.
    	* barrier.cc: Use new barriers definitions.
    	* config/x86/x86_sse.cc: Same.
    	* config/x86/x86_avx.cc: Same.
    	* Makefile.am: Don't build readonly and wbetl methods, memset.cc and
    	memcpy.cc.
    	* Makefile.in: Rebuild.
    	* method-readonly.cc: Remove.
    	* method-wbetl.cc: Rename GTM::abi_dispatch::lock_type to ls_modifier.
    	Rename GTM::abi_dispatch::NOLOCK to NONTXNAL.

diff --git a/libitm/Makefile.am b/libitm/Makefile.am
index 8ad4e5f..bf0180d 100644
--- a/libitm/Makefile.am
+++ b/libitm/Makefile.am
@@ -41,9 +41,9 @@ libitm_la_LDFLAGS = $(libitm_version_info) $(libitm_version_script) \
 
 libitm_la_SOURCES = \
 	aatree.cc alloc.cc alloc_c.cc alloc_cpp.cc barrier.cc beginend.cc \
-	clone.cc cacheline.cc cachepage.cc eh_cpp.cc local.cc memcpy.cc \
-	memset.cc query.cc retry.cc rwlock.cc useraction.cc util.cc \
-	sjlj.S tls.cc method-serial.cc method-readonly.cc method-wbetl.cc
+	clone.cc cacheline.cc cachepage.cc eh_cpp.cc local.cc \
+	query.cc retry.cc rwlock.cc useraction.cc util.cc \
+	sjlj.S tls.cc method-serial.cc
 
 if ARCH_X86
 libitm_la_SOURCES += x86_sse.cc x86_avx.cc
diff --git a/libitm/Makefile.in b/libitm/Makefile.in
index ad03c27..8fc3106 100644
--- a/libitm/Makefile.in
+++ b/libitm/Makefile.in
@@ -93,17 +93,15 @@ LTLIBRARIES = $(toolexeclib_LTLIBRARIES)
 libitm_la_LIBADD =
 am__libitm_la_SOURCES_DIST = aatree.cc alloc.cc alloc_c.cc \
 	alloc_cpp.cc barrier.cc beginend.cc clone.cc cacheline.cc \
-	cachepage.cc eh_cpp.cc local.cc memcpy.cc memset.cc query.cc \
+	cachepage.cc eh_cpp.cc local.cc query.cc \
 	retry.cc rwlock.cc useraction.cc util.cc sjlj.S tls.cc \
-	method-serial.cc method-readonly.cc method-wbetl.cc x86_sse.cc \
-	x86_avx.cc
+	method-serial.cc x86_sse.cc 86_avx.cc
 @ARCH_X86_TRUE@am__objects_1 = x86_sse.lo x86_avx.lo
 am_libitm_la_OBJECTS = aatree.lo alloc.lo alloc_c.lo alloc_cpp.lo \
 	barrier.lo beginend.lo clone.lo cacheline.lo cachepage.lo \
-	eh_cpp.lo local.lo memcpy.lo memset.lo query.lo retry.lo \
+	eh_cpp.lo local.lo query.lo retry.lo \
 	rwlock.lo useraction.lo util.lo sjlj.lo tls.lo \
-	method-serial.lo method-readonly.lo method-wbetl.lo \
-	$(am__objects_1)
+	method-serial.lo $(am__objects_1)
 libitm_la_OBJECTS = $(am_libitm_la_OBJECTS)
 DEFAULT_INCLUDES = -I.@am__isrc@
 depcomp = $(SHELL) $(top_srcdir)/../depcomp
@@ -342,6 +340,7 @@ AM_CPPFLAGS = $(addprefix -I, $(search_path))
 AM_CFLAGS = $(XCFLAGS)
 AM_CXXFLAGS = -std=gnu++0x -funwind-tables -fno-exceptions -fno-rtti \
 	$(XCFLAGS) $(abi_version)
+
 AM_CCASFLAGS = $(XCFLAGS)
 AM_LDFLAGS = $(XLDFLAGS) $(SECTION_LDFLAGS) $(OPT_LDFLAGS)
 toolexeclib_LTLIBRARIES = libitm.la
@@ -358,9 +357,9 @@ libitm_la_LDFLAGS = $(libitm_version_info) $(libitm_version_script) \
 
 libitm_la_SOURCES = aatree.cc alloc.cc alloc_c.cc alloc_cpp.cc \
 	barrier.cc beginend.cc clone.cc cacheline.cc cachepage.cc \
-	eh_cpp.cc local.cc memcpy.cc memset.cc query.cc retry.cc \
+	eh_cpp.cc local.cc query.cc retry.cc \
 	rwlock.cc useraction.cc util.cc sjlj.S tls.cc method-serial.cc \
-	method-readonly.cc method-wbetl.cc $(am__append_1)
+	$(am__append_1)
 all: config.h
 	$(MAKE) $(AM_MAKEFLAGS) all-recursive
 
@@ -470,11 +469,7 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/clone.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/eh_cpp.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/local.Plo@am__quote@
-@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/memcpy.Plo@am__quote@
-@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/memset.Plo@am__quote@
-@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/method-readonly.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/method-serial.Plo@am__quote@
-@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/method-wbetl.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/query.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/retry.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/rwlock.Plo@am__quote@
diff --git a/libitm/barrier.cc b/libitm/barrier.cc
index 52e5a08..948dd39 100644
--- a/libitm/barrier.cc
+++ b/libitm/barrier.cc
@@ -1,4 +1,4 @@
-/* Copyright (C) 2008, 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2008, 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU Transactional Memory Library (libitm).
@@ -23,15 +23,8 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include "libitm_i.h"
-#include "barrier.tpl"
-
-ITM_BARRIERS(U1)
-ITM_BARRIERS(U2)
-ITM_BARRIERS(U4)
-ITM_BARRIERS(U8)
-ITM_BARRIERS(F)
-ITM_BARRIERS(D)
-ITM_BARRIERS(E)
-ITM_BARRIERS(CF)
-ITM_BARRIERS(CD)
-ITM_BARRIERS(CE)
+
+using namespace GTM;
+
+CREATE_DISPATCH_FUNCTIONS(GTM::abi_disp()->_, )
+
diff --git a/libitm/beginend.cc b/libitm/beginend.cc
index 37e23e2..7ed5073 100644
--- a/libitm/beginend.cc
+++ b/libitm/beginend.cc
@@ -1,4 +1,4 @@
-/* Copyright (C) 2008, 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2008, 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU Transactional Memory Library (libitm).
@@ -128,13 +128,27 @@ GTM::gtm_transaction::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb)
 
   set_gtm_tx (tx);
 
+  // ??? pr_undoLogCode is not properly defined in the ABI. Are barriers
+  // omitted because they are not necessary (e.g., a transaction on thread-
+  // local data) or because the compiler thinks that some kind of global
+  // synchronization might perform better?
+  if (unlikely(prop & pr_undoLogCode))
+    GTM_fatal("pr_undoLogCode not supported");
+
   if ((prop & pr_doesGoIrrevocable) || !(prop & pr_instrumentedCode))
+    tx->state = (STATE_SERIAL | STATE_IRREVOCABLE);
+
+  else
+    disp = tx->decide_begin_dispatch ();
+
+  if (tx->state & STATE_SERIAL)
     {
       serial_lock.write_lock ();
 
-      tx->state = (STATE_SERIAL | STATE_IRREVOCABLE);
-
-      disp = dispatch_serial ();
+      if (tx->state & STATE_IRREVOCABLE)
+        disp = dispatch_serialirr ();
+      else
+        disp = dispatch_serial ();
 
       ret = a_runUninstrumentedCode;
       if ((prop & pr_multiwayCode) == pr_instrumentedCode)
@@ -143,14 +157,6 @@ GTM::gtm_transaction::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb)
   else
     {
       serial_lock.read_lock ();
-
-      // ??? Probably want some environment variable to choose the default
-      // STM implementation once we have more than one implemented.
-      if (prop & pr_readOnly)
-	disp = dispatch_readonly ();
-      else
-	disp = dispatch_wbetl ();
-
       ret = a_runInstrumentedCode | a_saveLiveVariables;
     }
 
diff --git a/libitm/common.h b/libitm/common.h
new file mode 100644
index 0000000..d94ca94
--- /dev/null
+++ b/libitm/common.h
@@ -0,0 +1,43 @@
+/* Copyright (C) 2008, 2009 Free Software Foundation, Inc.
+   Contributed by Richard Henderson <rth@redhat.com>.
+
+   This file is part of the GNU Transactional Memory Library (libitm).
+
+   Libitm is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   Libitm is distributed in the hope that it will be useful, but WITHOUT ANY
+   WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+   FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+   more details.
+
+   Under Section 7 of GPL version 3, you are granted additional
+   permissions described in the GCC Runtime Library Exception, version
+   3.1, as published by the Free Software Foundation.
+
+   You should have received a copy of the GNU General Public License and
+   a copy of the GCC Runtime Library Exception along with this program;
+   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* The following are internal implementation functions and definitions.
+   To distinguish them from those defined by the Intel ABI, they all
+   begin with GTM/gtm.  */
+
+#ifndef COMMON_H
+#define COMMON_H 1
+
+#define UNUSED		__attribute__((unused))
+#define ALWAYS_INLINE	__attribute__((always_inline))
+#ifdef HAVE_ATTRIBUTE_VISIBILITY
+# define HIDDEN		__attribute__((visibility("hidden")))
+#else
+# define HIDDEN
+#endif
+
+#define likely(X)	__builtin_expect((X) != 0, 1)
+#define unlikely(X)	__builtin_expect((X), 0)
+
+#endif // COMMON_H
diff --git a/libitm/config/x86/x86_avx.cc b/libitm/config/x86/x86_avx.cc
index ee073d5..f81e3b8 100644
--- a/libitm/config/x86/x86_avx.cc
+++ b/libitm/config/x86/x86_avx.cc
@@ -1,4 +1,4 @@
-/* Copyright (C) 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU Transactional Memory Library (libitm).
@@ -23,14 +23,16 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include "libitm_i.h"
-#include "barrier.tpl"
+#include "dispatch.h"
 
-ITM_BARRIERS(M256)
+// ??? Use memcpy for now, until we have figured out how to best instantiate
+// these loads/stores.
+CREATE_DISPATCH_FUNCTIONS_T_MEMCPY(M256, GTM::abi_disp()->_, )
 
 void ITM_REGPARM
 _ITM_LM256 (const _ITM_TYPE_M256 *ptr)
 {
-  GTM_LB (ptr, sizeof (*ptr));
+  GTM::GTM_LB (ptr, sizeof (*ptr));
 }
 
 // Helpers for re-aligning two 128-bit values.
diff --git a/libitm/config/x86/x86_sse.cc b/libitm/config/x86/x86_sse.cc
index 5bb9766..a8585e4 100644
--- a/libitm/config/x86/x86_sse.cc
+++ b/libitm/config/x86/x86_sse.cc
@@ -1,4 +1,4 @@
-/* Copyright (C) 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU Transactional Memory Library (libitm).
@@ -23,21 +23,23 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include "libitm_i.h"
-#include "barrier.tpl"
+#include "dispatch.h"
 
-ITM_BARRIERS(M64)
-ITM_BARRIERS(M128)
+// ??? Use memcpy for now, until we have figured out how to best instantiate
+// these loads/stores.
+CREATE_DISPATCH_FUNCTIONS_T_MEMCPY(M64, GTM::abi_disp()->_, )
+CREATE_DISPATCH_FUNCTIONS_T_MEMCPY(M128, GTM::abi_disp()->_, )
 
 void ITM_REGPARM
 _ITM_LM64 (const _ITM_TYPE_M64 *ptr)
 {
-  GTM_LB (ptr, sizeof (*ptr));
+  GTM::GTM_LB (ptr, sizeof (*ptr));
 }
 
 void ITM_REGPARM
 _ITM_LM128 (const _ITM_TYPE_M128 *ptr)
 {
-  GTM_LB (ptr, sizeof (*ptr));
+  GTM::GTM_LB (ptr, sizeof (*ptr));
 }
 
 // Helpers for re-aligning two 128-bit values.
diff --git a/libitm/dispatch.h b/libitm/dispatch.h
new file mode 100644
index 0000000..23a1453
--- /dev/null
+++ b/libitm/dispatch.h
@@ -0,0 +1,287 @@
+/* Copyright (C) 2011 Free Software Foundation, Inc.
+   Contributed by Torvald Riegel <triegel@redhat.com>.
+
+   This file is part of the GNU Transactional Memory Library (libitm).
+
+   Libitm is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   Libitm is distributed in the hope that it will be useful, but WITHOUT ANY
+   WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+   FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+   more details.
+
+   Under Section 7 of GPL version 3, you are granted additional
+   permissions described in the GCC Runtime Library Exception, version
+   3.1, as published by the Free Software Foundation.
+
+   You should have received a copy of the GNU General Public License and
+   a copy of the GCC Runtime Library Exception along with this program;
+   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef DISPATCH_H
+#define DISPATCH_H 1
+
+#include "libitm.h"
+#include "common.h"
+
+// Creates ABI load/store methods (can be made virtual or static using M).
+#define ITM_READ_M(T, LSMOD, M, M2)                                         \
+  M _ITM_TYPE_##T ITM_REGPARM _ITM_##LSMOD##T##M2 (const _ITM_TYPE_##T *ptr)\
+  {                                                                         \
+    return load(ptr, abi_dispatch::LSMOD);                                  \
+  }
+
+#define ITM_WRITE_M(T, LSMOD, M, M2)                         \
+  M void ITM_REGPARM _ITM_##LSMOD##T##M2 (_ITM_TYPE_##T *ptr,\
+                                         _ITM_TYPE_##T val)  \
+  {                                                          \
+    store(ptr, val, abi_dispatch::LSMOD);                    \
+  }
+
+// Creates ABI load/store methods for all load/store modifiers for a particular
+// type.
+#define CREATE_DISPATCH_METHODS_T(T, M, M2) \
+  ITM_READ_M(T, R, M, M2)                \
+  ITM_READ_M(T, RaR, M, M2)              \
+  ITM_READ_M(T, RaW, M, M2)              \
+  ITM_READ_M(T, RfW, M, M2)              \
+  ITM_WRITE_M(T, W, M, M2)               \
+  ITM_WRITE_M(T, WaR, M, M2)             \
+  ITM_WRITE_M(T, WaW, M, M2)
+
+// Creates ABI load/store methods for all types.
+// See CREATE_DISPATCH_FUNCTIONS for comments.
+#define CREATE_DISPATCH_METHODS(M, M2)  \
+  CREATE_DISPATCH_METHODS_T (U1, M, M2) \
+  CREATE_DISPATCH_METHODS_T (U2, M, M2) \
+  CREATE_DISPATCH_METHODS_T (U4, M, M2) \
+  CREATE_DISPATCH_METHODS_T (U8, M, M2) \
+  CREATE_DISPATCH_METHODS_T (F, M, M2)  \
+  CREATE_DISPATCH_METHODS_T (D, M, M2)  \
+  CREATE_DISPATCH_METHODS_T (E, M, M2)  \
+  CREATE_DISPATCH_METHODS_T (CF, M, M2) \
+  CREATE_DISPATCH_METHODS_T (CD, M, M2) \
+  CREATE_DISPATCH_METHODS_T (CE, M, M2)
+
+// Creates memcpy/memmove/memset methods.
+#define CREATE_DISPATCH_METHODS_MEM()  \
+virtual void _memtransfer(void *dst, const void* src, size_t size,     \
+bool may_overlap, ls_modifier dst_mod, ls_modifier src_mod)           \
+{                                                                     \
+  memtransfer_static(dst, src, size, may_overlap, dst_mod, src_mod);  \
+}                                                                     \
+virtual void _memset(void *dst, int c, size_t size, ls_modifier mod)   \
+{                                                                     \
+  memset_static(dst, c, size, mod);                                   \
+}
+
+
+// Creates ABI load/store functions that can target either a class or an
+// object.
+#define ITM_READ(T, LSMOD, TARGET, M2)                                 \
+  _ITM_TYPE_##T ITM_REGPARM _ITM_##LSMOD##T (const _ITM_TYPE_##T *ptr) \
+  {                                                                    \
+    return TARGET##ITM_##LSMOD##T##M2(ptr);                            \
+  }
+
+#define ITM_WRITE(T, LSMOD, TARGET, M2)                                    \
+  void ITM_REGPARM _ITM_##LSMOD##T (_ITM_TYPE_##T *ptr, _ITM_TYPE_##T val) \
+  {                                                                        \
+    TARGET##ITM_##LSMOD##T##M2(ptr, val);                                  \
+  }
+
+// Creates ABI load/store functions for all load/store modifiers for a
+// particular type.
+#define CREATE_DISPATCH_FUNCTIONS_T(T, TARGET, M2) \
+  ITM_READ(T, R, TARGET, M2)                \
+  ITM_READ(T, RaR, TARGET, M2)              \
+  ITM_READ(T, RaW, TARGET, M2)              \
+  ITM_READ(T, RfW, TARGET, M2)              \
+  ITM_WRITE(T, W, TARGET, M2)               \
+  ITM_WRITE(T, WaR, TARGET, M2)             \
+  ITM_WRITE(T, WaW, TARGET, M2)
+
+// Creates ABI memcpy/memmove/memset functions.
+#define ITM_MEMTRANSFER_DEF(TARGET, M2, NAME, READ, WRITE) \
+void ITM_REGPARM _ITM_memcpy##NAME(void *dst, const void *src, size_t size)  \
+{                                                                            \
+  TARGET##memtransfer##M2 (dst, src, size,                                   \
+             false, GTM::abi_dispatch::WRITE, GTM::abi_dispatch::READ);      \
+}                                                                            \
+void ITM_REGPARM _ITM_memmove##NAME(void *dst, const void *src, size_t size) \
+{                                                                            \
+  if (GTM::abi_dispatch::NONTXNAL == GTM::abi_dispatch::WRITE ||             \
+      GTM::abi_dispatch::NONTXNAL == GTM::abi_dispatch::READ)                \
+    {                                                                        \
+      if (((uintptr_t)dst <= (uintptr_t)src ?                                \
+          (uintptr_t)dst + size > (uintptr_t)src :                           \
+          (uintptr_t)src + size > (uintptr_t)dst))                           \
+        GTM::GTM_fatal("_ITM_memmove overlapping and t/nt is not allowed");  \
+      else                                                                   \
+        TARGET##memtransfer##M2 (dst, src, size,                             \
+                false, GTM::abi_dispatch::WRITE, GTM::abi_dispatch::READ);   \
+    }                                                                        \
+  TARGET##memtransfer##M2 (dst, src, size,                                   \
+              true, GTM::abi_dispatch::WRITE, GTM::abi_dispatch::READ);      \
+}
+
+#define ITM_MEMSET_DEF(TARGET, M2, WRITE) \
+void ITM_REGPARM _ITM_memset##WRITE(void *dst, int c, size_t size) \
+{                                                                  \
+  TARGET##memset##M2 (dst, c, size, GTM::abi_dispatch::WRITE);     \
+}                                                                  \
+
+
+// ??? The number of virtual methods is large (7*4 for integers, 7*6 for FP,
+// 7*3 for vectors). Is the cache footprint so costly that we should go for
+// a small table instead (i.e., only have two virtual load/store methods for
+// each supported type)? Note that this doesn't affect custom code paths at
+// all because these use only direct calls.
+// A large cache footprint could especially decrease HTM performance (due
+// to HTM capacity). We could add the modifier (RaR etc.) as parameter, which
+// would give us just 4*2+6*2+3*2 functions (so we'd just need one line for
+// the integer loads/stores), but then the modifier can be checked only at
+// runtime.
+// For memcpy/memmove/memset, we just have two virtual methods (memtransfer
+// and memset).
+#define CREATE_DISPATCH_FUNCTIONS(TARGET, M2)  \
+  CREATE_DISPATCH_FUNCTIONS_T (U1, TARGET, M2) \
+  CREATE_DISPATCH_FUNCTIONS_T (U2, TARGET, M2) \
+  CREATE_DISPATCH_FUNCTIONS_T (U4, TARGET, M2) \
+  CREATE_DISPATCH_FUNCTIONS_T (U8, TARGET, M2) \
+  CREATE_DISPATCH_FUNCTIONS_T (F, TARGET, M2)  \
+  CREATE_DISPATCH_FUNCTIONS_T (D, TARGET, M2)  \
+  CREATE_DISPATCH_FUNCTIONS_T (E, TARGET, M2)  \
+  CREATE_DISPATCH_FUNCTIONS_T (CF, TARGET, M2) \
+  CREATE_DISPATCH_FUNCTIONS_T (CD, TARGET, M2) \
+  CREATE_DISPATCH_FUNCTIONS_T (CE, TARGET, M2) \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RnWt,     NONTXNAL, W)      \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RnWtaR,   NONTXNAL, WaR)    \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RnWtaW,   NONTXNAL, WaW)    \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtWn,     R,      NONTXNAL) \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtWt,     R,      W)        \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtWtaR,   R,      WaR)      \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtWtaW,   R,      WaW)      \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtaRWn,   RaR,    NONTXNAL) \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtaRWt,   RaR,    W)        \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtaRWtaR, RaR,    WaR)      \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtaRWtaW, RaR,    WaW)      \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtaWWn,   RaW,    NONTXNAL) \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtaWWt,   RaW,    W)        \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtaWWtaR, RaW,    WaR)      \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtaWWtaW, RaW,    WaW)      \
+  ITM_MEMSET_DEF(TARGET, M2, W)   \
+  ITM_MEMSET_DEF(TARGET, M2, WaR) \
+  ITM_MEMSET_DEF(TARGET, M2, WaW)
+
+
+// Creates ABI load/store functions that delegate to a transactional memcpy.
+#define ITM_READ_MEMCPY(T, LSMOD, TARGET, M2)                         \
+  _ITM_TYPE_##T ITM_REGPARM _ITM_##LSMOD##T (const _ITM_TYPE_##T *ptr)\
+  {                                                                   \
+    _ITM_TYPE_##T v;                                                  \
+    TARGET##memtransfer##M2(&v, ptr, sizeof(_ITM_TYPE_##T), false,    \
+        GTM::abi_dispatch::NONTXNAL, GTM::abi_dispatch::LSMOD);       \
+    return v;                                                         \
+  }
+
+#define ITM_WRITE_MEMCPY(T, LSMOD, TARGET, M2)                            \
+  void ITM_REGPARM _ITM_##LSMOD##T (_ITM_TYPE_##T *ptr, _ITM_TYPE_##T val)\
+  {                                                                       \
+    TARGET##memtransfer##M2(ptr, &val, sizeof(_ITM_TYPE_##T), false,      \
+        GTM::abi_dispatch::LSMOD, GTM::abi_dispatch::NONTXNAL);           \
+  }
+
+#define CREATE_DISPATCH_FUNCTIONS_T_MEMCPY(T, TARGET, M2) \
+  ITM_READ_MEMCPY(T, R, TARGET, M2)                \
+  ITM_READ_MEMCPY(T, RaR, TARGET, M2)              \
+  ITM_READ_MEMCPY(T, RaW, TARGET, M2)              \
+  ITM_READ_MEMCPY(T, RfW, TARGET, M2)              \
+  ITM_WRITE_MEMCPY(T, W, TARGET, M2)               \
+  ITM_WRITE_MEMCPY(T, WaR, TARGET, M2)             \
+  ITM_WRITE_MEMCPY(T, WaW, TARGET, M2)
+
+
+namespace GTM HIDDEN {
+
+/// This pass-through method is the basis for other methods.
+/// It can be used for serial-irrevocable mode.
+struct abi_dispatch
+{
+public:
+  enum ls_modifier { NONTXNAL, R, RaR, RaW, RfW, W, WaR, WaW };
+
+private:
+  // Disallow copies
+  abi_dispatch(const abi_dispatch &) = delete;
+  abi_dispatch& operator=(const abi_dispatch &) = delete;
+
+public:
+  virtual bool trycommit() = 0;
+  virtual void rollback() = 0;
+  virtual void reinit() = 0;
+
+  // Use fini instead of dtor to support a static subclasses that uses
+  // a unique object and so we don't want to destroy it from common code.
+  virtual void fini() = 0;
+
+  bool read_only () const { return m_read_only; }
+  bool write_through() const { return m_write_through; }
+
+  static void *operator new(size_t s) { return xmalloc (s); }
+  static void operator delete(void *p) { free (p); }
+
+protected:
+  /// Transactional load. Will be called from the dispatch methods
+  /// created below.
+  template <typename V> static V load(const V* addr, ls_modifier mod)
+  {
+    return *addr;
+  }
+  /// Transactional store. Will be called from the dispatch methods
+  /// created below.
+  template <typename V> static void store(V* addr, const V value,
+      ls_modifier mod)
+  {
+    *addr = value;
+  }
+
+public:
+  /// Transactional memcpy/memmove. This method is static to avoid indirect
+  /// calls, and will be used by the virtual ABI dispatch methods.
+  static void memtransfer_static(void *dst, const void* src, size_t size,
+      bool may_overlap, ls_modifier dst_mod, ls_modifier src_mod)
+  {
+    if (!may_overlap)
+      ::memcpy(dst, src, size);
+    else
+      ::memmove(dst, src, size);
+  }
+
+  /// Transactional memset. This method is static to avoid indirect
+  /// calls, and will be used by the virtual ABI dispatch methods.
+  static void memset_static(void *dst, int c, size_t size, ls_modifier mod)
+  {
+    ::memset(dst, c, size);
+  }
+
+  // Creates the ABI dispatch methods for loads and stores.
+  // ??? Should the dispatch table instead be embedded in the dispatch object
+  // to avoid the indirect lookup in the vtable?
+  CREATE_DISPATCH_METHODS(virtual, )
+  // Creates the ABI dispatch methods for memcpy/memmove/memset.
+  CREATE_DISPATCH_METHODS_MEM()
+
+protected:
+  const bool m_read_only;
+  const bool m_write_through;
+  abi_dispatch(bool ro, bool wt) : m_read_only(ro), m_write_through(wt) { }
+};
+
+}
+
+#endif // DISPATCH_H
diff --git a/libitm/libitm_i.h b/libitm/libitm_i.h
index 3291f1b..d323d45 100644
--- a/libitm/libitm_i.h
+++ b/libitm/libitm_i.h
@@ -1,4 +1,4 @@
-/* Copyright (C) 2008, 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2008, 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU Transactional Memory Library (libitm).
@@ -38,16 +38,7 @@
 #include <unwind.h>
 #include <type_traits>
 
-#define UNUSED		__attribute__((unused))
-#define ALWAYS_INLINE	__attribute__((always_inline))
-#ifdef HAVE_ATTRIBUTE_VISIBILITY
-# define HIDDEN		__attribute__((visibility("hidden")))
-#else
-# define HIDDEN
-#endif
-
-#define likely(X)	__builtin_expect((X) != 0, 1)
-#define unlikely(X)	__builtin_expect((X), 0)
+#include "common.h"
 
 namespace GTM HIDDEN {
 
@@ -82,57 +73,12 @@ extern void * xrealloc (void *p, size_t s) __attribute__((malloc, nothrow));
 #include "cacheline.h"
 #include "cachepage.h"
 #include "stmlock.h"
+#include "dispatch.h"
 
 namespace GTM HIDDEN {
 
 // A dispatch table parameterizes the implementation of the STM.
-struct abi_dispatch
-{
- public:
-  enum lock_type { NOLOCK, R, RaR, RaW, RfW, W, WaR, WaW };
-
-  struct mask_pair
-  {
-    gtm_cacheline *line;
-    gtm_cacheline_mask *mask;
-
-    mask_pair() = default;
-    mask_pair(gtm_cacheline *l, gtm_cacheline_mask *m) : line(l), mask(m) { }
-  };
-
- private:
-  // Disallow copies
-  abi_dispatch(const abi_dispatch &) = delete;
-  abi_dispatch& operator=(const abi_dispatch &) = delete;
-
- public:
-  // The default version of these is pass-through.  This merely gives the
-  // a single location to instantiate the base class vtable.
-  virtual const gtm_cacheline *read_lock(const gtm_cacheline *, lock_type);
-  virtual mask_pair write_lock(gtm_cacheline *, lock_type);
-
-  virtual bool trycommit() = 0;
-  virtual void rollback() = 0;
-  virtual void reinit() = 0;
-  virtual bool trydropreference(void *, size_t) = 0;
-
-  // Use fini instead of dtor to support a static subclasses that uses
-  // a unique object and so we don't want to destroy it from common code.
-  virtual void fini() = 0;
-
-  bool read_only () const { return m_read_only; }
-  bool write_through() const { return m_write_through; }
-
-  static void *operator new(size_t s) { return xmalloc (s); }
-  static void operator delete(void *p) { free (p); }
-
- protected:
-  const bool m_read_only;
-  const bool m_write_through;
-  abi_dispatch(bool ro, bool wt) : m_read_only(ro), m_write_through(wt) { }
-
-  static gtm_cacheline_mask mask_sink;
-};
+struct abi_dispatch;
 
 // These values are given to GTM_restart_transaction and indicate the
 // reason for the restart.  The reason is used to decide what STM
@@ -256,6 +202,7 @@ struct gtm_transaction
 
   // In retry.cc
   void decide_retry_strategy (gtm_restart_reason);
+  abi_dispatch* decide_begin_dispatch ();
 
   // In method-serial.cc
   void serialirr_mode ();
@@ -287,9 +234,8 @@ extern void GTM_error (const char *fmt, ...)
 extern void GTM_fatal (const char *fmt, ...)
 	__attribute__((noreturn, format (printf, 1, 2)));
 
-extern abi_dispatch *dispatch_wbetl();
-extern abi_dispatch *dispatch_readonly();
 extern abi_dispatch *dispatch_serial();
+extern abi_dispatch *dispatch_serialirr();
 
 extern gtm_cacheline_mask gtm_mask_stack(gtm_cacheline *, gtm_cacheline_mask);
 
diff --git a/libitm/method-readonly.cc b/libitm/method-readonly.cc
deleted file mode 100644
index ed573e2..0000000
--- a/libitm/method-readonly.cc
+++ /dev/null
@@ -1,124 +0,0 @@
-/* Copyright (C) 2009 Free Software Foundation, Inc.
-   Contributed by Richard Henderson <rth@redhat.com>.
-
-   This file is part of the GNU Transactional Memory Library (libitm).
-
-   Libitm is free software; you can redistribute it and/or modify it
-   under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 3 of the License, or
-   (at your option) any later version.
-
-   Libitm is distributed in the hope that it will be useful, but WITHOUT ANY
-   WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
-   FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
-   more details.
-
-   Under Section 7 of GPL version 3, you are granted additional
-   permissions described in the GCC Runtime Library Exception, version
-   3.1, as published by the Free Software Foundation.
-
-   You should have received a copy of the GNU General Public License and
-   a copy of the GCC Runtime Library Exception along with this program;
-   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
-   <http://www.gnu.org/licenses/>.  */
-
-#include "libitm_i.h"
-
-namespace {
-
-using namespace GTM;
-
-class readonly_dispatch : public abi_dispatch
-{
- private:
-  gtm_version m_start;
-
- public:
-  readonly_dispatch();
-
-  virtual const gtm_cacheline *read_lock(const gtm_cacheline *, lock_type);
-  virtual mask_pair write_lock(gtm_cacheline *, lock_type);
-  virtual bool trycommit();
-  virtual void rollback();
-  virtual void reinit();
-  virtual void fini();
-  virtual bool trydropreference (void *ptr, size_t size) { return true; }
-};
-
-inline
-readonly_dispatch::readonly_dispatch()
-  : abi_dispatch(true, true), m_start(gtm_get_clock ())
-{ }
-
-
-const gtm_cacheline *
-readonly_dispatch::read_lock(const gtm_cacheline *line, lock_type lock)
-{
-  switch (lock)
-    {
-    case NOLOCK:
-    case R:
-    case RaR:
-      return line;
-
-    case RfW:
-      gtm_tx()->restart (RESTART_NOT_READONLY);
-
-    case RaW:
-    default:
-      abort ();
-    }
-}
-
-abi_dispatch::mask_pair
-readonly_dispatch::write_lock(gtm_cacheline *line, lock_type lock)
-{
-  switch (lock)
-    {
-    case NOLOCK:
-      {
-	abi_dispatch::mask_pair pair;
-	pair.line = line;
-	pair.mask = &mask_sink;
-	return pair;
-      }
-
-    case WaW:
-      abort ();
-
-    default:
-      gtm_tx()->restart (RESTART_NOT_READONLY);
-    }
-}
-
-bool
-readonly_dispatch::trycommit ()
-{
-  return gtm_get_clock () == m_start;
-}
-
-void
-readonly_dispatch::rollback ()
-{
-  /* Nothing to do.  */
-}
-
-void
-readonly_dispatch::reinit ()
-{
-  m_start = gtm_get_clock ();
-}
-
-void
-readonly_dispatch::fini ()
-{
-  delete this;
-}
-
-} // anon namespace
-
-abi_dispatch *
-GTM::dispatch_readonly ()
-{
-  return new readonly_dispatch();
-}
diff --git a/libitm/method-serial.cc b/libitm/method-serial.cc
index b46de4d..8e4fcd2 100644
--- a/libitm/method-serial.cc
+++ b/libitm/method-serial.cc
@@ -24,24 +24,6 @@
 
 #include "libitm_i.h"
 
-namespace GTM HIDDEN {
-
-gtm_cacheline_mask abi_dispatch::mask_sink;
-
-const gtm_cacheline *
-abi_dispatch::read_lock(const gtm_cacheline *addr, lock_type)
-{
-  return addr;
-}
-
-abi_dispatch::mask_pair
-abi_dispatch::write_lock(gtm_cacheline *addr, lock_type)
-{
-  return mask_pair (addr, &mask_sink);
-}
-
-} // namespace GTM
-
 // Avoid a dependency on libstdc++ for the pure virtuals in abi_dispatch.
 extern "C" void HIDDEN
 __cxa_pure_virtual ()
@@ -58,25 +40,80 @@ class serial_dispatch : public abi_dispatch
  public:
   serial_dispatch() : abi_dispatch(false, true) { }
 
-  // The read_lock and write_lock methods are implented by the base class.
+  // The transactional load/store/mem methods are implemented by the base
+  // class (simple direct memory accesses).
 
   virtual bool trycommit() { return true; }
   virtual void rollback() { abort(); }
   virtual void reinit() { }
   virtual void fini() { }
-  virtual bool trydropreference (void *ptr, size_t size) { return true; }
+};
+
+class serial_dispatch_ul : public serial_dispatch
+{
+protected:
+  static void log(const void *addr, size_t len)
+  {
+    // TODO Ensure that this gets inlined: Use internal log interface and LTO.
+    GTM_LB(addr, len);
+  }
+
+  template <typename V> static V load(const V* addr, ls_modifier mod)
+  {
+    return *addr;
+  }
+  template <typename V> static void store(V* addr, const V value,
+      ls_modifier mod)
+  {
+    if (mod != WaW)
+      log(addr, sizeof(V));
+    *addr = value;
+  }
+
+public:
+  static void memtransfer_static(void *dst, const void* src, size_t size,
+      bool may_overlap, ls_modifier dst_mod, ls_modifier src_mod)
+  {
+    if (dst_mod != WaW && dst_mod != NONTXNAL)
+      log(dst, size);
+    if (!may_overlap)
+      memcpy(dst, src, size);
+    else
+      memmove(dst, src, size);
+  }
+
+  static void memset_static(void *dst, int c, size_t size, ls_modifier mod)
+  {
+    if (mod != WaW)
+      log(dst, size);
+    memset(dst, c, size);
+  }
+
+  // Local undo will handle this.
+  // trydropreference() need not be changed either.
+  virtual void rollback() { }
+
+  CREATE_DISPATCH_METHODS(virtual, )
+  CREATE_DISPATCH_METHODS_MEM()
 };
 
 } // anon namespace
 
 static const serial_dispatch o_serial_dispatch;
+static const serial_dispatch_ul o_serial_dispatch_ul;
 
 abi_dispatch *
-GTM::dispatch_serial ()
+GTM::dispatch_serialirr ()
 {
   return const_cast<serial_dispatch *>(&o_serial_dispatch);
 }
 
+abi_dispatch *
+GTM::dispatch_serial ()
+{
+  return const_cast<serial_dispatch_ul *>(&o_serial_dispatch_ul);
+}
+
 // Put the transaction into serial-irrevocable mode.
 
 void
diff --git a/libitm/method-wbetl.cc b/libitm/method-wbetl.cc
index 7ce317e..aff6397 100644
--- a/libitm/method-wbetl.cc
+++ b/libitm/method-wbetl.cc
@@ -83,8 +83,8 @@ class wbetl_dispatch : public abi_dispatch
  public:
   wbetl_dispatch();
 
-  virtual const gtm_cacheline *read_lock(const gtm_cacheline *, lock_type);
-  virtual mask_pair write_lock(gtm_cacheline *, lock_type);
+  virtual const gtm_cacheline *read_lock(const gtm_cacheline *, ls_modifier);
+  virtual mask_pair write_lock(gtm_cacheline *, ls_modifier);
 
   virtual bool trycommit();
   virtual void rollback();
@@ -375,11 +375,11 @@ wbetl_dispatch::do_read_lock (const gtm_cacheline *addr, bool after_read)
 }
 
 const gtm_cacheline *
-wbetl_dispatch::read_lock (const gtm_cacheline *addr, lock_type ltype)
+wbetl_dispatch::read_lock (const gtm_cacheline *addr, ls_modifier ltype)
 {
   switch (ltype)
     {
-    case NOLOCK:
+    case NONTXNAL:
       return addr;
     case R:
       return do_read_lock (addr, false);
@@ -395,13 +395,13 @@ wbetl_dispatch::read_lock (const gtm_cacheline *addr, lock_type ltype)
 }
 
 abi_dispatch::mask_pair
-wbetl_dispatch::write_lock (gtm_cacheline *addr, lock_type ltype)
+wbetl_dispatch::write_lock (gtm_cacheline *addr, ls_modifier ltype)
 {
   gtm_cacheline *line;
 
   switch (ltype)
     {
-    case NOLOCK:
+    case NONTXNAL:
       return mask_pair (addr, &mask_sink);
     case W:
     case WaR:
diff --git a/libitm/retry.cc b/libitm/retry.cc
index a25b282..31757d5 100644
--- a/libitm/retry.cc
+++ b/libitm/retry.cc
@@ -42,6 +42,10 @@ GTM::gtm_transaction::decide_retry_strategy (gtm_restart_reason r)
       // write lock is not yet held, grab it.  Don't do this with
       // an upgrade, since we've no need to preserve the state we
       // acquired with the read.
+      // FIXME this might be dangerous if we use serial mode to change TM
+      // meta data (e.g., reallocate the lock array). Likewise, for
+      // privatization, we must get rid of old references (that is, abort)
+      // or let privatizers know we're still there by not releasing the lock.
       if ((this->state & STATE_SERIAL) == 0)
 	{
 	  this->state |= STATE_SERIAL;
@@ -65,17 +69,23 @@ GTM::gtm_transaction::decide_retry_strategy (gtm_restart_reason r)
     }
   else
     {
-      if (r == RESTART_NOT_READONLY)
-	{
-	  assert ((this->prop & pr_readOnly) == 0);
-	  if (disp->read_only ())
-	    {
-	      disp->fini ();
-	      disp = dispatch_wbetl ();
-	      set_abi_disp (disp);
-	      return;
-	    }
-	}
-      disp->reinit ();
+      GTM_fatal("internal error: unsupported mode");
     }
 }
+
+
+/// Decides which TM method should be used on the first attempt to run this
+/// transaction, setting this->disp accordingly.
+/// serial_lock will not have been acquired if this is the outer-most
+/// transaction. If the state is set to STATE_SERIAL, the caller will set the
+/// dispatch.
+GTM::abi_dispatch*
+GTM::gtm_transaction::decide_begin_dispatch ()
+{
+  // ??? Probably want some environment variable to choose the default
+  // STM implementation once we have more than one implemented.
+  state = STATE_SERIAL;
+  if (prop & pr_hasNoAbort)
+    state |= STATE_IRREVOCABLE;
+  return 0;
+}

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [trans-mem] Beginning of refactoring
  2011-05-25 22:45 [trans-mem] Beginning of refactoring Torvald Riegel
@ 2011-06-29 21:22 ` Torvald Riegel
  2011-06-29 21:50   ` Richard Henderson
  2011-06-29 23:18 ` Richard Henderson
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Torvald Riegel @ 2011-06-29 21:22 UTC (permalink / raw)
  To: GCC Patches; +Cc: Richard Henderson

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

Patch contains more clean-up: the trycommit, rollback and
registerThrownObject functions in the ABI are unnecessary for how we
support exception handling.

Ok for branch?


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 8143 bytes --]

commit 79ae722913f40715882b4c7b3fb798306f1ad3ae
Author: Torvald Riegel <triegel@redhat.com>
Date:   Wed Jun 29 23:05:28 2011 +0200

    Removed unnecessary trycommit, rollback, and registerThrownObject ABI functions.

diff --git a/libitm/beginend.cc b/libitm/beginend.cc
index 7ed5073..9b16973 100644
--- a/libitm/beginend.cc
+++ b/libitm/beginend.cc
@@ -184,25 +184,12 @@ GTM::gtm_transaction::rollback ()
 }
 
 void ITM_REGPARM
-_ITM_rollbackTransaction (void)
-{
-  gtm_transaction *tx = gtm_tx();
-  
-  assert ((tx->prop & pr_hasNoAbort) == 0);
-  assert ((tx->state & gtm_transaction::STATE_ABORTING) == 0);
-
-  tx->rollback ();
-  tx->state |= gtm_transaction::STATE_ABORTING;
-}
-
-void ITM_REGPARM
 _ITM_abortTransaction (_ITM_abortReason reason)
 {
   gtm_transaction *tx = gtm_tx();
 
   assert (reason == userAbort);
   assert ((tx->prop & pr_hasNoAbort) == 0);
-  assert ((tx->state & gtm_transaction::STATE_ABORTING) == 0);
 
   if (tx->state & gtm_transaction::STATE_IRREVOCABLE)
     abort ();
@@ -238,7 +225,7 @@ GTM::gtm_transaction::trycommit ()
 bool
 GTM::gtm_transaction::trycommit_and_finalize ()
 {
-  if ((this->state & gtm_transaction::STATE_ABORTING) || trycommit ())
+  if (trycommit ())
     {
       abi_disp()->fini ();
       set_gtm_tx (this->prev);
@@ -252,14 +239,6 @@ GTM::gtm_transaction::trycommit_and_finalize ()
   return false;
 }
 
-bool ITM_REGPARM
-_ITM_tryCommitTransaction (void)
-{
-  gtm_transaction *tx = gtm_tx();
-  assert ((tx->state & gtm_transaction::STATE_ABORTING) == 0);
-  return tx->trycommit ();
-}
-
 void ITM_NORETURN
 GTM::gtm_transaction::restart (gtm_restart_reason r)
 {
diff --git a/libitm/libitm.h b/libitm/libitm.h
index df89d33..abd4274 100644
--- a/libitm/libitm.h
+++ b/libitm/libitm.h
@@ -45,7 +45,7 @@ extern "C" {
 
 #define ITM_NORETURN	__attribute__((noreturn))
 #define ITM_PURE __attribute__((transaction_pure))
-\f
+
 /* The following are externally visible definitions and functions, though
    only very few of these should be called by user code.  */
 
@@ -91,6 +91,7 @@ typedef enum
    pr_RaRBarriersOmitted	= 0x0200,
    pr_undoLogCode		= 0x0400,
    pr_preferUninstrumented	= 0x0800,
+   /* Exception blocks are not used nor supported. */
    pr_exceptionBlock		= 0x1000,
    pr_readOnly			= 0x4000,
    pr_hasElse			= 0x200000,
@@ -138,12 +139,8 @@ extern _ITM_transactionId_t _ITM_getTransactionId(void) ITM_REGPARM;
 extern uint32_t _ITM_beginTransaction(uint32_t, ...) ITM_REGPARM;
 
 extern void _ITM_abortTransaction(_ITM_abortReason) ITM_REGPARM ITM_NORETURN;
-extern void _ITM_rollbackTransaction (void) ITM_REGPARM;
 
 extern void _ITM_commitTransaction (void) ITM_REGPARM;
-extern bool _ITM_tryCommitTransaction(void) ITM_REGPARM;
-
-extern void _ITM_registerThrownObject (const void *, size_t) ITM_REGPARM;
 
 extern void _ITM_changeTransactionMode (_ITM_transactionState) ITM_REGPARM;
 
diff --git a/libitm/libitm.map b/libitm/libitm.map
index 0d52a7c..49d0b1b 100644
--- a/libitm/libitm.map
+++ b/libitm/libitm.map
@@ -12,9 +12,6 @@ LIBITM_1.0 {
 	_ITM_getTransactionId;
 	_ITM_inTransaction;
 	_ITM_libraryVersion;
-	_ITM_registerThrownObject;
-	_ITM_rollbackTransaction;
-	_ITM_tryCommitTransaction;
 	_ITM_versionCompatible;
 
 	_ITM_registerTMCloneTable;
diff --git a/libitm/libitm.texi b/libitm/libitm.texi
index 8d8de8c..046b0bd 100644
--- a/libitm/libitm.texi
+++ b/libitm/libitm.texi
@@ -221,25 +221,34 @@ reported for the dynamic scope as well, not just for the lexical scope
 (@code{hasNoAbort}). Without this, a library cannot exploit this together
 with flat nesting.
 
-@strong{TODO} @code{exceptionBlock?}
+@code{exceptionBlock} is not supported because exception blocks are not used.
 
 @subsubsection [No changes] Windows exception state
 @subsubsection [No changes] Other machine state
 
-@subsubsection Results from beginTransaction
-@strong{TODO} @code{abortTransaction} supported?
+@subsubsection [No changes] Results from beginTransaction
 
 @subsection Aborting a transaction
-@strong{TODO} make consistent with EH.
 
-@subsection Committing a transaction
-@strong{TODO} make consistent with EH.
+@code{_ITM_rollbackTransaction} is not supported. @code{_ITM_abortTransaction}
+is supported but the abort reason @code{exceptionBlockAbort} is not (and there
+are no exception blocks in general, so the related cases also do not have to
+be considered).
 
-@subsection Exception handling support
+@subsection Committing a transaction
 
-@strong{TODO} Document wrappers. Document code generated for commit, perhaps
-with examples similar to those in the specification. What can be removed from
-the ABI in turn? Document requirements on libstdc++ (@code{_cxa_tm_cleanup()}).
+The exception handling (EH) scheme is different. The Intel ABI requires the
+@code{_ITM_tryCommitTransaction} function that will return even when the
+commit failed and will have to be matched with calls to either
+@code{_ITM_abortTransaction} or @code{_ITM_commitTransaction}. In contrast,
+gcc relies on transactional wrappers for the functions of the Exception
+Handling ABI and on one additional commit function (shown below). This allows
+the TM to keep track of EH internally and thus it does not have to embed the
+cleanup of EH state into the existing EH code in the program.
+@code{_ITM_tryCommitTransaction} is not supported.
+@code{_ITM_commitTransactionToId} is also not supported because the
+propagation of thrown exceptions will not bypass commits of nested
+transactions.
 
 @example
 void _ITM_commitTransactionEH(void *exc_ptr) ITM_REGPARM;
@@ -249,6 +258,42 @@ void *_ITM_cxa_begin_catch (void *exc_ptr);
 void _ITM_cxa_end_catch (void);
 @end example
 
+@code{_ITM_commitTransactionEH} must be called to commit a transaction if an
+exception could be in flight at this position in the code. @code{exc_ptr} is
+the current exception or zero if there is no current exception.
+The @code{_ITM_cxa...} functions are transactional wrappers for the respective
+@code{__cxa...} functions and must be called instead of these in transactional
+code.
+
+To support this EH scheme, libstdc++ needs to provide one additional function
+(@code{_cxa_tm_cleanup}), which is used by the TM to clean up the exception
+handling state while rolling back a transaction:
+
+@example
+void __cxa_tm_cleanup (void *unthrown_obj, void *cleanup_exc,
+                       unsigned int caught_count);
+@end example
+
+@code{unthrown_obj} is non-null if the program called
+@code{__cxa_allocate_exception} for this exception but did not yet called
+@code{__cxa_throw} for it. @code{cleanup_exc} is non-null if the program is
+currently processing a cleanup along an exception path but has not caught this
+exception yet. @code{caught_count} is the nesting depth of
+@code{__cxa_begin_catch} within the transaction (which can be counted by the TM
+using @code{_ITM_cxa_begin_catch} and @code{_ITM_cxa_end_catch});
+@code{__cxa_tm_cleanup} then performs rollback by essentially performing
+@code{__cxa_end_catch} that many times.
+
+
+
+@subsection Exception handling support
+
+Currently, there is no support for functionality like
+@code{__transaction_cancel throw} as described in the C++ TM specification.
+Supporting this should be possible with the EH scheme explained previously
+because via the transactional wrappers for the EH ABI, the TM is able to
+observe and intercept EH.
+
 
 @subsection [No changes] Transition to serial--irrevocable mode
 @subsection [No changes] Data transfer functions
diff --git a/libitm/libitm_i.h b/libitm/libitm_i.h
index d323d45..98c2c75 100644
--- a/libitm/libitm_i.h
+++ b/libitm/libitm_i.h
@@ -146,10 +146,6 @@ struct gtm_transaction
   // Set if the serial-irrevocable dispatch table is installed.
   // Implies that no logging is being done, and abort is not possible.
   static const uint32_t STATE_IRREVOCABLE	= 0x0002;
-  // Set if we're in the process of aborting the transaction.  This is
-  // used when _ITM_rollbackTransaction is called to begin the abort
-  // and ends with _ITM_commitTransaction.
-  static const uint32_t STATE_ABORTING		= 0x0004;
 
   // A bitmask of the above.
   uint32_t state;

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [trans-mem] Beginning of refactoring
  2011-06-29 21:22 ` Torvald Riegel
@ 2011-06-29 21:50   ` Richard Henderson
  0 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2011-06-29 21:50 UTC (permalink / raw)
  To: Torvald Riegel; +Cc: GCC Patches

On 06/29/2011 02:12 PM, Torvald Riegel wrote:
> Patch contains more clean-up: the trycommit, rollback and
> registerThrownObject functions in the ABI are unnecessary for how we
> support exception handling.
> 
> Ok for branch?
> 

Looks good.


r~

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [trans-mem] Beginning of refactoring
  2011-05-25 22:45 [trans-mem] Beginning of refactoring Torvald Riegel
  2011-06-29 21:22 ` Torvald Riegel
@ 2011-06-29 23:18 ` Richard Henderson
  2011-06-30 12:19   ` Torvald Riegel
  2011-07-01 20:23 ` Torvald Riegel
  2011-07-09 15:35 ` Torvald Riegel
  3 siblings, 1 reply; 19+ messages in thread
From: Richard Henderson @ 2011-06-29 23:18 UTC (permalink / raw)
  To: Torvald Riegel; +Cc: GCC Patches

On 05/25/2011 02:10 PM, Torvald Riegel wrote:
> Here's the beginning of a refactoring aimed at being able to merge more
> TM algorithms later on.
> 
> Patch 1: Just a straightfoward rename to make it clear that we're
> dispatching on the level of ABI calls, not internals.

Ok, I guess.  I don't think it matters that much.

> Patch 2: _ITM_dropReferences is not sufficiently defined in the ABI. It
> seems to target some form of open nesting for txnal wrappers, but the
> prose in the ABI specification is unclear. Thus, disable this for now
> (aka fatal runtime error), and expect the related tests to fail. Pick it
> up again once that the ABI has been improved and the use cases are
> clear.

Sure, but please actually delete the code rather than just comment it out.

> Patch 3: The actual change in how ABI calls are dispatched. Also,
> removed method-readonly (broken, will in a similar form reappear in the
> family of globallock-based algorithms), and disabled method-wbetl (needs
> larger refactoring, will be revived/remerged later).

> +CREATE_DISPATCH_FUNCTIONS_T_MEMCPY(M256, GTM::abi_disp()->_, )
> +CREATE_DISPATCH_FUNCTIONS_T_MEMCPY(M64, GTM::abi_disp()->_, )
> +CREATE_DISPATCH_FUNCTIONS_T_MEMCPY(M128, GTM::abi_disp()->_, )
> +CREATE_DISPATCH_FUNCTIONS(GTM::abi_disp()->_, )

What's the point of using "GTM::abi_disp()->_" as a mandatory argument?

Further, that second "M2" argument is universally empty.  What's that?

> +// Creates memcpy/memmove/memset methods.
> +#define CREATE_DISPATCH_METHODS_MEM()  \
> +virtual void _memtransfer(void *dst, const void* src, size_t size,     \
> +bool may_overlap, ls_modifier dst_mod, ls_modifier src_mod)           \
> +{                                                                     \
> +  memtransfer_static(dst, src, size, may_overlap, dst_mod, src_mod);  \
> +}                                                                     \
> +virtual void _memset(void *dst, int c, size_t size, ls_modifier mod)   \
> +{                                                                     \
> +  memset_static(dst, c, size, mod);                                   \
> +}

Why are the memtransfer and memset virtuals distinguished from the statics?
For the patch as written it would seem to be ok to merge them.

> +  if (GTM::abi_dispatch::NONTXNAL == GTM::abi_dispatch::WRITE ||             \
> +      GTM::abi_dispatch::NONTXNAL == GTM::abi_dispatch::READ)                \

Formatting.

> +#define ITM_MEMTRANSFER_DEF(TARGET, M2, NAME, READ, WRITE) \
> +void ITM_REGPARM _ITM_memcpy##NAME(void *dst, const void *src, size_t size)  \
> +{                                                                            \
> +  TARGET##memtransfer##M2 (dst, src, size,                                   \
> +             false, GTM::abi_dispatch::WRITE, GTM::abi_dispatch::READ);      \
> +}                                                                            \
> +void ITM_REGPARM _ITM_memmove##NAME(void *dst, const void *src, size_t size) \
> +{                                                                            \
> +  if (GTM::abi_dispatch::NONTXNAL == GTM::abi_dispatch::WRITE ||             \
> +      GTM::abi_dispatch::NONTXNAL == GTM::abi_dispatch::READ)                \
> +    {                                                                        \
> +      if (((uintptr_t)dst <= (uintptr_t)src ?                                \
> +          (uintptr_t)dst + size > (uintptr_t)src :                           \
> +          (uintptr_t)src + size > (uintptr_t)dst))                           \
> +        GTM::GTM_fatal("_ITM_memmove overlapping and t/nt is not allowed");  \
> +      else                                                                   \
> +        TARGET##memtransfer##M2 (dst, src, size,                             \
> +                false, GTM::abi_dispatch::WRITE, GTM::abi_dispatch::READ);   \
> +    }                                                                        \
> +  TARGET##memtransfer##M2 (dst, src, size,                                   \
> +              true, GTM::abi_dispatch::WRITE, GTM::abi_dispatch::READ);      \
> +}

Ok, I realize we need macros to generate the ABI names both here and in
CREATE_DISPATCH_FUNCTIONS, but can we limit the code within macros to as
little as absolutely possible?

For instance,

  template<abi_dispatch::ls_modifier dst_mod, abi_dispatch::ls_modifier src_mod>
  void abi_memmove(void *dst, const void *src, size_t size)
  {
    if (dst_mod == NONTXNAL || src_mod == NONTXNAL)
      ...
  }

where the actual implementation under macro is limited to a function call.

Missing return/else in there?  Surely not two calls to memtransfer...

> +protected:
> +  /// Transactional load. Will be called from the dispatch methods
> +  /// created below.
> +  template <typename V> static V load(const V* addr, ls_modifier mod)
> +  {
> +    return *addr;
> +  }
> +  /// Transactional store. Will be called from the dispatch methods
> +  /// created below.
> +  template <typename V> static void store(V* addr, const V value,
> +      ls_modifier mod)
> +  {
> +    *addr = value;
> +  }

Why are these here in the base class?  I'm not sure I like having static
functions that are required to be overridden at each instance, wherein if
you forget to implement them the build silently succeeds, but of course
does the wrong thing.

What's with the 3 /// comments?  Can we stick with existing gnu standards?
I'm pretty sure we've got some documentation for at least one of libstdc++,
gold, and the go front end.

> -#define UNUSED		__attribute__((unused))
> -#define ALWAYS_INLINE	__attribute__((always_inline))
> -#ifdef HAVE_ATTRIBUTE_VISIBILITY
> -# define HIDDEN		__attribute__((visibility("hidden")))
> -#else
> -# define HIDDEN
> -#endif
> -
> -#define likely(X)	__builtin_expect((X) != 0, 1)
> -#define unlikely(X)	__builtin_expect((X), 0)
> +#include "common.h"

Why?  We're already in a header that's clearly marked "internal".


r~

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [trans-mem] Beginning of refactoring
  2011-06-29 23:18 ` Richard Henderson
@ 2011-06-30 12:19   ` Torvald Riegel
  2011-06-30 15:03     ` Richard Henderson
  0 siblings, 1 reply; 19+ messages in thread
From: Torvald Riegel @ 2011-06-30 12:19 UTC (permalink / raw)
  To: GCC Patches; +Cc: Richard Henderson

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

On Wed, 2011-06-29 at 16:01 -0700, Richard Henderson wrote:
> On 05/25/2011 02:10 PM, Torvald Riegel wrote:
> > Patch 2: _ITM_dropReferences is not sufficiently defined in the ABI. It
> > seems to target some form of open nesting for txnal wrappers, but the
> > prose in the ABI specification is unclear. Thus, disable this for now
> > (aka fatal runtime error), and expect the related tests to fail. Pick it
> > up again once that the ABI has been improved and the use cases are
> > clear.
> 
> Sure, but please actually delete the code rather than just comment it out.

Fixed in updated patch2.

> > Patch 3: The actual change in how ABI calls are dispatched. Also,
> > removed method-readonly (broken, will in a similar form reappear in the
> > family of globallock-based algorithms), and disabled method-wbetl (needs
> > larger refactoring, will be revived/remerged later).
> 
> > +CREATE_DISPATCH_FUNCTIONS_T_MEMCPY(M256, GTM::abi_disp()->_, )
> > +CREATE_DISPATCH_FUNCTIONS_T_MEMCPY(M64, GTM::abi_disp()->_, )
> > +CREATE_DISPATCH_FUNCTIONS_T_MEMCPY(M128, GTM::abi_disp()->_, )
> > +CREATE_DISPATCH_FUNCTIONS(GTM::abi_disp()->_, )
> 
> What's the point of using "GTM::abi_disp()->_" as a mandatory argument?
> 
> Further, that second "M2" argument is universally empty.  What's that?

This prepares us to be later able to instantiate ABI mem access
functions without having to go through the vtable dispatch (thus,
avoiding the overhead of the vtable dispatch / indirection). This will
become useful as soon as GCC can create several code paths with
different instrumentation (ie, calling different sets of _ITM_*
functions) for each transaction. Together with LTO, this will allow us
to have the TM implementation in the library and still be as fast as
direct TM support in the compiler.

How it works is that in a TM-method-specific dispatch class we would add
  CREATE_DISPATCH_METHODS(static, _static)
in addition to the 
  CREATE_DISPATCH_METHODS(virtual, )
that is currently there. In barrier.cc we can then do
  CREATE_DISPATCH_FUNCTIONS(GTM::serial_dispatch::_, _static)
and thus bypass the virtual functions, but still keep one implementation
of the TM method (in the static template functions). This example would
create functions that just use the serial-mode accesses.

Right now, CREATE_DISPATCH_FUNCTIONS will always create the same set of
functions (with names as specified in the ABI), but this is because we
haven't defined yet how those we would separate different
instrumentations (eg, name mangling vs. sth. else).

The trailing underscore at the end of the first param is to make the
preprocessor happen. Is there a better way to let it accept
"GTM::abi_disp()->" as argument?

> 
> > +// Creates memcpy/memmove/memset methods.
> > +#define CREATE_DISPATCH_METHODS_MEM()  \
> > +virtual void _memtransfer(void *dst, const void* src, size_t size,     \
> > +bool may_overlap, ls_modifier dst_mod, ls_modifier src_mod)           \
> > +{                                                                     \
> > +  memtransfer_static(dst, src, size, may_overlap, dst_mod, src_mod);  \
> > +}                                                                     \
> > +virtual void _memset(void *dst, int c, size_t size, ls_modifier mod)   \
> > +{                                                                     \
> > +  memset_static(dst, c, size, mod);                                   \
> > +}
> 
> Why are the memtransfer and memset virtuals distinguished from the statics?
> For the patch as written it would seem to be ok to merge them.

For the same reason as above. We seem to need two entry points (virtual
and static) to make that work. In the updated patch3, I add leading
underscores to make the _static version above actually work.

> > +#define ITM_MEMTRANSFER_DEF(TARGET, M2, NAME, READ, WRITE) \
> > +void ITM_REGPARM _ITM_memcpy##NAME(void *dst, const void *src, size_t size)  \
> > +{                                                                            \
> > +  TARGET##memtransfer##M2 (dst, src, size,                                   \
> > +             false, GTM::abi_dispatch::WRITE, GTM::abi_dispatch::READ);      \
> > +}                                                                            \
> > +void ITM_REGPARM _ITM_memmove##NAME(void *dst, const void *src, size_t size) \
> > +{                                                                            \
> > +  if (GTM::abi_dispatch::NONTXNAL == GTM::abi_dispatch::WRITE ||             \
> > +      GTM::abi_dispatch::NONTXNAL == GTM::abi_dispatch::READ)                \
> > +    {                                                                        \
> > +      if (((uintptr_t)dst <= (uintptr_t)src ?                                \
> > +          (uintptr_t)dst + size > (uintptr_t)src :                           \
> > +          (uintptr_t)src + size > (uintptr_t)dst))                           \
> > +        GTM::GTM_fatal("_ITM_memmove overlapping and t/nt is not allowed");  \
> > +      else                                                                   \
> > +        TARGET##memtransfer##M2 (dst, src, size,                             \
> > +                false, GTM::abi_dispatch::WRITE, GTM::abi_dispatch::READ);   \
> > +    }                                                                        \
> > +  TARGET##memtransfer##M2 (dst, src, size,                                   \
> > +              true, GTM::abi_dispatch::WRITE, GTM::abi_dispatch::READ);      \
> > +}
> 
> Ok, I realize we need macros to generate the ABI names both here and in
> CREATE_DISPATCH_FUNCTIONS, but can we limit the code within macros to as
> little as absolutely possible?

The check is now in one of abi_dispatch's methods (implemented in
barrier.cc).

> Missing return/else in there?  Surely not two calls to memtransfer...

Thanks, fixed as well.

> > +protected:
> > +  /// Transactional load. Will be called from the dispatch methods
> > +  /// created below.
> > +  template <typename V> static V load(const V* addr, ls_modifier mod)
> > +  {
> > +    return *addr;
> > +  }
> > +  /// Transactional store. Will be called from the dispatch methods
> > +  /// created below.
> > +  template <typename V> static void store(V* addr, const V value,
> > +      ls_modifier mod)
> > +  {
> > +    *addr = value;
> > +  }
> 
> Why are these here in the base class?  I'm not sure I like having static
> functions that are required to be overridden at each instance, wherein if
> you forget to implement them the build silently succeeds, but of course
> does the wrong thing.

I added macros to generate pure-virtual methods for the base class.
Write/read-through access got moved to the serial_dispatch class.

> What's with the 3 /// comments?  Can we stick with existing gnu standards?

Old doxygen habits. Removed.

> > -#define UNUSED		__attribute__((unused))
> > -#define ALWAYS_INLINE	__attribute__((always_inline))
> > -#ifdef HAVE_ATTRIBUTE_VISIBILITY
> > -# define HIDDEN		__attribute__((visibility("hidden")))
> > -#else
> > -# define HIDDEN
> > -#endif
> > -
> > -#define likely(X)	__builtin_expect((X) != 0, 1)
> > -#define unlikely(X)	__builtin_expect((X), 0)
> > +#include "common.h"
> 
> Why?  We're already in a header that's clearly marked "internal".

I moved these macros to a common header so libitm_i.h doesn't have to be
included everywhere. There was some circular dependency after the
changes (I think in via the barrier definitions for SSE etc.). What's
the issue that you see with this change?

Are the attached updated patches okay for the branch?


Torvald

[-- Attachment #2: patch2 --]
[-- Type: text/plain, Size: 2348 bytes --]

commit f00a5faa853c268c6d17f2c458f9aa53dfbff9dc
Author: Torvald Riegel <triegel@redhat.com>
Date:   Tue May 24 18:30:28 2011 +0200

    Do not support _ITM_dropReferences anymore
    
    	* alloc_c.cc (_ITM_dropReferences): Don't support it anymore.
    	* testsuite/libitm.c++/dropref.C: _ITM_dropReferences is expected to fail.
    	* testsuite/libitm.c/dropref-2.c: Same.
    	* testsuite/libitm.c/dropref.c: Same.

diff --git a/libitm/alloc_c.cc b/libitm/alloc_c.cc
index 66d1109..b87b304 100644
--- a/libitm/alloc_c.cc
+++ b/libitm/alloc_c.cc
@@ -1,4 +1,4 @@
-/* Copyright (C) 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU Transactional Memory Library (libitm).
@@ -63,11 +63,10 @@ __attribute__((transaction_pure))
 void ITM_REGPARM
 _ITM_dropReferences (void *ptr, size_t len)
 {
-  gtm_transaction *tx = gtm_tx();
-  if (!abi_disp()->trydropreference (ptr, len))
-    tx->restart (RESTART_VALIDATE_READ);
-  tx->drop_references_local (ptr, len);
-  tx->drop_references_allocations (ptr);
+  // The semantics of _ITM_dropReferences are not sufficiently defined in the
+  // ABI specification, so it does not make sense to support it right now. See
+  // the libitm documentation for details.
+  GTM_fatal("_ITM_dropReferences is not supported");
 }
 
 } // extern "C"
diff --git a/libitm/testsuite/libitm.c++/dropref.C b/libitm/testsuite/libitm.c++/dropref.C
index 92c8812..ee4f1bb 100644
--- a/libitm/testsuite/libitm.c++/dropref.C
+++ b/libitm/testsuite/libitm.c++/dropref.C
@@ -1,3 +1,4 @@
+/* { dg-xfail-run-if "unsupported" { *-*-* } } */
 #include <libitm.h>
 
 char *pp;
diff --git a/libitm/testsuite/libitm.c/dropref-2.c b/libitm/testsuite/libitm.c/dropref-2.c
index ddd4eae..2386b18 100644
--- a/libitm/testsuite/libitm.c/dropref-2.c
+++ b/libitm/testsuite/libitm.c/dropref-2.c
@@ -1,3 +1,4 @@
+/* { dg-xfail-run-if "unsupported" { *-*-* } } */
 #include <stdlib.h>
 #include <libitm.h>
 
diff --git a/libitm/testsuite/libitm.c/dropref.c b/libitm/testsuite/libitm.c/dropref.c
index 92c8812..ee4f1bb 100644
--- a/libitm/testsuite/libitm.c/dropref.c
+++ b/libitm/testsuite/libitm.c/dropref.c
@@ -1,3 +1,4 @@
+/* { dg-xfail-run-if "unsupported" { *-*-* } } */
 #include <libitm.h>
 
 char *pp;

[-- Attachment #3: patch3 --]
[-- Type: text/plain, Size: 38518 bytes --]

commit be07f99348ea793529230c5a2238625b826d77ae
Author: Torvald Riegel <triegel@redhat.com>
Date:   Wed May 25 17:21:53 2011 +0200

    New dispatch class/functions. Remove/disable old methods (readonly, wbetl).
    
    	* libitm_i.h: Move parts to common.h and dispatch.h.
    	* common.h: New file.
    	* dispatch.h: New file, new dispatch class.
    	Rename GTM::abi_dispatch::lock_type to ls_modifier.
    	RenameGTM::abi_dispatch::NOLOCK to NONTXNAL.
    	* beginend.cc (GTM::gtm_transaction::begin_transaction): Delegate mode
    	decision to retry.cc.
    	* retry.cc (GTM::gtm_transaction::decide_retry_strategy): Use serial mode
    	only.
    	(GTM::gtm_transaction::decide_begin_dispatch): Same.
    	* method-serial.cc: Adapt to new dispatch. Add serial mode with undo
    	logging.
    	* barrier.cc: Use new barriers definitions.
    	* config/x86/x86_sse.cc: Same.
    	* config/x86/x86_avx.cc: Same.
    	* Makefile.am: Don't build readonly and wbetl methods, memset.cc and
    	memcpy.cc.
    	* Makefile.in: Rebuild.
    	* method-readonly.cc: Remove.
    	* method-wbetl.cc: Rename GTM::abi_dispatch::lock_type to ls_modifier.
    	Rename GTM::abi_dispatch::NOLOCK to NONTXNAL.

diff --git a/libitm/Makefile.am b/libitm/Makefile.am
index 8ad4e5f..bf0180d 100644
--- a/libitm/Makefile.am
+++ b/libitm/Makefile.am
@@ -41,9 +41,9 @@ libitm_la_LDFLAGS = $(libitm_version_info) $(libitm_version_script) \
 
 libitm_la_SOURCES = \
 	aatree.cc alloc.cc alloc_c.cc alloc_cpp.cc barrier.cc beginend.cc \
-	clone.cc cacheline.cc cachepage.cc eh_cpp.cc local.cc memcpy.cc \
-	memset.cc query.cc retry.cc rwlock.cc useraction.cc util.cc \
-	sjlj.S tls.cc method-serial.cc method-readonly.cc method-wbetl.cc
+	clone.cc cacheline.cc cachepage.cc eh_cpp.cc local.cc \
+	query.cc retry.cc rwlock.cc useraction.cc util.cc \
+	sjlj.S tls.cc method-serial.cc
 
 if ARCH_X86
 libitm_la_SOURCES += x86_sse.cc x86_avx.cc
diff --git a/libitm/Makefile.in b/libitm/Makefile.in
index ad03c27..8fc3106 100644
--- a/libitm/Makefile.in
+++ b/libitm/Makefile.in
@@ -93,17 +93,15 @@ LTLIBRARIES = $(toolexeclib_LTLIBRARIES)
 libitm_la_LIBADD =
 am__libitm_la_SOURCES_DIST = aatree.cc alloc.cc alloc_c.cc \
 	alloc_cpp.cc barrier.cc beginend.cc clone.cc cacheline.cc \
-	cachepage.cc eh_cpp.cc local.cc memcpy.cc memset.cc query.cc \
+	cachepage.cc eh_cpp.cc local.cc query.cc \
 	retry.cc rwlock.cc useraction.cc util.cc sjlj.S tls.cc \
-	method-serial.cc method-readonly.cc method-wbetl.cc x86_sse.cc \
-	x86_avx.cc
+	method-serial.cc x86_sse.cc 86_avx.cc
 @ARCH_X86_TRUE@am__objects_1 = x86_sse.lo x86_avx.lo
 am_libitm_la_OBJECTS = aatree.lo alloc.lo alloc_c.lo alloc_cpp.lo \
 	barrier.lo beginend.lo clone.lo cacheline.lo cachepage.lo \
-	eh_cpp.lo local.lo memcpy.lo memset.lo query.lo retry.lo \
+	eh_cpp.lo local.lo query.lo retry.lo \
 	rwlock.lo useraction.lo util.lo sjlj.lo tls.lo \
-	method-serial.lo method-readonly.lo method-wbetl.lo \
-	$(am__objects_1)
+	method-serial.lo $(am__objects_1)
 libitm_la_OBJECTS = $(am_libitm_la_OBJECTS)
 DEFAULT_INCLUDES = -I.@am__isrc@
 depcomp = $(SHELL) $(top_srcdir)/../depcomp
@@ -342,6 +340,7 @@ AM_CPPFLAGS = $(addprefix -I, $(search_path))
 AM_CFLAGS = $(XCFLAGS)
 AM_CXXFLAGS = -std=gnu++0x -funwind-tables -fno-exceptions -fno-rtti \
 	$(XCFLAGS) $(abi_version)
+
 AM_CCASFLAGS = $(XCFLAGS)
 AM_LDFLAGS = $(XLDFLAGS) $(SECTION_LDFLAGS) $(OPT_LDFLAGS)
 toolexeclib_LTLIBRARIES = libitm.la
@@ -358,9 +357,9 @@ libitm_la_LDFLAGS = $(libitm_version_info) $(libitm_version_script) \
 
 libitm_la_SOURCES = aatree.cc alloc.cc alloc_c.cc alloc_cpp.cc \
 	barrier.cc beginend.cc clone.cc cacheline.cc cachepage.cc \
-	eh_cpp.cc local.cc memcpy.cc memset.cc query.cc retry.cc \
+	eh_cpp.cc local.cc query.cc retry.cc \
 	rwlock.cc useraction.cc util.cc sjlj.S tls.cc method-serial.cc \
-	method-readonly.cc method-wbetl.cc $(am__append_1)
+	$(am__append_1)
 all: config.h
 	$(MAKE) $(AM_MAKEFLAGS) all-recursive
 
@@ -470,11 +469,7 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/clone.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/eh_cpp.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/local.Plo@am__quote@
-@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/memcpy.Plo@am__quote@
-@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/memset.Plo@am__quote@
-@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/method-readonly.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/method-serial.Plo@am__quote@
-@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/method-wbetl.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/query.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/retry.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/rwlock.Plo@am__quote@
diff --git a/libitm/barrier.cc b/libitm/barrier.cc
index 52e5a08..10424d0 100644
--- a/libitm/barrier.cc
+++ b/libitm/barrier.cc
@@ -1,4 +1,4 @@
-/* Copyright (C) 2008, 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2008, 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU Transactional Memory Library (libitm).
@@ -23,15 +23,23 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include "libitm_i.h"
-#include "barrier.tpl"
-
-ITM_BARRIERS(U1)
-ITM_BARRIERS(U2)
-ITM_BARRIERS(U4)
-ITM_BARRIERS(U8)
-ITM_BARRIERS(F)
-ITM_BARRIERS(D)
-ITM_BARRIERS(E)
-ITM_BARRIERS(CF)
-ITM_BARRIERS(CD)
-ITM_BARRIERS(CE)
+
+using namespace GTM;
+
+bool abi_dispatch::memmove_overlap_check(void *dst, const void *src,
+    size_t size, ls_modifier dst_mod, ls_modifier src_mod)
+{
+  if (dst_mod == NONTXNAL || src_mod == NONTXNAL)
+    {
+      if (((uintptr_t)dst <= (uintptr_t)src ?
+          (uintptr_t)dst + size > (uintptr_t)src :
+          (uintptr_t)src + size > (uintptr_t)dst))
+        GTM::GTM_fatal("_ITM_memmove overlapping and t/nt is not allowed");
+      return false;
+    }
+  return true;
+}
+
+CREATE_DISPATCH_FUNCTIONS(GTM::abi_disp()->_, )
+//CREATE_DISPATCH_FUNCTIONS(GTM::abi_dispatch::_, _static)
+
diff --git a/libitm/beginend.cc b/libitm/beginend.cc
index 37e23e2..7ed5073 100644
--- a/libitm/beginend.cc
+++ b/libitm/beginend.cc
@@ -1,4 +1,4 @@
-/* Copyright (C) 2008, 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2008, 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU Transactional Memory Library (libitm).
@@ -128,13 +128,27 @@ GTM::gtm_transaction::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb)
 
   set_gtm_tx (tx);
 
+  // ??? pr_undoLogCode is not properly defined in the ABI. Are barriers
+  // omitted because they are not necessary (e.g., a transaction on thread-
+  // local data) or because the compiler thinks that some kind of global
+  // synchronization might perform better?
+  if (unlikely(prop & pr_undoLogCode))
+    GTM_fatal("pr_undoLogCode not supported");
+
   if ((prop & pr_doesGoIrrevocable) || !(prop & pr_instrumentedCode))
+    tx->state = (STATE_SERIAL | STATE_IRREVOCABLE);
+
+  else
+    disp = tx->decide_begin_dispatch ();
+
+  if (tx->state & STATE_SERIAL)
     {
       serial_lock.write_lock ();
 
-      tx->state = (STATE_SERIAL | STATE_IRREVOCABLE);
-
-      disp = dispatch_serial ();
+      if (tx->state & STATE_IRREVOCABLE)
+        disp = dispatch_serialirr ();
+      else
+        disp = dispatch_serial ();
 
       ret = a_runUninstrumentedCode;
       if ((prop & pr_multiwayCode) == pr_instrumentedCode)
@@ -143,14 +157,6 @@ GTM::gtm_transaction::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb)
   else
     {
       serial_lock.read_lock ();
-
-      // ??? Probably want some environment variable to choose the default
-      // STM implementation once we have more than one implemented.
-      if (prop & pr_readOnly)
-	disp = dispatch_readonly ();
-      else
-	disp = dispatch_wbetl ();
-
       ret = a_runInstrumentedCode | a_saveLiveVariables;
     }
 
diff --git a/libitm/common.h b/libitm/common.h
new file mode 100644
index 0000000..d94ca94
--- /dev/null
+++ b/libitm/common.h
@@ -0,0 +1,43 @@
+/* Copyright (C) 2008, 2009 Free Software Foundation, Inc.
+   Contributed by Richard Henderson <rth@redhat.com>.
+
+   This file is part of the GNU Transactional Memory Library (libitm).
+
+   Libitm is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   Libitm is distributed in the hope that it will be useful, but WITHOUT ANY
+   WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+   FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+   more details.
+
+   Under Section 7 of GPL version 3, you are granted additional
+   permissions described in the GCC Runtime Library Exception, version
+   3.1, as published by the Free Software Foundation.
+
+   You should have received a copy of the GNU General Public License and
+   a copy of the GCC Runtime Library Exception along with this program;
+   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* The following are internal implementation functions and definitions.
+   To distinguish them from those defined by the Intel ABI, they all
+   begin with GTM/gtm.  */
+
+#ifndef COMMON_H
+#define COMMON_H 1
+
+#define UNUSED		__attribute__((unused))
+#define ALWAYS_INLINE	__attribute__((always_inline))
+#ifdef HAVE_ATTRIBUTE_VISIBILITY
+# define HIDDEN		__attribute__((visibility("hidden")))
+#else
+# define HIDDEN
+#endif
+
+#define likely(X)	__builtin_expect((X) != 0, 1)
+#define unlikely(X)	__builtin_expect((X), 0)
+
+#endif // COMMON_H
diff --git a/libitm/config/x86/x86_avx.cc b/libitm/config/x86/x86_avx.cc
index ee073d5..f81e3b8 100644
--- a/libitm/config/x86/x86_avx.cc
+++ b/libitm/config/x86/x86_avx.cc
@@ -1,4 +1,4 @@
-/* Copyright (C) 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU Transactional Memory Library (libitm).
@@ -23,14 +23,16 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include "libitm_i.h"
-#include "barrier.tpl"
+#include "dispatch.h"
 
-ITM_BARRIERS(M256)
+// ??? Use memcpy for now, until we have figured out how to best instantiate
+// these loads/stores.
+CREATE_DISPATCH_FUNCTIONS_T_MEMCPY(M256, GTM::abi_disp()->_, )
 
 void ITM_REGPARM
 _ITM_LM256 (const _ITM_TYPE_M256 *ptr)
 {
-  GTM_LB (ptr, sizeof (*ptr));
+  GTM::GTM_LB (ptr, sizeof (*ptr));
 }
 
 // Helpers for re-aligning two 128-bit values.
diff --git a/libitm/config/x86/x86_sse.cc b/libitm/config/x86/x86_sse.cc
index 5bb9766..a8585e4 100644
--- a/libitm/config/x86/x86_sse.cc
+++ b/libitm/config/x86/x86_sse.cc
@@ -1,4 +1,4 @@
-/* Copyright (C) 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU Transactional Memory Library (libitm).
@@ -23,21 +23,23 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include "libitm_i.h"
-#include "barrier.tpl"
+#include "dispatch.h"
 
-ITM_BARRIERS(M64)
-ITM_BARRIERS(M128)
+// ??? Use memcpy for now, until we have figured out how to best instantiate
+// these loads/stores.
+CREATE_DISPATCH_FUNCTIONS_T_MEMCPY(M64, GTM::abi_disp()->_, )
+CREATE_DISPATCH_FUNCTIONS_T_MEMCPY(M128, GTM::abi_disp()->_, )
 
 void ITM_REGPARM
 _ITM_LM64 (const _ITM_TYPE_M64 *ptr)
 {
-  GTM_LB (ptr, sizeof (*ptr));
+  GTM::GTM_LB (ptr, sizeof (*ptr));
 }
 
 void ITM_REGPARM
 _ITM_LM128 (const _ITM_TYPE_M128 *ptr)
 {
-  GTM_LB (ptr, sizeof (*ptr));
+  GTM::GTM_LB (ptr, sizeof (*ptr));
 }
 
 // Helpers for re-aligning two 128-bit values.
diff --git a/libitm/dispatch.h b/libitm/dispatch.h
new file mode 100644
index 0000000..955cfab
--- /dev/null
+++ b/libitm/dispatch.h
@@ -0,0 +1,283 @@
+/* Copyright (C) 2011 Free Software Foundation, Inc.
+   Contributed by Torvald Riegel <triegel@redhat.com>.
+
+   This file is part of the GNU Transactional Memory Library (libitm).
+
+   Libitm is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   Libitm is distributed in the hope that it will be useful, but WITHOUT ANY
+   WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+   FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+   more details.
+
+   Under Section 7 of GPL version 3, you are granted additional
+   permissions described in the GCC Runtime Library Exception, version
+   3.1, as published by the Free Software Foundation.
+
+   You should have received a copy of the GNU General Public License and
+   a copy of the GCC Runtime Library Exception along with this program;
+   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef DISPATCH_H
+#define DISPATCH_H 1
+
+#include "libitm.h"
+#include "common.h"
+
+// Creates ABI load/store methods (can be made virtual or static using M,
+// use M2 to create separate methods names for virtual and static)
+// The _PV variants are for the pure-virtual methods in the base class.
+#define ITM_READ_M(T, LSMOD, M, M2)                                         \
+  M _ITM_TYPE_##T ITM_REGPARM _ITM_##LSMOD##T##M2 (const _ITM_TYPE_##T *ptr)\
+  {                                                                         \
+    return load(ptr, abi_dispatch::LSMOD);                                  \
+  }
+
+#define ITM_READ_M_PV(T, LSMOD, M, M2)                                      \
+  M _ITM_TYPE_##T ITM_REGPARM _ITM_##LSMOD##T##M2 (const _ITM_TYPE_##T *ptr)\
+  = 0;
+
+#define ITM_WRITE_M(T, LSMOD, M, M2)                         \
+  M void ITM_REGPARM _ITM_##LSMOD##T##M2 (_ITM_TYPE_##T *ptr,\
+                                         _ITM_TYPE_##T val)  \
+  {                                                          \
+    store(ptr, val, abi_dispatch::LSMOD);                    \
+  }
+
+#define ITM_WRITE_M_PV(T, LSMOD, M, M2)                      \
+  M void ITM_REGPARM _ITM_##LSMOD##T##M2 (_ITM_TYPE_##T *ptr,\
+                                         _ITM_TYPE_##T val)  \
+  = 0;
+
+// Creates ABI load/store methods for all load/store modifiers for a particular
+// type.
+#define CREATE_DISPATCH_METHODS_T(T, M, M2) \
+  ITM_READ_M(T, R, M, M2)                \
+  ITM_READ_M(T, RaR, M, M2)              \
+  ITM_READ_M(T, RaW, M, M2)              \
+  ITM_READ_M(T, RfW, M, M2)              \
+  ITM_WRITE_M(T, W, M, M2)               \
+  ITM_WRITE_M(T, WaR, M, M2)             \
+  ITM_WRITE_M(T, WaW, M, M2)
+#define CREATE_DISPATCH_METHODS_T_PV(T, M, M2) \
+  ITM_READ_M_PV(T, R, M, M2)                \
+  ITM_READ_M_PV(T, RaR, M, M2)              \
+  ITM_READ_M_PV(T, RaW, M, M2)              \
+  ITM_READ_M_PV(T, RfW, M, M2)              \
+  ITM_WRITE_M_PV(T, W, M, M2)               \
+  ITM_WRITE_M_PV(T, WaR, M, M2)             \
+  ITM_WRITE_M_PV(T, WaW, M, M2)
+
+// Creates ABI load/store methods for all types.
+// See CREATE_DISPATCH_FUNCTIONS for comments.
+#define CREATE_DISPATCH_METHODS(M, M2)  \
+  CREATE_DISPATCH_METHODS_T (U1, M, M2) \
+  CREATE_DISPATCH_METHODS_T (U2, M, M2) \
+  CREATE_DISPATCH_METHODS_T (U4, M, M2) \
+  CREATE_DISPATCH_METHODS_T (U8, M, M2) \
+  CREATE_DISPATCH_METHODS_T (F, M, M2)  \
+  CREATE_DISPATCH_METHODS_T (D, M, M2)  \
+  CREATE_DISPATCH_METHODS_T (E, M, M2)  \
+  CREATE_DISPATCH_METHODS_T (CF, M, M2) \
+  CREATE_DISPATCH_METHODS_T (CD, M, M2) \
+  CREATE_DISPATCH_METHODS_T (CE, M, M2)
+#define CREATE_DISPATCH_METHODS_PV(M, M2)  \
+  CREATE_DISPATCH_METHODS_T_PV (U1, M, M2) \
+  CREATE_DISPATCH_METHODS_T_PV (U2, M, M2) \
+  CREATE_DISPATCH_METHODS_T_PV (U4, M, M2) \
+  CREATE_DISPATCH_METHODS_T_PV (U8, M, M2) \
+  CREATE_DISPATCH_METHODS_T_PV (F, M, M2)  \
+  CREATE_DISPATCH_METHODS_T_PV (D, M, M2)  \
+  CREATE_DISPATCH_METHODS_T_PV (E, M, M2)  \
+  CREATE_DISPATCH_METHODS_T_PV (CF, M, M2) \
+  CREATE_DISPATCH_METHODS_T_PV (CD, M, M2) \
+  CREATE_DISPATCH_METHODS_T_PV (CE, M, M2)
+
+// Creates memcpy/memmove/memset methods.
+#define CREATE_DISPATCH_METHODS_MEM()  \
+virtual void _memtransfer(void *dst, const void* src, size_t size,    \
+    bool may_overlap, ls_modifier dst_mod, ls_modifier src_mod)       \
+{                                                                     \
+  _memtransfer_static(dst, src, size, may_overlap, dst_mod, src_mod); \
+}                                                                     \
+virtual void _memset(void *dst, int c, size_t size, ls_modifier mod)  \
+{                                                                     \
+  _memset_static(dst, c, size, mod);                                  \
+}
+
+#define CREATE_DISPATCH_METHODS_MEM_PV()  \
+virtual void _memtransfer(void *dst, const void* src, size_t size,       \
+    bool may_overlap, ls_modifier dst_mod, ls_modifier src_mod) = 0;     \
+virtual void _memset(void *dst, int c, size_t size, ls_modifier mod) = 0;
+
+
+// Creates ABI load/store functions that can target either a class or an
+// object.
+#define ITM_READ(T, LSMOD, TARGET, M2)                                 \
+  _ITM_TYPE_##T ITM_REGPARM _ITM_##LSMOD##T (const _ITM_TYPE_##T *ptr) \
+  {                                                                    \
+    return TARGET##ITM_##LSMOD##T##M2(ptr);                            \
+  }
+
+#define ITM_WRITE(T, LSMOD, TARGET, M2)                                    \
+  void ITM_REGPARM _ITM_##LSMOD##T (_ITM_TYPE_##T *ptr, _ITM_TYPE_##T val) \
+  {                                                                        \
+    TARGET##ITM_##LSMOD##T##M2(ptr, val);                                  \
+  }
+
+// Creates ABI load/store functions for all load/store modifiers for a
+// particular type.
+#define CREATE_DISPATCH_FUNCTIONS_T(T, TARGET, M2) \
+  ITM_READ(T, R, TARGET, M2)                \
+  ITM_READ(T, RaR, TARGET, M2)              \
+  ITM_READ(T, RaW, TARGET, M2)              \
+  ITM_READ(T, RfW, TARGET, M2)              \
+  ITM_WRITE(T, W, TARGET, M2)               \
+  ITM_WRITE(T, WaR, TARGET, M2)             \
+  ITM_WRITE(T, WaW, TARGET, M2)
+
+// Creates ABI memcpy/memmove/memset functions.
+#define ITM_MEMTRANSFER_DEF(TARGET, M2, NAME, READ, WRITE) \
+void ITM_REGPARM _ITM_memcpy##NAME(void *dst, const void *src, size_t size)  \
+{                                                                            \
+  TARGET##memtransfer##M2 (dst, src, size,                                   \
+             false, GTM::abi_dispatch::WRITE, GTM::abi_dispatch::READ);      \
+}                                                                            \
+void ITM_REGPARM _ITM_memmove##NAME(void *dst, const void *src, size_t size) \
+{                                                                            \
+  TARGET##memtransfer##M2 (dst, src, size,                                   \
+      GTM::abi_dispatch::memmove_overlap_check(dst, src, size,               \
+          GTM::abi_dispatch::WRITE, GTM::abi_dispatch::READ),                \
+      GTM::abi_dispatch::WRITE, GTM::abi_dispatch::READ);                    \
+}
+
+#define ITM_MEMSET_DEF(TARGET, M2, WRITE) \
+void ITM_REGPARM _ITM_memset##WRITE(void *dst, int c, size_t size) \
+{                                                                  \
+  TARGET##memset##M2 (dst, c, size, GTM::abi_dispatch::WRITE);     \
+}                                                                  \
+
+
+// ??? The number of virtual methods is large (7*4 for integers, 7*6 for FP,
+// 7*3 for vectors). Is the cache footprint so costly that we should go for
+// a small table instead (i.e., only have two virtual load/store methods for
+// each supported type)? Note that this doesn't affect custom code paths at
+// all because these use only direct calls.
+// A large cache footprint could especially decrease HTM performance (due
+// to HTM capacity). We could add the modifier (RaR etc.) as parameter, which
+// would give us just 4*2+6*2+3*2 functions (so we'd just need one line for
+// the integer loads/stores), but then the modifier can be checked only at
+// runtime.
+// For memcpy/memmove/memset, we just have two virtual methods (memtransfer
+// and memset).
+#define CREATE_DISPATCH_FUNCTIONS(TARGET, M2)  \
+  CREATE_DISPATCH_FUNCTIONS_T (U1, TARGET, M2) \
+  CREATE_DISPATCH_FUNCTIONS_T (U2, TARGET, M2) \
+  CREATE_DISPATCH_FUNCTIONS_T (U4, TARGET, M2) \
+  CREATE_DISPATCH_FUNCTIONS_T (U8, TARGET, M2) \
+  CREATE_DISPATCH_FUNCTIONS_T (F, TARGET, M2)  \
+  CREATE_DISPATCH_FUNCTIONS_T (D, TARGET, M2)  \
+  CREATE_DISPATCH_FUNCTIONS_T (E, TARGET, M2)  \
+  CREATE_DISPATCH_FUNCTIONS_T (CF, TARGET, M2) \
+  CREATE_DISPATCH_FUNCTIONS_T (CD, TARGET, M2) \
+  CREATE_DISPATCH_FUNCTIONS_T (CE, TARGET, M2) \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RnWt,     NONTXNAL, W)      \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RnWtaR,   NONTXNAL, WaR)    \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RnWtaW,   NONTXNAL, WaW)    \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtWn,     R,      NONTXNAL) \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtWt,     R,      W)        \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtWtaR,   R,      WaR)      \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtWtaW,   R,      WaW)      \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtaRWn,   RaR,    NONTXNAL) \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtaRWt,   RaR,    W)        \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtaRWtaR, RaR,    WaR)      \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtaRWtaW, RaR,    WaW)      \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtaWWn,   RaW,    NONTXNAL) \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtaWWt,   RaW,    W)        \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtaWWtaR, RaW,    WaR)      \
+  ITM_MEMTRANSFER_DEF(TARGET, M2, RtaWWtaW, RaW,    WaW)      \
+  ITM_MEMSET_DEF(TARGET, M2, W)   \
+  ITM_MEMSET_DEF(TARGET, M2, WaR) \
+  ITM_MEMSET_DEF(TARGET, M2, WaW)
+
+
+// Creates ABI load/store functions that delegate to a transactional memcpy.
+#define ITM_READ_MEMCPY(T, LSMOD, TARGET, M2)                         \
+  _ITM_TYPE_##T ITM_REGPARM _ITM_##LSMOD##T (const _ITM_TYPE_##T *ptr)\
+  {                                                                   \
+    _ITM_TYPE_##T v;                                                  \
+    TARGET##memtransfer##M2(&v, ptr, sizeof(_ITM_TYPE_##T), false,    \
+        GTM::abi_dispatch::NONTXNAL, GTM::abi_dispatch::LSMOD);       \
+    return v;                                                         \
+  }
+
+#define ITM_WRITE_MEMCPY(T, LSMOD, TARGET, M2)                            \
+  void ITM_REGPARM _ITM_##LSMOD##T (_ITM_TYPE_##T *ptr, _ITM_TYPE_##T val)\
+  {                                                                       \
+    TARGET##memtransfer##M2(ptr, &val, sizeof(_ITM_TYPE_##T), false,      \
+        GTM::abi_dispatch::LSMOD, GTM::abi_dispatch::NONTXNAL);           \
+  }
+
+#define CREATE_DISPATCH_FUNCTIONS_T_MEMCPY(T, TARGET, M2) \
+  ITM_READ_MEMCPY(T, R, TARGET, M2)                \
+  ITM_READ_MEMCPY(T, RaR, TARGET, M2)              \
+  ITM_READ_MEMCPY(T, RaW, TARGET, M2)              \
+  ITM_READ_MEMCPY(T, RfW, TARGET, M2)              \
+  ITM_WRITE_MEMCPY(T, W, TARGET, M2)               \
+  ITM_WRITE_MEMCPY(T, WaR, TARGET, M2)             \
+  ITM_WRITE_MEMCPY(T, WaW, TARGET, M2)
+
+
+namespace GTM HIDDEN {
+
+// This pass-through method is the basis for other methods.
+// It can be used for serial-irrevocable mode.
+struct abi_dispatch
+{
+public:
+  enum ls_modifier { NONTXNAL, R, RaR, RaW, RfW, W, WaR, WaW };
+
+private:
+  // Disallow copies
+  abi_dispatch(const abi_dispatch &) = delete;
+  abi_dispatch& operator=(const abi_dispatch &) = delete;
+
+public:
+  virtual bool trycommit() = 0;
+  virtual void rollback() = 0;
+  virtual void reinit() = 0;
+
+  // Use fini instead of dtor to support a static subclasses that uses
+  // a unique object and so we don't want to destroy it from common code.
+  virtual void fini() = 0;
+
+  bool read_only () const { return m_read_only; }
+  bool write_through() const { return m_write_through; }
+
+  static void *operator new(size_t s) { return xmalloc (s); }
+  static void operator delete(void *p) { free (p); }
+
+public:
+  static bool memmove_overlap_check(void *dst, const void *src, size_t size,
+      ls_modifier dst_mod, ls_modifier src_mod);
+
+  // Creates the ABI dispatch methods for loads and stores.
+  // ??? Should the dispatch table instead be embedded in the dispatch object
+  // to avoid the indirect lookup in the vtable?
+  CREATE_DISPATCH_METHODS_PV(virtual, )
+  // Creates the ABI dispatch methods for memcpy/memmove/memset.
+  CREATE_DISPATCH_METHODS_MEM_PV()
+
+protected:
+  const bool m_read_only;
+  const bool m_write_through;
+  abi_dispatch(bool ro, bool wt) : m_read_only(ro), m_write_through(wt) { }
+};
+
+}
+
+#endif // DISPATCH_H
diff --git a/libitm/libitm_i.h b/libitm/libitm_i.h
index 3291f1b..d323d45 100644
--- a/libitm/libitm_i.h
+++ b/libitm/libitm_i.h
@@ -1,4 +1,4 @@
-/* Copyright (C) 2008, 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2008, 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU Transactional Memory Library (libitm).
@@ -38,16 +38,7 @@
 #include <unwind.h>
 #include <type_traits>
 
-#define UNUSED		__attribute__((unused))
-#define ALWAYS_INLINE	__attribute__((always_inline))
-#ifdef HAVE_ATTRIBUTE_VISIBILITY
-# define HIDDEN		__attribute__((visibility("hidden")))
-#else
-# define HIDDEN
-#endif
-
-#define likely(X)	__builtin_expect((X) != 0, 1)
-#define unlikely(X)	__builtin_expect((X), 0)
+#include "common.h"
 
 namespace GTM HIDDEN {
 
@@ -82,57 +73,12 @@ extern void * xrealloc (void *p, size_t s) __attribute__((malloc, nothrow));
 #include "cacheline.h"
 #include "cachepage.h"
 #include "stmlock.h"
+#include "dispatch.h"
 
 namespace GTM HIDDEN {
 
 // A dispatch table parameterizes the implementation of the STM.
-struct abi_dispatch
-{
- public:
-  enum lock_type { NOLOCK, R, RaR, RaW, RfW, W, WaR, WaW };
-
-  struct mask_pair
-  {
-    gtm_cacheline *line;
-    gtm_cacheline_mask *mask;
-
-    mask_pair() = default;
-    mask_pair(gtm_cacheline *l, gtm_cacheline_mask *m) : line(l), mask(m) { }
-  };
-
- private:
-  // Disallow copies
-  abi_dispatch(const abi_dispatch &) = delete;
-  abi_dispatch& operator=(const abi_dispatch &) = delete;
-
- public:
-  // The default version of these is pass-through.  This merely gives the
-  // a single location to instantiate the base class vtable.
-  virtual const gtm_cacheline *read_lock(const gtm_cacheline *, lock_type);
-  virtual mask_pair write_lock(gtm_cacheline *, lock_type);
-
-  virtual bool trycommit() = 0;
-  virtual void rollback() = 0;
-  virtual void reinit() = 0;
-  virtual bool trydropreference(void *, size_t) = 0;
-
-  // Use fini instead of dtor to support a static subclasses that uses
-  // a unique object and so we don't want to destroy it from common code.
-  virtual void fini() = 0;
-
-  bool read_only () const { return m_read_only; }
-  bool write_through() const { return m_write_through; }
-
-  static void *operator new(size_t s) { return xmalloc (s); }
-  static void operator delete(void *p) { free (p); }
-
- protected:
-  const bool m_read_only;
-  const bool m_write_through;
-  abi_dispatch(bool ro, bool wt) : m_read_only(ro), m_write_through(wt) { }
-
-  static gtm_cacheline_mask mask_sink;
-};
+struct abi_dispatch;
 
 // These values are given to GTM_restart_transaction and indicate the
 // reason for the restart.  The reason is used to decide what STM
@@ -256,6 +202,7 @@ struct gtm_transaction
 
   // In retry.cc
   void decide_retry_strategy (gtm_restart_reason);
+  abi_dispatch* decide_begin_dispatch ();
 
   // In method-serial.cc
   void serialirr_mode ();
@@ -287,9 +234,8 @@ extern void GTM_error (const char *fmt, ...)
 extern void GTM_fatal (const char *fmt, ...)
 	__attribute__((noreturn, format (printf, 1, 2)));
 
-extern abi_dispatch *dispatch_wbetl();
-extern abi_dispatch *dispatch_readonly();
 extern abi_dispatch *dispatch_serial();
+extern abi_dispatch *dispatch_serialirr();
 
 extern gtm_cacheline_mask gtm_mask_stack(gtm_cacheline *, gtm_cacheline_mask);
 
diff --git a/libitm/method-readonly.cc b/libitm/method-readonly.cc
deleted file mode 100644
index ed573e2..0000000
--- a/libitm/method-readonly.cc
+++ /dev/null
@@ -1,124 +0,0 @@
-/* Copyright (C) 2009 Free Software Foundation, Inc.
-   Contributed by Richard Henderson <rth@redhat.com>.
-
-   This file is part of the GNU Transactional Memory Library (libitm).
-
-   Libitm is free software; you can redistribute it and/or modify it
-   under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 3 of the License, or
-   (at your option) any later version.
-
-   Libitm is distributed in the hope that it will be useful, but WITHOUT ANY
-   WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
-   FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
-   more details.
-
-   Under Section 7 of GPL version 3, you are granted additional
-   permissions described in the GCC Runtime Library Exception, version
-   3.1, as published by the Free Software Foundation.
-
-   You should have received a copy of the GNU General Public License and
-   a copy of the GCC Runtime Library Exception along with this program;
-   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
-   <http://www.gnu.org/licenses/>.  */
-
-#include "libitm_i.h"
-
-namespace {
-
-using namespace GTM;
-
-class readonly_dispatch : public abi_dispatch
-{
- private:
-  gtm_version m_start;
-
- public:
-  readonly_dispatch();
-
-  virtual const gtm_cacheline *read_lock(const gtm_cacheline *, lock_type);
-  virtual mask_pair write_lock(gtm_cacheline *, lock_type);
-  virtual bool trycommit();
-  virtual void rollback();
-  virtual void reinit();
-  virtual void fini();
-  virtual bool trydropreference (void *ptr, size_t size) { return true; }
-};
-
-inline
-readonly_dispatch::readonly_dispatch()
-  : abi_dispatch(true, true), m_start(gtm_get_clock ())
-{ }
-
-
-const gtm_cacheline *
-readonly_dispatch::read_lock(const gtm_cacheline *line, lock_type lock)
-{
-  switch (lock)
-    {
-    case NOLOCK:
-    case R:
-    case RaR:
-      return line;
-
-    case RfW:
-      gtm_tx()->restart (RESTART_NOT_READONLY);
-
-    case RaW:
-    default:
-      abort ();
-    }
-}
-
-abi_dispatch::mask_pair
-readonly_dispatch::write_lock(gtm_cacheline *line, lock_type lock)
-{
-  switch (lock)
-    {
-    case NOLOCK:
-      {
-	abi_dispatch::mask_pair pair;
-	pair.line = line;
-	pair.mask = &mask_sink;
-	return pair;
-      }
-
-    case WaW:
-      abort ();
-
-    default:
-      gtm_tx()->restart (RESTART_NOT_READONLY);
-    }
-}
-
-bool
-readonly_dispatch::trycommit ()
-{
-  return gtm_get_clock () == m_start;
-}
-
-void
-readonly_dispatch::rollback ()
-{
-  /* Nothing to do.  */
-}
-
-void
-readonly_dispatch::reinit ()
-{
-  m_start = gtm_get_clock ();
-}
-
-void
-readonly_dispatch::fini ()
-{
-  delete this;
-}
-
-} // anon namespace
-
-abi_dispatch *
-GTM::dispatch_readonly ()
-{
-  return new readonly_dispatch();
-}
diff --git a/libitm/method-serial.cc b/libitm/method-serial.cc
index b46de4d..71208ec 100644
--- a/libitm/method-serial.cc
+++ b/libitm/method-serial.cc
@@ -24,24 +24,6 @@
 
 #include "libitm_i.h"
 
-namespace GTM HIDDEN {
-
-gtm_cacheline_mask abi_dispatch::mask_sink;
-
-const gtm_cacheline *
-abi_dispatch::read_lock(const gtm_cacheline *addr, lock_type)
-{
-  return addr;
-}
-
-abi_dispatch::mask_pair
-abi_dispatch::write_lock(gtm_cacheline *addr, lock_type)
-{
-  return mask_pair (addr, &mask_sink);
-}
-
-} // namespace GTM
-
 // Avoid a dependency on libstdc++ for the pure virtuals in abi_dispatch.
 extern "C" void HIDDEN
 __cxa_pure_virtual ()
@@ -58,25 +40,110 @@ class serial_dispatch : public abi_dispatch
  public:
   serial_dispatch() : abi_dispatch(false, true) { }
 
-  // The read_lock and write_lock methods are implented by the base class.
+ protected:
+  // Transactional loads and stores simply access memory directly.
+  // These methods are static to avoid indirect calls, and will be used by the
+  // virtual ABI dispatch methods or by static direct-access methods created
+  // below.
+  template <typename V> static V load(const V* addr, ls_modifier mod)
+  {
+    return *addr;
+  }
+  template <typename V> static void store(V* addr, const V value,
+      ls_modifier mod)
+  {
+    *addr = value;
+  }
+
+  static void _memtransfer_static(void *dst, const void* src, size_t size,
+      bool may_overlap, ls_modifier dst_mod, ls_modifier src_mod)
+  {
+    if (!may_overlap)
+      ::memcpy(dst, src, size);
+    else
+      ::memmove(dst, src, size);
+  }
+
+  static void _memset_static(void *dst, int c, size_t size, ls_modifier mod)
+  {
+    ::memset(dst, c, size);
+  }
+
+  CREATE_DISPATCH_METHODS(virtual, )
+  CREATE_DISPATCH_METHODS_MEM()
 
+ public:
   virtual bool trycommit() { return true; }
   virtual void rollback() { abort(); }
   virtual void reinit() { }
   virtual void fini() { }
-  virtual bool trydropreference (void *ptr, size_t size) { return true; }
+};
+
+class serial_dispatch_ul : public serial_dispatch
+{
+protected:
+  static void log(const void *addr, size_t len)
+  {
+    // TODO Ensure that this gets inlined: Use internal log interface and LTO.
+    GTM_LB(addr, len);
+  }
+
+  template <typename V> static V load(const V* addr, ls_modifier mod)
+  {
+    return *addr;
+  }
+  template <typename V> static void store(V* addr, const V value,
+      ls_modifier mod)
+  {
+    if (mod != WaW)
+      log(addr, sizeof(V));
+    *addr = value;
+  }
+
+public:
+  static void memtransfer_static(void *dst, const void* src, size_t size,
+      bool may_overlap, ls_modifier dst_mod, ls_modifier src_mod)
+  {
+    if (dst_mod != WaW && dst_mod != NONTXNAL)
+      log(dst, size);
+    if (!may_overlap)
+      memcpy(dst, src, size);
+    else
+      memmove(dst, src, size);
+  }
+
+  static void memset_static(void *dst, int c, size_t size, ls_modifier mod)
+  {
+    if (mod != WaW)
+      log(dst, size);
+    memset(dst, c, size);
+  }
+
+  // Local undo will handle this.
+  // trydropreference() need not be changed either.
+  virtual void rollback() { }
+
+  CREATE_DISPATCH_METHODS(virtual, )
+  CREATE_DISPATCH_METHODS_MEM()
 };
 
 } // anon namespace
 
 static const serial_dispatch o_serial_dispatch;
+static const serial_dispatch_ul o_serial_dispatch_ul;
 
 abi_dispatch *
-GTM::dispatch_serial ()
+GTM::dispatch_serialirr ()
 {
   return const_cast<serial_dispatch *>(&o_serial_dispatch);
 }
 
+abi_dispatch *
+GTM::dispatch_serial ()
+{
+  return const_cast<serial_dispatch_ul *>(&o_serial_dispatch_ul);
+}
+
 // Put the transaction into serial-irrevocable mode.
 
 void
diff --git a/libitm/method-wbetl.cc b/libitm/method-wbetl.cc
index 7ce317e..aff6397 100644
--- a/libitm/method-wbetl.cc
+++ b/libitm/method-wbetl.cc
@@ -83,8 +83,8 @@ class wbetl_dispatch : public abi_dispatch
  public:
   wbetl_dispatch();
 
-  virtual const gtm_cacheline *read_lock(const gtm_cacheline *, lock_type);
-  virtual mask_pair write_lock(gtm_cacheline *, lock_type);
+  virtual const gtm_cacheline *read_lock(const gtm_cacheline *, ls_modifier);
+  virtual mask_pair write_lock(gtm_cacheline *, ls_modifier);
 
   virtual bool trycommit();
   virtual void rollback();
@@ -375,11 +375,11 @@ wbetl_dispatch::do_read_lock (const gtm_cacheline *addr, bool after_read)
 }
 
 const gtm_cacheline *
-wbetl_dispatch::read_lock (const gtm_cacheline *addr, lock_type ltype)
+wbetl_dispatch::read_lock (const gtm_cacheline *addr, ls_modifier ltype)
 {
   switch (ltype)
     {
-    case NOLOCK:
+    case NONTXNAL:
       return addr;
     case R:
       return do_read_lock (addr, false);
@@ -395,13 +395,13 @@ wbetl_dispatch::read_lock (const gtm_cacheline *addr, lock_type ltype)
 }
 
 abi_dispatch::mask_pair
-wbetl_dispatch::write_lock (gtm_cacheline *addr, lock_type ltype)
+wbetl_dispatch::write_lock (gtm_cacheline *addr, ls_modifier ltype)
 {
   gtm_cacheline *line;
 
   switch (ltype)
     {
-    case NOLOCK:
+    case NONTXNAL:
       return mask_pair (addr, &mask_sink);
     case W:
     case WaR:
diff --git a/libitm/retry.cc b/libitm/retry.cc
index a25b282..8e586de 100644
--- a/libitm/retry.cc
+++ b/libitm/retry.cc
@@ -42,6 +42,10 @@ GTM::gtm_transaction::decide_retry_strategy (gtm_restart_reason r)
       // write lock is not yet held, grab it.  Don't do this with
       // an upgrade, since we've no need to preserve the state we
       // acquired with the read.
+      // FIXME this might be dangerous if we use serial mode to change TM
+      // meta data (e.g., reallocate the lock array). Likewise, for
+      // privatization, we must get rid of old references (that is, abort)
+      // or let privatizers know we're still there by not releasing the lock.
       if ((this->state & STATE_SERIAL) == 0)
 	{
 	  this->state |= STATE_SERIAL;
@@ -65,17 +69,23 @@ GTM::gtm_transaction::decide_retry_strategy (gtm_restart_reason r)
     }
   else
     {
-      if (r == RESTART_NOT_READONLY)
-	{
-	  assert ((this->prop & pr_readOnly) == 0);
-	  if (disp->read_only ())
-	    {
-	      disp->fini ();
-	      disp = dispatch_wbetl ();
-	      set_abi_disp (disp);
-	      return;
-	    }
-	}
-      disp->reinit ();
+      GTM_fatal("internal error: unsupported mode");
     }
 }
+
+
+// Decides which TM method should be used on the first attempt to run this
+// transaction, setting this->disp accordingly.
+// serial_lock will not have been acquired if this is the outer-most
+// transaction. If the state is set to STATE_SERIAL, the caller will set the
+// dispatch.
+GTM::abi_dispatch*
+GTM::gtm_transaction::decide_begin_dispatch ()
+{
+  // ??? Probably want some environment variable to choose the default
+  // STM implementation once we have more than one implemented.
+  state = STATE_SERIAL;
+  if (prop & pr_hasNoAbort)
+    state |= STATE_IRREVOCABLE;
+  return 0;
+}

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [trans-mem] Beginning of refactoring
  2011-06-30 12:19   ` Torvald Riegel
@ 2011-06-30 15:03     ` Richard Henderson
  2011-06-30 17:02       ` Torvald Riegel
  0 siblings, 1 reply; 19+ messages in thread
From: Richard Henderson @ 2011-06-30 15:03 UTC (permalink / raw)
  To: Torvald Riegel; +Cc: GCC Patches

On 06/30/2011 04:15 AM, Torvald Riegel wrote:
> On Wed, 2011-06-29 at 16:01 -0700, Richard Henderson wrote:
>> On 05/25/2011 02:10 PM, Torvald Riegel wrote:
>>> Patch 2: _ITM_dropReferences is not sufficiently defined in the ABI. It
>>> seems to target some form of open nesting for txnal wrappers, but the
>>> prose in the ABI specification is unclear. Thus, disable this for now
>>> (aka fatal runtime error), and expect the related tests to fail. Pick it
>>> up again once that the ABI has been improved and the use cases are
>>> clear.
>>
>> Sure, but please actually delete the code rather than just comment it out.
> 
> Fixed in updated patch2.

This patch is ok.

> How it works is that in a TM-method-specific dispatch class we would add
>   CREATE_DISPATCH_METHODS(static, _static)
> in addition to the 
>   CREATE_DISPATCH_METHODS(virtual, )
> that is currently there. In barrier.cc we can then do
>   CREATE_DISPATCH_FUNCTIONS(GTM::serial_dispatch::_, _static)
> and thus bypass the virtual functions, but still keep one implementation
> of the TM method (in the static template functions). This example would
> create functions that just use the serial-mode accesses.

Ah yes, I remember discussing this.

> The trailing underscore at the end of the first param is to make the
> preprocessor happen. Is there a better way to let it accept
> "GTM::abi_disp()->" as argument?

It depends on what else you're passing to the other methods above.

It does seem odd, though, that you're passing some bit of global
state to the virtual dispatch methods (the result of abi_disp())
whereas you're arranging to pass nothing but a name concatenation
to the others.

I'm not sure what to make of your first two examples above, but
let's suppose that the third example is the real one.  In which
case you don't actually need concatenation.  Remove the ## from
the macro and then you can pass in

  GTM::abi_disp()->
and
  GTM::serial_dispatch::

which require no more than adjacency.

>> Why?  We're already in a header that's clearly marked "internal".
> 
> I moved these macros to a common header so libitm_i.h doesn't have to be
> included everywhere. There was some circular dependency after the
> changes (I think in via the barrier definitions for SSE etc.). What's
> the issue that you see with this change?

Not really an issue, it just seemed odd.

> Are the attached updated patches okay for the branch?

Yeah, we can iterate on the points above without re-sending
the entire patch3 again.


r~

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [trans-mem] Beginning of refactoring
  2011-06-30 15:03     ` Richard Henderson
@ 2011-06-30 17:02       ` Torvald Riegel
  0 siblings, 0 replies; 19+ messages in thread
From: Torvald Riegel @ 2011-06-30 17:02 UTC (permalink / raw)
  To: Richard Henderson; +Cc: GCC Patches

On Thu, 2011-06-30 at 07:59 -0700, Richard Henderson wrote:
> I'm not sure what to make of your first two examples above, but
> let's suppose that the third example is the real one.  In which
> case you don't actually need concatenation.  Remove the ## from
> the macro and then you can pass in
> 
>   GTM::abi_disp()->
> and
>   GTM::serial_dispatch::
> 
> which require no more than adjacency.

Good point, thanks. Fixed it, and will commit this and the others to the
branch.

Torvald

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [trans-mem] Beginning of refactoring
  2011-05-25 22:45 [trans-mem] Beginning of refactoring Torvald Riegel
  2011-06-29 21:22 ` Torvald Riegel
  2011-06-29 23:18 ` Richard Henderson
@ 2011-07-01 20:23 ` Torvald Riegel
  2011-07-01 20:32   ` Richard Henderson
  2011-07-09 15:35 ` Torvald Riegel
  3 siblings, 1 reply; 19+ messages in thread
From: Torvald Riegel @ 2011-07-01 20:23 UTC (permalink / raw)
  To: GCC Patches; +Cc: Richard Henderson

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

Attached patch adds a vector-like container implementation and uses it
for a transaction's undo log. Also, adds a flag to xmalloc/xrealloc that
requests the allocated data to be on cache lines that are not shared
with data accessed by other threads (this will get an actual
implementation in a later patch).

Ok for branch?

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 9609 bytes --]

commit f8953dec8a791e691938cd2abc96eff3b9712024
Author: Torvald Riegel <triegel@redhat.com>
Date:   Fri Jul 1 22:18:33 2011 +0200

    Add vector-like container, use it for gtm_transaction::undo_log.
    
    	* containers.h: New file.
    	* util.cc (xmalloc, xrealloc): Accept cacheline-alloc flag.
    	* libitm_i.h (xmalloc, xrealloc): Moved declarations from here ...
    	* common.h: ... to here.
    	(local_undo): Use GTM::vector for gtm_transaction::local_undo.
    	* local.cc: Same.

diff --git a/libitm/common.h b/libitm/common.h
index d94ca94..9db566a 100644
--- a/libitm/common.h
+++ b/libitm/common.h
@@ -40,4 +40,24 @@
 #define likely(X)	__builtin_expect((X) != 0, 1)
 #define unlikely(X)	__builtin_expect((X), 0)
 
+namespace GTM HIDDEN {
+
+// Locally defined protected allocation functions.
+//
+// To avoid dependency on libstdc++ new/delete, as well as to not
+// interfere with the wrapping of the global new/delete we wrap for
+// the user in alloc_cpp.cc, use class-local versions that defer
+// to malloc/free.  Recall that operator new/delete does not go through
+// normal lookup and so we cannot simply inject a version into the
+// GTM namespace.
+// If separate_cl is true, the allocator will try to return memory that is on
+// cache lines that are not shared with any object used by another thread.
+extern void * xmalloc (size_t s, bool separate_cl = false)
+  __attribute__((malloc, nothrow));
+extern void * xrealloc (void *p, size_t s, bool separate_cl = false)
+  __attribute__((malloc, nothrow));
+
+} // namespace GTM
+
+
 #endif // COMMON_H
diff --git a/libitm/containers.h b/libitm/containers.h
new file mode 100644
index 0000000..13a6cbc
--- /dev/null
+++ b/libitm/containers.h
@@ -0,0 +1,110 @@
+/* Copyright (C) 2011 Free Software Foundation, Inc.
+   Contributed by Torvald Riegel <triegel@redhat.com>.
+
+   This file is part of the GNU Transactional Memory Library (libitm).
+
+   Libitm is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   Libitm is distributed in the hope that it will be useful, but WITHOUT ANY
+   WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+   FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+   more details.
+
+   Under Section 7 of GPL version 3, you are granted additional
+   permissions described in the GCC Runtime Library Exception, version
+   3.1, as published by the Free Software Foundation.
+
+   You should have received a copy of the GNU General Public License and
+   a copy of the GCC Runtime Library Exception along with this program;
+   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef LIBITM_CONTAINERS_H
+#define LIBITM_CONTAINERS_H 1
+
+#include "common.h"
+#include <stdio.h>
+
+namespace GTM HIDDEN {
+
+// A simple vector-like container.
+// If alloc_seperate_cl is true, allocations will happen on separate cache
+// lines.
+template <typename T, bool alloc_separate_cl = true>
+class vector
+{
+ private:
+  size_t m_capacity;
+  size_t m_size;
+  T* entries;
+
+  // Initial capacity of the vector.
+  static const size_t default_initial_capacity = 32;
+  // Above that capacity, grow vector by that size for each call.
+  static const size_t default_resize_max = 2048;
+  // Resize vector to at least this capacity.
+  static const size_t default_resize_min = 32;
+
+  // Don't try to copy this vector.
+  vector<T, alloc_separate_cl>(const vector<T, alloc_separate_cl>& x);
+
+ public:
+  typedef T datatype;
+  typedef T* iterator;
+
+  iterator begin() const { return entries; }
+  iterator end() const { return entries + m_size; }
+  T& operator[] (size_t pos) { return entries[pos]; }
+  const T& operator[] (size_t pos) const  { return entries[pos]; }
+
+  vector<T, alloc_separate_cl>(size_t initial_size = default_initial_capacity)
+    : m_capacity(initial_size),
+      m_size(0)
+  {
+    if (m_capacity > 0)
+      entries = (T*) xmalloc(sizeof(T) * m_capacity, alloc_separate_cl);
+    else
+      entries = 0;
+  }
+  ~vector<T, alloc_separate_cl>() { if (m_capacity) free(entries); }
+
+  void resize()
+  {
+    if (m_capacity >= default_resize_max)
+      m_capacity = m_capacity + default_resize_max;
+    else
+      m_capacity = m_capacity * 2;
+    if (m_capacity < default_resize_min)
+      m_capacity = default_resize_min;
+    entries = (T*) xrealloc(entries, sizeof(T) * m_capacity, alloc_separate_cl);
+  }
+  void resize_noinline() __attribute__((noinline)) { resize(); }
+
+  size_t size() const { return m_size; }
+  size_t capacity() const { return this->capacity; }
+
+  void clear() { m_size = 0; }
+
+  iterator push() {
+    // We don't want inlining here since push() is often on the fast path.
+    if (m_size == m_capacity) resize_noinline();
+    fprintf(stdout, "cap=%zu size=%zu ent=%p\n", m_capacity, m_size, entries);
+    return &entries[m_size++];
+  }
+
+  iterator pop() {
+    if (m_size > 0)
+      {
+        m_size--;
+        return entries + m_size;
+      }
+    else return 0;
+  }
+};
+
+} // namespace GTM
+
+#endif // LIBITM_CONTAINERS_H
diff --git a/libitm/libitm_i.h b/libitm/libitm_i.h
index 98c2c75..01b2e4a 100644
--- a/libitm/libitm_i.h
+++ b/libitm/libitm_i.h
@@ -53,18 +53,6 @@ template<> struct sized_integral<8> { typedef uint64_t type; };
 
 typedef unsigned int gtm_word __attribute__((mode (word)));
 
-// Locally defined protected allocation functions.
-//
-// To avoid dependency on libstdc++ new/delete, as well as to not
-// interfere with the wrapping of the global new/delete we wrap for
-// the user in alloc_cpp.cc, use class-local versions that defer
-// to malloc/free.  Recall that operator new/delete does not go through
-// normal lookup and so we cannot simply inject a version into the
-// GTM namespace.
-
-extern void * xmalloc (size_t s) __attribute__((malloc, nothrow));
-extern void * xrealloc (void *p, size_t s) __attribute__((malloc, nothrow));
-
 } // namespace GTM
 
 #include "target.h"
@@ -74,6 +62,7 @@ extern void * xrealloc (void *p, size_t s) __attribute__((malloc, nothrow));
 #include "cachepage.h"
 #include "stmlock.h"
 #include "dispatch.h"
+#include "containers.h"
 
 namespace GTM HIDDEN {
 
@@ -118,9 +107,7 @@ struct gtm_transaction
   gtm_jmpbuf jb;
 
   // Data used by local.c for the local memory undo log.
-  struct gtm_local_undo **local_undo;
-  size_t n_local_undo;
-  size_t size_local_undo;
+  vector<gtm_local_undo*> local_undo;
 
   // Data used by alloc.c for the malloc/free undo log.
   aa_tree<uintptr_t, gtm_alloc_action> alloc_actions;
diff --git a/libitm/local.cc b/libitm/local.cc
index 1681222..a1a5655 100644
--- a/libitm/local.cc
+++ b/libitm/local.cc
@@ -37,28 +37,20 @@ struct gtm_local_undo
 void
 gtm_transaction::commit_local ()
 {
-  gtm_local_undo **local_undo = this->local_undo;
-  size_t i, n = this->n_local_undo;
+  size_t i, n = local_undo.size();
 
   if (n > 0)
     {
       for (i = 0; i < n; ++i)
 	free (local_undo[i]);
-      this->n_local_undo = 0;
-    }
-  if (local_undo)
-    {
-      free (local_undo);
-      this->local_undo = NULL;
-      this->size_local_undo = 0;
+      this->local_undo.clear();
     }
 }
 
 void
 gtm_transaction::rollback_local (void)
 {
-  gtm_local_undo **local_undo = this->local_undo;
-  size_t i, n = this->n_local_undo;
+  size_t i, n = local_undo.size();
 
   if (n > 0)
     {
@@ -71,7 +63,7 @@ gtm_transaction::rollback_local (void)
 	      free (u);
 	    }
 	}
-      this->n_local_undo = 0;
+      local_undo.clear();
     }
 }
 
@@ -80,8 +72,7 @@ gtm_transaction::rollback_local (void)
 void
 gtm_transaction::drop_references_local (const void *ptr, size_t len)
 {
-  gtm_local_undo **local_undo = this->local_undo;
-  size_t i, n = this->n_local_undo;
+  size_t i, n = local_undo.size();
 
   if (n > 0)
     {
@@ -110,19 +101,7 @@ GTM_LB (const void *ptr, size_t len)
   undo->addr = (void *) ptr;
   undo->len = len;
 
-  if (tx->local_undo == NULL)
-    {
-      tx->size_local_undo = 32;
-      tx->local_undo = (gtm_local_undo **)
-	xmalloc (sizeof (undo) * tx->size_local_undo);
-    }
-  else if (tx->n_local_undo == tx->size_local_undo)
-    {
-      tx->size_local_undo *= 2;
-      tx->local_undo = (gtm_local_undo **)
-	xrealloc (tx->local_undo, sizeof (undo) * tx->size_local_undo);
-    }
-  tx->local_undo[tx->n_local_undo++] = undo;
+  tx->local_undo.push()[0] = undo;
 
   memcpy (undo->saved, ptr, len);
 }
diff --git a/libitm/util.cc b/libitm/util.cc
index b9a8727..e4ff778 100644
--- a/libitm/util.cc
+++ b/libitm/util.cc
@@ -59,8 +59,11 @@ GTM_fatal (const char *fmt, ...)
 }
 
 void *
-xmalloc (size_t size)
+xmalloc (size_t size, bool separate_cl)
 {
+  // TODO Use posix_memalign if separate_cl is true, or some other allocation
+  // method that will avoid sharing cache lines with data used by other
+  // threads.
   void *r = malloc (size);
   if (r == 0)
     GTM_fatal ("Out of memory allocating %lu bytes", (unsigned long) size);
@@ -68,8 +71,11 @@ xmalloc (size_t size)
 }
 
 void *
-xrealloc (void *old, size_t size)
+xrealloc (void *old, size_t size, bool separate_cl)
 {
+  // TODO Use posix_memalign if separate_cl is true, or some other allocation
+  // method that will avoid sharing cache lines with data used by other
+  // threads.
   void *r = realloc (old, size);
   if (r == 0)
     GTM_fatal ("Out of memory allocating %lu bytes", (unsigned long) size);

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [trans-mem] Beginning of refactoring
  2011-07-01 20:23 ` Torvald Riegel
@ 2011-07-01 20:32   ` Richard Henderson
  0 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2011-07-01 20:32 UTC (permalink / raw)
  To: Torvald Riegel; +Cc: GCC Patches

On 07/01/2011 01:23 PM, Torvald Riegel wrote:
>     Add vector-like container, use it for gtm_transaction::undo_log.
>     
>     	* containers.h: New file.
>     	* util.cc (xmalloc, xrealloc): Accept cacheline-alloc flag.
>     	* libitm_i.h (xmalloc, xrealloc): Moved declarations from here ...
>     	* common.h: ... to here.
>     	(local_undo): Use GTM::vector for gtm_transaction::local_undo.
>     	* local.cc: Same.

Ok.


r~

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [trans-mem] Beginning of refactoring
  2011-05-25 22:45 [trans-mem] Beginning of refactoring Torvald Riegel
                   ` (2 preceding siblings ...)
  2011-07-01 20:23 ` Torvald Riegel
@ 2011-07-09 15:35 ` Torvald Riegel
  2011-07-18 23:38   ` Torvald Riegel
  3 siblings, 1 reply; 19+ messages in thread
From: Torvald Riegel @ 2011-07-09 15:35 UTC (permalink / raw)
  To: GCC Patches; +Cc: Richard Henderson

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

The attached patch makes flat nesting the default, while still
supporting closed nesting on demand (for user-controlled aborts via
__transaction_cancel).
Previously, a new transaction object was created for each nested
transaction. The patch changes this to using one transaction object
during the whole lifetime of a thread and keeping checkpoints of the
transaction state when starting closed nested transactions. This allows
us to easily use flat nesting and decreases the transaction start/commit
overheads to some extent.

OK for branch?

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 36990 bytes --]

commit a6b608b3b7e942b2ffaff7f7b8a897dd7cfd62c1
Author: Torvald Riegel <triegel@redhat.com>
Date:   Sat Jul 9 16:22:21 2011 +0200

    Use flat nesting by default and support closed nesting via checkpoints.
    
    	* local.cc (gtm_transaction::rollback_local): Support closed nesting.
    	* retry.cc (GTM::gtm_transaction::decide_retry_strategy): Same.
    	* eh_cpp.cc (GTM::gtm_transaction::revert_cpp_exceptions): Same.
    	* useraction.cc: Same. Also use vector instead of list to store actions.
    	* dispatch.h (GTM::abi_dispatch): Add can_run_uninstrumented_code and
    	closed_nesting flags, as well as a closed nesting alternative.
    	* method-serial.cc: Same.
    	* beginend.cc (GTM::gtm_transaction::begin_transaction): Change to
    	flat nesting as default, and closed nesting on demand.
    	(GTM::gtm_transaction::rollback): Same.
    	(_ITM_abortTransaction): Same.
    	(GTM::gtm_transaction::restart): Same.
    	(GTM::gtm_transaction::trycommit): Same.
    	(GTM::gtm_transaction::trycommit_and_finalize): Removed.
    	(choose_code_path): New.
    	(GTM::gtm_transaction_cp::save): New.
    	(GTM::gtm_transaction_cp::commit): New.
    	* query.cc (_ITM_inTransaction): Support flat nesting.
    	* aatree.h (aa_tree::remove): New.
    	(aa_tree::operator new): Add placement new.
    	* libitm_i.h: Add closed nesting as restart reason.
    	(GTM::gtm_transaction_cp): New helper struct for nesting.
    	(GTM::gtm_transaction::user_action): Same.
    	(GTM::gtm_transaction): Support flat and closed nesting.
    	* alloc.cc (commit_allocations_2): New.
    	(commit_cb_data): New helper struct.
    	(GTM::gtm_transaction::commit_allocations): Handle nested
    	commits/rollbacks.
    	* libitm.h (_ITM_codeProperties): Change pr_hasElse to the ABI's value.
    	* libitm.texi: Update user action section, add description of nesting.

diff --git a/libitm/aatree.h b/libitm/aatree.h
index 6d28890..3e11b95 100644
--- a/libitm/aatree.h
+++ b/libitm/aatree.h
@@ -1,4 +1,4 @@
-/* Copyright (C) 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU Transactional Memory Library (libitm).
@@ -141,6 +141,8 @@ class aa_tree : public aa_tree_key<KEY>
   aa_tree() = default;
   ~aa_tree() { clear(); }
 
+  static void *operator new (size_t s, aa_tree<KEY, DATA>* p) { return p; }
+
   DATA *find(KEY k) const
   {
     node_ptr n = static_cast<node_ptr>(base::find (k));
@@ -160,6 +162,13 @@ class aa_tree : public aa_tree_key<KEY>
     delete n;
   }
 
+  node_ptr remove(KEY k, DATA** data)
+  {
+    node_ptr n = static_cast<node_ptr>(base::erase (k));
+    *data = (n ? &n->data : 0);
+    return n;
+  }
+
   void clear()
   {
     node_ptr n = static_cast<node_ptr>(this->m_tree);
diff --git a/libitm/alloc.cc b/libitm/alloc.cc
index 5c70144..9b60835 100644
--- a/libitm/alloc.cc
+++ b/libitm/alloc.cc
@@ -1,4 +1,4 @@
-/* Copyright (C) 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU Transactional Memory Library (libitm).
@@ -52,6 +52,55 @@ gtm_transaction::forget_allocation (void *ptr, void (*free_fn)(void *))
   a->allocated = false;
 }
 
+namespace {
+struct commit_cb_data {
+  aa_tree<uintptr_t, gtm_alloc_action>* parent;
+  bool revert_p;
+};
+}
+
+static void
+commit_allocations_2 (uintptr_t key, gtm_alloc_action *a, void *data)
+{
+  void *ptr = (void *)key;
+  commit_cb_data *cb_data = static_cast<commit_cb_data *>(data);
+
+  if (cb_data->revert_p)
+    {
+      // Roll back nested allocations.
+      if (a->allocated)
+        a->free_fn (ptr);
+    }
+  else
+    {
+      if (a->allocated)
+        {
+          // Add nested allocations to parent transaction.
+          gtm_alloc_action* a_parent = cb_data->parent->insert(key);
+          *a_parent = *a;
+        }
+      else
+        {
+          // Eliminate a parent allocation if it matches this memory release,
+          // otherwise just add it to the parent.
+          gtm_alloc_action* a_parent;
+          aa_tree<uintptr_t, gtm_alloc_action>::node_ptr node_ptr =
+              cb_data->parent->remove(key, &a_parent);
+          if (node_ptr)
+            {
+              assert(a_parent->allocated);
+              a_parent->free_fn(ptr);
+              delete node_ptr;
+            }
+          else
+            {
+              a_parent = cb_data->parent->insert(key);
+              *a_parent = *a;
+            }
+        }
+    }
+}
+
 static void
 commit_allocations_1 (uintptr_t key, gtm_alloc_action *a, void *cb_data)
 {
@@ -67,10 +116,19 @@ commit_allocations_1 (uintptr_t key, gtm_alloc_action *a, void *cb_data)
    REVERT_P is true if instead of committing the allocations, we want
    to roll them back (and vice versa).  */
 void
-gtm_transaction::commit_allocations (bool revert_p)
+gtm_transaction::commit_allocations (bool revert_p,
+    aa_tree<uintptr_t, gtm_alloc_action>* parent)
 {
-  this->alloc_actions.traverse (commit_allocations_1,
-				(void *)(uintptr_t)revert_p);
+  if (parent)
+    {
+      commit_cb_data cb_data;
+      cb_data.parent = parent;
+      cb_data.revert_p = revert_p;
+      this->alloc_actions.traverse (commit_allocations_2, &cb_data);
+    }
+  else
+    this->alloc_actions.traverse (commit_allocations_1,
+				  (void *)(uintptr_t)revert_p);
   this->alloc_actions.clear ();
 }
 
diff --git a/libitm/beginend.cc b/libitm/beginend.cc
index 9b16973..6023390 100644
--- a/libitm/beginend.cc
+++ b/libitm/beginend.cc
@@ -88,6 +88,14 @@ GTM::gtm_transaction::operator delete(void *tx)
 static pthread_mutex_t global_tid_lock = PTHREAD_MUTEX_INITIALIZER;
 #endif
 
+static inline uint32_t choose_code_path(uint32_t prop, abi_dispatch *disp)
+{
+  if ((prop & pr_uninstrumentedCode) && disp->can_run_uninstrumented_code())
+    return a_runUninstrumentedCode;
+  else
+    return a_runInstrumentedCode;
+}
+
 uint32_t
 GTM::gtm_transaction::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb)
 {
@@ -97,14 +105,112 @@ GTM::gtm_transaction::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb)
   abi_dispatch *disp;
   uint32_t ret;
 
+  // ??? pr_undoLogCode is not properly defined in the ABI. Are barriers
+  // omitted because they are not necessary (e.g., a transaction on thread-
+  // local data) or because the compiler thinks that some kind of global
+  // synchronization might perform better?
+  if (unlikely(prop & pr_undoLogCode))
+    GTM_fatal("pr_undoLogCode not supported");
+
   gtm_thread *thr = setup_gtm_thr ();
 
-  tx = new gtm_transaction;
+  tx = gtm_tx();
+  if (tx == NULL)
+    {
+      tx = new gtm_transaction;
+      set_gtm_tx(tx);
+    }
+
+  if (tx->nesting > 0)
+    {
+      // This is a nested transaction.
+      // Check prop compatibility:
+      // The ABI requires pr_hasNoFloatUpdate, pr_hasNoVectorUpdate,
+      // pr_hasNoIrrevocable, pr_aWBarriersOmitted, pr_RaRBarriersOmitted, and
+      // pr_hasNoSimpleReads to hold for the full dynamic scope of a
+      // transaction. We could check that these are set for the nested
+      // transaction if they are also set for the parent transaction, but the
+      // ABI does not require these flags to be set if they could be set,
+      // so the check could be too strict.
+      // ??? For pr_readOnly, lexical or dynamic scope is unspecified.
+
+      if (prop & pr_hasNoAbort)
+        {
+          // We can use flat nesting, so elide this transaction.
+          if (!(prop & pr_instrumentedCode))
+            {
+              if (!(tx->state & STATE_SERIAL) ||
+                  !(tx->state & STATE_IRREVOCABLE))
+                tx->serialirr_mode();
+            }
+          // Increment nesting level after checking that we have a method that
+          // allows us to continue.
+          tx->nesting++;
+          return choose_code_path(prop, abi_disp());
+        }
+
+      // The transaction might abort, so use closed nesting if possible.
+      // pr_hasNoAbort has lexical scope, so the compiler should really have
+      // generated an instrumented code path.
+      assert(prop & pr_instrumentedCode);
+
+      // Create a checkpoint of the current transaction.
+      gtm_transaction_cp *cp = tx->parent_txns.push();
+      cp->save(tx);
+      new (&tx->alloc_actions) aa_tree<uintptr_t, gtm_alloc_action>();
+
+      // Check whether the current method actually supports closed nesting.
+      // If we can switch to another one, do so.
+      // If not, we assume that actual aborts are infrequent, and rather
+      // restart in _ITM_abortTransaction when we really have to.
+      disp = abi_disp();
+      if (!disp->closed_nesting())
+        {
+          // ??? Should we elide the transaction if there is no alternative
+          // method that supports closed nesting? If we do, we need to set
+          // some flag to prevent _ITM_abortTransaction from aborting the
+          // wrong transaction (i.e., some parent transaction).
+          abi_dispatch *cn_disp = disp->closed_nesting_alternative();
+          if (cn_disp)
+            {
+              disp = cn_disp;
+              set_abi_disp(disp);
+            }
+        }
+    }
+  else
+    {
+      // Outermost transaction
+      // TODO Pay more attention to prop flags (eg, *omitted) when selecting
+      // dispatch.
+      if ((prop & pr_doesGoIrrevocable) || !(prop & pr_instrumentedCode))
+        tx->state = (STATE_SERIAL | STATE_IRREVOCABLE);
+
+      else
+        disp = tx->decide_begin_dispatch ();
+
+      if (tx->state & STATE_SERIAL)
+        {
+          serial_lock.write_lock ();
 
+          if (tx->state & STATE_IRREVOCABLE)
+            disp = dispatch_serialirr ();
+          else
+            disp = dispatch_serial ();
+        }
+      else
+        {
+          serial_lock.read_lock ();
+        }
+
+      set_abi_disp (disp);
+    }
+
+  // Initialization that is common for outermost and nested transactions.
   tx->prop = prop;
-  tx->prev = gtm_tx();
-  if (tx->prev)
-    tx->nesting = tx->prev->nesting + 1;
+  tx->nesting++;
+
+  tx->jb = *jb;
 
   // As long as we have not exhausted a previously allocated block of TIDs,
   // we can avoid an atomic operation on a shared cacheline.
@@ -124,58 +230,80 @@ GTM::gtm_transaction::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb)
 #endif
     }
 
-  tx->jb = *jb;
+  // Determine the code path to run. Only irrevocable transactions cannot be
+  // restarted, so all other transactions need to save live variables.
+  ret = choose_code_path(prop, disp);
+  if (!(tx->state & STATE_IRREVOCABLE)) ret |= a_saveLiveVariables;
+  return ret;
+}
 
-  set_gtm_tx (tx);
 
-  // ??? pr_undoLogCode is not properly defined in the ABI. Are barriers
-  // omitted because they are not necessary (e.g., a transaction on thread-
-  // local data) or because the compiler thinks that some kind of global
-  // synchronization might perform better?
-  if (unlikely(prop & pr_undoLogCode))
-    GTM_fatal("pr_undoLogCode not supported");
+void
+GTM::gtm_transaction_cp::save(gtm_transaction* tx)
+{
+  // Save everything that we might have to restore on restarts or aborts.
+  jb = tx->jb;
+  local_undo_size = tx->local_undo.size();
+  memcpy(&alloc_actions, &tx->alloc_actions, sizeof(alloc_actions));
+  user_actions_size = tx->user_actions.size();
+  id = tx->id;
+  prop = tx->prop;
+  cxa_catch_count = tx->cxa_catch_count;
+  cxa_unthrown = tx->cxa_unthrown;
+  disp = abi_disp();
+  nesting = tx->nesting;
+}
 
-  if ((prop & pr_doesGoIrrevocable) || !(prop & pr_instrumentedCode))
-    tx->state = (STATE_SERIAL | STATE_IRREVOCABLE);
+void
+GTM::gtm_transaction_cp::commit(gtm_transaction* tx)
+{
+  // Restore state that is not persistent across commits. Exception handling,
+  // information, nesting level, and any logs do not need to be restored on
+  // commits of nested transactions. Allocation actions must be committed
+  // before committing the snapshot.
+  tx->jb = jb;
+  memcpy(&tx->alloc_actions, &alloc_actions, sizeof(alloc_actions));
+  tx->id = id;
+  tx->prop = prop;
+}
 
-  else
-    disp = tx->decide_begin_dispatch ();
 
-  if (tx->state & STATE_SERIAL)
-    {
-      serial_lock.write_lock ();
+void
+GTM::gtm_transaction::rollback (gtm_transaction_cp *cp)
+{
+  abi_disp()->rollback (cp);
 
-      if (tx->state & STATE_IRREVOCABLE)
-        disp = dispatch_serialirr ();
-      else
-        disp = dispatch_serial ();
+  rollback_local (cp ? cp->local_undo_size : 0);
+  rollback_user_actions (cp ? cp->user_actions_size : 0);
+  commit_allocations (true, (cp ? &cp->alloc_actions : 0));
+  revert_cpp_exceptions (cp);
 
-      ret = a_runUninstrumentedCode;
-      if ((prop & pr_multiwayCode) == pr_instrumentedCode)
-	ret = a_runInstrumentedCode;
+  if (cp)
+    {
+      // Roll back the rest of the state to the checkpoint.
+      jb = cp->jb;
+      id = cp->id;
+      prop = cp->prop;
+      if (cp->disp != abi_disp())
+        set_abi_disp(cp->disp);
+      memcpy(&alloc_actions, &cp->alloc_actions, sizeof(alloc_actions));
+      nesting = cp->nesting;
     }
   else
     {
-      serial_lock.read_lock ();
-      ret = a_runInstrumentedCode | a_saveLiveVariables;
+      // Restore the jump buffer and transaction properties, which we will
+      // need for the longjmp used to restart or abort the transaction.
+      if (parent_txns.size() > 0)
+        {
+          jb = parent_txns[0].jb;
+          prop = parent_txns[0].prop;
+        }
+      // Reset the transaction. Do not reset state, which is handled by the
+      // callers.
+      nesting = 0;
+      parent_txns.clear();
     }
 
-  set_abi_disp (disp);
-
-  return ret;
-}
-
-void
-GTM::gtm_transaction::rollback ()
-{
-  abi_disp()->rollback ();
-  rollback_local ();
-
-  free_actions (&this->commit_actions);
-  run_actions (&this->undo_actions);
-  commit_allocations (true);
-  revert_cpp_exceptions ();
-
   if (this->eh_in_flight)
     {
       _Unwind_DeleteException ((_Unwind_Exception *) this->eh_in_flight);
@@ -194,46 +322,87 @@ _ITM_abortTransaction (_ITM_abortReason reason)
   if (tx->state & gtm_transaction::STATE_IRREVOCABLE)
     abort ();
 
-  tx->rollback ();
-  abi_disp()->fini ();
+  // If the current method does not support closed nesting, we are nested, and
+  // we can restart, then restart with a method that supports closed nesting.
+  abi_dispatch *disp = abi_disp();
+  if (!disp->closed_nesting())
+    tx->restart(RESTART_CLOSED_NESTING);
+
+  // Roll back to innermost transaction.
+  if (tx->parent_txns.size() > 0)
+    {
+      // The innermost transaction is a nested transaction.
+      gtm_transaction_cp *cp = tx->parent_txns.pop();
+      uint32_t longjmp_prop = tx->prop;
+      gtm_jmpbuf longjmp_jb = tx->jb;
+
+      tx->rollback (cp);
+      abi_disp()->fini ();
 
-  if (tx->state & gtm_transaction::STATE_SERIAL)
-    gtm_transaction::serial_lock.write_unlock ();
+      // Jump to nested transaction (use the saved jump buffer).
+      GTM_longjmp (&longjmp_jb, a_abortTransaction | a_restoreLiveVariables,
+          longjmp_prop);
+    }
   else
-    gtm_transaction::serial_lock.read_unlock ();
+    {
+      // There is no nested transaction, so roll back to outermost transaction.
+      tx->rollback ();
+      abi_disp()->fini ();
 
-  set_gtm_tx (tx->prev);
-  delete tx;
+      // Aborting an outermost transaction finishes execution of the whole
+      // transaction. Therefore, reset transaction state.
+      if (tx->state & gtm_transaction::STATE_SERIAL)
+        gtm_transaction::serial_lock.write_unlock ();
+      else
+        gtm_transaction::serial_lock.read_unlock ();
+      tx->state = 0;
 
-  GTM_longjmp (&tx->jb, a_abortTransaction | a_restoreLiveVariables, tx->prop);
+      GTM_longjmp (&tx->jb, a_abortTransaction | a_restoreLiveVariables,
+          tx->prop);
+    }
 }
 
 bool
 GTM::gtm_transaction::trycommit ()
 {
-  if (abi_disp()->trycommit ())
+  nesting--;
+
+  // Skip any real commit for elided transactions.
+  if (nesting > 0 && (parent_txns.size() == 0 ||
+      nesting > parent_txns[parent_txns.size() - 1].nesting))
+    return true;
+
+  if (nesting > 0)
     {
-      commit_local ();
-      free_actions (&this->undo_actions);
-      run_actions (&this->commit_actions);
-      commit_allocations (false);
+      // Commit of a closed-nested transaction. Remove one checkpoint and add
+      // any effects of this transaction to the parent transaction.
+      gtm_transaction_cp *cp = parent_txns.pop();
+      commit_allocations(false, &cp->alloc_actions);
+      cp->commit(this);
       return true;
     }
-  return false;
-}
 
-bool
-GTM::gtm_transaction::trycommit_and_finalize ()
-{
-  if (trycommit ())
+  // Commit of an outermost transaction.
+  if (abi_disp()->trycommit ())
     {
+      commit_local ();
+      // FIXME: run after ensuring privatization safety:
+      commit_user_actions ();
+      commit_allocations (false, 0);
       abi_disp()->fini ();
-      set_gtm_tx (this->prev);
-      delete this;
-      if (this->state & gtm_transaction::STATE_SERIAL)
+
+      // Reset transaction state.
+      cxa_catch_count = 0;
+      cxa_unthrown = NULL;
+
+      // TODO can release SI mode before committing user actions? If so,
+      // we can release before ensuring privatization safety too.
+      if (state & gtm_transaction::STATE_SERIAL)
 	gtm_transaction::serial_lock.write_unlock ();
       else
 	gtm_transaction::serial_lock.read_unlock ();
+      state = 0;
+
       return true;
     }
   return false;
@@ -242,24 +411,21 @@ GTM::gtm_transaction::trycommit_and_finalize ()
 void ITM_NORETURN
 GTM::gtm_transaction::restart (gtm_restart_reason r)
 {
-  uint32_t actions;
-
+  // Roll back to outermost transaction. Do not reset transaction state because
+  // we will continue executing this transaction.
   rollback ();
   decide_retry_strategy (r);
 
-  actions = a_runInstrumentedCode | a_restoreLiveVariables;
-  if ((this->prop & pr_uninstrumentedCode)
-      && (this->state & gtm_transaction::STATE_IRREVOCABLE))
-    actions = a_runUninstrumentedCode | a_restoreLiveVariables;
-
-  GTM_longjmp (&this->jb, actions, this->prop);
+  GTM_longjmp (&this->jb,
+      choose_code_path(prop, abi_disp()) | a_restoreLiveVariables,
+      this->prop);
 }
 
 void ITM_REGPARM
 _ITM_commitTransaction(void)
 {
   gtm_transaction *tx = gtm_tx();
-  if (!tx->trycommit_and_finalize ())
+  if (!tx->trycommit ())
     tx->restart (RESTART_VALIDATE_COMMIT);
 }
 
@@ -267,7 +433,7 @@ void ITM_REGPARM
 _ITM_commitTransactionEH(void *exc_ptr)
 {
   gtm_transaction *tx = gtm_tx();
-  if (!tx->trycommit_and_finalize ())
+  if (!tx->trycommit ())
     {
       tx->eh_in_flight = exc_ptr;
       tx->restart (RESTART_VALIDATE_COMMIT);
diff --git a/libitm/dispatch.h b/libitm/dispatch.h
index 3643503..0ab5c4e 100644
--- a/libitm/dispatch.h
+++ b/libitm/dispatch.h
@@ -234,6 +234,8 @@ void ITM_REGPARM _ITM_memset##WRITE(void *dst, int c, size_t size) \
 
 namespace GTM HIDDEN {
 
+struct gtm_transaction_cp;
+
 // This pass-through method is the basis for other methods.
 // It can be used for serial-irrevocable mode.
 struct abi_dispatch
@@ -248,15 +250,24 @@ private:
 
 public:
   virtual bool trycommit() = 0;
-  virtual void rollback() = 0;
+  virtual void rollback(gtm_transaction_cp *cp = 0) = 0;
   virtual void reinit() = 0;
 
   // Use fini instead of dtor to support a static subclasses that uses
   // a unique object and so we don't want to destroy it from common code.
   virtual void fini() = 0;
 
+  // Return an alternative method that is compatible with the current
+  // method but supports closed nesting. Return zero if there is none.
+  virtual abi_dispatch* closed_nesting_alternative() { return 0; }
+
   bool read_only () const { return m_read_only; }
   bool write_through() const { return m_write_through; }
+  bool can_run_uninstrumented_code() const
+  {
+    return m_can_run_uninstrumented_code;
+  }
+  bool closed_nesting() const { return m_closed_nesting; }
 
   static void *operator new(size_t s) { return xmalloc (s); }
   static void operator delete(void *p) { free (p); }
@@ -275,7 +286,13 @@ public:
 protected:
   const bool m_read_only;
   const bool m_write_through;
-  abi_dispatch(bool ro, bool wt) : m_read_only(ro), m_write_through(wt) { }
+  const bool m_can_run_uninstrumented_code;
+  const bool m_closed_nesting;
+  abi_dispatch(bool ro, bool wt, bool uninstrumented, bool closed_nesting) :
+    m_read_only(ro), m_write_through(wt),
+    m_can_run_uninstrumented_code(uninstrumented),
+    m_closed_nesting(closed_nesting)
+  { }
 };
 
 }
diff --git a/libitm/eh_cpp.cc b/libitm/eh_cpp.cc
index bd83f72..35c0b50 100644
--- a/libitm/eh_cpp.cc
+++ b/libitm/eh_cpp.cc
@@ -72,14 +72,37 @@ _ITM_cxa_end_catch (void)
 }
 
 void
-GTM::gtm_transaction::revert_cpp_exceptions (void)
+GTM::gtm_transaction::revert_cpp_exceptions (gtm_transaction_cp *cp)
 {
-  if (this->cxa_unthrown || this->cxa_catch_count)
+  if (cp)
     {
-      __cxa_tm_cleanup (this->cxa_unthrown, this->eh_in_flight,
-			this->cxa_catch_count);
-      this->cxa_catch_count = 0;
-      this->cxa_unthrown = NULL;
-      this->eh_in_flight = NULL;
+      // If rolling back a nested transaction, only clean up unthrown
+      // exceptions since the last checkpoint. Always reset eh_in_flight
+      // because it just contains the argument provided to
+      // _ITM_commitTransactionEH
+      void *unthrown =
+          (cxa_unthrown != cp->cxa_unthrown ? cxa_unthrown : NULL);
+      assert (cxa_catch_count > cp->cxa_catch_count);
+      uint32_t catch_count = cxa_catch_count - cp->cxa_catch_count;
+      if (unthrown || catch_count)
+        {
+          __cxa_tm_cleanup (unthrown, this->eh_in_flight, catch_count);
+          cxa_catch_count = cp->cxa_catch_count;
+          cxa_unthrown = cp->cxa_unthrown;
+          this->eh_in_flight = NULL;
+        }
+    }
+  else
+    {
+      // Both cxa_catch_count and cxa_unthrown are maximal because EH regions
+      // and transactions are properly nested.
+      if (this->cxa_unthrown || this->cxa_catch_count)
+        {
+          __cxa_tm_cleanup (this->cxa_unthrown, this->eh_in_flight,
+              this->cxa_catch_count);
+          this->cxa_catch_count = 0;
+          this->cxa_unthrown = NULL;
+          this->eh_in_flight = NULL;
+        }
     }
 }
diff --git a/libitm/libitm.h b/libitm/libitm.h
index abd4274..e961e81 100644
--- a/libitm/libitm.h
+++ b/libitm/libitm.h
@@ -93,8 +93,8 @@ typedef enum
    pr_preferUninstrumented	= 0x0800,
    /* Exception blocks are not used nor supported. */
    pr_exceptionBlock		= 0x1000,
+   pr_hasElse			= 0x2000,
    pr_readOnly			= 0x4000,
-   pr_hasElse			= 0x200000,
    pr_hasNoSimpleReads		= 0x400000
 } _ITM_codeProperties;
 
diff --git a/libitm/libitm.texi b/libitm/libitm.texi
index 046b0bd..8d7aef4 100644
--- a/libitm/libitm.texi
+++ b/libitm/libitm.texi
@@ -314,13 +314,16 @@ nontransactionally).
 
 @subsection User-registered commit and undo actions
 
-The order in which commit or undo actions are executed is undefined. It is also
-undefined whether privatization safety has been ensured by the time the
-transactions are executed. The ordering of undo actions and the roll-back of
-data transfers is undefined.
+Commit actions will get executed in the same order in which the respective
+calls to @code{_ITM_addUserCommitAction} happened. Only
+@code{_ITM_noTransactionId} is allowed as value for the
+@code{resumingTransactionId} argument. Commit actions get executed after
+privatization safety has been ensured.
 
-However, the ABI should specify ordering guarantees where necessary (in
-particular, w.r.t. privatization safety).
+Undo actions will get executed in reverse order compared to the order in which
+the respective calls to @code{_ITM_addUserUndoAction} happened. The ordering of
+undo actions w.r.t. the roll-back of other actions (e.g., data transfers or
+memory allocations) is undefined.
 
 @code{_ITM_dropReferences} is not supported currently because its semantics and
 the intention behind it is not entirely clear. The
@@ -415,7 +418,31 @@ specification. Likewise, the TM runtime must ensure privatization safety.
 @node Internals
 @chapter Internals
 
-TODO: SI-mode implementation, method sets, ...
+@section Nesting: flat vs. closed
+
+We support two different kinds of nesting of transactions. In the case of
+@emph{flat nesting}, the nesting structure is flattened and all nested
+transactions are subsumed by the enclosing transaction. In contrast,
+with @emph{closed nesting}, nested transactions that have not yet committed
+can be rolled back separately from the enclosing transactions; when they
+commit, they are subsumed by the enclosing transaction, and their effects
+will be finally committed when the outermost transaction commits.
+@emph{Open nesting} (where nested transactions can commit independently of the
+enclosing transactions) are not supported.
+
+Flat nesting is the default nesting mode, but closed nesting is supported and
+used when transactions contain user-controlled aborts
+(@code{__transaction_cancel} statements). We assume that user-controlled
+aborts are rare in typical code and used mostly in exceptional situations.
+Thus, it makes more sense to use flat nesting by default to avoid the
+performance overhead of the additional checkpoints required for closed
+nesting. User-controlled aborts will correctly abort the innermost enclosing
+transaction, whereas the whole (i.e., outermost) transaction will be restarted
+otherwise (e.g., when a transaction encounters data conflicts during
+optimistic execution).
+
+
+@strong{TODO}: SI-mode implementation, method sets, ...
 
 @c ---------------------------------------------------------------------
 @c GNU General Public License
diff --git a/libitm/libitm_i.h b/libitm/libitm_i.h
index 01b2e4a..4cc0a70 100644
--- a/libitm/libitm_i.h
+++ b/libitm/libitm_i.h
@@ -82,6 +82,7 @@ enum gtm_restart_reason
   RESTART_VALIDATE_COMMIT,
   RESTART_SERIAL_IRR,
   RESTART_NOT_READONLY,
+  RESTART_CLOSED_NESTING,
   NUM_RESTARTS
 };
 
@@ -96,12 +97,43 @@ struct gtm_alloc_action
 // This type is private to local.c.
 struct gtm_local_undo;
 
-// This type is private to useraction.c.
-struct gtm_user_action;
+struct gtm_transaction;
+
+// A transaction checkpoint: data that has to saved and restored when doing
+// closed nesting.
+struct gtm_transaction_cp
+{
+  gtm_jmpbuf jb;
+  size_t local_undo_size;
+  aa_tree<uintptr_t, gtm_alloc_action> alloc_actions;
+  size_t user_actions_size;
+  _ITM_transactionId_t id;
+  uint32_t prop;
+  uint32_t cxa_catch_count;
+  void *cxa_unthrown;
+  // We might want to use a different but compatible dispatch method for
+  // a nested transaction.
+  abi_dispatch *disp;
+  // Nesting level of this checkpoint (1 means that this is a checkpoint of
+  // the outermost transaction).
+  uint32_t nesting;
+
+  void save(gtm_transaction* tx);
+  void commit(gtm_transaction* tx);
+};
 
 // All data relevant to a single transaction.
 struct gtm_transaction
 {
+
+  struct user_action
+  {
+    _ITM_userCommitFunction fn;
+    void *arg;
+    bool on_commit;
+    _ITM_transactionId_t resuming_id;
+  };
+
   // The jump buffer by which GTM_longjmp restarts the transaction.
   // This field *must* be at the beginning of the transaction.
   gtm_jmpbuf jb;
@@ -112,12 +144,8 @@ struct gtm_transaction
   // Data used by alloc.c for the malloc/free undo log.
   aa_tree<uintptr_t, gtm_alloc_action> alloc_actions;
 
-  // Data used by useraction.c for the user defined undo log.
-  struct gtm_user_action *commit_actions;
-  struct gtm_user_action *undo_actions;
-
-  // A pointer to the "outer" transaction.
-  struct gtm_transaction *prev;
+  // Data used by useraction.c for the user-defined commit/abort handlers.
+  vector<user_action> user_actions;
 
   // A numerical identifier for this transaction.
   _ITM_transactionId_t id;
@@ -125,13 +153,16 @@ struct gtm_transaction
   // The _ITM_codeProperties of this transaction as given by the compiler.
   uint32_t prop;
 
-  // The nesting depth of this transaction.
+  // The nesting depth for subsequently started transactions. This variable
+  // will be set to 1 when starting an outermost transaction.
   uint32_t nesting;
 
   // Set if this transaction owns the serial write lock.
+  // Can be reset only when restarting the outermost transaction.
   static const uint32_t STATE_SERIAL		= 0x0001;
   // Set if the serial-irrevocable dispatch table is installed.
   // Implies that no logging is being done, and abort is not possible.
+  // Can be reset only when restarting the outermost transaction.
   static const uint32_t STATE_IRREVOCABLE	= 0x0002;
 
   // A bitmask of the above.
@@ -142,6 +173,9 @@ struct gtm_transaction
   void *cxa_unthrown;
   void *eh_in_flight;
 
+  // Checkpoints for closed nesting.
+  vector<gtm_transaction_cp> parent_txns;
+
   // Data used by retry.c for deciding what STM implementation should
   // be used for the next iteration of the transaction.
   uint32_t restart_reason[NUM_RESTARTS];
@@ -153,7 +187,7 @@ struct gtm_transaction
   static gtm_rwlock serial_lock;
 
   // In alloc.cc
-  void commit_allocations (bool);
+  void commit_allocations (bool, aa_tree<uintptr_t, gtm_alloc_action>*);
   void record_allocation (void *, void (*)(void *));
   void forget_allocation (void *, void (*)(void *));
   void drop_references_allocations (const void *ptr)
@@ -162,9 +196,8 @@ struct gtm_transaction
   }
 
   // In beginend.cc
-  void rollback ();
+  void rollback (gtm_transaction_cp *cp = 0);
   bool trycommit ();
-  bool trycommit_and_finalize ();
   void restart (gtm_restart_reason) ITM_NORETURN;
 
   static void *operator new(size_t);
@@ -176,11 +209,11 @@ struct gtm_transaction
 	__asm__("GTM_begin_transaction") ITM_REGPARM;
 
   // In eh_cpp.cc
-  void revert_cpp_exceptions ();
+  void revert_cpp_exceptions (gtm_transaction_cp *cp = 0);
 
   // In local.cc
   void commit_local (void);
-  void rollback_local (void);
+  void rollback_local (size_t until_size = 0);
   void drop_references_local (const void *, size_t);
 
   // In retry.cc
@@ -191,8 +224,8 @@ struct gtm_transaction
   void serialirr_mode ();
 
   // In useraction.cc
-  static void run_actions (struct gtm_user_action **);
-  static void free_actions (struct gtm_user_action **);
+  void rollback_user_actions (size_t until_size = 0);
+  void commit_user_actions ();
 };
 
 } // namespace GTM
diff --git a/libitm/local.cc b/libitm/local.cc
index a1a5655..3da67ab 100644
--- a/libitm/local.cc
+++ b/libitm/local.cc
@@ -48,22 +48,21 @@ gtm_transaction::commit_local ()
 }
 
 void
-gtm_transaction::rollback_local (void)
+gtm_transaction::rollback_local (size_t until_size)
 {
   size_t i, n = local_undo.size();
 
   if (n > 0)
     {
-      for (i = n; i-- > 0; )
+      for (i = n; i-- > until_size; )
 	{
-	  gtm_local_undo *u = local_undo[i];
+	  gtm_local_undo *u = *local_undo.pop();
 	  if (u)
 	    {
 	      memcpy (u->addr, u->saved, u->len);
 	      free (u);
 	    }
 	}
-      local_undo.clear();
     }
 }
 
diff --git a/libitm/method-serial.cc b/libitm/method-serial.cc
index a6fbc7f..a93b991 100644
--- a/libitm/method-serial.cc
+++ b/libitm/method-serial.cc
@@ -38,7 +38,7 @@ namespace {
 class serial_dispatch : public abi_dispatch
 {
  public:
-  serial_dispatch() : abi_dispatch(false, true) { }
+  serial_dispatch() : abi_dispatch(false, true, true, false) { }
 
  protected:
   // Transactional loads and stores simply access memory directly.
@@ -74,12 +74,19 @@ class serial_dispatch : public abi_dispatch
   CREATE_DISPATCH_METHODS_MEM()
 
   virtual bool trycommit() { return true; }
-  virtual void rollback() { abort(); }
+  virtual void rollback(gtm_transaction_cp *cp) { abort(); }
   virtual void reinit() { }
   virtual void fini() { }
+
+  virtual abi_dispatch* closed_nesting_alternative()
+  {
+    // For nested transactions with an instrumented code path, we can do
+    // undo logging.
+    return GTM::dispatch_serial();
+  }
 };
 
-class serial_dispatch_ul : public serial_dispatch
+class serial_dispatch_ul : public abi_dispatch
 {
 protected:
   static void log(const void *addr, size_t len)
@@ -119,12 +126,17 @@ public:
     ::memset(dst, c, size);
   }
 
+  virtual bool trycommit() { return true; }
   // Local undo will handle this.
   // trydropreference() need not be changed either.
-  virtual void rollback() { }
+  virtual void rollback(gtm_transaction_cp *cp) { }
+  virtual void reinit() { }
+  virtual void fini() { }
 
   CREATE_DISPATCH_METHODS(virtual, )
   CREATE_DISPATCH_METHODS_MEM()
+
+  serial_dispatch_ul() : abi_dispatch(false, true, false, true) { }
 };
 
 } // anon namespace
diff --git a/libitm/query.cc b/libitm/query.cc
index 1f03ddf..4905fc6 100644
--- a/libitm/query.cc
+++ b/libitm/query.cc
@@ -44,7 +44,7 @@ _ITM_howExecuting ITM_REGPARM
 _ITM_inTransaction (void)
 {
   struct gtm_transaction *tx = gtm_tx();
-  if (tx)
+  if (tx && (tx->nesting > 0))
     {
       if (tx->state & gtm_transaction::STATE_IRREVOCABLE)
 	return inIrrevocableTransaction;
diff --git a/libitm/retry.cc b/libitm/retry.cc
index 8e586de..c3b3a1d 100644
--- a/libitm/retry.cc
+++ b/libitm/retry.cc
@@ -35,6 +35,8 @@ GTM::gtm_transaction::decide_retry_strategy (gtm_restart_reason r)
   bool retry_irr = (r == RESTART_SERIAL_IRR);
   bool retry_serial = (this->restart_total > 100 || retry_irr);
 
+  if (r == RESTART_CLOSED_NESTING) retry_serial = true;
+
   if (retry_serial)
     {
       // In serialirr_mode we can succeed with the upgrade to
@@ -56,7 +58,7 @@ GTM::gtm_transaction::decide_retry_strategy (gtm_restart_reason r)
       // ??? We can only retry with dispatch_serial when the transaction
       // doesn't contain an abort.  TODO: Create a serial mode dispatch
       // that does logging in order to support abort.
-      if (this->prop & pr_hasNoAbort)
+      if ((this->prop & pr_hasNoAbort) && (r != RESTART_CLOSED_NESTING))
 	retry_irr = true;
     }
 
@@ -64,12 +66,13 @@ GTM::gtm_transaction::decide_retry_strategy (gtm_restart_reason r)
     {
       this->state = (STATE_SERIAL | STATE_IRREVOCABLE);
       disp->fini ();
-      disp = dispatch_serial ();
+      disp = dispatch_serialirr ();
       set_abi_disp (disp);
     }
   else
     {
-      GTM_fatal("internal error: unsupported mode");
+      disp = dispatch_serial();
+      set_abi_disp (disp);
     }
 }
 
diff --git a/libitm/useraction.cc b/libitm/useraction.cc
index adbe470..4042366 100644
--- a/libitm/useraction.cc
+++ b/libitm/useraction.cc
@@ -26,50 +26,28 @@
 
 namespace GTM HIDDEN {
 
-struct gtm_user_action
-{
-  struct gtm_user_action *next;
-  _ITM_userCommitFunction fn;
-  void *arg;
-};
-
-
 void
-gtm_transaction::run_actions (gtm_user_action **list)
+gtm_transaction::rollback_user_actions(size_t until_size)
 {
-  gtm_user_action *a = *list;
-
-  if (a == NULL)
-    return;
-  *list = NULL;
-
-  do
+  for (size_t s = user_actions.size(); s > until_size; s--)
     {
-      gtm_user_action *n = a->next;
-      a->fn (a->arg);
-      free (a);
-      a = n;
+      user_action *a = user_actions.pop();
+      if (!a->on_commit)
+        a->fn (a->arg);
     }
-  while (a);
 }
 
 
 void
-gtm_transaction::free_actions (gtm_user_action **list)
+gtm_transaction::commit_user_actions()
 {
-  gtm_user_action *a = *list;
-
-  if (a == NULL)
-    return;
-  *list = NULL;
-
-  do
+  for (vector<user_action>::iterator i = user_actions.begin(),
+      ie = user_actions.end(); i != ie; i++)
     {
-      gtm_user_action *n = a->next;
-      free (a);
-      a = n;
+      if (i->on_commit)
+        i->fn (i->arg);
     }
-  while (a);
+  user_actions.clear();
 }
 
 } // namespace GTM
@@ -80,17 +58,15 @@ void ITM_REGPARM
 _ITM_addUserCommitAction(_ITM_userCommitFunction fn,
 			 _ITM_transactionId_t tid, void *arg)
 {
-  gtm_transaction *tx;
-  gtm_user_action *a;
-
-  for (tx = gtm_tx(); tx->id != tid; tx = tx->prev)
-    continue;
-
-  a = (gtm_user_action *) xmalloc (sizeof (*a));
-  a->next = tx->commit_actions;
+  gtm_transaction *tx = gtm_tx();
+  if (tid != _ITM_noTransactionId)
+    GTM_fatal("resumingTransactionId in _ITM_addUserCommitAction must be "
+              "_ITM_noTransactionId");
+  gtm_transaction::user_action *a = tx->user_actions.push();
   a->fn = fn;
   a->arg = arg;
-  tx->commit_actions = a;
+  a->on_commit = true;
+  a->resuming_id = tid;
 }
 
 
@@ -98,11 +74,8 @@ void ITM_REGPARM
 _ITM_addUserUndoAction(_ITM_userUndoFunction fn, void * arg)
 {
   gtm_transaction *tx = gtm_tx();
-  gtm_user_action *a;
-
-  a = (gtm_user_action *) xmalloc (sizeof (*a));
-  a->next = tx->undo_actions;
+  gtm_transaction::user_action *a = tx->user_actions.push();
   a->fn = fn;
   a->arg = arg;
-  tx->undo_actions = a;
+  a->on_commit = false;
 }

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [trans-mem] Beginning of refactoring
  2011-07-09 15:35 ` Torvald Riegel
@ 2011-07-18 23:38   ` Torvald Riegel
  2011-07-18 23:44     ` Torvald Riegel
  0 siblings, 1 reply; 19+ messages in thread
From: Torvald Riegel @ 2011-07-18 23:38 UTC (permalink / raw)
  To: GCC Patches; +Cc: Richard Henderson

On Sat, 2011-07-09 at 16:30 +0200, Torvald Riegel wrote:
> The attached patch makes flat nesting the default, while still
> supporting closed nesting on demand (for user-controlled aborts via
> __transaction_cancel).
> Previously, a new transaction object was created for each nested
> transaction. The patch changes this to using one transaction object
> during the whole lifetime of a thread and keeping checkpoints of the
> transaction state when starting closed nested transactions. This allows
> us to easily use flat nesting and decreases the transaction start/commit
> overheads to some extent.
> 
> OK for branch?

I split the patch, as requested.

First three parts are smaller changes:

patch1: New erase method and placement new for aatree.
patch2: Change pr_hasElse to the value specified in the ABI.
patch3: Add information to dispatch about closed nesting and
uninstrumented code.

Then, in preparation for the following flat-transaction object design:
patch4: Use vector instead of list to store user actions.
patch5: Add closed nesting as restart reason.

And the actual core change:
patch6: Make flat nesting the default, use closed nesting on demand.

This last patch is still rather large, but it is one logical piece.
beginend.cc has most changes, but splitting this one up would rather
create incomplete intermediate steps than make the direction of this
whole patch clear.

As discussed offline, reducing the one extra copying of jmpbuf for
actual closed-nested transactions can be addressed in a future patch by
modifying _ITM_beginTransaction, which could additionally remove the
extra copying from stack to jmpbuf for all transactions as it existed
before this patch here.

Ok for branch, or is further splitting required?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [trans-mem] Beginning of refactoring
  2011-07-18 23:38   ` Torvald Riegel
@ 2011-07-18 23:44     ` Torvald Riegel
  2011-07-27 10:57       ` Torvald Riegel
  2011-07-28 16:52       ` Richard Henderson
  0 siblings, 2 replies; 19+ messages in thread
From: Torvald Riegel @ 2011-07-18 23:44 UTC (permalink / raw)
  To: GCC Patches; +Cc: Richard Henderson

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

On Tue, 2011-07-19 at 01:19 +0200, Torvald Riegel wrote:
> On Sat, 2011-07-09 at 16:30 +0200, Torvald Riegel wrote:
> > The attached patch makes flat nesting the default, while still
> > supporting closed nesting on demand (for user-controlled aborts via
> > __transaction_cancel).
> > Previously, a new transaction object was created for each nested
> > transaction. The patch changes this to using one transaction object
> > during the whole lifetime of a thread and keeping checkpoints of the
> > transaction state when starting closed nested transactions. This allows
> > us to easily use flat nesting and decreases the transaction start/commit
> > overheads to some extent.
> > 
> > OK for branch?
> 
> I split the patch, as requested.
> 
> First three parts are smaller changes:
> 
> patch1: New erase method and placement new for aatree.
> patch2: Change pr_hasElse to the value specified in the ABI.
> patch3: Add information to dispatch about closed nesting and
> uninstrumented code.
> 
> Then, in preparation for the following flat-transaction object design:
> patch4: Use vector instead of list to store user actions.
> patch5: Add closed nesting as restart reason.
> 
> And the actual core change:
> patch6: Make flat nesting the default, use closed nesting on demand.
> 
> This last patch is still rather large, but it is one logical piece.
> beginend.cc has most changes, but splitting this one up would rather
> create incomplete intermediate steps than make the direction of this
> whole patch clear.
> 
> As discussed offline, reducing the one extra copying of jmpbuf for
> actual closed-nested transactions can be addressed in a future patch by
> modifying _ITM_beginTransaction, which could additionally remove the
> extra copying from stack to jmpbuf for all transactions as it existed
> before this patch here.
> 
> Ok for branch, or is further splitting required?
> 

This time with patches attached, sorry.

[-- Attachment #2: patch1 --]
[-- Type: text/plain, Size: 1269 bytes --]

commit 267504991b7b6724f5fee633045dfbdd363cc754
Author: Torvald Riegel <triegel@redhat.com>
Date:   Mon Jul 18 22:54:34 2011 +0200

    New erase method and placement new for aatree.
    
            * aatree.h (aa_tree::remove): New.
            (aa_tree::operator new): Add placement new.

diff --git a/libitm/aatree.h b/libitm/aatree.h
index 6d28890..3e11b95 100644
--- a/libitm/aatree.h
+++ b/libitm/aatree.h
@@ -1,4 +1,4 @@
-/* Copyright (C) 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU Transactional Memory Library (libitm).
@@ -141,6 +141,8 @@ class aa_tree : public aa_tree_key<KEY>
   aa_tree() = default;
   ~aa_tree() { clear(); }
 
+  static void *operator new (size_t s, aa_tree<KEY, DATA>* p) { return p; }
+
   DATA *find(KEY k) const
   {
     node_ptr n = static_cast<node_ptr>(base::find (k));
@@ -160,6 +162,13 @@ class aa_tree : public aa_tree_key<KEY>
     delete n;
   }
 
+  node_ptr remove(KEY k, DATA** data)
+  {
+    node_ptr n = static_cast<node_ptr>(base::erase (k));
+    *data = (n ? &n->data : 0);
+    return n;
+  }
+
   void clear()
   {
     node_ptr n = static_cast<node_ptr>(this->m_tree);

[-- Attachment #3: patch2 --]
[-- Type: text/plain, Size: 703 bytes --]

commit ca8b345e703109038617bf75693f6b5e8cb43019
Author: Torvald Riegel <triegel@redhat.com>
Date:   Mon Jul 18 23:06:19 2011 +0200

    Change pr_hasElse to the value specified in the ABI.
    
            * libitm.h (_ITM_codeProperties): Change pr_hasElse to the ABI's value.

diff --git a/libitm/libitm.h b/libitm/libitm.h
index abd4274..e961e81 100644
--- a/libitm/libitm.h
+++ b/libitm/libitm.h
@@ -93,8 +93,8 @@ typedef enum
    pr_preferUninstrumented	= 0x0800,
    /* Exception blocks are not used nor supported. */
    pr_exceptionBlock		= 0x1000,
+   pr_hasElse			= 0x2000,
    pr_readOnly			= 0x4000,
-   pr_hasElse			= 0x200000,
    pr_hasNoSimpleReads		= 0x400000
 } _ITM_codeProperties;
 

[-- Attachment #4: patch3 --]
[-- Type: text/plain, Size: 3178 bytes --]

commit 7bc4fad41e8a5a2efaba6fc0d272e79357d3877f
Author: Torvald Riegel <triegel@redhat.com>
Date:   Mon Jul 18 23:18:13 2011 +0200

    Add information to dispatch about closed nesting and uninstrumented code.
    
            * dispatch.h (GTM::abi_dispatch): Add can_run_uninstrumented_code and
            closed_nesting flags, as well as a closed nesting alternative.
            * method-serial.cc: Same.

diff --git a/libitm/dispatch.h b/libitm/dispatch.h
index 3643503..38f1342 100644
--- a/libitm/dispatch.h
+++ b/libitm/dispatch.h
@@ -255,8 +255,17 @@ public:
   // a unique object and so we don't want to destroy it from common code.
   virtual void fini() = 0;
 
+  // Return an alternative method that is compatible with the current
+  // method but supports closed nesting. Return zero if there is none.
+  virtual abi_dispatch* closed_nesting_alternative() { return 0; }
+
   bool read_only () const { return m_read_only; }
   bool write_through() const { return m_write_through; }
+  bool can_run_uninstrumented_code() const
+  {
+    return m_can_run_uninstrumented_code;
+  }
+  bool closed_nesting() const { return m_closed_nesting; }
 
   static void *operator new(size_t s) { return xmalloc (s); }
   static void operator delete(void *p) { free (p); }
@@ -275,7 +284,13 @@ public:
 protected:
   const bool m_read_only;
   const bool m_write_through;
-  abi_dispatch(bool ro, bool wt) : m_read_only(ro), m_write_through(wt) { }
+  const bool m_can_run_uninstrumented_code;
+  const bool m_closed_nesting;
+  abi_dispatch(bool ro, bool wt, bool uninstrumented, bool closed_nesting) :
+    m_read_only(ro), m_write_through(wt),
+    m_can_run_uninstrumented_code(uninstrumented),
+    m_closed_nesting(closed_nesting)
+  { }
 };
 
 }
diff --git a/libitm/method-serial.cc b/libitm/method-serial.cc
index a6fbc7f..009d221 100644
--- a/libitm/method-serial.cc
+++ b/libitm/method-serial.cc
@@ -38,7 +38,7 @@ namespace {
 class serial_dispatch : public abi_dispatch
 {
  public:
-  serial_dispatch() : abi_dispatch(false, true) { }
+  serial_dispatch() : abi_dispatch(false, true, true, false) { }
 
  protected:
   // Transactional loads and stores simply access memory directly.
@@ -77,9 +77,16 @@ class serial_dispatch : public abi_dispatch
   virtual void rollback() { abort(); }
   virtual void reinit() { }
   virtual void fini() { }
+
+  virtual abi_dispatch* closed_nesting_alternative()
+  {
+    // For nested transactions with an instrumented code path, we can do
+    // undo logging.
+    return GTM::dispatch_serial();
+  }
 };
 
-class serial_dispatch_ul : public serial_dispatch
+class serial_dispatch_ul : public abi_dispatch
 {
 protected:
   static void log(const void *addr, size_t len)
@@ -119,12 +126,17 @@ public:
     ::memset(dst, c, size);
   }
 
+  virtual bool trycommit() { return true; }
   // Local undo will handle this.
   // trydropreference() need not be changed either.
   virtual void rollback() { }
+  virtual void reinit() { }
+  virtual void fini() { }
 
   CREATE_DISPATCH_METHODS(virtual, )
   CREATE_DISPATCH_METHODS_MEM()
+
+  serial_dispatch_ul() : abi_dispatch(false, true, false, true) { }
 };
 
 } // anon namespace

[-- Attachment #5: patch4 --]
[-- Type: text/plain, Size: 5070 bytes --]

commit 10732b9b900e63d4b38e054b1175080ccdd2fa52
Author: Torvald Riegel <triegel@redhat.com>
Date:   Tue Jul 19 00:10:11 2011 +0200

    Use vector instead of list to store user actions.
    
    	* useraction.cc: Use vector instead of list to store actions.
    	Also support partial rollbacks for closed nesting.
    	* libitm_i.h (GTM::gtm_transaction::user_action): Same.
    	* beginend.cc: Same.

diff --git a/libitm/beginend.cc b/libitm/beginend.cc
index 9b16973..26dbaf1 100644
--- a/libitm/beginend.cc
+++ b/libitm/beginend.cc
@@ -171,8 +171,7 @@ GTM::gtm_transaction::rollback ()
   abi_disp()->rollback ();
   rollback_local ();
 
-  free_actions (&this->commit_actions);
-  run_actions (&this->undo_actions);
+  rollback_user_actions();
   commit_allocations (true);
   revert_cpp_exceptions ();
 
@@ -214,8 +213,7 @@ GTM::gtm_transaction::trycommit ()
   if (abi_disp()->trycommit ())
     {
       commit_local ();
-      free_actions (&this->undo_actions);
-      run_actions (&this->commit_actions);
+      commit_user_actions();
       commit_allocations (false);
       return true;
     }
diff --git a/libitm/libitm_i.h b/libitm/libitm_i.h
index 01b2e4a..31584ac 100644
--- a/libitm/libitm_i.h
+++ b/libitm/libitm_i.h
@@ -96,12 +96,19 @@ struct gtm_alloc_action
 // This type is private to local.c.
 struct gtm_local_undo;
 
-// This type is private to useraction.c.
-struct gtm_user_action;
 
 // All data relevant to a single transaction.
 struct gtm_transaction
 {
+
+  struct user_action
+  {
+    _ITM_userCommitFunction fn;
+    void *arg;
+    bool on_commit;
+    _ITM_transactionId_t resuming_id;
+  };
+
   // The jump buffer by which GTM_longjmp restarts the transaction.
   // This field *must* be at the beginning of the transaction.
   gtm_jmpbuf jb;
@@ -112,12 +119,10 @@ struct gtm_transaction
   // Data used by alloc.c for the malloc/free undo log.
   aa_tree<uintptr_t, gtm_alloc_action> alloc_actions;
 
-  // Data used by useraction.c for the user defined undo log.
-  struct gtm_user_action *commit_actions;
-  struct gtm_user_action *undo_actions;
-
   // A pointer to the "outer" transaction.
   struct gtm_transaction *prev;
+  // Data used by useraction.c for the user-defined commit/abort handlers.
+  vector<user_action> user_actions;
 
   // A numerical identifier for this transaction.
   _ITM_transactionId_t id;
@@ -191,8 +196,8 @@ struct gtm_transaction
   void serialirr_mode ();
 
   // In useraction.cc
-  static void run_actions (struct gtm_user_action **);
-  static void free_actions (struct gtm_user_action **);
+  void rollback_user_actions (size_t until_size = 0);
+  void commit_user_actions ();
 };
 
 } // namespace GTM
diff --git a/libitm/useraction.cc b/libitm/useraction.cc
index adbe470..4042366 100644
--- a/libitm/useraction.cc
+++ b/libitm/useraction.cc
@@ -26,50 +26,28 @@
 
 namespace GTM HIDDEN {
 
-struct gtm_user_action
-{
-  struct gtm_user_action *next;
-  _ITM_userCommitFunction fn;
-  void *arg;
-};
-
-
 void
-gtm_transaction::run_actions (gtm_user_action **list)
+gtm_transaction::rollback_user_actions(size_t until_size)
 {
-  gtm_user_action *a = *list;
-
-  if (a == NULL)
-    return;
-  *list = NULL;
-
-  do
+  for (size_t s = user_actions.size(); s > until_size; s--)
     {
-      gtm_user_action *n = a->next;
-      a->fn (a->arg);
-      free (a);
-      a = n;
+      user_action *a = user_actions.pop();
+      if (!a->on_commit)
+        a->fn (a->arg);
     }
-  while (a);
 }
 
 
 void
-gtm_transaction::free_actions (gtm_user_action **list)
+gtm_transaction::commit_user_actions()
 {
-  gtm_user_action *a = *list;
-
-  if (a == NULL)
-    return;
-  *list = NULL;
-
-  do
+  for (vector<user_action>::iterator i = user_actions.begin(),
+      ie = user_actions.end(); i != ie; i++)
     {
-      gtm_user_action *n = a->next;
-      free (a);
-      a = n;
+      if (i->on_commit)
+        i->fn (i->arg);
     }
-  while (a);
+  user_actions.clear();
 }
 
 } // namespace GTM
@@ -80,17 +58,15 @@ void ITM_REGPARM
 _ITM_addUserCommitAction(_ITM_userCommitFunction fn,
 			 _ITM_transactionId_t tid, void *arg)
 {
-  gtm_transaction *tx;
-  gtm_user_action *a;
-
-  for (tx = gtm_tx(); tx->id != tid; tx = tx->prev)
-    continue;
-
-  a = (gtm_user_action *) xmalloc (sizeof (*a));
-  a->next = tx->commit_actions;
+  gtm_transaction *tx = gtm_tx();
+  if (tid != _ITM_noTransactionId)
+    GTM_fatal("resumingTransactionId in _ITM_addUserCommitAction must be "
+              "_ITM_noTransactionId");
+  gtm_transaction::user_action *a = tx->user_actions.push();
   a->fn = fn;
   a->arg = arg;
-  tx->commit_actions = a;
+  a->on_commit = true;
+  a->resuming_id = tid;
 }
 
 
@@ -98,11 +74,8 @@ void ITM_REGPARM
 _ITM_addUserUndoAction(_ITM_userUndoFunction fn, void * arg)
 {
   gtm_transaction *tx = gtm_tx();
-  gtm_user_action *a;
-
-  a = (gtm_user_action *) xmalloc (sizeof (*a));
-  a->next = tx->undo_actions;
+  gtm_transaction::user_action *a = tx->user_actions.push();
   a->fn = fn;
   a->arg = arg;
-  tx->undo_actions = a;
+  a->on_commit = false;
 }

[-- Attachment #6: patch5 --]
[-- Type: text/plain, Size: 1964 bytes --]

commit 21dc1b25dccfc5bec97886c80f135623c38a0aba
Author: Torvald Riegel <triegel@redhat.com>
Date:   Tue Jul 19 00:35:56 2011 +0200

    Add closed nesting as restart reason.
    
    	* libitm_i.h: Add closed nesting as restart reason.
    	* retry.cc (GTM::gtm_transaction::decide_retry_strategy): Same.

diff --git a/libitm/libitm_i.h b/libitm/libitm_i.h
index 31584ac..9857087 100644
--- a/libitm/libitm_i.h
+++ b/libitm/libitm_i.h
@@ -82,6 +82,7 @@ enum gtm_restart_reason
   RESTART_VALIDATE_COMMIT,
   RESTART_SERIAL_IRR,
   RESTART_NOT_READONLY,
+  RESTART_CLOSED_NESTING,
   NUM_RESTARTS
 };
 
diff --git a/libitm/retry.cc b/libitm/retry.cc
index 8e586de..bfca2ca 100644
--- a/libitm/retry.cc
+++ b/libitm/retry.cc
@@ -35,6 +35,8 @@ GTM::gtm_transaction::decide_retry_strategy (gtm_restart_reason r)
   bool retry_irr = (r == RESTART_SERIAL_IRR);
   bool retry_serial = (this->restart_total > 100 || retry_irr);
 
+  if (r == RESTART_CLOSED_NESTING) retry_serial = true;
+
   if (retry_serial)
     {
       // In serialirr_mode we can succeed with the upgrade to
@@ -54,9 +56,8 @@ GTM::gtm_transaction::decide_retry_strategy (gtm_restart_reason r)
 	}
 
       // ??? We can only retry with dispatch_serial when the transaction
-      // doesn't contain an abort.  TODO: Create a serial mode dispatch
-      // that does logging in order to support abort.
-      if (this->prop & pr_hasNoAbort)
+      // doesn't contain an abort.
+      if ((this->prop & pr_hasNoAbort) && (r != RESTART_CLOSED_NESTING))
 	retry_irr = true;
     }
 
@@ -64,12 +65,13 @@ GTM::gtm_transaction::decide_retry_strategy (gtm_restart_reason r)
     {
       this->state = (STATE_SERIAL | STATE_IRREVOCABLE);
       disp->fini ();
-      disp = dispatch_serial ();
+      disp = dispatch_serialirr ();
       set_abi_disp (disp);
     }
   else
     {
-      GTM_fatal("internal error: unsupported mode");
+      disp = dispatch_serial();
+      set_abi_disp (disp);
     }
 }
 

[-- Attachment #7: patch6 --]
[-- Type: text/plain, Size: 28247 bytes --]

commit c3fc59da237c51dfa2aea25069080a74cb2fdc12
Author: Torvald Riegel <triegel@redhat.com>
Date:   Tue Jul 19 00:54:23 2011 +0200

    Make flat nesting the default, use closed nesting on demand.
    
            * local.cc (gtm_transaction::rollback_local): Support closed nesting.
            * eh_cpp.cc (GTM::gtm_transaction::revert_cpp_exceptions): Same.
    	* dispatch.h: Same.
    	* method-serial.cc: Same.
            * beginend.cc (GTM::gtm_transaction::begin_transaction): Change to
            flat nesting as default, and closed nesting on demand.
            (GTM::gtm_transaction::rollback): Same.
            (_ITM_abortTransaction): Same.
            (GTM::gtm_transaction::restart): Same.
            (GTM::gtm_transaction::trycommit): Same.
            (GTM::gtm_transaction::trycommit_and_finalize): Removed.
            (choose_code_path): New.
            (GTM::gtm_transaction_cp::save): New.
            (GTM::gtm_transaction_cp::commit): New.
            * query.cc (_ITM_inTransaction): Support flat nesting.
            * libitm_i.h (GTM::gtm_transaction_cp): New helper struct for nesting.
            (GTM::gtm_transaction): Support flat and closed nesting.
            * alloc.cc (commit_allocations_2): New.
            (commit_cb_data): New helper struct.
            (GTM::gtm_transaction::commit_allocations): Handle nested
            commits/rollbacks.
            * libitm.texi: Update user action section, add description of nesting.

diff --git a/libitm/alloc.cc b/libitm/alloc.cc
index 5c70144..9b60835 100644
--- a/libitm/alloc.cc
+++ b/libitm/alloc.cc
@@ -1,4 +1,4 @@
-/* Copyright (C) 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU Transactional Memory Library (libitm).
@@ -52,6 +52,55 @@ gtm_transaction::forget_allocation (void *ptr, void (*free_fn)(void *))
   a->allocated = false;
 }
 
+namespace {
+struct commit_cb_data {
+  aa_tree<uintptr_t, gtm_alloc_action>* parent;
+  bool revert_p;
+};
+}
+
+static void
+commit_allocations_2 (uintptr_t key, gtm_alloc_action *a, void *data)
+{
+  void *ptr = (void *)key;
+  commit_cb_data *cb_data = static_cast<commit_cb_data *>(data);
+
+  if (cb_data->revert_p)
+    {
+      // Roll back nested allocations.
+      if (a->allocated)
+        a->free_fn (ptr);
+    }
+  else
+    {
+      if (a->allocated)
+        {
+          // Add nested allocations to parent transaction.
+          gtm_alloc_action* a_parent = cb_data->parent->insert(key);
+          *a_parent = *a;
+        }
+      else
+        {
+          // Eliminate a parent allocation if it matches this memory release,
+          // otherwise just add it to the parent.
+          gtm_alloc_action* a_parent;
+          aa_tree<uintptr_t, gtm_alloc_action>::node_ptr node_ptr =
+              cb_data->parent->remove(key, &a_parent);
+          if (node_ptr)
+            {
+              assert(a_parent->allocated);
+              a_parent->free_fn(ptr);
+              delete node_ptr;
+            }
+          else
+            {
+              a_parent = cb_data->parent->insert(key);
+              *a_parent = *a;
+            }
+        }
+    }
+}
+
 static void
 commit_allocations_1 (uintptr_t key, gtm_alloc_action *a, void *cb_data)
 {
@@ -67,10 +116,19 @@ commit_allocations_1 (uintptr_t key, gtm_alloc_action *a, void *cb_data)
    REVERT_P is true if instead of committing the allocations, we want
    to roll them back (and vice versa).  */
 void
-gtm_transaction::commit_allocations (bool revert_p)
+gtm_transaction::commit_allocations (bool revert_p,
+    aa_tree<uintptr_t, gtm_alloc_action>* parent)
 {
-  this->alloc_actions.traverse (commit_allocations_1,
-				(void *)(uintptr_t)revert_p);
+  if (parent)
+    {
+      commit_cb_data cb_data;
+      cb_data.parent = parent;
+      cb_data.revert_p = revert_p;
+      this->alloc_actions.traverse (commit_allocations_2, &cb_data);
+    }
+  else
+    this->alloc_actions.traverse (commit_allocations_1,
+				  (void *)(uintptr_t)revert_p);
   this->alloc_actions.clear ();
 }
 
diff --git a/libitm/beginend.cc b/libitm/beginend.cc
index 26dbaf1..6023390 100644
--- a/libitm/beginend.cc
+++ b/libitm/beginend.cc
@@ -88,6 +88,14 @@ GTM::gtm_transaction::operator delete(void *tx)
 static pthread_mutex_t global_tid_lock = PTHREAD_MUTEX_INITIALIZER;
 #endif
 
+static inline uint32_t choose_code_path(uint32_t prop, abi_dispatch *disp)
+{
+  if ((prop & pr_uninstrumentedCode) && disp->can_run_uninstrumented_code())
+    return a_runUninstrumentedCode;
+  else
+    return a_runInstrumentedCode;
+}
+
 uint32_t
 GTM::gtm_transaction::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb)
 {
@@ -97,14 +105,112 @@ GTM::gtm_transaction::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb)
   abi_dispatch *disp;
   uint32_t ret;
 
+  // ??? pr_undoLogCode is not properly defined in the ABI. Are barriers
+  // omitted because they are not necessary (e.g., a transaction on thread-
+  // local data) or because the compiler thinks that some kind of global
+  // synchronization might perform better?
+  if (unlikely(prop & pr_undoLogCode))
+    GTM_fatal("pr_undoLogCode not supported");
+
   gtm_thread *thr = setup_gtm_thr ();
 
-  tx = new gtm_transaction;
+  tx = gtm_tx();
+  if (tx == NULL)
+    {
+      tx = new gtm_transaction;
+      set_gtm_tx(tx);
+    }
+
+  if (tx->nesting > 0)
+    {
+      // This is a nested transaction.
+      // Check prop compatibility:
+      // The ABI requires pr_hasNoFloatUpdate, pr_hasNoVectorUpdate,
+      // pr_hasNoIrrevocable, pr_aWBarriersOmitted, pr_RaRBarriersOmitted, and
+      // pr_hasNoSimpleReads to hold for the full dynamic scope of a
+      // transaction. We could check that these are set for the nested
+      // transaction if they are also set for the parent transaction, but the
+      // ABI does not require these flags to be set if they could be set,
+      // so the check could be too strict.
+      // ??? For pr_readOnly, lexical or dynamic scope is unspecified.
+
+      if (prop & pr_hasNoAbort)
+        {
+          // We can use flat nesting, so elide this transaction.
+          if (!(prop & pr_instrumentedCode))
+            {
+              if (!(tx->state & STATE_SERIAL) ||
+                  !(tx->state & STATE_IRREVOCABLE))
+                tx->serialirr_mode();
+            }
+          // Increment nesting level after checking that we have a method that
+          // allows us to continue.
+          tx->nesting++;
+          return choose_code_path(prop, abi_disp());
+        }
+
+      // The transaction might abort, so use closed nesting if possible.
+      // pr_hasNoAbort has lexical scope, so the compiler should really have
+      // generated an instrumented code path.
+      assert(prop & pr_instrumentedCode);
+
+      // Create a checkpoint of the current transaction.
+      gtm_transaction_cp *cp = tx->parent_txns.push();
+      cp->save(tx);
+      new (&tx->alloc_actions) aa_tree<uintptr_t, gtm_alloc_action>();
+
+      // Check whether the current method actually supports closed nesting.
+      // If we can switch to another one, do so.
+      // If not, we assume that actual aborts are infrequent, and rather
+      // restart in _ITM_abortTransaction when we really have to.
+      disp = abi_disp();
+      if (!disp->closed_nesting())
+        {
+          // ??? Should we elide the transaction if there is no alternative
+          // method that supports closed nesting? If we do, we need to set
+          // some flag to prevent _ITM_abortTransaction from aborting the
+          // wrong transaction (i.e., some parent transaction).
+          abi_dispatch *cn_disp = disp->closed_nesting_alternative();
+          if (cn_disp)
+            {
+              disp = cn_disp;
+              set_abi_disp(disp);
+            }
+        }
+    }
+  else
+    {
+      // Outermost transaction
+      // TODO Pay more attention to prop flags (eg, *omitted) when selecting
+      // dispatch.
+      if ((prop & pr_doesGoIrrevocable) || !(prop & pr_instrumentedCode))
+        tx->state = (STATE_SERIAL | STATE_IRREVOCABLE);
+
+      else
+        disp = tx->decide_begin_dispatch ();
+
+      if (tx->state & STATE_SERIAL)
+        {
+          serial_lock.write_lock ();
 
+          if (tx->state & STATE_IRREVOCABLE)
+            disp = dispatch_serialirr ();
+          else
+            disp = dispatch_serial ();
+        }
+      else
+        {
+          serial_lock.read_lock ();
+        }
+
+      set_abi_disp (disp);
+    }
+
+  // Initialization that is common for outermost and nested transactions.
   tx->prop = prop;
-  tx->prev = gtm_tx();
-  if (tx->prev)
-    tx->nesting = tx->prev->nesting + 1;
+  tx->nesting++;
+
+  tx->jb = *jb;
 
   // As long as we have not exhausted a previously allocated block of TIDs,
   // we can avoid an atomic operation on a shared cacheline.
@@ -124,57 +230,80 @@ GTM::gtm_transaction::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb)
 #endif
     }
 
-  tx->jb = *jb;
+  // Determine the code path to run. Only irrevocable transactions cannot be
+  // restarted, so all other transactions need to save live variables.
+  ret = choose_code_path(prop, disp);
+  if (!(tx->state & STATE_IRREVOCABLE)) ret |= a_saveLiveVariables;
+  return ret;
+}
 
-  set_gtm_tx (tx);
 
-  // ??? pr_undoLogCode is not properly defined in the ABI. Are barriers
-  // omitted because they are not necessary (e.g., a transaction on thread-
-  // local data) or because the compiler thinks that some kind of global
-  // synchronization might perform better?
-  if (unlikely(prop & pr_undoLogCode))
-    GTM_fatal("pr_undoLogCode not supported");
+void
+GTM::gtm_transaction_cp::save(gtm_transaction* tx)
+{
+  // Save everything that we might have to restore on restarts or aborts.
+  jb = tx->jb;
+  local_undo_size = tx->local_undo.size();
+  memcpy(&alloc_actions, &tx->alloc_actions, sizeof(alloc_actions));
+  user_actions_size = tx->user_actions.size();
+  id = tx->id;
+  prop = tx->prop;
+  cxa_catch_count = tx->cxa_catch_count;
+  cxa_unthrown = tx->cxa_unthrown;
+  disp = abi_disp();
+  nesting = tx->nesting;
+}
 
-  if ((prop & pr_doesGoIrrevocable) || !(prop & pr_instrumentedCode))
-    tx->state = (STATE_SERIAL | STATE_IRREVOCABLE);
+void
+GTM::gtm_transaction_cp::commit(gtm_transaction* tx)
+{
+  // Restore state that is not persistent across commits. Exception handling,
+  // information, nesting level, and any logs do not need to be restored on
+  // commits of nested transactions. Allocation actions must be committed
+  // before committing the snapshot.
+  tx->jb = jb;
+  memcpy(&tx->alloc_actions, &alloc_actions, sizeof(alloc_actions));
+  tx->id = id;
+  tx->prop = prop;
+}
 
-  else
-    disp = tx->decide_begin_dispatch ();
 
-  if (tx->state & STATE_SERIAL)
-    {
-      serial_lock.write_lock ();
+void
+GTM::gtm_transaction::rollback (gtm_transaction_cp *cp)
+{
+  abi_disp()->rollback (cp);
 
-      if (tx->state & STATE_IRREVOCABLE)
-        disp = dispatch_serialirr ();
-      else
-        disp = dispatch_serial ();
+  rollback_local (cp ? cp->local_undo_size : 0);
+  rollback_user_actions (cp ? cp->user_actions_size : 0);
+  commit_allocations (true, (cp ? &cp->alloc_actions : 0));
+  revert_cpp_exceptions (cp);
 
-      ret = a_runUninstrumentedCode;
-      if ((prop & pr_multiwayCode) == pr_instrumentedCode)
-	ret = a_runInstrumentedCode;
+  if (cp)
+    {
+      // Roll back the rest of the state to the checkpoint.
+      jb = cp->jb;
+      id = cp->id;
+      prop = cp->prop;
+      if (cp->disp != abi_disp())
+        set_abi_disp(cp->disp);
+      memcpy(&alloc_actions, &cp->alloc_actions, sizeof(alloc_actions));
+      nesting = cp->nesting;
     }
   else
     {
-      serial_lock.read_lock ();
-      ret = a_runInstrumentedCode | a_saveLiveVariables;
+      // Restore the jump buffer and transaction properties, which we will
+      // need for the longjmp used to restart or abort the transaction.
+      if (parent_txns.size() > 0)
+        {
+          jb = parent_txns[0].jb;
+          prop = parent_txns[0].prop;
+        }
+      // Reset the transaction. Do not reset state, which is handled by the
+      // callers.
+      nesting = 0;
+      parent_txns.clear();
     }
 
-  set_abi_disp (disp);
-
-  return ret;
-}
-
-void
-GTM::gtm_transaction::rollback ()
-{
-  abi_disp()->rollback ();
-  rollback_local ();
-
-  rollback_user_actions();
-  commit_allocations (true);
-  revert_cpp_exceptions ();
-
   if (this->eh_in_flight)
     {
       _Unwind_DeleteException ((_Unwind_Exception *) this->eh_in_flight);
@@ -193,45 +322,87 @@ _ITM_abortTransaction (_ITM_abortReason reason)
   if (tx->state & gtm_transaction::STATE_IRREVOCABLE)
     abort ();
 
-  tx->rollback ();
-  abi_disp()->fini ();
+  // If the current method does not support closed nesting, we are nested, and
+  // we can restart, then restart with a method that supports closed nesting.
+  abi_dispatch *disp = abi_disp();
+  if (!disp->closed_nesting())
+    tx->restart(RESTART_CLOSED_NESTING);
+
+  // Roll back to innermost transaction.
+  if (tx->parent_txns.size() > 0)
+    {
+      // The innermost transaction is a nested transaction.
+      gtm_transaction_cp *cp = tx->parent_txns.pop();
+      uint32_t longjmp_prop = tx->prop;
+      gtm_jmpbuf longjmp_jb = tx->jb;
+
+      tx->rollback (cp);
+      abi_disp()->fini ();
 
-  if (tx->state & gtm_transaction::STATE_SERIAL)
-    gtm_transaction::serial_lock.write_unlock ();
+      // Jump to nested transaction (use the saved jump buffer).
+      GTM_longjmp (&longjmp_jb, a_abortTransaction | a_restoreLiveVariables,
+          longjmp_prop);
+    }
   else
-    gtm_transaction::serial_lock.read_unlock ();
+    {
+      // There is no nested transaction, so roll back to outermost transaction.
+      tx->rollback ();
+      abi_disp()->fini ();
 
-  set_gtm_tx (tx->prev);
-  delete tx;
+      // Aborting an outermost transaction finishes execution of the whole
+      // transaction. Therefore, reset transaction state.
+      if (tx->state & gtm_transaction::STATE_SERIAL)
+        gtm_transaction::serial_lock.write_unlock ();
+      else
+        gtm_transaction::serial_lock.read_unlock ();
+      tx->state = 0;
 
-  GTM_longjmp (&tx->jb, a_abortTransaction | a_restoreLiveVariables, tx->prop);
+      GTM_longjmp (&tx->jb, a_abortTransaction | a_restoreLiveVariables,
+          tx->prop);
+    }
 }
 
 bool
 GTM::gtm_transaction::trycommit ()
 {
-  if (abi_disp()->trycommit ())
+  nesting--;
+
+  // Skip any real commit for elided transactions.
+  if (nesting > 0 && (parent_txns.size() == 0 ||
+      nesting > parent_txns[parent_txns.size() - 1].nesting))
+    return true;
+
+  if (nesting > 0)
     {
-      commit_local ();
-      commit_user_actions();
-      commit_allocations (false);
+      // Commit of a closed-nested transaction. Remove one checkpoint and add
+      // any effects of this transaction to the parent transaction.
+      gtm_transaction_cp *cp = parent_txns.pop();
+      commit_allocations(false, &cp->alloc_actions);
+      cp->commit(this);
       return true;
     }
-  return false;
-}
 
-bool
-GTM::gtm_transaction::trycommit_and_finalize ()
-{
-  if (trycommit ())
+  // Commit of an outermost transaction.
+  if (abi_disp()->trycommit ())
     {
+      commit_local ();
+      // FIXME: run after ensuring privatization safety:
+      commit_user_actions ();
+      commit_allocations (false, 0);
       abi_disp()->fini ();
-      set_gtm_tx (this->prev);
-      delete this;
-      if (this->state & gtm_transaction::STATE_SERIAL)
+
+      // Reset transaction state.
+      cxa_catch_count = 0;
+      cxa_unthrown = NULL;
+
+      // TODO can release SI mode before committing user actions? If so,
+      // we can release before ensuring privatization safety too.
+      if (state & gtm_transaction::STATE_SERIAL)
 	gtm_transaction::serial_lock.write_unlock ();
       else
 	gtm_transaction::serial_lock.read_unlock ();
+      state = 0;
+
       return true;
     }
   return false;
@@ -240,24 +411,21 @@ GTM::gtm_transaction::trycommit_and_finalize ()
 void ITM_NORETURN
 GTM::gtm_transaction::restart (gtm_restart_reason r)
 {
-  uint32_t actions;
-
+  // Roll back to outermost transaction. Do not reset transaction state because
+  // we will continue executing this transaction.
   rollback ();
   decide_retry_strategy (r);
 
-  actions = a_runInstrumentedCode | a_restoreLiveVariables;
-  if ((this->prop & pr_uninstrumentedCode)
-      && (this->state & gtm_transaction::STATE_IRREVOCABLE))
-    actions = a_runUninstrumentedCode | a_restoreLiveVariables;
-
-  GTM_longjmp (&this->jb, actions, this->prop);
+  GTM_longjmp (&this->jb,
+      choose_code_path(prop, abi_disp()) | a_restoreLiveVariables,
+      this->prop);
 }
 
 void ITM_REGPARM
 _ITM_commitTransaction(void)
 {
   gtm_transaction *tx = gtm_tx();
-  if (!tx->trycommit_and_finalize ())
+  if (!tx->trycommit ())
     tx->restart (RESTART_VALIDATE_COMMIT);
 }
 
@@ -265,7 +433,7 @@ void ITM_REGPARM
 _ITM_commitTransactionEH(void *exc_ptr)
 {
   gtm_transaction *tx = gtm_tx();
-  if (!tx->trycommit_and_finalize ())
+  if (!tx->trycommit ())
     {
       tx->eh_in_flight = exc_ptr;
       tx->restart (RESTART_VALIDATE_COMMIT);
diff --git a/libitm/dispatch.h b/libitm/dispatch.h
index 38f1342..0ab5c4e 100644
--- a/libitm/dispatch.h
+++ b/libitm/dispatch.h
@@ -234,6 +234,8 @@ void ITM_REGPARM _ITM_memset##WRITE(void *dst, int c, size_t size) \
 
 namespace GTM HIDDEN {
 
+struct gtm_transaction_cp;
+
 // This pass-through method is the basis for other methods.
 // It can be used for serial-irrevocable mode.
 struct abi_dispatch
@@ -248,7 +250,7 @@ private:
 
 public:
   virtual bool trycommit() = 0;
-  virtual void rollback() = 0;
+  virtual void rollback(gtm_transaction_cp *cp = 0) = 0;
   virtual void reinit() = 0;
 
   // Use fini instead of dtor to support a static subclasses that uses
diff --git a/libitm/eh_cpp.cc b/libitm/eh_cpp.cc
index bd83f72..35c0b50 100644
--- a/libitm/eh_cpp.cc
+++ b/libitm/eh_cpp.cc
@@ -72,14 +72,37 @@ _ITM_cxa_end_catch (void)
 }
 
 void
-GTM::gtm_transaction::revert_cpp_exceptions (void)
+GTM::gtm_transaction::revert_cpp_exceptions (gtm_transaction_cp *cp)
 {
-  if (this->cxa_unthrown || this->cxa_catch_count)
+  if (cp)
     {
-      __cxa_tm_cleanup (this->cxa_unthrown, this->eh_in_flight,
-			this->cxa_catch_count);
-      this->cxa_catch_count = 0;
-      this->cxa_unthrown = NULL;
-      this->eh_in_flight = NULL;
+      // If rolling back a nested transaction, only clean up unthrown
+      // exceptions since the last checkpoint. Always reset eh_in_flight
+      // because it just contains the argument provided to
+      // _ITM_commitTransactionEH
+      void *unthrown =
+          (cxa_unthrown != cp->cxa_unthrown ? cxa_unthrown : NULL);
+      assert (cxa_catch_count > cp->cxa_catch_count);
+      uint32_t catch_count = cxa_catch_count - cp->cxa_catch_count;
+      if (unthrown || catch_count)
+        {
+          __cxa_tm_cleanup (unthrown, this->eh_in_flight, catch_count);
+          cxa_catch_count = cp->cxa_catch_count;
+          cxa_unthrown = cp->cxa_unthrown;
+          this->eh_in_flight = NULL;
+        }
+    }
+  else
+    {
+      // Both cxa_catch_count and cxa_unthrown are maximal because EH regions
+      // and transactions are properly nested.
+      if (this->cxa_unthrown || this->cxa_catch_count)
+        {
+          __cxa_tm_cleanup (this->cxa_unthrown, this->eh_in_flight,
+              this->cxa_catch_count);
+          this->cxa_catch_count = 0;
+          this->cxa_unthrown = NULL;
+          this->eh_in_flight = NULL;
+        }
     }
 }
diff --git a/libitm/libitm.texi b/libitm/libitm.texi
index 046b0bd..8d7aef4 100644
--- a/libitm/libitm.texi
+++ b/libitm/libitm.texi
@@ -314,13 +314,16 @@ nontransactionally).
 
 @subsection User-registered commit and undo actions
 
-The order in which commit or undo actions are executed is undefined. It is also
-undefined whether privatization safety has been ensured by the time the
-transactions are executed. The ordering of undo actions and the roll-back of
-data transfers is undefined.
+Commit actions will get executed in the same order in which the respective
+calls to @code{_ITM_addUserCommitAction} happened. Only
+@code{_ITM_noTransactionId} is allowed as value for the
+@code{resumingTransactionId} argument. Commit actions get executed after
+privatization safety has been ensured.
 
-However, the ABI should specify ordering guarantees where necessary (in
-particular, w.r.t. privatization safety).
+Undo actions will get executed in reverse order compared to the order in which
+the respective calls to @code{_ITM_addUserUndoAction} happened. The ordering of
+undo actions w.r.t. the roll-back of other actions (e.g., data transfers or
+memory allocations) is undefined.
 
 @code{_ITM_dropReferences} is not supported currently because its semantics and
 the intention behind it is not entirely clear. The
@@ -415,7 +418,31 @@ specification. Likewise, the TM runtime must ensure privatization safety.
 @node Internals
 @chapter Internals
 
-TODO: SI-mode implementation, method sets, ...
+@section Nesting: flat vs. closed
+
+We support two different kinds of nesting of transactions. In the case of
+@emph{flat nesting}, the nesting structure is flattened and all nested
+transactions are subsumed by the enclosing transaction. In contrast,
+with @emph{closed nesting}, nested transactions that have not yet committed
+can be rolled back separately from the enclosing transactions; when they
+commit, they are subsumed by the enclosing transaction, and their effects
+will be finally committed when the outermost transaction commits.
+@emph{Open nesting} (where nested transactions can commit independently of the
+enclosing transactions) are not supported.
+
+Flat nesting is the default nesting mode, but closed nesting is supported and
+used when transactions contain user-controlled aborts
+(@code{__transaction_cancel} statements). We assume that user-controlled
+aborts are rare in typical code and used mostly in exceptional situations.
+Thus, it makes more sense to use flat nesting by default to avoid the
+performance overhead of the additional checkpoints required for closed
+nesting. User-controlled aborts will correctly abort the innermost enclosing
+transaction, whereas the whole (i.e., outermost) transaction will be restarted
+otherwise (e.g., when a transaction encounters data conflicts during
+optimistic execution).
+
+
+@strong{TODO}: SI-mode implementation, method sets, ...
 
 @c ---------------------------------------------------------------------
 @c GNU General Public License
diff --git a/libitm/libitm_i.h b/libitm/libitm_i.h
index 9857087..4cc0a70 100644
--- a/libitm/libitm_i.h
+++ b/libitm/libitm_i.h
@@ -97,6 +97,30 @@ struct gtm_alloc_action
 // This type is private to local.c.
 struct gtm_local_undo;
 
+struct gtm_transaction;
+
+// A transaction checkpoint: data that has to saved and restored when doing
+// closed nesting.
+struct gtm_transaction_cp
+{
+  gtm_jmpbuf jb;
+  size_t local_undo_size;
+  aa_tree<uintptr_t, gtm_alloc_action> alloc_actions;
+  size_t user_actions_size;
+  _ITM_transactionId_t id;
+  uint32_t prop;
+  uint32_t cxa_catch_count;
+  void *cxa_unthrown;
+  // We might want to use a different but compatible dispatch method for
+  // a nested transaction.
+  abi_dispatch *disp;
+  // Nesting level of this checkpoint (1 means that this is a checkpoint of
+  // the outermost transaction).
+  uint32_t nesting;
+
+  void save(gtm_transaction* tx);
+  void commit(gtm_transaction* tx);
+};
 
 // All data relevant to a single transaction.
 struct gtm_transaction
@@ -120,8 +144,6 @@ struct gtm_transaction
   // Data used by alloc.c for the malloc/free undo log.
   aa_tree<uintptr_t, gtm_alloc_action> alloc_actions;
 
-  // A pointer to the "outer" transaction.
-  struct gtm_transaction *prev;
   // Data used by useraction.c for the user-defined commit/abort handlers.
   vector<user_action> user_actions;
 
@@ -131,13 +153,16 @@ struct gtm_transaction
   // The _ITM_codeProperties of this transaction as given by the compiler.
   uint32_t prop;
 
-  // The nesting depth of this transaction.
+  // The nesting depth for subsequently started transactions. This variable
+  // will be set to 1 when starting an outermost transaction.
   uint32_t nesting;
 
   // Set if this transaction owns the serial write lock.
+  // Can be reset only when restarting the outermost transaction.
   static const uint32_t STATE_SERIAL		= 0x0001;
   // Set if the serial-irrevocable dispatch table is installed.
   // Implies that no logging is being done, and abort is not possible.
+  // Can be reset only when restarting the outermost transaction.
   static const uint32_t STATE_IRREVOCABLE	= 0x0002;
 
   // A bitmask of the above.
@@ -148,6 +173,9 @@ struct gtm_transaction
   void *cxa_unthrown;
   void *eh_in_flight;
 
+  // Checkpoints for closed nesting.
+  vector<gtm_transaction_cp> parent_txns;
+
   // Data used by retry.c for deciding what STM implementation should
   // be used for the next iteration of the transaction.
   uint32_t restart_reason[NUM_RESTARTS];
@@ -159,7 +187,7 @@ struct gtm_transaction
   static gtm_rwlock serial_lock;
 
   // In alloc.cc
-  void commit_allocations (bool);
+  void commit_allocations (bool, aa_tree<uintptr_t, gtm_alloc_action>*);
   void record_allocation (void *, void (*)(void *));
   void forget_allocation (void *, void (*)(void *));
   void drop_references_allocations (const void *ptr)
@@ -168,9 +196,8 @@ struct gtm_transaction
   }
 
   // In beginend.cc
-  void rollback ();
+  void rollback (gtm_transaction_cp *cp = 0);
   bool trycommit ();
-  bool trycommit_and_finalize ();
   void restart (gtm_restart_reason) ITM_NORETURN;
 
   static void *operator new(size_t);
@@ -182,11 +209,11 @@ struct gtm_transaction
 	__asm__("GTM_begin_transaction") ITM_REGPARM;
 
   // In eh_cpp.cc
-  void revert_cpp_exceptions ();
+  void revert_cpp_exceptions (gtm_transaction_cp *cp = 0);
 
   // In local.cc
   void commit_local (void);
-  void rollback_local (void);
+  void rollback_local (size_t until_size = 0);
   void drop_references_local (const void *, size_t);
 
   // In retry.cc
diff --git a/libitm/local.cc b/libitm/local.cc
index a1a5655..3da67ab 100644
--- a/libitm/local.cc
+++ b/libitm/local.cc
@@ -48,22 +48,21 @@ gtm_transaction::commit_local ()
 }
 
 void
-gtm_transaction::rollback_local (void)
+gtm_transaction::rollback_local (size_t until_size)
 {
   size_t i, n = local_undo.size();
 
   if (n > 0)
     {
-      for (i = n; i-- > 0; )
+      for (i = n; i-- > until_size; )
 	{
-	  gtm_local_undo *u = local_undo[i];
+	  gtm_local_undo *u = *local_undo.pop();
 	  if (u)
 	    {
 	      memcpy (u->addr, u->saved, u->len);
 	      free (u);
 	    }
 	}
-      local_undo.clear();
     }
 }
 
diff --git a/libitm/method-serial.cc b/libitm/method-serial.cc
index 009d221..a93b991 100644
--- a/libitm/method-serial.cc
+++ b/libitm/method-serial.cc
@@ -74,7 +74,7 @@ class serial_dispatch : public abi_dispatch
   CREATE_DISPATCH_METHODS_MEM()
 
   virtual bool trycommit() { return true; }
-  virtual void rollback() { abort(); }
+  virtual void rollback(gtm_transaction_cp *cp) { abort(); }
   virtual void reinit() { }
   virtual void fini() { }
 
@@ -129,7 +129,7 @@ public:
   virtual bool trycommit() { return true; }
   // Local undo will handle this.
   // trydropreference() need not be changed either.
-  virtual void rollback() { }
+  virtual void rollback(gtm_transaction_cp *cp) { }
   virtual void reinit() { }
   virtual void fini() { }
 
diff --git a/libitm/query.cc b/libitm/query.cc
index 1f03ddf..4905fc6 100644
--- a/libitm/query.cc
+++ b/libitm/query.cc
@@ -44,7 +44,7 @@ _ITM_howExecuting ITM_REGPARM
 _ITM_inTransaction (void)
 {
   struct gtm_transaction *tx = gtm_tx();
-  if (tx)
+  if (tx && (tx->nesting > 0))
     {
       if (tx->state & gtm_transaction::STATE_IRREVOCABLE)
 	return inIrrevocableTransaction;

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [trans-mem] Beginning of refactoring
  2011-07-18 23:44     ` Torvald Riegel
@ 2011-07-27 10:57       ` Torvald Riegel
  2011-07-28 17:13         ` Richard Henderson
  2011-07-28 16:52       ` Richard Henderson
  1 sibling, 1 reply; 19+ messages in thread
From: Torvald Riegel @ 2011-07-27 10:57 UTC (permalink / raw)
  To: GCC Patches; +Cc: Richard Henderson

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

On Tue, 2011-07-19 at 01:22 +0200, Torvald Riegel wrote:
> On Tue, 2011-07-19 at 01:19 +0200, Torvald Riegel wrote:
> > On Sat, 2011-07-09 at 16:30 +0200, Torvald Riegel wrote:
> > > The attached patch makes flat nesting the default, while still
> > > supporting closed nesting on demand (for user-controlled aborts via
> > > __transaction_cancel).
> > > Previously, a new transaction object was created for each nested
> > > transaction. The patch changes this to using one transaction object
> > > during the whole lifetime of a thread and keeping checkpoints of the
> > > transaction state when starting closed nested transactions. This allows
> > > us to easily use flat nesting and decreases the transaction start/commit
> > > overheads to some extent.
> > > 
> > > OK for branch?
> > 
> > I split the patch, as requested.
> > 
> > First three parts are smaller changes:
> > 
> > patch1: New erase method and placement new for aatree.
> > patch2: Change pr_hasElse to the value specified in the ABI.
> > patch3: Add information to dispatch about closed nesting and
> > uninstrumented code.
> > 
> > Then, in preparation for the following flat-transaction object design:
> > patch4: Use vector instead of list to store user actions.
> > patch5: Add closed nesting as restart reason.
> > 
> > And the actual core change:
> > patch6: Make flat nesting the default, use closed nesting on demand.
> > 
> > This last patch is still rather large, but it is one logical piece.
> > beginend.cc has most changes, but splitting this one up would rather
> > create incomplete intermediate steps than make the direction of this
> > whole patch clear.
> > 
> > As discussed offline, reducing the one extra copying of jmpbuf for
> > actual closed-nested transactions can be addressed in a future patch by
> > modifying _ITM_beginTransaction, which could additionally remove the
> > extra copying from stack to jmpbuf for all transactions as it existed
> > before this patch here.
> > 
> > Ok for branch, or is further splitting required?
> > 
> 
> This time with patches attached, sorry.

Attached are two small (in size) fixes for this patch set.

patch7: gtm_transaction::decide_begin_dispatch() gets the transaction
properties from the caller instead of reading from
gtm_transaction::prop, which might not have been updated by the caller
yet.

patch8: Fix nesting level reset when rolling back the outermost
transaction. Before, this was incorrectly reset to zero, which caused
transactions to not get committed. This didn't show up in previous
testing because previously, only serial-mode TM methods were available.

OK for branch, together with the previous patches?

[-- Attachment #2: patch7 --]
[-- Type: text/plain, Size: 2063 bytes --]

commit f3d24b9933d2124fe330e0513f14ce0711402e88
Author: Torvald Riegel <triegel@redhat.com>
Date:   Tue Jul 26 20:28:54 2011 +0200

    Fix gtm_transaction::decide_begin_dispatch().
    
    	* retry.cc (GTM::gtm_transaction::decide_begin_dispatch): Get transaction
    	properties from the caller instead of from the transaction object.
    	* libitm_i.h: Same.
    	* beginend.cc (GTM::gtm_transaction::begin_transaction): Same.

diff --git a/libitm/beginend.cc b/libitm/beginend.cc
index 6023390..417b41a 100644
--- a/libitm/beginend.cc
+++ b/libitm/beginend.cc
@@ -187,7 +187,7 @@ GTM::gtm_transaction::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb)
         tx->state = (STATE_SERIAL | STATE_IRREVOCABLE);
 
       else
-        disp = tx->decide_begin_dispatch ();
+        disp = tx->decide_begin_dispatch (prop);
 
       if (tx->state & STATE_SERIAL)
         {
diff --git a/libitm/libitm_i.h b/libitm/libitm_i.h
index 4cc0a70..093fc0e 100644
--- a/libitm/libitm_i.h
+++ b/libitm/libitm_i.h
@@ -218,7 +218,7 @@ struct gtm_transaction
 
   // In retry.cc
   void decide_retry_strategy (gtm_restart_reason);
-  abi_dispatch* decide_begin_dispatch ();
+  abi_dispatch* decide_begin_dispatch (uint32_t prop);
 
   // In method-serial.cc
   void serialirr_mode ();
diff --git a/libitm/retry.cc b/libitm/retry.cc
index bfca2ca..91778e6 100644
--- a/libitm/retry.cc
+++ b/libitm/retry.cc
@@ -77,12 +77,12 @@ GTM::gtm_transaction::decide_retry_strategy (gtm_restart_reason r)
 
 
 // Decides which TM method should be used on the first attempt to run this
-// transaction, setting this->disp accordingly.
+// transaction.
 // serial_lock will not have been acquired if this is the outer-most
 // transaction. If the state is set to STATE_SERIAL, the caller will set the
 // dispatch.
 GTM::abi_dispatch*
-GTM::gtm_transaction::decide_begin_dispatch ()
+GTM::gtm_transaction::decide_begin_dispatch (uint32_t prop)
 {
   // ??? Probably want some environment variable to choose the default
   // STM implementation once we have more than one implemented.

[-- Attachment #3: patch8 --]
[-- Type: text/plain, Size: 1377 bytes --]

commit f85b6ce7b67cecfd9c056620c7ef2aaa173fe5bc
Author: Torvald Riegel <triegel@redhat.com>
Date:   Wed Jul 27 12:27:10 2011 +0200

    Fix nesting level reset when rolling back the outermost transaction.
    
    	* beginend.cc (GTM::gtm_transaction::rollback): Fix nesting level
    	rollback.

diff --git a/libitm/beginend.cc b/libitm/beginend.cc
index 417b41a..ba90af2 100644
--- a/libitm/beginend.cc
+++ b/libitm/beginend.cc
@@ -291,6 +291,7 @@ GTM::gtm_transaction::rollback (gtm_transaction_cp *cp)
     }
   else
     {
+      // Roll back to the outermost transaction.
       // Restore the jump buffer and transaction properties, which we will
       // need for the longjmp used to restart or abort the transaction.
       if (parent_txns.size() > 0)
@@ -298,9 +299,11 @@ GTM::gtm_transaction::rollback (gtm_transaction_cp *cp)
           jb = parent_txns[0].jb;
           prop = parent_txns[0].prop;
         }
-      // Reset the transaction. Do not reset state, which is handled by the
-      // callers.
-      nesting = 0;
+      // Reset the transaction. Do not reset this->state, which is handled by
+      // the callers. Note that we reset the transaction to the point after
+      // having executed begin_transaction (we will return from it), so the
+      // nesting level must be one, not zero.
+      nesting = 1;
       parent_txns.clear();
     }
 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [trans-mem] Beginning of refactoring
  2011-07-18 23:44     ` Torvald Riegel
  2011-07-27 10:57       ` Torvald Riegel
@ 2011-07-28 16:52       ` Richard Henderson
  2011-07-28 19:42         ` Torvald Riegel
  1 sibling, 1 reply; 19+ messages in thread
From: Richard Henderson @ 2011-07-28 16:52 UTC (permalink / raw)
  To: Torvald Riegel; +Cc: GCC Patches

>     New erase method and placement new for aatree.
>     
>             * aatree.h (aa_tree::remove): New.
>             (aa_tree::operator new): Add placement new.

Ok.

>     Change pr_hasElse to the value specified in the ABI.
>     
>             * libitm.h (_ITM_codeProperties): Change pr_hasElse to the ABI's value.

Ok.

>     Add information to dispatch about closed nesting and uninstrumented code.
>     
>             * dispatch.h (GTM::abi_dispatch): Add can_run_uninstrumented_code and
>             closed_nesting flags, as well as a closed nesting alternative.
>             * method-serial.cc: Same.

Nearly...

> +  virtual abi_dispatch* closed_nesting_alternative()
> +  {
> +    // For nested transactions with an instrumented code path, we can do
> +    // undo logging.
> +    return GTM::dispatch_serial();

Surely you really mean dispatch_serial_ul here?
Otherwise ok.

>     Use vector instead of list to store user actions.
>     
>     	* useraction.cc: Use vector instead of list to store actions.
>     	Also support partial rollbacks for closed nesting.
>     	* libitm_i.h (GTM::gtm_transaction::user_action): Same.
>     	* beginend.cc: Same.

Ok.

>     Add closed nesting as restart reason.
>     
>     	* libitm_i.h: Add closed nesting as restart reason.
>     	* retry.cc (GTM::gtm_transaction::decide_retry_strategy): Same.

Ok, except

> +  if (r == RESTART_CLOSED_NESTING) retry_serial = true;

Coding style.  THEN statement on the next line, even for small THEN.

>     Make flat nesting the default, use closed nesting on demand.
>     
>             * local.cc (gtm_transaction::rollback_local): Support closed nesting.
>             * eh_cpp.cc (GTM::gtm_transaction::revert_cpp_exceptions): Same.
>     	* dispatch.h: Same.
>     	* method-serial.cc: Same.
>             * beginend.cc (GTM::gtm_transaction::begin_transaction): Change to
>             flat nesting as default, and closed nesting on demand.
>             (GTM::gtm_transaction::rollback): Same.
>             (_ITM_abortTransaction): Same.
>             (GTM::gtm_transaction::restart): Same.
>             (GTM::gtm_transaction::trycommit): Same.
>             (GTM::gtm_transaction::trycommit_and_finalize): Removed.
>             (choose_code_path): New.
>             (GTM::gtm_transaction_cp::save): New.
>             (GTM::gtm_transaction_cp::commit): New.
>             * query.cc (_ITM_inTransaction): Support flat nesting.
>             * libitm_i.h (GTM::gtm_transaction_cp): New helper struct for nesting.
>             (GTM::gtm_transaction): Support flat and closed nesting.
>             * alloc.cc (commit_allocations_2): New.
>             (commit_cb_data): New helper struct.
>             (GTM::gtm_transaction::commit_allocations): Handle nested
>             commits/rollbacks.
>             * libitm.texi: Update user action section, add description of nesting.

Nearly...

> +          abi_dispatch *cn_disp = disp->closed_nesting_alternative();
> +          if (cn_disp)
> +            {
> +              disp = cn_disp;
> +              set_abi_disp(disp);
> +            }

Don't we need to fini the old disp?  Seems there's a leak here, though
not visible until we re-instate the non-serial methods.

> +  if (!(tx->state & STATE_IRREVOCABLE)) ret |= a_saveLiveVariables;

Coding style.



r~

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [trans-mem] Beginning of refactoring
  2011-07-27 10:57       ` Torvald Riegel
@ 2011-07-28 17:13         ` Richard Henderson
  0 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2011-07-28 17:13 UTC (permalink / raw)
  To: Torvald Riegel; +Cc: GCC Patches

On 07/27/2011 03:35 AM, Torvald Riegel wrote:
> patch7: gtm_transaction::decide_begin_dispatch() gets the transaction
> properties from the caller instead of reading from
> gtm_transaction::prop, which might not have been updated by the caller
> yet.
> 
> patch8: Fix nesting level reset when rolling back the outermost
> transaction. Before, this was incorrectly reset to zero, which caused
> transactions to not get committed. This didn't show up in previous
> testing because previously, only serial-mode TM methods were available.
> 
> OK for branch, together with the previous patches?

Both ok.


r~

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [trans-mem] Beginning of refactoring
  2011-07-28 16:52       ` Richard Henderson
@ 2011-07-28 19:42         ` Torvald Riegel
  2011-07-28 20:26           ` Richard Henderson
  0 siblings, 1 reply; 19+ messages in thread
From: Torvald Riegel @ 2011-07-28 19:42 UTC (permalink / raw)
  To: Richard Henderson; +Cc: GCC Patches

On Thu, 2011-07-28 at 09:02 -0700, Richard Henderson wrote:
> >     Add information to dispatch about closed nesting and uninstrumented code.
> >     
> >             * dispatch.h (GTM::abi_dispatch): Add can_run_uninstrumented_code and
> >             closed_nesting flags, as well as a closed nesting alternative.
> >             * method-serial.cc: Same.
> 
> Nearly...
> 
> > +  virtual abi_dispatch* closed_nesting_alternative()
> > +  {
> > +    // For nested transactions with an instrumented code path, we can do
> > +    // undo logging.
> > +    return GTM::dispatch_serial();
> 
> Surely you really mean dispatch_serial_ul here?
> Otherwise ok.

No, this is correct because it calls the factory function in libitm_i.h.
However, the classes in method-serial.cc were named differently than
those factory functions, so I renamed them like this:

-class serial_dispatch : public abi_dispatch
+class serialirr_dispatch : public abi_dispatch

-class serial_dispatch_ul : public abi_dispatch
+class serial_dispatch : public abi_dispatch

This should avoid confusion in the future.


> >     Add closed nesting as restart reason.
> >     
> >     	* libitm_i.h: Add closed nesting as restart reason.
> >     	* retry.cc (GTM::gtm_transaction::decide_retry_strategy): Same.
> 
> Ok, except
> 
> > +  if (r == RESTART_CLOSED_NESTING) retry_serial = true;
> 
> Coding style.  THEN statement on the next line, even for small THEN.

Will try to keep that in mind ...

> >     Make flat nesting the default, use closed nesting on demand.
> >     
> >             * local.cc (gtm_transaction::rollback_local): Support closed nesting.
> >             * eh_cpp.cc (GTM::gtm_transaction::revert_cpp_exceptions): Same.
> >     	* dispatch.h: Same.
> >     	* method-serial.cc: Same.
> >             * beginend.cc (GTM::gtm_transaction::begin_transaction): Change to
> >             flat nesting as default, and closed nesting on demand.
> >             (GTM::gtm_transaction::rollback): Same.
> >             (_ITM_abortTransaction): Same.
> >             (GTM::gtm_transaction::restart): Same.
> >             (GTM::gtm_transaction::trycommit): Same.
> >             (GTM::gtm_transaction::trycommit_and_finalize): Removed.
> >             (choose_code_path): New.
> >             (GTM::gtm_transaction_cp::save): New.
> >             (GTM::gtm_transaction_cp::commit): New.
> >             * query.cc (_ITM_inTransaction): Support flat nesting.
> >             * libitm_i.h (GTM::gtm_transaction_cp): New helper struct for nesting.
> >             (GTM::gtm_transaction): Support flat and closed nesting.
> >             * alloc.cc (commit_allocations_2): New.
> >             (commit_cb_data): New helper struct.
> >             (GTM::gtm_transaction::commit_allocations): Handle nested
> >             commits/rollbacks.
> >             * libitm.texi: Update user action section, add description of nesting.
> 
> Nearly...
> 
> > +          abi_dispatch *cn_disp = disp->closed_nesting_alternative();
> > +          if (cn_disp)
> > +            {
> > +              disp = cn_disp;
> > +              set_abi_disp(disp);
> > +            }
> 
> Don't we need to fini the old disp?  Seems there's a leak here, though
> not visible until we re-instate the non-serial methods.

Yes, probably. However, one of the next steps on my refactoring list is
to document and change the TM method lifecycle callbacks. This will
include grouping several compatible methods (ie, those that can run
together) into method sets (e.g., global lock, multiple locks).
Switching a method within the current method set would then require no
fini(), whereas switching the method set would require a more
heavy-weight callback.

I have put the case you raised on my to-do list, and will revisit it
when working on these lifecycle management changes.

> 
> > +  if (!(tx->state & STATE_IRREVOCABLE)) ret |= a_saveLiveVariables;

Fixed.

Committing to branch, together with the two more recent changes.


Torvald

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [trans-mem] Beginning of refactoring
  2011-07-28 19:42         ` Torvald Riegel
@ 2011-07-28 20:26           ` Richard Henderson
  2011-08-03 10:50             ` Torvald Riegel
  0 siblings, 1 reply; 19+ messages in thread
From: Richard Henderson @ 2011-07-28 20:26 UTC (permalink / raw)
  To: Torvald Riegel; +Cc: GCC Patches

On 07/28/2011 12:36 PM, Torvald Riegel wrote:
> No, this is correct because it calls the factory function in libitm_i.h.
> However, the classes in method-serial.cc were named differently than
> those factory functions, so I renamed them like this:
> 
> -class serial_dispatch : public abi_dispatch
> +class serialirr_dispatch : public abi_dispatch
> 
> -class serial_dispatch_ul : public abi_dispatch
> +class serial_dispatch : public abi_dispatch
> 
> This should avoid confusion in the future.

Excellent.

>> Don't we need to fini the old disp?  Seems there's a leak here, though
>> not visible until we re-instate the non-serial methods.
> 
> Yes, probably. However, one of the next steps on my refactoring list is
> to document and change the TM method lifecycle callbacks. This will
> include grouping several compatible methods (ie, those that can run
> together) into method sets (e.g., global lock, multiple locks).
> Switching a method within the current method set would then require no
> fini(), whereas switching the method set would require a more
> heavy-weight callback.
> 
> I have put the case you raised on my to-do list, and will revisit it
> when working on these lifecycle management changes.

Sounds good.


r~

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [trans-mem] Beginning of refactoring
  2011-07-28 20:26           ` Richard Henderson
@ 2011-08-03 10:50             ` Torvald Riegel
  2011-08-03 15:47               ` Richard Henderson
  0 siblings, 1 reply; 19+ messages in thread
From: Torvald Riegel @ 2011-08-03 10:50 UTC (permalink / raw)
  To: Richard Henderson, Aldy Hernandez; +Cc: GCC Patches

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

This small patch fixes the rollback of the transaction ID as well as
queries for it.

OK for branch?

[-- Attachment #2: patch1 --]
[-- Type: text/plain, Size: 1473 bytes --]

commit bd83b4594c1f6eee09e0fa7b369557f986291f9b
Author: Torvald Riegel <triegel@redhat.com>
Date:   Fri Jul 29 15:19:04 2011 +0200

    Fix rollback and queries of transaction ID.
    
    	* beginend.cc (GTM::gtm_transaction::rollback): Roll back tx id as well.
    	* query.cc (_ITM_getTransactionId): There is no active transaction if
    	the current nesting level is zero.

diff --git a/libitm/beginend.cc b/libitm/beginend.cc
index 9f84404..d336d60 100644
--- a/libitm/beginend.cc
+++ b/libitm/beginend.cc
@@ -298,6 +298,7 @@ GTM::gtm_transaction::rollback (gtm_transaction_cp *cp)
       if (parent_txns.size() > 0)
         {
           jb = parent_txns[0].jb;
+          id = parent_txns[0].id;
           prop = parent_txns[0].prop;
         }
       // Reset the transaction. Do not reset this->state, which is handled by
diff --git a/libitm/query.cc b/libitm/query.cc
index 4905fc6..9a95211 100644
--- a/libitm/query.cc
+++ b/libitm/query.cc
@@ -1,4 +1,4 @@
-/* Copyright (C) 2008, 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2008, 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU Transactional Memory Library (libitm).
@@ -59,7 +59,7 @@ _ITM_transactionId_t ITM_REGPARM
 _ITM_getTransactionId (void)
 {
   struct gtm_transaction *tx = gtm_tx();
-  return tx ? tx->id : _ITM_noTransactionId;
+  return (tx && (tx->nesting > 0)) ? tx->id : _ITM_noTransactionId;
 }
 
 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [trans-mem] Beginning of refactoring
  2011-08-03 10:50             ` Torvald Riegel
@ 2011-08-03 15:47               ` Richard Henderson
  0 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2011-08-03 15:47 UTC (permalink / raw)
  To: Torvald Riegel; +Cc: Aldy Hernandez, GCC Patches

On 08/03/2011 03:50 AM, Torvald Riegel wrote:
>     Fix rollback and queries of transaction ID.
>     
>     	* beginend.cc (GTM::gtm_transaction::rollback): Roll back tx id as well.
>     	* query.cc (_ITM_getTransactionId): There is no active transaction if
>     	the current nesting level is zero.

Ok.


r~

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2011-08-03 15:47 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-25 22:45 [trans-mem] Beginning of refactoring Torvald Riegel
2011-06-29 21:22 ` Torvald Riegel
2011-06-29 21:50   ` Richard Henderson
2011-06-29 23:18 ` Richard Henderson
2011-06-30 12:19   ` Torvald Riegel
2011-06-30 15:03     ` Richard Henderson
2011-06-30 17:02       ` Torvald Riegel
2011-07-01 20:23 ` Torvald Riegel
2011-07-01 20:32   ` Richard Henderson
2011-07-09 15:35 ` Torvald Riegel
2011-07-18 23:38   ` Torvald Riegel
2011-07-18 23:44     ` Torvald Riegel
2011-07-27 10:57       ` Torvald Riegel
2011-07-28 17:13         ` Richard Henderson
2011-07-28 16:52       ` Richard Henderson
2011-07-28 19:42         ` Torvald Riegel
2011-07-28 20:26           ` Richard Henderson
2011-08-03 10:50             ` Torvald Riegel
2011-08-03 15:47               ` Richard Henderson

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).