libitm: Fix HTM fastpath.

	* beginend.cc (GTM::gtm_thread::serial_lock): Put on cacheline
	boundary.
	(htm_fastpath): Remove.
	(gtm_thread::begin_transaction): Fix HTM fastpath.
	(_ITM_commitTransaction): Adapt.
	(_ITM_commitTransactionEH): Adapt.
	* libitm/config/linux/rwlock.h (gtm_rwlock): Add htm_fastpath member
	and accessors.
	* libitm/config/posix/rwlock.h (gtm_rwlock): Likewise.
	* libitm/config/posix/rwlock.cc (gtm_rwlock::gtm_rwlock): Adapt.
	* libitm/config/x86/sjlj.S (_ITM_beginTransaction): Fix HTM fastpath.
	* libitm/libitm_i.h (htm_fastpath): Remove declaration.
	* libitm/method-serial.cc (htm_mg): Adapt.
	(gtm_thread::serialirr_mode): Adapt.
	* libitm/query.cc (_ITM_inTransaction, _ITM_getTransactionId): Adapt.

From-SVN: r232735

4 files changed, 74 insertions, 20 deletions

diff --git a/libitm/config/linux/rwlock.h b/libitm/config/linux/rwlock.h
index d9f364cefe0..4dd1dab80cb 100644
--- a/libitm/config/linux/rwlock.h
+++ b/libitm/config/linux/rwlock.h
@@ -41,19 +41,27 @@ struct gtm_thread;
 // read-to-write upgrades do not have a higher priority than writers.
 //
 // Do not change the layout of this class; it must remain a POD type with
-// standard layout, and the WRITERS field must be first (i.e., so the
+// standard layout, and the writers field must be first (i.e., so the
 // assembler code can assume that its address is equal to the address of the
