From patchwork Mon Feb 21 14:29:03 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 83847 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 5BC13B6EEE for ; Tue, 22 Feb 2011 01:29:27 +1100 (EST) Received: (qmail 22216 invoked by alias); 21 Feb 2011 14:29:21 -0000 Received: (qmail 22198 invoked by uid 22791); 21 Feb 2011 14:29:19 -0000 X-SWARE-Spam-Status: No, hits=-6.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, TW_FN, TW_TM, T_RP_MATCHES_RCVD 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, 21 Feb 2011 14:29:06 +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 p1LET48d014168 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 21 Feb 2011 09:29:04 -0500 Received: from houston.quesejoda.com (vpn-224-150.phx2.redhat.com [10.3.224.150]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p1LET3u0014298; Mon, 21 Feb 2011 09:29:04 -0500 Message-ID: <4D6276AF.9040709@redhat.com> Date: Mon, 21 Feb 2011 08:29:03 -0600 From: Aldy Hernandez User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101209 Fedora/3.1.7-0.35.b3pre.fc14 Lightning/1.0b3pre Thunderbird/3.1.7 MIME-Version: 1.0 To: Richard Henderson CC: gcc-patches Subject: Re: [trans-mem] do not go through the run-time for clone calls References: <4D5C026F.8020402@redhat.com> <4D5C243D.2070701@redhat.com> In-Reply-To: <4D5C243D.2070701@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/16/11 13:23, Richard Henderson wrote: > On 02/16/2011 08:59 AM, Aldy Hernandez wrote: >> And now for the actual optimization... >> >> There is no need to go through the run-time when calling a clone. We can just call the clone directly. >> >> OK? > > I think this is a lot closer to correct. In the process I found some bits > that look... odd to say the least. > Funny how we're moving all the logic back into ipa_tm_transform_calls_redirect(). I should've left ipa_tm_transform_calls_1() alone :). Thanks for looking into this. I have tested your patch, and added a changelog entry. Below is the patch I am committing. Aldy * trans-mem.c (ipa_tm_transform_calls_redirect): Optimize for recursive calls inside clones. Abstract the logic from... (ipa_tm_transform_calls_1): ...here. Index: testsuite/gcc.dg/tm/20110216.c =================================================================== --- testsuite/gcc.dg/tm/20110216.c (revision 0) +++ testsuite/gcc.dg/tm/20110216.c (revision 0) @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-fgnu-tm" } */ + +int george; + +__attribute__((transaction_callable)) +void q1() +{ + __transaction [[atomic]] { + george=999; + } + q1(); +} + +/* { dg-final { scan-assembler-not "_ITM_getTMCloneOrIrrevocable" } } */ Index: trans-mem.c =================================================================== --- trans-mem.c (revision 170359) +++ trans-mem.c (working copy) @@ -4294,7 +4294,28 @@ ipa_tm_transform_calls_redirect (struct struct cgraph_edge *e = cgraph_edge (node, stmt); tree fndecl = gimple_call_fndecl (stmt); - /* If there is a replacement, use it, otherwise use the clone. */ + /* For indirect calls, pass the address through the runtime. */ + if (fndecl == NULL) + { + *need_ssa_rename_p |= + ipa_tm_insert_gettmclone_call (node, region, gsi, stmt); + return; + } + + /* If the call is to the TM runtime, do nothing further. */ + if (flags_from_decl_or_type (fndecl) & ECF_TM_OPS) + return; + + /* Fixup recursive calls inside clones. */ + /* ??? Why did cgraph_copy_node_for_versioning update the call edges + for recursion but not update the call statements themselves? */ + if (e->caller == e->callee && DECL_IS_TM_CLONE (current_function_decl)) + { + gimple_call_set_fndecl (stmt, current_function_decl); + return; + } + + /* If there is a replacement, use it. */ fndecl = find_tm_replacement_function (fndecl); if (fndecl) { @@ -4313,6 +4334,11 @@ ipa_tm_transform_calls_redirect (struct We do need to mark these nodes so that we get the proper result in expand_call_tm. */ + /* ??? This seems broken. How is it that we're marking the + CALLEE as may_enter_irr? Surely we should be marking the + CALLER. Also note that find_tm_replacement_function also + contains mappings into the TM runtime, e.g. memcpy. These + we know won't go irrevocable. */ new_node->local.tm_may_enter_irr = 1; } else @@ -4358,27 +4384,12 @@ ipa_tm_transform_calls_1 (struct cgraph_ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) { gimple stmt = gsi_stmt (gsi); - tree fndecl; if (!is_gimple_call (stmt)) continue; if (is_tm_pure_call (stmt)) continue; - fndecl = gimple_call_fndecl (stmt); - - /* For indirect calls, pass the address through the runtime. */ - if (fndecl == NULL) - { - need_ssa_rename |= - ipa_tm_insert_gettmclone_call (node, region, &gsi, stmt); - continue; - } - - /* Don't scan past the end of the transaction. */ - if (is_tm_ending_fndecl (fndecl)) - break; - /* Redirect edges to the appropriate replacement or clone. */ ipa_tm_transform_calls_redirect (node, region, &gsi, &need_ssa_rename); }