From patchwork Mon Feb 25 22:52:26 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 223064 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 3CBFD2C0092 for ; Tue, 26 Feb 2013 09:52:42 +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=1362437562; 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=uloR7K3Gq1F7LIxuGE/RX3J8Uxk=; b=wBGcAs46LLe+IH4 un4sFJV7BxzLOYVpJyVIJ1LoUeXt1NHmIPaM9N49i+zPMxfE9aayZ5QLqYn47/7W dIr58/TO979x9OBEagpHFEAtcm/H7Bcr0TmCgZUo1NK2egXHNW2wB4sCfg54E63h O/v1+CJf7IdVWS7++wZeL9D6+Flo= 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:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=e9nDo+Q/x4pD9VURluoFKqLqVPXDh15QvriS3rPkbGpLWYV2fmrMEVrbq8cxgl TJMIAkuHCuM+7D6jvrbI98M/Z4dimp/uCE8BRwi5dxchpinSyWp8twdyV1Ppxa9p 8WpyCdSngYm/r9A8d9CkuSm+AVqEju2YDl2pJf0G8aYWg=; Received: (qmail 20589 invoked by alias); 25 Feb 2013 22:52:35 -0000 Received: (qmail 20580 invoked by uid 22791); 25 Feb 2013 22:52:34 -0000 X-SWARE-Spam-Status: No, hits=-7.4 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 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; Mon, 25 Feb 2013 22:52:27 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r1PMqRLj030256 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 25 Feb 2013 17:52:27 -0500 Received: from houston.quesejoda.com (vpn-49-31.rdu2.redhat.com [10.10.49.31]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r1PMqQo0024309; Mon, 25 Feb 2013 17:52:26 -0500 Message-ID: <512BEB2A.9090007@redhat.com> Date: Mon, 25 Feb 2013 16:52:26 -0600 From: Aldy Hernandez User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: Richard Henderson CC: gcc-patches Subject: Re: do not pass PR_INSTRUMENTEDCODE if there is no instrumentation References: <51269C36.3070203@redhat.com> <5127AA82.2070108@redhat.com> <512BEA59.2040903@redhat.com> In-Reply-To: <512BEA59.2040903@redhat.com> 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 Whoops, wrong patch. This is the latest revision. On 02/25/13 16:48, Aldy Hernandez wrote: > On 02/22/13 11:27, Richard Henderson wrote: >> On 02/21/2013 02:14 PM, Aldy Hernandez wrote: >>> I suppose we could cheat and avoid passing PR_INSTRUMENTEDCODE if we >>> ever enter expand_block_tm(), but perhaps we could do a little better as >>> with the attached patch. >> >> I assume you meant "never enter" there. >> >> Yes, you don't need to "accumulate" GTMA_INSTRUMENTED_CODE. >> >> Probably what should happen is that either >> >>> /* If we're sure to go irrevocable, there won't be >>> anything to expand, since the run-time will go >>> irrevocable right away. */ >>> if (sub & GTMA_DOES_GO_IRREVOCABLE >>> && sub & GTMA_MAY_ENTER_IRREVOCABLE) >>> continue; >> >> should in fact set EDGE_TM_UNINSTRUMENTED, or we should set that bit >> even earlier > > I think it's best to do this here at tmmark time, instead of at IPA-tm. > Don't we have problems when ipa inlining runs after ipa_tm, thus > creating more instrumented code later on? > > If so, the attached patch does it at tmmark. I also cleaned up the > instrumented edges, thus generating: > > : > tm_state.2_5 = __builtin__ITM_beginTransaction (16458); [ > uninstrumentedCode hasNoAbort doesGoIrrevocable readOnly ] > __asm__ __volatile__(""); > __builtin__ITM_commitTransaction (); > > : > return; > > Which is way better than what we've ever generated... removing the > alternative path altogether. Yay. > > How does this look? > Aldy > >> >>> /* If we're sure to go irrevocable, don't transform anything. */ >>> if (d->irrevocable_blocks_normal >>> && bitmap_bit_p (d->irrevocable_blocks_normal, >>> region->entry_block->index)) >>> { >>> transaction_subcode_ior (region, GTMA_DOES_GO_IRREVOCABLE); >>> transaction_subcode_ior (region, GTMA_MAY_ENTER_IRREVOCABLE); >>> continue; >>> } >> >> And while we're at it, ipa_tm_scan_calls_transaction should probably add >> a check to skip doing ipa_uninstrument_transaction on transactions that >> have already been so marked. > + * trans-mem.c (execute_tm_mark): Set the region's irrevocable bit + if appropriate. + (tm_region_init_0): Initialize irrevocable field. + (expand_transaction): Remove instrumented edge and blocks if we + are sure to go irrevocable. + (execute_tm_edges): Skip irrevocable transactions. + (execute_tm_memopt): Skip optimization for irrevocable regions. 2013-02-20 Aldy Hernandez PR middle-end/56108 diff --git a/gcc/testsuite/gcc.dg/tm/instrumented-mask.c b/gcc/testsuite/gcc.dg/tm/instrumented-mask.c new file mode 100644 index 0000000..6cfd3e4 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tm/instrumented-mask.c @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-fgnu-tm -fdump-tree-tmmark" } */ + +/* If we're sure to go irrevocable, as in the case below, do not pass + PR_INSTRUMENTEDCODE to the run-time if there is nothing + instrumented within the transaction. */ + +int +main() +{ + __transaction_relaxed { __asm__(""); } + return 0; +} + +/* { dg-final { scan-tree-dump-times " instrumentedCode" 0 "tmmark" } } */ +/* { dg-final { cleanup-tree-dump "tmmark" } } */ diff --git a/gcc/trans-mem.c b/gcc/trans-mem.c index 71eaa44..8fe1133 100644 --- a/gcc/trans-mem.c +++ b/gcc/trans-mem.c @@ -1737,6 +1737,11 @@ struct tm_region /* The set of all blocks that have an TM_IRREVOCABLE call. */ bitmap irr_blocks; + + /* True if this transaction has any instrumented code. False if + transaction will immediately go irrevocable upon start. This bit + is set in the tmmark stage, and is not valid before. */ + bool has_instrumented_code; }; typedef struct tm_region *tm_region_p; @@ -1785,6 +1790,7 @@ tm_region_init_0 (struct tm_region *outer, basic_block bb, gimple stmt) region->exit_blocks = BITMAP_ALLOC (&tm_obstack); region->irr_blocks = BITMAP_ALLOC (&tm_obstack); + region->has_instrumented_code = true; return region; } @@ -2586,6 +2592,21 @@ expand_transaction (struct tm_region *region, void *data ATTRIBUTE_UNUSED) if (e->flags & EDGE_FALLTHRU) fallthru_edge = e; } + + /* If transaction will immediately go irrevocable, thus requiring + no instrumented code, get rid of all the instrumentation + blocks. */ + if (!region->has_instrumented_code) + { + remove_edge_and_dominated_blocks (inst_edge); + + /* The `inst_edge' was the fallthru edge, and we've just + removed it. Use the uninst_edge from here on to make + everybody CFG happy. */ + uninst_edge->flags |= EDGE_FALLTHRU; + + inst_edge = NULL; + } } /* ??? There are plenty of bits here we're not computing. */ @@ -2631,7 +2652,7 @@ expand_transaction (struct tm_region *region, void *data ATTRIBUTE_UNUSED) region->restart_block = region->entry_block; // Generate log restores. - if (!tm_log_save_addresses.is_empty ()) + if (region->has_instrumented_code && !tm_log_save_addresses.is_empty ()) { basic_block test_bb = create_empty_bb (transaction_bb); basic_block code_bb = create_empty_bb (test_bb); @@ -2679,7 +2700,7 @@ expand_transaction (struct tm_region *region, void *data ATTRIBUTE_UNUSED) } // If we have an ABORT edge, create a test to perform the abort. - if (abort_edge) + if (region->has_instrumented_code && abort_edge) { basic_block test_bb = create_empty_bb (transaction_bb); if (current_loops && transaction_bb->loop_father) @@ -2772,7 +2793,8 @@ expand_transaction (struct tm_region *region, void *data ATTRIBUTE_UNUSED) // atomic region that shares the first block. This can cause problems with // the transaction restart abnormal edges to be added in the tm_edges pass. // Solve this by adding a new empty block to receive the abnormal edges. - if (region->restart_block == region->entry_block + if (region->has_instrumented_code + && region->restart_block == region->entry_block && phi_nodes (region->entry_block)) { basic_block empty_bb = create_empty_bb (transaction_bb); @@ -2871,7 +2893,10 @@ execute_tm_mark (void) irrevocable right away. */ if (sub & GTMA_DOES_GO_IRREVOCABLE && sub & GTMA_MAY_ENTER_IRREVOCABLE) - continue; + { + r->has_instrumented_code = false; + continue; + } } expand_block_tm (r, BASIC_BLOCK (i)); } @@ -3047,7 +3072,7 @@ execute_tm_edges (void) unsigned i; FOR_EACH_VEC_ELT (bb_regions, i, r) - if (r != NULL) + if (r != NULL && r->has_instrumented_code) expand_block_edges (r, BASIC_BLOCK (i)); bb_regions.release (); @@ -3741,6 +3766,9 @@ execute_tm_memopt (void) size_t i; basic_block bb; + if (!region->has_instrumented_code) + continue; + bitmap_obstack_initialize (&tm_memopt_obstack); /* Save all BBs for the current region. */