On 2012-11-08 15:32, Torvald Riegel wrote:
> On Thu, 2012-11-08 at 15:38 +0000, Lai, Konrad wrote:
>> The xabort argument is the only "escape" currently allowed in RTM. So it is
>> not possible to use a separate Boolean in memory.
>
> No, that's not what I meant. The boolean would be used in libitm's
> htm_abort(), which the architecture-specific code (eg,
> config/x86/target.h) would then change into whatever the particular HTM
> uses as abort reasons (eg, true would become 0xff). That's just to keep
> as much of libitm portable as possible, nothing more.
>
>> [Roman just posted an example in
>> http://software.intel.com/en-us/blogs/2012/11/06/exploring-intel-transactional-synchronization-extensions-with-intel-software
>> ]
>>
>> I don't know "any" large code that uses cancel. Someone claim there is one
>> in STAMP, and one got speed up if it was removed. I think this discussion
>> potentially explains why.
>
> The abort in STAMP is bogus. They didn't instrument all of the code (if
> you use the manual instrumentation), and the abort is just there to
> "catch" race conditions and other inconsistencies.
>
> My suggestions for next steps are to move the begin stuff to assembly.
> After that, we can go for the abort branch, if removing it really makes
> a non-negligible performance difference.
I believe this is the sort of patch that Torvald was talking about
for handling abortTransaction with RTM.
Andi, can you please test?
r~
diff --git a/libitm/beginend.cc b/libitm/beginend.cc
index 4369946..8f5c4ef 100644
--- a/libitm/beginend.cc
+++ b/libitm/beginend.cc
@@ -166,18 +166,16 @@ GTM::gtm_thread::begin_transaction (uint32_t prop, const
gtm_jmpbuf *jb)
GTM_fatal("pr_undoLogCode not supported");
#if defined(USE_HTM_FASTPATH) && !defined(HTM_CUSTOM_FASTPATH)
- // HTM fastpath. Only chosen in the absence of transaction_cancel to allow
- // using an uninstrumented code path.
- // The fastpath is enabled only by dispatch_htm's method group, which uses
- // serial-mode methods as fallback. Serial-mode transactions cannot execute
- // concurrently with HW transactions because the latter monitor the serial
- // lock's writer flag and thus abort if another thread is or becomes a
- // serial transaction. Therefore, if the fastpath is enabled, then a
- // transaction is not executing as a HW transaction iff the serial lock is
- // write-locked. This allows us to use htm_fastpath and the serial lock's
- // writer flag to reliable determine whether the current thread runs a HW
- // transaction, and thus we do not need to maintain this information in
- // per-thread state.
+ // HTM fastpath. The fastpath is enabled only by dispatch_htm's method
+ // group, which uses serial-mode methods as fallback. Serial-mode
+ // transactions cannot execute concurrently with HW transactions because
+ // the latter monitor the serial lock's writer flag and thus abort if
+ // another thread is or becomes a serial transaction. Therefore, if the
+ // fastpath is enabled, then a transaction is not executing as a HW
+ // transaction iff the serial lock is write-locked. This allows us to
+ // use htm_fastpath and the serial lock's writer flag to reliable determine
+ // whether the current thread runs a HW transaction, and thus we do not
+ // need to maintain this information in per-thread state.
// If an uninstrumented code path is not available, we can still run
// instrumented code from a HW transaction because the HTM fastpath kicks
// in early in both begin and commit, and the transaction is not canceled.
@@ -187,7 +185,7 @@ 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(htm_fastpath))
{
for (uint32_t t = htm_fastpath; t; t--)
{
@@ -198,33 +196,41 @@ GTM::gtm_thread::begin_transaction (uint32_t prop, const
gtm_jmpbuf *jb)
// 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();
+ htm_abort_retry();
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
+ // The transaction has aborted. Retry if it's likely 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())
+ if (htm_abort_should_retry(ret))
{
- tx = gtm_thr();
- if (unlikely(tx == NULL))
- {
- // See below.
- tx = new gtm_thread();
- set_gtm_thr(tx);
- }
- serial_lock.read_lock(tx);
- serial_lock.read_unlock(tx);
- // TODO We should probably reset the retry count t here, unless
- // we have retried so often that we should go serial to avoid
- // starvation.
+ // 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())
+ {
+ tx = gtm_thr();
+ if (unlikely(tx == NULL))
+ {
+ // See below.
+ tx = new gtm_thread();
+ set_gtm_thr(tx);
+ }
+ serial_lock.read_lock(tx);
+ serial_lock.read_unlock(tx);
+ // TODO We should probably reset the retry count t here,
+ // unless we have retried so often that we should go
+ // serial to avoid starvation.
+ }
}
+ // Honor an abort from abortTransaction.
+ else if (htm_abort_is_cancel(ret))
+ return a_abortTransaction | a_restoreLiveVariables;
+ // Otherwise quit using HTM and fall back to STM.
+ else
+ break;
}
}
#endif
@@ -438,6 +444,11 @@ GTM::gtm_thread::rollback (gtm_transaction_cp *cp, bool
aborting)
void ITM_REGPARM
_ITM_abortTransaction (_ITM_abortReason reason)
{
+#if defined(USE_HTM_FASTPATH)
+ if (likely(htm_fastpath && !gtm_thread::serial_lock.is_write_locked()))
+ htm_abort_cancel();
+#endif
+
gtm_thread *tx = gtm_thr();
assert (reason == userAbort || reason == (userAbort | outerAbort));
diff --git a/libitm/config/x86/target.h b/libitm/config/x86/target.h
index 41ae2eb..1d2e822 100644
--- a/libitm/config/x86/target.h
+++ b/libitm/config/x86/target.h
@@ -125,18 +125,32 @@ htm_commit ()
}
static inline void
-htm_abort ()
+htm_abort_retry ()
{
// ??? According to a yet unpublished ABI rule, 0xff is reserved and
// supposed to signal a busy lock. Source: [email protected]
_xabort(0xff);
}
+static inline void
+htm_abort_cancel ()
+{
+ // ??? What's the unpublished ABI rule for this, Andi?
+ _xabort(0);
+}
+
static inline bool
htm_abort_should_retry (uint32_t begin_ret)
{
return begin_ret & _XABORT_RETRY;
}
+
+static inline bool
+htm_abort_is_cancel (uint32_t begin_ret)
+{
+ // return (begin_ret & _XABORT_EXPLICIT) && _XABORT_CODE(begin_ret) == 0;
+ return begin_ret == _XABORT_EXPLICIT;
+}
#endif
