From patchwork Wed Feb 27 16:43:10 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 223650 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 9C5D02C007E for ; Thu, 28 Feb 2013 03:43:31 +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=1362588213; 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=/eg+SGJwNPYhWRCSvRhPvNT4STQ=; b=bdbb1C1oaVS6UE9 Cj5/JI2M8kTk3/OOzBCY38dBzSNOqqJr0FJ7xuMnnYuYEk8M0e+59pivWRG6/j6/ kTZ+lAo/5BD7fmbBok01Y+fX/4q/Y+bhRZKSanW8YiYNOHCkfi9F9PdwDb+JboLW EoL1I2FLQLXJl/cmTLlEMwSJezVE= 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=RmEPpd/G3lYcRrvXwopeEoMpDvzypYY8iNC5ClJFRLA2zUGqyWnK7Q4yZoBxsN CERNlU9+Bfkr7EUSTfMBxUUbBv87AGZ+4ZcesuPZtDyZDz+6D/aOUpkilUp1coSY Daeq0R6lavosN8qZkjdblxCb1Zh8cru51EuPJCs8u+lkk=; Received: (qmail 3483 invoked by alias); 27 Feb 2013 16:43:21 -0000 Received: (qmail 3363 invoked by uid 22791); 27 Feb 2013 16:43:19 -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; Wed, 27 Feb 2013 16:43:12 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r1RGhBBJ006311 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 27 Feb 2013 11:43:11 -0500 Received: from houston.quesejoda.com (vpn-52-118.rdu2.redhat.com [10.10.52.118]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r1RGhAUF021815; Wed, 27 Feb 2013 11:43:11 -0500 Message-ID: <512E379E.2060906@redhat.com> Date: Wed, 27 Feb 2013 10:43:10 -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> <512BEB2A.9090007@redhat.com> <512CFDC2.5080508@redhat.com> In-Reply-To: <512CFDC2.5080508@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 On 02/26/13 12:24, Richard Henderson wrote: > On 02/25/2013 02:52 PM, Aldy Hernandez wrote: >> 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? > > No, I shouldn't think so. Inlining doesn't change the decision we made during > IPA_TM about whether or not any one transaction doesGoIrr, which is the *only* > bit that's relevant to eliding the uninstrumented code path during IPA_TM, and > thus should be the only bit that's relevant to deciding that the sole code path > is actually supposed to be instrumented or uninstrumented. If inlining doesn't change anything then doing it at IPA time is definitely cleaner. See attached patch. > I'm not fond of how much extra code and tests this patch is adding. Is it > really really required? Is my analysis above wrong in some way? Well, I know you wanted me to avoid calling ipa_uninstrument_transaction in ipa_tm_scan_calls_transaction, but the problem is that ipa_tm_scan_calls_transaction gets called prior to ipa_tm_transform_transaction which is the one setting the GTMA bits. If you're ok with this revision, I could commit it, and figure something out for eliding the call to ipa_uninstrument_transaction as a followup patch. I'm thinking either: a) Something like the previous patch (which you clearly did not like), where we remove the edges ex post facto. b) Rearrange things somehow so we do ipa_uninstrument_transaction after ipa_tm_scan_calls_transaction. Aldy + * trans-mem.c (expand_transaction): Do not set PR_INSTRUMENTEDCODE + if GTMA_HAS_NO_INSTRUMENTATION. + (generate_tm_state): Keep GTMA_HAS_NO_INSTRUMENTATION bit. + (ipa_tm_transform_transaction): Set GTMA_HAS_NO_INSTRUMENTATION. + * gimple.h (GTMA_HAS_NO_INSTRUMENTATION): Define. + * gimple-pretty-print.c (dump_gimple_transaction): Handle + GTMA_HAS_NO_INSTRUMENTATION. diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c index e7e821d..8c24a57 100644 --- a/gcc/gimple-pretty-print.c +++ b/gcc/gimple-pretty-print.c @@ -1399,6 +1399,11 @@ dump_gimple_transaction (pretty_printer *buffer, gimple gs, int spc, int flags) pp_string (buffer, "GTMA_DOES_GO_IRREVOCABLE "); subcode &= ~GTMA_DOES_GO_IRREVOCABLE; } + if (subcode & GTMA_HAS_NO_INSTRUMENTATION) + { + pp_string (buffer, "GTMA_HAS_NO_INSTRUMENTATION "); + subcode &= ~GTMA_HAS_NO_INSTRUMENTATION; + } if (subcode) pp_printf (buffer, "0x%x ", subcode); pp_string (buffer, "]"); diff --git a/gcc/gimple.h b/gcc/gimple.h index 4bd6b3d..1bbd7d7 100644 --- a/gcc/gimple.h +++ b/gcc/gimple.h @@ -661,6 +661,9 @@ struct GTY(()) gimple_statement_omp_atomic_store { tell the runtime that it should begin the transaction in serial-irrevocable mode. */ #define GTMA_DOES_GO_IRREVOCABLE (1u << 6) +/* The transaction contains no instrumentation code whatsover, most + likely because it is guaranteed to go irrevocable upon entry. */ +#define GTMA_HAS_NO_INSTRUMENTATION (1u << 7) struct GTY(()) gimple_statement_transaction { 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..b0f18b5 100644 --- a/gcc/trans-mem.c +++ b/gcc/trans-mem.c @@ -2602,7 +2602,7 @@ expand_transaction (struct tm_region *region, void *data ATTRIBUTE_UNUSED) flags |= PR_HASNOABORT; if ((subcode & GTMA_HAVE_STORE) == 0) flags |= PR_READONLY; - if (inst_edge) + if (inst_edge && !(subcode & GTMA_HAS_NO_INSTRUMENTATION)) flags |= PR_INSTRUMENTEDCODE; if (uninst_edge) flags |= PR_UNINSTRUMENTEDCODE; @@ -2806,7 +2806,8 @@ generate_tm_state (struct tm_region *region, void *data ATTRIBUTE_UNUSED) if (subcode & GTMA_DOES_GO_IRREVOCABLE) subcode &= (GTMA_DECLARATION_MASK | GTMA_DOES_GO_IRREVOCABLE - | GTMA_MAY_ENTER_IRREVOCABLE); + | GTMA_MAY_ENTER_IRREVOCABLE + | GTMA_HAS_NO_INSTRUMENTATION); else subcode &= GTMA_DECLARATION_MASK; gimple_transaction_set_subcode (region->transaction_stmt, subcode); @@ -5069,8 +5070,9 @@ ipa_tm_transform_transaction (struct cgraph_node *node) && 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); + transaction_subcode_ior (region, GTMA_DOES_GO_IRREVOCABLE + | GTMA_MAY_ENTER_IRREVOCABLE + | GTMA_HAS_NO_INSTRUMENTATION); continue; }