From patchwork Fri Nov 9 01:23:50 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Henderson X-Patchwork-Id: 197906 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 4D5842C0097 for ; Fri, 9 Nov 2012 12:24:05 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1353029046; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject: References:In-Reply-To:Content-Type:Mailing-List:Precedence: List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=BiKTEFzNkEj5HU8WgmwRVWMszNs=; b=hSAqvrdFtaTwsU0 bLYsafq1Tn2Etjq4I6ynpfgfelX34rrRjB7jFQfYp2Gx/8u1ifZuI/50+Lq3WMkZ yhRBqiDFSDvYTv9XuI8jPB9iYQlKsvDytFi/CWsuUmyig68Ozj/tNDheES2nJwSY bSeWyEVk0UigCrFJ7g4J/moxwaaE= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject:References:In-Reply-To:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=MOGPXzEDA382BlDie6Xu7DB6el5kXD6HMVAWfeXebybZN088NoKgQbbqi05lc5 i6bZcj9IlZSjc5cqPPBU1zsOZpzU1uZBnE1O0od/Sp24qn8Vl+9K6/s3vXLcZxLe rsIOGseS2fqUr8pxsbUHctat9mw4jXnNIxzlUQR77rQ3k=; Received: (qmail 31044 invoked by alias); 9 Nov 2012 01:24:00 -0000 Received: (qmail 31034 invoked by uid 22791); 9 Nov 2012 01:23:59 -0000 X-SWARE-Spam-Status: No, hits=-7.2 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, KHOP_THREADED, RCVD_IN_DNSWL_HI, RCVD_IN_HOSTKARMA_W, RP_MATCHES_RCVD, SPF_HELO_PASS, TW_XF X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 09 Nov 2012 01:23:49 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qA91NnOL031951 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 8 Nov 2012 20:23:49 -0500 Received: from pebble.twiddle.home (ovpn-113-50.phx2.redhat.com [10.3.113.50]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id qA91NlQA016511; Thu, 8 Nov 2012 20:23:48 -0500 Message-ID: <509C5B26.6000204@redhat.com> Date: Thu, 08 Nov 2012 17:23:50 -0800 From: Richard Henderson User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/20121016 Thunderbird/16.0.1 MIME-Version: 1.0 To: "Kleen, Andi" CC: Torvald Riegel , "Lai, Konrad" , Aldy Hernandez , GCC Patches Subject: [PATCH] Handle abortTransaction with RTM References: <1352372508.3374.36558.camel@triegel.csb> <1352417547.3374.39611.camel@triegel.csb> In-Reply-To: <1352417547.3374.39611.camel@triegel.csb> X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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: andi.kleen@intel.com _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