-// respective instance of the class).
+// respective instance of the class), and htm_fastpath must be second.
 
 class gtm_rwlock
 {
-  // TODO Put futexes on different cachelines?
   std::atomic<int> writers;       // Writers' futex.
+  // We put the HTM fastpath control variable here so that HTM fastpath
+  // transactions can check efficiently whether they are allowed to run.
+  // This must be accessed atomically because threads can load this value
+  // when they are neither a registered reader nor writer (i.e., when they
+  // attempt to execute the HTM fastpath).
+  std::atomic<uint32_t> htm_fastpath;
+  // TODO Put these futexes on different cachelines?  (writers and htm_fastpath
+  // should remain on the same cacheline.
   std::atomic<int> writer_readers;// A confirmed writer waits here for readers.
   std::atomic<int> readers;       // Readers wait here for writers (iff true).
 
  public:
-  gtm_rwlock() : writers(0), writer_readers(0), readers(0) {};
+  gtm_rwlock() : writers(0), htm_fastpath(0), writer_readers(0), readers(0)
+  { }
 
   void read_lock (gtm_thread *tx);
   void read_unlock (gtm_thread *tx);
@@ -64,12 +72,28 @@ class gtm_rwlock
   bool write_upgrade (gtm_thread *tx);
   void write_upgrade_finish (gtm_thread *tx);
 
-  // Returns true iff there is a concurrent active or waiting writer.
-  // This is primarily useful for simple HyTM approaches, and the value being
-  // checked is loaded with memory_order_relaxed.
-  bool is_write_locked()
+  // Returns true iff there is a concurrent active or waiting writer, or
+  // htm_fastpath is zero. This is primarily useful for simple HyTM
+  // approaches, and the values being checked are loaded with
+  // memory_order_relaxed.
+  bool htm_fastpath_disabled ()
   {
-    return writers.load (memory_order_relaxed) != 0;
+    return writers.load (memory_order_relaxed) != 0
+	|| htm_fastpath.load (memory_order_relaxed) == 0;
+  }
+
+  // This does not need to return an exact value, hence relaxed MO is
+  // sufficient.
+  uint32_t get_htm_fastpath ()
+  {
+    return htm_fastpath.load (memory_order_relaxed);
+  }
+  // This must only be called while having acquired the write lock, and other
+  // threads do not need to load an exact value; hence relaxed MO is
+  // sufficient.
+  void set_htm_fastpath (uint32_t val)
+  {
+    htm_fastpath.store (val, memory_order_relaxed);
   }
 
  protected:
diff --git a/libitm/config/posix/rwlock.cc b/libitm/config/posix/rwlock.cc
index 1e1eea820f2..86f71058525 100644
--- a/libitm/config/posix/rwlock.cc
+++ b/libitm/config/posix/rwlock.cc
@@ -31,6 +31,7 @@ namespace GTM HIDDEN {
 
 gtm_rwlock::gtm_rwlock()
   : summary (0),
+    htm_fastpath (0),
     mutex (PTHREAD_MUTEX_INITIALIZER),
     c_readers (PTHREAD_COND_INITIALIZER),
     c_writers (PTHREAD_COND_INITIALIZER),
diff --git a/libitm/config/posix/rwlock.h b/libitm/config/posix/rwlock.h
index 81c29a6f426..1e74e64b87d 100644
--- a/libitm/config/posix/rwlock.h
+++ b/libitm/config/posix/rwlock.h
@@ -46,9 +46,9 @@ struct gtm_thread;
 // read-to-write upgrades do not have a higher priority than writers.
 //
 // Do not change the layout of this class; it must remain a POD type with
-// standard layout, and the SUMMARY field must be first (i.e., so the
+// standard layout, and the summary field must be first (i.e., so the
 // assembler code can assume that its address is equal to the address of the
-// respective instance of the class).
+// respective instance of the class), and htm_fastpath must be second.
 
 class gtm_rwlock
 {
@@ -58,6 +58,13 @@ class gtm_rwlock
 
   std::atomic<unsigned int> summary;	// Bitmask of the above.
 
+  // We put the HTM fastpath control variable here so that HTM fastpath
+  // transactions can check efficiently whether they are allowed to run.
+  // This must be accessed atomically because threads can load this value
+  // when they are neither a registered reader nor writer (i.e., when they
+  // attempt to execute the HTM fastpath).
+  std::atomic<uint32_t> htm_fastpath;
+
   pthread_mutex_t mutex;	        // Held if manipulating any field.
   pthread_cond_t c_readers;	        // Readers wait here
   pthread_cond_t c_writers;	        // Writers wait here for writers
@@ -80,12 +87,28 @@ class gtm_rwlock
   bool write_upgrade (gtm_thread *tx);
   void write_upgrade_finish (gtm_thread *tx);
 
-  // Returns true iff there is a concurrent active or waiting writer.
-  // This is primarily useful for simple HyTM approaches, and the value being
-  // checked is loaded with memory_order_relaxed.
-  bool is_write_locked()
+  // Returns true iff there is a concurrent active or waiting writer, or
+  // htm_fastpath is zero. This is primarily useful for simple HyTM
+  // approaches, and the values being checked are loaded with
+  // memory_order_relaxed.
+  bool htm_fastpath_disabled ()
+  {
+    return (summary.load (memory_order_relaxed) & (a_writer | w_writer))
+	|| htm_fastpath.load (memory_order_relaxed) == 0;
+  }
+
+  // This does not need to return an exact value, hence relaxed MO is
+  // sufficient.
+  uint32_t get_htm_fastpath ()
+  {
+    return htm_fastpath.load (memory_order_relaxed);
+  }
+  // This must only be called while having acquired the write lock, and other
+  // threads do not need to load an exact value; hence relaxed MO is
+  // sufficient.
+  void set_htm_fastpath (uint32_t val)
   {
-    return summary.load (memory_order_relaxed) & (a_writer | w_writer);
+    htm_fastpath.store (val, memory_order_relaxed);
   }
 
  protected:
diff --git a/libitm/config/x86/sjlj.S b/libitm/config/x86/sjlj.S
index 4b79db778f7..3d2a922c82c 100644
--- a/libitm/config/x86/sjlj.S
+++ b/libitm/config/x86/sjlj.S
@@ -81,8 +81,9 @@ SYM(_ITM_beginTransaction):
 	   back to another execution method.  RTM restores all registers after
 	   a HW transaction abort, so we can do the SW setjmp after aborts,
 	   and we have to because we might choose a SW fall back.  However,
-	   we have to explicitly save/restore the first argument (edi).  */
-	cmpl	$0, SYM(gtm_htm_fastpath)(%rip)
+	   we have to explicitly save/restore the first argument (edi).
+	   The htm_fastpath field is the second int in gtm_rwlock.  */
+	cmpl	$0, (SYM(gtm_serial_lock)+4)(%rip)
 	jz	.Lno_htm
 	testl	$pr_hasNoAbort, %edi
 	jz	.Lno_htm
@@ -95,6 +96,10 @@ SYM(_ITM_beginTransaction):
 	   this case in the retry policy implementation.  */
 	cmpl	$0, SYM(gtm_serial_lock)(%rip)
 	jnz	1f
+	/* Now also check that HW transactions are still allowed to run (see
+	   gtm_thread::begin_transaction for why this is necessary).  */
+	cmpl	$0, (SYM(gtm_serial_lock)+4)(%rip)
+	jz	1f
 	/* Everything is good.  Run the transaction, preferably using the
 	   uninstrumented code path.  Note that the following works because
 	   pr_uninstrumentedCode == a_runUninstrumentedCode.  */
@@ -102,8 +107,9 @@ SYM(_ITM_beginTransaction):
 	mov	$a_runInstrumentedCode, %eax
 	cmovnz	%edi, %eax
 	ret
-	/* There is a serial-mode transaction, so abort (see htm_abort()
-	   regarding the abort code).  */
+	/* There is a serial-mode transaction or HW transactions are not
+	   allowed anymore, so abort (see htm_abort() regarding the abort
+	   code).  */
 1:	xabort	$0xff
 .Ltxn_abort:
 	/* If it might make sense to retry the HTM fast path, let the C++