Patchwork [trans-mem] Streamline the in-use stack check

login
register
mail settings
Submitter Richard Henderson
Date June 23, 2010, 8:42 p.m.
Message ID <4C2271AC.50300@redhat.com>
Download mbox | patch
Permalink /patch/56716/
State New
Headers show

Comments

Richard Henderson - June 23, 2010, 8:42 p.m.
I over-thought the patch I posted last week.  The portion of the 
run-time stack that needs to be protected from update is not as
dynamic as it seems from that patch.

The begin/commit pair *must* be at the same stack level; something
that is enforced by the compiler in the way it generates the calls.
Any rollbacks that need protecting will be at a lower or equal
stack level and will be immediately followed by a longjmp back to
the begin stack level, so again we can use the original begin 
stack level as our cutoff point.

This partially reverts the commit from the 16th.  We no longer
record the incomming stack level at each entry point, but instead
use the value that was recorded during the setjmp.


r~


        * barrier.tpl, beginend.cc, clone.cc, tls.h, memcpy.cc,
        memset.cc, method-serial.cc: Revert the 2010-06-16 change.
        * config/x86/target.h (struct gtm_jmpbuf): Change CFA type to void*.
        * config/alpha/target.h: Likewise.
        * config/generic/tls.cc (gtm_mask_stack): Use it.

Patch

diff --git a/libitm/barrier.tpl b/libitm/barrier.tpl
index b174791..7d4e882 100644
--- a/libitm/barrier.tpl
+++ b/libitm/barrier.tpl
@@ -123,14 +123,12 @@  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)	\
   {									\
-    gtm_stack_marker marker;						\
     return do_read (ptr, gtm_dispatch::LOCK);				\
   }
 
 #define ITM_WRITE(T, LOCK)						\
   void ITM_REGPARM _ITM_##LOCK##T (_ITM_TYPE_##T *ptr, _ITM_TYPE_##T val) \
   {									\
-    gtm_stack_marker marker;						\
     do_write (ptr, val, gtm_dispatch::LOCK);				\
   }
 
diff --git a/libitm/beginend.cc b/libitm/beginend.cc
index 4bebb1e..e1589ac 100644
--- a/libitm/beginend.cc
+++ b/libitm/beginend.cc
@@ -161,8 +161,6 @@  _ITM_rollbackTransaction (void)
   assert ((tx->prop & pr_hasNoAbort) == 0);
   assert ((tx->state & gtm_transaction::STATE_ABORTING) == 0);
 
-  gtm_stack_marker marker;
-
   tx->rollback ();
   tx->state |= gtm_transaction::STATE_ABORTING;
 }
@@ -179,8 +177,6 @@  _ITM_abortTransaction (_ITM_abortReason reason)
   if (tx->state & gtm_transaction::STATE_IRREVOCABLE)
     abort ();
 
-  gtm_stack_marker marker;
-
   tx->rollback ();
   gtm_disp()->fini ();
 
@@ -254,8 +250,6 @@  void ITM_REGPARM
 _ITM_commitTransaction(void)
 {
   gtm_transaction *tx = gtm_tx();
-  gtm_stack_marker marker;
-
   if (!tx->trycommit_and_finalize ())
     tx->restart (RESTART_VALIDATE_COMMIT);
 }
