From patchwork Wed Nov 7 23:03:40 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Henderson X-Patchwork-Id: 197737 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 D283E2C00BD for ; Thu, 8 Nov 2012 10:03:55 +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=1352934236; 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=0j/yrqSV3595mzGEM0c9VjR3Hx8=; b=rUXGcDCPH4Pyagt wifpLY9GVMbsH08f+asqLzmUHi4jXhLK0XEuuLCAOIVDZv/0wqCOS14OigpR+E6w 8OK/V4MfszaJQVU279tP/Nlqi6GiUWYz/0t5znU/sCFg1M3bP33JUIdywzEcmOpk iFeFwW2w8ClZ7eNI109X3ZiuTF7w= 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=llQPH1UDU4KB5GxROgxA3zB/GCOgQHYLWv39PL2TCkGE+V7ZMUQ5ht7ZrPA7O8 o/AKVIcYLsscfaSzFiCSfZj8B6C/uat1teWoJ8YdCoF9NSLvEzzJd+e9IvPlAt5c bnAqKXEPnehUzmXqgoOJIi+VGVPKrUGTVOYpJfIS683oI=; Received: (qmail 3902 invoked by alias); 7 Nov 2012 23:03:49 -0000 Received: (qmail 3894 invoked by uid 22791); 7 Nov 2012 23:03:48 -0000 X-SWARE-Spam-Status: No, hits=-7.1 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, KHOP_SPAMHAUS_DROP, KHOP_THREADED, RCVD_IN_DNSWL_HI, RCVD_IN_HOSTKARMA_W, RP_MATCHES_RCVD, SPF_HELO_PASS, TW_TM 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, 07 Nov 2012 23:03:41 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qA7N3eF9025587 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 7 Nov 2012 18:03:41 -0500 Received: from anchor.twiddle.home (vpn-228-97.phx2.redhat.com [10.3.228.97]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id qA7N3eAi024525; Wed, 7 Nov 2012 18:03:40 -0500 Message-ID: <509AE8CC.4050602@redhat.com> Date: Wed, 07 Nov 2012 15:03:40 -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: Torvald Riegel CC: GCC Patches , Aldy Hernandez Subject: Re: [trans-mem][rfc] Improvements to uninstrumented code paths References: <509AE85C.2000107@redhat.com> In-Reply-To: <509AE85C.2000107@redhat.com> 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 11/07/2012 03:01 PM, Richard Henderson wrote: > Thoughts? Now with 100% more patches per mail! r~ From 9253d4138a0cdb76c40345a1e32694968f375a86 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Wed, 7 Nov 2012 14:36:01 -0800 Subject: [PATCH 2/2] tm: Optimize nested transactions in an uninstrumented code path * trans-mem.c (tm_region_init_0): Consider all edges when looking for the entry_block. (collect_bb2reg): Handle uninstrumented only transactions. (generate_tm_state): Likewise. (expand_transaction): Likewise. (execute_tm_memopt): Likewise. (ipa_uninstrument_transaction0): Convert nested transactions in the uninstrumented code path to uninstrumented only. --- gcc/ChangeLog | 9 ++++++++ gcc/trans-mem.c | 66 ++++++++++++++++++++++++++++++++++++++++++++------------- 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index dc1909c..8555e8c 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,5 +1,14 @@ 2012-11-07 Richard Henderson + * trans-mem.c (tm_region_init_0): Consider all edges when looking + for the entry_block. + (collect_bb2reg): Handle uninstrumented only transactions. + (generate_tm_state): Likewise. + (expand_transaction): Likewise. + (execute_tm_memopt): Likewise. + (ipa_uninstrument_transaction0): Convert nested transactions in + the uninstrumented code path to uninstrumented only. + * trans-mem.c (ipa_uninstrument_transaction0): Rename from ipa_uninstrument_transaction. (ipa_uninstrument_transaction): New function. diff --git a/gcc/trans-mem.c b/gcc/trans-mem.c index 478ce71..c0987dc 100644 --- a/gcc/trans-mem.c +++ b/gcc/trans-mem.c @@ -868,10 +868,13 @@ typedef struct tm_log_entry { /* Address to save. */ tree addr; + /* Entry block for the transaction this address occurs in. */ basic_block entry_block; + /* Dominating statements the store occurs in. */ gimple_vec stmts; + /* Initially, while we are building the log, we place a nonzero value here to mean that this address *will* be saved with a save/restore sequence. Later, when generating the save sequence @@ -1721,8 +1724,8 @@ struct tm_region /* Return value from BUILT_IN_TM_START. */ tree tm_state; - /* The entry block to this region. This will always be the first - block of the body of the transaction. */ + /* The entry block to the instrumented code path for this region. + This will always be the first block of the body of the transaction. */ basic_block entry_block; /* The first block after an expanded call to _ITM_beginTransaction. */ @@ -1732,7 +1735,7 @@ struct tm_region These blocks are still a part of the region (i.e., the border is inclusive). Note that this set is only complete for paths in the CFG starting at ENTRY_BLOCK, and that there is no exit block recorded for - the edge to the "over" label. */ + the TM_ABORT edge nor for the body of the uninstrumented code path. */ bitmap exit_blocks; /* The set of all blocks that have an TM_IRREVOCABLE call. */ @@ -1779,11 +1782,19 @@ tm_region_init_0 (struct tm_region *outer, basic_block bb, gimple stmt) region->original_transaction_was_outer = false; region->tm_state = NULL; - /* There are either one or two edges out of the block containing - the GIMPLE_TRANSACTION, one to the actual region and one to the - "over" label if the region contains an abort. The former will - always be the one marked FALLTHRU. */ - region->entry_block = FALLTHRU_EDGE (bb)->dest; + // There are three possible edges out of GIMPLE_TRANSACTION. + // The "entry_block" always refers to the instrumented code path. + region->entry_block = NULL; + { + edge_iterator ei; + edge e; + FOR_EACH_EDGE (e, ei, bb->succs) + if (!(e->flags & (EDGE_TM_ABORT | EDGE_TM_UNINSTRUMENTED))) + { + region->entry_block = e->dest; + break; + } + } region->exit_blocks = BITMAP_ALLOC (&tm_obstack); region->irr_blocks = BITMAP_ALLOC (&tm_obstack); @@ -2453,6 +2464,10 @@ collect_bb2reg (struct tm_region *region, void *data) unsigned int i; basic_block bb; + // Don't run on transactions with only uninstrumented code paths. + if (region->entry_block == NULL) + return NULL; + queue = get_tm_region_blocks (region->entry_block, region->exit_blocks, region->irr_blocks, @@ -2564,9 +2579,8 @@ expand_transaction (struct tm_region *region, void *data ATTRIBUTE_UNUSED) uninst_edge = e; else inst_edge = e; - if (e->flags & EDGE_FALLTHRU) - fallthru_edge = e; } + fallthru_edge = (inst_edge ? inst_edge : uninst_edge); } /* ??? There are plenty of bits here we're not computing. */ @@ -2780,7 +2794,7 @@ generate_tm_state (struct tm_region *region, void *data ATTRIBUTE_UNUSED) // Reset the subcode, post optimizations. We'll fill this in // again as we process blocks. - if (region->exit_blocks) + if (region->entry_block && region->exit_blocks) { unsigned int subcode = gimple_transaction_subcode (region->transaction_stmt); @@ -3695,6 +3709,10 @@ execute_tm_memopt (void) size_t i; basic_block bb; + // Don't run on transactions with only uninstrumented code paths. + if (region->entry_block == NULL) + continue; + bitmap_obstack_initialize (&tm_memopt_obstack); /* Save all BBs for the current region. */ @@ -3894,15 +3912,33 @@ ipa_uninstrument_transaction0 (struct tm_region *region, int n = VEC_length (basic_block, queue); basic_block *new_bbs = XNEWVEC (basic_block, n); + // We will have a GIMPLE_ATOMIC with 3 possible edges out of it. + // a) EDGE_FALLTHRU to the instrumented blocks, + // b) EDGE_TM_UNINSTRUMENTED to the uninstrumented blocks, + // c) EDGE_TM_ABORT out of the transaction + copy_bbs (VEC_address (basic_block, queue), n, new_bbs, NULL, 0, NULL, NULL, transaction_bb); edge e = make_edge (transaction_bb, new_bbs[0], EDGE_TM_UNINSTRUMENTED); add_phi_args_after_copy (new_bbs, n, e); - // Now we will have a GIMPLE_ATOMIC with 3 possible edges out of it. - // a) EDGE_FALLTHRU into the transaction - // b) EDGE_TM_ABORT out of the transaction - // c) EDGE_TM_UNINSTRUMENTED into the uninstrumented blocks. + // Note that the region that we copied can contain transactions. Given + // that these are on a code path that we've just designated uninstrumented, + // there's no point in having these nested transactions have instrumented + // code paths. Which means that there's no point processing these new + // transactions into new tm_regions so that they could be processed by + // ipa_tm_scan_calls_transaction. What we would like to do is change the + // fallthru (instrumented) edges to uninstrumented. + for (int i = 0; i < n; ++i) + { + gimple_stmt_iterator gsi = gsi_last_bb (new_bbs[i]); + if (!gsi_end_p (gsi) + && gimple_code (gsi_stmt (gsi)) == GIMPLE_TRANSACTION) + { + edge e = FALLTHRU_EDGE (new_bbs[i]); + e->flags = EDGE_TM_UNINSTRUMENTED; + } + } free (new_bbs); } -- 1.7.11.7