From patchwork Wed Jul 27 10:35:49 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Torvald Riegel X-Patchwork-Id: 107022 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 B022FB6F77 for ; Wed, 27 Jul 2011 20:36:16 +1000 (EST) Received: (qmail 1643 invoked by alias); 27 Jul 2011 10:36:12 -0000 Received: (qmail 1299 invoked by uid 22791); 27 Jul 2011 10:36:11 -0000 X-SWARE-Spam-Status: No, hits=-6.7 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, SPF_HELO_PASS, TW_GJ 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; Wed, 27 Jul 2011 10:35:51 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p6RAZpp2003246 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 27 Jul 2011 06:35:51 -0400 Received: from [10.36.7.145] (vpn1-7-145.ams2.redhat.com [10.36.7.145]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p6RAZnTQ017034; Wed, 27 Jul 2011 06:35:50 -0400 Subject: Re: [trans-mem] Beginning of refactoring From: Torvald Riegel To: GCC Patches Cc: Richard Henderson In-Reply-To: <1311031322.19033.1049.camel@triegel.csb> References: <1306357828.13348.423.camel@triegel.csb> <1310221819.5106.1054.camel@triegel.csb> <1311031188.19033.1045.camel@triegel.csb> <1311031322.19033.1049.camel@triegel.csb> Date: Wed, 27 Jul 2011 12:35:49 +0200 Message-ID: <1311762949.4292.546.camel@triegel.csb> Mime-Version: 1.0 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 Tue, 2011-07-19 at 01:22 +0200, Torvald Riegel wrote: > On Tue, 2011-07-19 at 01:19 +0200, Torvald Riegel wrote: > > On Sat, 2011-07-09 at 16:30 +0200, Torvald Riegel wrote: > > > The attached patch makes flat nesting the default, while still > > > supporting closed nesting on demand (for user-controlled aborts via > > > __transaction_cancel). > > > Previously, a new transaction object was created for each nested > > > transaction. The patch changes this to using one transaction object > > > during the whole lifetime of a thread and keeping checkpoints of the > > > transaction state when starting closed nested transactions. This allows > > > us to easily use flat nesting and decreases the transaction start/commit > > > overheads to some extent. > > > > > > OK for branch? > > > > I split the patch, as requested. > > > > First three parts are smaller changes: > > > > patch1: New erase method and placement new for aatree. > > patch2: Change pr_hasElse to the value specified in the ABI. > > patch3: Add information to dispatch about closed nesting and > > uninstrumented code. > > > > Then, in preparation for the following flat-transaction object design: > > patch4: Use vector instead of list to store user actions. > > patch5: Add closed nesting as restart reason. > > > > And the actual core change: > > patch6: Make flat nesting the default, use closed nesting on demand. > > > > This last patch is still rather large, but it is one logical piece. > > beginend.cc has most changes, but splitting this one up would rather > > create incomplete intermediate steps than make the direction of this > > whole patch clear. > > > > As discussed offline, reducing the one extra copying of jmpbuf for > > actual closed-nested transactions can be addressed in a future patch by > > modifying _ITM_beginTransaction, which could additionally remove the > > extra copying from stack to jmpbuf for all transactions as it existed > > before this patch here. > > > > Ok for branch, or is further splitting required? > > > > This time with patches attached, sorry. Attached are two small (in size) fixes for this patch set. patch7: gtm_transaction::decide_begin_dispatch() gets the transaction properties from the caller instead of reading from gtm_transaction::prop, which might not have been updated by the caller yet. patch8: Fix nesting level reset when rolling back the outermost transaction. Before, this was incorrectly reset to zero, which caused transactions to not get committed. This didn't show up in previous testing because previously, only serial-mode TM methods were available. OK for branch, together with the previous patches? commit f3d24b9933d2124fe330e0513f14ce0711402e88 Author: Torvald Riegel Date: Tue Jul 26 20:28:54 2011 +0200 Fix gtm_transaction::decide_begin_dispatch(). * retry.cc (GTM::gtm_transaction::decide_begin_dispatch): Get transaction properties from the caller instead of from the transaction object. * libitm_i.h: Same. * beginend.cc (GTM::gtm_transaction::begin_transaction): Same. commit f85b6ce7b67cecfd9c056620c7ef2aaa173fe5bc Author: Torvald Riegel Date: Wed Jul 27 12:27:10 2011 +0200 Fix nesting level reset when rolling back the outermost transaction. * beginend.cc (GTM::gtm_transaction::rollback): Fix nesting level rollback. diff --git a/libitm/beginend.cc b/libitm/beginend.cc index 417b41a..ba90af2 100644 --- a/libitm/beginend.cc +++ b/libitm/beginend.cc @@ -291,6 +291,7 @@ GTM::gtm_transaction::rollback (gtm_transaction_cp *cp) } else { + // Roll back to the outermost transaction. // Restore the jump buffer and transaction properties, which we will // need for the longjmp used to restart or abort the transaction. if (parent_txns.size() > 0) @@ -298,9 +299,11 @@ GTM::gtm_transaction::rollback (gtm_transaction_cp *cp) jb = parent_txns[0].jb; prop = parent_txns[0].prop; } - // Reset the transaction. Do not reset state, which is handled by the - // callers. - nesting = 0; + // Reset the transaction. Do not reset this->state, which is handled by + // the callers. Note that we reset the transaction to the point after + // having executed begin_transaction (we will return from it), so the + // nesting level must be one, not zero. + nesting = 1; parent_txns.clear(); } diff --git a/libitm/beginend.cc b/libitm/beginend.cc index 6023390..417b41a 100644 --- a/libitm/beginend.cc +++ b/libitm/beginend.cc @@ -187,7 +187,7 @@ GTM::gtm_transaction::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb) tx->state = (STATE_SERIAL | STATE_IRREVOCABLE); else - disp = tx->decide_begin_dispatch (); + disp = tx->decide_begin_dispatch (prop); if (tx->state & STATE_SERIAL) { diff --git a/libitm/libitm_i.h b/libitm/libitm_i.h index 4cc0a70..093fc0e 100644 --- a/libitm/libitm_i.h +++ b/libitm/libitm_i.h @@ -218,7 +218,7 @@ struct gtm_transaction // In retry.cc void decide_retry_strategy (gtm_restart_reason); - abi_dispatch* decide_begin_dispatch (); + abi_dispatch* decide_begin_dispatch (uint32_t prop); // In method-serial.cc void serialirr_mode (); diff --git a/libitm/retry.cc b/libitm/retry.cc index bfca2ca..91778e6 100644 --- a/libitm/retry.cc +++ b/libitm/retry.cc @@ -77,12 +77,12 @@ GTM::gtm_transaction::decide_retry_strategy (gtm_restart_reason r) // Decides which TM method should be used on the first attempt to run this -// transaction, setting this->disp accordingly. +// transaction. // serial_lock will not have been acquired if this is the outer-most // transaction. If the state is set to STATE_SERIAL, the caller will set the // dispatch. GTM::abi_dispatch* -GTM::gtm_transaction::decide_begin_dispatch () +GTM::gtm_transaction::decide_begin_dispatch (uint32_t prop) { // ??? Probably want some environment variable to choose the default // STM implementation once we have more than one implemented.