@@ -264,8 +258,6 @@  void ITM_REGPARM
 _ITM_commitTransactionEH(void *exc_ptr)
 {
   gtm_transaction *tx = gtm_tx();
-  gtm_stack_marker marker;
-
   if (!tx->trycommit_and_finalize ())
     {
       tx->eh_in_flight = exc_ptr;
diff --git a/libitm/clone.cc b/libitm/clone.cc
index 46d6328..7d77814 100644
--- a/libitm/clone.cc
+++ b/libitm/clone.cc
@@ -92,7 +92,6 @@  _ITM_getTMCloneOrIrrevocable (void *ptr)
   if (ret)
     return ret;
 
-  gtm_stack_marker marker;
   gtm_tx()->serialirr_mode ();
 
   return ptr;
diff --git a/libitm/config/alpha/target.h b/libitm/config/alpha/target.h
index 2c0f448..94ac59b 100644
--- a/libitm/config/alpha/target.h
+++ b/libitm/config/alpha/target.h
@@ -28,7 +28,7 @@  typedef struct gtm_jmpbuf
 {
   unsigned long pc;
   unsigned long s[7];
-  unsigned long cfa;
+  void *cfa;
   unsigned long f[8];
 } gtm_jmpbuf;
 
diff --git a/libitm/config/generic/tls.cc b/libitm/config/generic/tls.cc
index 1e541b6..204e84c 100644
--- a/libitm/config/generic/tls.cc
+++ b/libitm/config/generic/tls.cc
@@ -33,7 +33,7 @@  namespace GTM HIDDEN {
 gtm_cacheline_mask __attribute__((noinline))
 gtm_mask_stack(gtm_cacheline *line, gtm_cacheline_mask mask)
 {
-  void *top = gtm_stack_top();
+  void *top = gtm_tx()->jb.cfa;
   void *bot = __builtin_dwarf_cfa();
 
   // We must have come through an entry point that set TOP.
diff --git a/libitm/config/generic/tls.h b/libitm/config/generic/tls.h
index 3bfad74..8ea0b90 100644
--- a/libitm/config/generic/tls.h
+++ b/libitm/config/generic/tls.h
@@ -40,10 +40,6 @@  struct gtm_thread
   // if the target provides some efficient mechanism for storing this.
   gtm_dispatch *disp;
 #endif
-#ifndef HAVE_ARCH_GTM_THREAD_STACK_TOP
-  // The top of stack on entry to the STM library.
-  void *stack_top;
-#endif
 
   // The maximum number of free gtm_transaction structs to be kept.
   // This number must be greater than 1 in order for transaction abort
@@ -89,17 +85,6 @@  static inline gtm_dispatch * gtm_disp() { return gtm_thr()->disp; }
 static inline void set_gtm_disp(gtm_dispatch *x) { gtm_thr()->disp = x; }
 #endif
 
-#ifndef HAVE_ARCH_GTM_THREAD_STACK_TOP
-static inline void *gtm_stack_top() { return gtm_thr()->stack_top; }
-static inline void set_gtm_stack_top(void *t) { gtm_thr()->stack_top = t; }
-#endif
-
-struct gtm_stack_marker
-{
-  gtm_stack_marker(void *t = __builtin_dwarf_cfa()) { set_gtm_stack_top (t); }
-  ~gtm_stack_marker() { set_gtm_stack_top (0); }
-};
-
 } // namespace GTM
 
 #endif // LIBITM_TLS_H
diff --git a/libitm/config/x86/target.h b/libitm/config/x86/target.h
index 375891b..758e773 100644
--- a/libitm/config/x86/target.h
+++ b/libitm/config/x86/target.h
@@ -28,7 +28,7 @@  namespace GTM HIDDEN {
 /* ??? This doesn't work for Win64.  */
 typedef struct gtm_jmpbuf
 {
-  unsigned long cfa;
+  void *cfa;
   unsigned long rip;
   unsigned long rbx;
   unsigned long rbp;
@@ -40,7 +40,7 @@  typedef struct gtm_jmpbuf
 #else
 typedef struct gtm_jmpbuf
 {
-  unsigned long cfa;
+  void *cfa;
   unsigned long ebx;
   unsigned long esi;
   unsigned long edi;
diff --git a/libitm/memcpy.cc b/libitm/memcpy.cc
index c435294..672af98 100644
--- a/libitm/memcpy.cc
+++ b/libitm/memcpy.cc
@@ -302,13 +302,11 @@  do_memmove (uintptr_t idst, uintptr_t isrc, size_t size,
 #define ITM_MEM_DEF(NAME, READ, WRITE) \
 void ITM_REGPARM _ITM_memcpy##NAME(void *dst, const void *src, size_t size)  \
 {									     \
-  gtm_stack_marker marker;						     \
   do_memcpy ((uintptr_t)dst, (uintptr_t)src, size,			     \
 	     gtm_dispatch::WRITE, gtm_dispatch::READ);			     \
 }									     \
 void ITM_REGPARM _ITM_memmove##NAME(void *dst, const void *src, size_t size) \
 {									     \
-  gtm_stack_marker marker;						     \
   do_memmove ((uintptr_t)dst, (uintptr_t)src, size,			     \
 	      gtm_dispatch::WRITE, gtm_dispatch::READ);			     \
 }
diff --git a/libitm/memset.cc b/libitm/memset.cc
index 05584ec..39ee72b 100644
--- a/libitm/memset.cc
+++ b/libitm/memset.cc
@@ -70,7 +70,6 @@  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)	\
 {									\
-  gtm_stack_marker marker;						\
   do_memset ((uintptr_t)dst, c, size, gtm_dispatch::WRITE);		\
 }
 
diff --git a/libitm/method-serial.cc b/libitm/method-serial.cc
index c7c79f0..2a69d48 100644
--- a/libitm/method-serial.cc
+++ b/libitm/method-serial.cc
@@ -117,7 +117,6 @@  GTM::gtm_transaction::serialirr_mode ()
 void ITM_REGPARM
 _ITM_changeTransactionMode (_ITM_transactionState state)
 {
-  gtm_stack_marker marker;
   assert (state == modeSerialIrrevocable);
   gtm_tx()->serialirr_mode ();
 }