* [PATCH] Add faster HTM fastpath for libitm TSX @ 2013-01-12 21:03 Andi Kleen 2013-01-21 4:41 ` [PING] " Andi Kleen 2013-01-24 9:42 ` Torvald Riegel 0 siblings, 2 replies; 6+ messages in thread From: Andi Kleen @ 2013-01-12 21:03 UTC (permalink / raw) To: gcc-patches; +Cc: triegel, Andi Kleen From: Andi Kleen <ak@linux.intel.com> The libitm TSX hardware transaction fast path currently does quite a bit of unnecessary work (saving registers etc.) before even trying to start a hardware transaction. This patch moves the initial attempt at a transaction early into the assembler stub. Complicated work like retries is still done in C. So this is essentially a fast path for the fast path. The assembler code doesn't understand the layout of "serial_lock", but it needs to check that serial_lock is free. We export just the lock variable as a separate pointer for this. The assembler code controls the HTM fast path with a new pr_tryHTM flag. I had to reorganize the retry loop slightly so that the waiting for the lock to be free again is done first (because the first transaction is already done elsewhere). This makes the patch look larger than it really is. This just moves one basic block around. Passes bootstrap/testsuite on x86_64 libitm/: 2013-01-12 Andi Kleen <ak@linux.intel.com> * beginend.cc (GTM::htm_fastpath): Remove. (__gtm_htm_fastpath, __gtm_global_lock): Add. (begin_transaction): Check pr_tryHTM. Remove one iteration from HTM retry loop. Move lock free check to the beginning of loop body. (_ITM_commitTransaction): Check __gtm_htm_fastpath instead of htm_fastpath. (_ITM_commitTransactionEH): dito. * config/linux/rwlock.h (gtm_rwlock::get_lock_var): Add. * config/posix/rwlock.h (tm_rwlock::get_lock_var): Add. * config/x86/sjlj.S: Include config.h. (pr_tryHTM, pr_uninstrumentedCode, pr_hasNoAbort, a_runInstrumentedCode, a_runUninstrumentedCode, _XABORT_EXPLICIT, _XABORT_RETRY): Add defines. (_ITM_beginTransaction): Add HTM fast path. * libitm.h (pr_tryHTM): Add. * libitm_i.h (GTM::htm_fastpath): Remove. (__gtm_htm_fastpath, __gtm_global_lock): Add. * method-serial.cc (method_group::init, fini): Initialize __gtm_global_lock and __gtm_htm_fastpath. (gtm_thread::serialirr_mode): Check __gtm_htm_fastpath. --- libitm/beginend.cc | 50 +++++++++++++++++++++++------------------- libitm/config/linux/rwlock.h | 5 +++++ libitm/config/posix/rwlock.h | 5 +++++ libitm/config/x86/sjlj.S | 47 +++++++++++++++++++++++++++++++++++++++ libitm/libitm.h | 3 ++- libitm/libitm_i.h | 7 +++--- libitm/method-serial.cc | 8 ++++--- 7 files changed, 96 insertions(+), 29 deletions(-) diff --git a/libitm/beginend.cc b/libitm/beginend.cc index 4369946..96ee1b3 100644 --- a/libitm/beginend.cc +++ b/libitm/beginend.cc @@ -55,7 +55,11 @@ static pthread_key_t thr_release_key; static pthread_once_t thr_release_once = PTHREAD_ONCE_INIT; // See gtm_thread::begin_transaction. -uint32_t GTM::htm_fastpath = 0; +extern "C" +{ + uint32_t __gtm_htm_fastpath = 0; + uint32_t *__gtm_global_lock; +}; /* Allocate a transaction structure. */ void * @@ -187,27 +191,11 @@ GTM::gtm_thread::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb) // indeed in serial mode, and HW transactions should never need serial mode // for any internal changes (e.g., they never abort visibly to the STM code // and thus do not trigger the standard retry handling). - if (likely(htm_fastpath && (prop & pr_hasNoAbort))) + if (likely(prop & pr_tryHTM)) { - for (uint32_t t = htm_fastpath; t; t--) + // One transaction has been already tried by the assembler fast path. + for (uint32_t t = __gtm_htm_fastpath - 1; t; t--) { - uint32_t ret = htm_begin(); - if (htm_begin_success(ret)) - { - // We are executing a transaction now. - // Monitor the writer flag in the serial-mode lock, and abort - // if there is an active or waiting serial-mode transaction. - if (unlikely(serial_lock.is_write_locked())) - htm_abort(); - else - // We do not need to set a_saveLiveVariables because of HTM. - return (prop & pr_uninstrumentedCode) ? - a_runUninstrumentedCode : a_runInstrumentedCode; - } - // The transaction has aborted. Don't retry if it's unlikely that - // retrying the transaction will be successful. - if (!htm_abort_should_retry(ret)) - break; // Wait until any concurrent serial-mode transactions have finished. // This is an empty critical section, but won't be elided. if (serial_lock.is_write_locked()) @@ -225,6 +213,24 @@ GTM::gtm_thread::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb) // we have retried so often that we should go serial to avoid // starvation. } + + uint32_t ret = htm_begin(); + if (htm_begin_success(ret)) + { + // We are executing a transaction now. + // Monitor the writer flag in the serial-mode lock, and abort + // if there is an active or waiting serial-mode transaction. + if (unlikely(serial_lock.is_write_locked())) + htm_abort(); + else + // We do not need to set a_saveLiveVariables because of HTM. + return (prop & pr_uninstrumentedCode) ? + a_runUninstrumentedCode : a_runInstrumentedCode; + } + // The transaction has aborted. Don't retry if it's unlikely that + // retrying the transaction will be successful. + if (!htm_abort_should_retry(ret)) + break; } } #endif @@ -608,7 +614,7 @@ _ITM_commitTransaction(void) // a serial-mode transaction. If we are, then there will be no other // concurrent serial-mode transaction. // See gtm_thread::begin_transaction. - if (likely(htm_fastpath && !gtm_thread::serial_lock.is_write_locked())) + if (likely(__gtm_htm_fastpath && !gtm_thread::serial_lock.is_write_locked())) { htm_commit(); return; @@ -624,7 +630,7 @@ _ITM_commitTransactionEH(void *exc_ptr) { #if defined(USE_HTM_FASTPATH) // See _ITM_commitTransaction. - if (likely(htm_fastpath && !gtm_thread::serial_lock.is_write_locked())) + if (likely(__gtm_htm_fastpath && !gtm_thread::serial_lock.is_write_locked())) { htm_commit(); return; diff --git a/libitm/config/linux/rwlock.h b/libitm/config/linux/rwlock.h index f13d287..b27b785 100644 --- a/libitm/config/linux/rwlock.h +++ b/libitm/config/linux/rwlock.h @@ -67,6 +67,11 @@ class gtm_rwlock return writers.load (memory_order_relaxed) != 0; } + uint32_t *get_lock_var() + { + return (uint32_t *)&writers; + } + protected: bool write_lock_generic (gtm_thread *tx); }; diff --git a/libitm/config/posix/rwlock.h b/libitm/config/posix/rwlock.h index 79f1429..4481378 100644 --- a/libitm/config/posix/rwlock.h +++ b/libitm/config/posix/rwlock.h @@ -82,6 +82,11 @@ class gtm_rwlock return summary.load (memory_order_relaxed) & (a_writer | w_writer); } + uint32_t *get_lock_var() + { + return (uint32_t *)&summmary; + } + protected: bool write_lock_generic (gtm_thread *tx); }; diff --git a/libitm/config/x86/sjlj.S b/libitm/config/x86/sjlj.S index 4d733f4..be24e7e 100644 --- a/libitm/config/x86/sjlj.S +++ b/libitm/config/x86/sjlj.S @@ -24,6 +24,7 @@ #include "asmcfi.h" +#include "config.h" #define CONCAT1(a, b) CONCAT2(a, b) #define CONCAT2(a, b) a ## b @@ -52,6 +53,19 @@ # endif #endif +/* Should share those somewhere, but they are an ABI anyways, so shouldn't + change. */ + +#define pr_tryHTM 0x800000 +#define pr_uninstrumentedCode 0x0002 +#define pr_hasNoAbort 0x0008 + +#define a_runInstrumentedCode 0x01 +#define a_runUninstrumentedCode 0x02 + +#define _XABORT_EXPLICIT (1 << 0) +#define _XABORT_RETRY (1 << 1) + .text .align 4 @@ -60,6 +74,39 @@ SYM(_ITM_beginTransaction): cfi_startproc #ifdef __x86_64__ +#ifdef HAVE_AS_RTM + /* Hardware Transactional fast path. + Try to run the transaction once here. If it doesn't work + call the C code which retries a few more times and does other + things. This is merely a fast path to optimize small transactions, + anything complicated is done in C++. */ + cmpl $0,__gtm_htm_fastpath(%rip) + jz no_txn + testl $pr_hasNoAbort,%edi + jnz no_txn + xbegin txn_abort + /* Would be better to have a tool to generate offsetof(). */ + movq __gtm_global_lock(%rip),%rax + /* global lock is free? */ + cmpl $0,(%rax) + jnz 1f + /* Everything good. start running transaction */ + testl $pr_uninstrumentedCode,%edi + mov $a_runInstrumentedCode,%eax + mov $a_runUninstrumentedCode,%ecx + cmovnz %ecx,%eax + ret + /* Lock is busy: abort */ +1: xabort $0xff +txn_abort: + /* if an internal abort, but not the xabort above, + tell the C code to not retry */ + testl $(_XABORT_RETRY|_XABORT_EXPLICIT),%eax + jz no_txn + orl $pr_tryHTM,%edi + /* go on to the C code now for further retries */ +no_txn: +#endif leaq 8(%rsp), %rax subq $56, %rsp cfi_def_cfa_offset(64) diff --git a/libitm/libitm.h b/libitm/libitm.h index 436eb94..72e4585 100644 --- a/libitm/libitm.h +++ b/libitm/libitm.h @@ -95,7 +95,8 @@ typedef enum pr_exceptionBlock = 0x1000, pr_hasElse = 0x2000, pr_readOnly = 0x4000, - pr_hasNoSimpleReads = 0x400000 + pr_hasNoSimpleReads = 0x400000, + pr_tryHTM = 0x800000 } _ITM_codeProperties; /* Result from startTransaction that describes what actions to take. */ diff --git a/libitm/libitm_i.h b/libitm/libitm_i.h index 4dfcda9..9bad7df 100644 --- a/libitm/libitm_i.h +++ b/libitm/libitm_i.h @@ -336,10 +336,11 @@ extern abi_dispatch *dispatch_htm(); extern gtm_cacheline_mask gtm_mask_stack(gtm_cacheline *, gtm_cacheline_mask); +} // namespace GTM + // Control variable for the HTM fastpath that uses serial mode as fallback. // Non-zero if the HTM fastpath is enabled. See gtm_thread::begin_transaction. -extern uint32_t htm_fastpath; - -} // namespace GTM +extern "C" uint32_t __gtm_htm_fastpath; +extern "C" uint32_t *__gtm_global_lock; #endif // LIBITM_I_H diff --git a/libitm/method-serial.cc b/libitm/method-serial.cc index 38857dc..ed7517b 100644 --- a/libitm/method-serial.cc +++ b/libitm/method-serial.cc @@ -222,13 +222,15 @@ struct htm_mg : public method_group // Enable the HTM fastpath if the HW is available. The fastpath is // initially disabled. #ifdef USE_HTM_FASTPATH - htm_fastpath = htm_init(); + __gtm_global_lock = gtm_thread::serial_lock.get_lock_var(); + __gtm_htm_fastpath = htm_init(); #endif } virtual void fini() { // Disable the HTM fastpath. - htm_fastpath = 0; + __gtm_htm_fastpath = 0; + __gtm_global_lock = NULL; } }; @@ -288,7 +290,7 @@ GTM::gtm_thread::serialirr_mode () #if defined(USE_HTM_FASTPATH) // HTM fastpath. If we are executing a HW transaction, don't go serial but // continue. See gtm_thread::begin_transaction. - if (likely(htm_fastpath && !gtm_thread::serial_lock.is_write_locked())) + if (likely(__gtm_htm_fastpath && !gtm_thread::serial_lock.is_write_locked())) return; #endif -- 1.7.10.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PING] Re: [PATCH] Add faster HTM fastpath for libitm TSX 2013-01-12 21:03 [PATCH] Add faster HTM fastpath for libitm TSX Andi Kleen @ 2013-01-21 4:41 ` Andi Kleen 2013-01-24 9:42 ` Torvald Riegel 1 sibling, 0 replies; 6+ messages in thread From: Andi Kleen @ 2013-01-21 4:41 UTC (permalink / raw) To: gcc-patches; +Cc: triegel Andi Kleen <andi@firstfloor.org> writes: PING! This patch needs review. -Andi > From: Andi Kleen <ak@linux.intel.com> > > The libitm TSX hardware transaction fast path currently does quite a bit of > unnecessary work (saving registers etc.) before even trying to start > a hardware transaction. This patch moves the initial attempt at a > transaction early into the assembler stub. Complicated work like retries > is still done in C. So this is essentially a fast path for the fast > path. -- ak@linux.intel.com -- Speaking for myself only ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add faster HTM fastpath for libitm TSX 2013-01-12 21:03 [PATCH] Add faster HTM fastpath for libitm TSX Andi Kleen 2013-01-21 4:41 ` [PING] " Andi Kleen @ 2013-01-24 9:42 ` Torvald Riegel 2013-01-24 16:45 ` Andi Kleen 1 sibling, 1 reply; 6+ messages in thread From: Torvald Riegel @ 2013-01-24 9:42 UTC (permalink / raw) To: Andi Kleen; +Cc: gcc-patches, Andi Kleen, Richard Henderson On Sat, 2013-01-12 at 13:03 -0800, Andi Kleen wrote: > From: Andi Kleen <ak@linux.intel.com> > > The libitm TSX hardware transaction fast path currently does quite a bit of > unnecessary work (saving registers etc.) before even trying to start > a hardware transaction. This patch moves the initial attempt at a > transaction early into the assembler stub. Complicated work like retries > is still done in C. So this is essentially a fast path for the fast > path. > > The assembler code doesn't understand the layout of "serial_lock", but > it needs to check that serial_lock is free. We export just the lock > variable as a separate pointer for this. > > The assembler code controls the HTM fast path with a new pr_tryHTM flag. > > I had to reorganize the retry loop slightly so that the waiting for the > lock to be free again is done first (because the first transaction > is already done elsewhere). This makes the patch look larger than > it really is. This just moves one basic block around. > > Passes bootstrap/testsuite on x86_64 > > libitm/: > 2013-01-12 Andi Kleen <ak@linux.intel.com> > > * beginend.cc (GTM::htm_fastpath): Remove. > (__gtm_htm_fastpath, __gtm_global_lock): Add. > (begin_transaction): Check pr_tryHTM. Remove one iteration > from HTM retry loop. Move lock free check to the beginning of > loop body. > (_ITM_commitTransaction): Check __gtm_htm_fastpath instead of > htm_fastpath. > (_ITM_commitTransactionEH): dito. > * config/linux/rwlock.h (gtm_rwlock::get_lock_var): Add. > * config/posix/rwlock.h (tm_rwlock::get_lock_var): Add. > * config/x86/sjlj.S: Include config.h. > (pr_tryHTM, pr_uninstrumentedCode, pr_hasNoAbort, > a_runInstrumentedCode, a_runUninstrumentedCode, > _XABORT_EXPLICIT, _XABORT_RETRY): Add defines. > (_ITM_beginTransaction): Add HTM fast path. > * libitm.h (pr_tryHTM): Add. > * libitm_i.h (GTM::htm_fastpath): Remove. > (__gtm_htm_fastpath, __gtm_global_lock): Add. > * method-serial.cc (method_group::init, fini): Initialize > __gtm_global_lock and __gtm_htm_fastpath. > (gtm_thread::serialirr_mode): Check __gtm_htm_fastpath. > --- > libitm/beginend.cc | 50 +++++++++++++++++++++++------------------- > libitm/config/linux/rwlock.h | 5 +++++ > libitm/config/posix/rwlock.h | 5 +++++ > libitm/config/x86/sjlj.S | 47 +++++++++++++++++++++++++++++++++++++++ > libitm/libitm.h | 3 ++- > libitm/libitm_i.h | 7 +++--- > libitm/method-serial.cc | 8 ++++--- > 7 files changed, 96 insertions(+), 29 deletions(-) > > diff --git a/libitm/beginend.cc b/libitm/beginend.cc > index 4369946..96ee1b3 100644 > --- a/libitm/beginend.cc > +++ b/libitm/beginend.cc > @@ -55,7 +55,11 @@ static pthread_key_t thr_release_key; > static pthread_once_t thr_release_once = PTHREAD_ONCE_INIT; > > // See gtm_thread::begin_transaction. > -uint32_t GTM::htm_fastpath = 0; > +extern "C" > +{ > + uint32_t __gtm_htm_fastpath = 0; Please keep this as-is and rather use the same approach as for gtm_thread::begin_transaction. > + uint32_t *__gtm_global_lock; Same, and rearrange the serial lock's fields (see below). > +}; > > /* Allocate a transaction structure. */ > void * > @@ -187,27 +191,11 @@ GTM::gtm_thread::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb) > // indeed in serial mode, and HW transactions should never need serial mode > // for any internal changes (e.g., they never abort visibly to the STM code > // and thus do not trigger the standard retry handling). Add a comment as second paragraph in the previous block of comments, briefly explaining when pr_tryHTM is set (ie, under which condition), and that this happens in the assembler implementation of _ITM_beginTransaction. > - if (likely(htm_fastpath && (prop & pr_hasNoAbort))) > + if (likely(prop & pr_tryHTM)) > { > - for (uint32_t t = htm_fastpath; t; t--) > + // One transaction has been already tried by the assembler fast path. > + for (uint32_t t = __gtm_htm_fastpath - 1; t; t--) > { > - uint32_t ret = htm_begin(); > - if (htm_begin_success(ret)) > - { > - // We are executing a transaction now. > - // Monitor the writer flag in the serial-mode lock, and abort > - // if there is an active or waiting serial-mode transaction. > - if (unlikely(serial_lock.is_write_locked())) > - htm_abort(); > - else > - // We do not need to set a_saveLiveVariables because of HTM. > - return (prop & pr_uninstrumentedCode) ? > - a_runUninstrumentedCode : a_runInstrumentedCode; > - } > - // The transaction has aborted. Don't retry if it's unlikely that > - // retrying the transaction will be successful. > - if (!htm_abort_should_retry(ret)) > - break; > // Wait until any concurrent serial-mode transactions have finished. > // This is an empty critical section, but won't be elided. > if (serial_lock.is_write_locked()) > @@ -225,6 +213,24 @@ GTM::gtm_thread::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb) > // we have retried so often that we should go serial to avoid > // starvation. > } > + > + uint32_t ret = htm_begin(); > + if (htm_begin_success(ret)) > + { > + // We are executing a transaction now. > + // Monitor the writer flag in the serial-mode lock, and abort > + // if there is an active or waiting serial-mode transaction. > + if (unlikely(serial_lock.is_write_locked())) > + htm_abort(); > + else > + // We do not need to set a_saveLiveVariables because of HTM. > + return (prop & pr_uninstrumentedCode) ? > + a_runUninstrumentedCode : a_runInstrumentedCode; > + } > + // The transaction has aborted. Don't retry if it's unlikely that > + // retrying the transaction will be successful. > + if (!htm_abort_should_retry(ret)) > + break; > } > } > #endif > @@ -608,7 +614,7 @@ _ITM_commitTransaction(void) > // a serial-mode transaction. If we are, then there will be no other > // concurrent serial-mode transaction. > // See gtm_thread::begin_transaction. > - if (likely(htm_fastpath && !gtm_thread::serial_lock.is_write_locked())) > + if (likely(__gtm_htm_fastpath && !gtm_thread::serial_lock.is_write_locked())) > { > htm_commit(); > return; > @@ -624,7 +630,7 @@ _ITM_commitTransactionEH(void *exc_ptr) > { > #if defined(USE_HTM_FASTPATH) > // See _ITM_commitTransaction. > - if (likely(htm_fastpath && !gtm_thread::serial_lock.is_write_locked())) > + if (likely(__gtm_htm_fastpath && !gtm_thread::serial_lock.is_write_locked())) > { > htm_commit(); > return; > diff --git a/libitm/config/linux/rwlock.h b/libitm/config/linux/rwlock.h > index f13d287..b27b785 100644 > --- a/libitm/config/linux/rwlock.h > +++ b/libitm/config/linux/rwlock.h > @@ -67,6 +67,11 @@ class gtm_rwlock > return writers.load (memory_order_relaxed) != 0; > } > > + uint32_t *get_lock_var() > + { > + return (uint32_t *)&writers; > + } > + Instead of this, please add a comment that the writers field must remain at the start of the struct because the assembler HTM fast path will assume that there is no writer if this field's value is zero. > protected: > bool write_lock_generic (gtm_thread *tx); > }; > diff --git a/libitm/config/posix/rwlock.h b/libitm/config/posix/rwlock.h > index 79f1429..4481378 100644 > --- a/libitm/config/posix/rwlock.h > +++ b/libitm/config/posix/rwlock.h > @@ -82,6 +82,11 @@ class gtm_rwlock > return summary.load (memory_order_relaxed) & (a_writer | w_writer); > } > > + uint32_t *get_lock_var() > + { > + return (uint32_t *)&summmary; > + } > + Remove this, move the summary field to the start of the struct, and add a comment as above. > protected: > bool write_lock_generic (gtm_thread *tx); > }; > diff --git a/libitm/config/x86/sjlj.S b/libitm/config/x86/sjlj.S > index 4d733f4..be24e7e 100644 > --- a/libitm/config/x86/sjlj.S > +++ b/libitm/config/x86/sjlj.S > @@ -24,6 +24,7 @@ > > > #include "asmcfi.h" > +#include "config.h" > > #define CONCAT1(a, b) CONCAT2(a, b) > #define CONCAT2(a, b) a ## b > @@ -52,6 +53,19 @@ > # endif > #endif > > +/* Should share those somewhere, but they are an ABI anyways, so shouldn't > + change. */ > + Trailing space here? > +#define pr_tryHTM 0x800000 > +#define pr_uninstrumentedCode 0x0002 > +#define pr_hasNoAbort 0x0008 > + > +#define a_runInstrumentedCode 0x01 > +#define a_runUninstrumentedCode 0x02 Add brief comments to the respective enums in libitm.h, stating that if these values change, the assembler bits have to be reviewed. > + > +#define _XABORT_EXPLICIT (1 << 0) > +#define _XABORT_RETRY (1 << 1) > + > .text > > .align 4 > @@ -60,6 +74,39 @@ > SYM(_ITM_beginTransaction): > cfi_startproc > #ifdef __x86_64__ > +#ifdef HAVE_AS_RTM > + /* Hardware Transactional fast path. "HTM fast path" ? > + Try to run the transaction once here. If it doesn't work > + call the C code which retries a few more times and does other > + things. This is merely a fast path to optimize small transactions, > + anything complicated is done in C++. */ Pun intended? ;) (Either use C or C++ everywhere...) > + cmpl $0,__gtm_htm_fastpath(%rip) > + jz no_txn > + testl $pr_hasNoAbort,%edi > + jnz no_txn > + xbegin txn_abort > + /* Would be better to have a tool to generate offsetof(). */ > + movq __gtm_global_lock(%rip),%rax Use the serial lock directly. > + /* global lock is free? */ > + cmpl $0,(%rax) > + jnz 1f > + /* Everything good. start running transaction */ > + testl $pr_uninstrumentedCode,%edi > + mov $a_runInstrumentedCode,%eax > + mov $a_runUninstrumentedCode,%ecx > + cmovnz %ecx,%eax > + ret > + /* Lock is busy: abort */ > +1: xabort $0xff > +txn_abort: > + /* if an internal abort, but not the xabort above, > + tell the C code to not retry */ > + testl $(_XABORT_RETRY|_XABORT_EXPLICIT),%eax > + jz no_txn > + orl $pr_tryHTM,%edi > + /* go on to the C code now for further retries */ > +no_txn: > +#endif > leaq 8(%rsp), %rax > subq $56, %rsp > cfi_def_cfa_offset(64) > diff --git a/libitm/libitm.h b/libitm/libitm.h > index 436eb94..72e4585 100644 > --- a/libitm/libitm.h > +++ b/libitm/libitm.h > @@ -95,7 +95,8 @@ typedef enum > pr_exceptionBlock = 0x1000, > pr_hasElse = 0x2000, > pr_readOnly = 0x4000, > - pr_hasNoSimpleReads = 0x400000 > + pr_hasNoSimpleReads = 0x400000, > + pr_tryHTM = 0x800000 Add a comment like "Non-ABI bit used for the HTM fast path. See gtm_thread::begin_transaction." > } _ITM_codeProperties; > > /* Result from startTransaction that describes what actions to take. */ > diff --git a/libitm/libitm_i.h b/libitm/libitm_i.h > index 4dfcda9..9bad7df 100644 > --- a/libitm/libitm_i.h > +++ b/libitm/libitm_i.h > @@ -336,10 +336,11 @@ extern abi_dispatch *dispatch_htm(); > > extern gtm_cacheline_mask gtm_mask_stack(gtm_cacheline *, gtm_cacheline_mask); > > +} // namespace GTM > + > // Control variable for the HTM fastpath that uses serial mode as fallback. > // Non-zero if the HTM fastpath is enabled. See gtm_thread::begin_transaction. > -extern uint32_t htm_fastpath; > - > -} // namespace GTM > +extern "C" uint32_t __gtm_htm_fastpath; > +extern "C" uint32_t *__gtm_global_lock; > > #endif // LIBITM_I_H > diff --git a/libitm/method-serial.cc b/libitm/method-serial.cc > index 38857dc..ed7517b 100644 > --- a/libitm/method-serial.cc > +++ b/libitm/method-serial.cc > @@ -222,13 +222,15 @@ struct htm_mg : public method_group > // Enable the HTM fastpath if the HW is available. The fastpath is > // initially disabled. > #ifdef USE_HTM_FASTPATH > - htm_fastpath = htm_init(); > + __gtm_global_lock = gtm_thread::serial_lock.get_lock_var(); > + __gtm_htm_fastpath = htm_init(); > #endif > } > virtual void fini() > { > // Disable the HTM fastpath. > - htm_fastpath = 0; > + __gtm_htm_fastpath = 0; > + __gtm_global_lock = NULL; > } > }; > > @@ -288,7 +290,7 @@ GTM::gtm_thread::serialirr_mode () > #if defined(USE_HTM_FASTPATH) > // HTM fastpath. If we are executing a HW transaction, don't go serial but > // continue. See gtm_thread::begin_transaction. > - if (likely(htm_fastpath && !gtm_thread::serial_lock.is_write_locked())) > + if (likely(__gtm_htm_fastpath && !gtm_thread::serial_lock.is_write_locked())) > return; > #endif > Adapt these. Thanks. Torvald ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add faster HTM fastpath for libitm TSX 2013-01-24 9:42 ` Torvald Riegel @ 2013-01-24 16:45 ` Andi Kleen 2013-01-24 18:02 ` Richard Henderson 2013-01-29 13:05 ` Torvald Riegel 0 siblings, 2 replies; 6+ messages in thread From: Andi Kleen @ 2013-01-24 16:45 UTC (permalink / raw) To: Torvald Riegel; +Cc: Andi Kleen, gcc-patches, Andi Kleen, Richard Henderson > > // See gtm_thread::begin_transaction. > > -uint32_t GTM::htm_fastpath = 0; > > +extern "C" > > +{ > > + uint32_t __gtm_htm_fastpath = 0; > > Please keep this as-is and rather use the same approach as for > gtm_thread::begin_transaction. What approach? I don't want to hardcode C++ mangling in the assembler file, that just's horrible. > > > + uint32_t *__gtm_global_lock; > > Same, and rearrange the serial lock's fields (see below). My understanding is that C++ makes no guarantees of class layout. That is why i ended up not relying on the layout. -Andi -- ak@linux.intel.com -- Speaking for myself only. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add faster HTM fastpath for libitm TSX 2013-01-24 16:45 ` Andi Kleen @ 2013-01-24 18:02 ` Richard Henderson 2013-01-29 13:05 ` Torvald Riegel 1 sibling, 0 replies; 6+ messages in thread From: Richard Henderson @ 2013-01-24 18:02 UTC (permalink / raw) To: Andi Kleen; +Cc: Torvald Riegel, gcc-patches, Andi Kleen On 01/24/2013 08:45 AM, Andi Kleen wrote: >> >Please keep this as-is and rather use the same approach as for >> >gtm_thread::begin_transaction. > What approach? Using __asm__ to set the assembler name. r~ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add faster HTM fastpath for libitm TSX 2013-01-24 16:45 ` Andi Kleen 2013-01-24 18:02 ` Richard Henderson @ 2013-01-29 13:05 ` Torvald Riegel 1 sibling, 0 replies; 6+ messages in thread From: Torvald Riegel @ 2013-01-29 13:05 UTC (permalink / raw) To: Andi Kleen; +Cc: gcc-patches, Andi Kleen, Richard Henderson On Thu, 2013-01-24 at 17:45 +0100, Andi Kleen wrote: > > > > > + uint32_t *__gtm_global_lock; > > > > Same, and rearrange the serial lock's fields (see below). > > My understanding is that C++ makes no guarantees of class layout. > That is why i ended up not relying on the layout. gtm_rwlock is a POD type, so you get the guarantees that C would provide. Torvald ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-01-29 13:05 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-01-12 21:03 [PATCH] Add faster HTM fastpath for libitm TSX Andi Kleen 2013-01-21 4:41 ` [PING] " Andi Kleen 2013-01-24 9:42 ` Torvald Riegel 2013-01-24 16:45 ` Andi Kleen 2013-01-24 18:02 ` Richard Henderson 2013-01-29 13:05 ` Torvald Riegel
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).