From patchwork Thu Jun 24 22:28:00 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 56865 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 E12B3B707E for ; Fri, 25 Jun 2010 08:25:03 +1000 (EST) Received: (qmail 408 invoked by alias); 24 Jun 2010 22:25:00 -0000 Received: (qmail 399 invoked by uid 22791); 24 Jun 2010 22:24:59 -0000 X-SWARE-Spam-Status: No, hits=-6.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, TW_TM X-Spam-Check-By: sourceware.org Received: from cantor.suse.de (HELO mx1.suse.de) (195.135.220.2) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 24 Jun 2010 22:24:52 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.221.2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.suse.de (Postfix) with ESMTP id DEE0193987; Fri, 25 Jun 2010 00:24:49 +0200 (CEST) Date: Fri, 25 Jun 2010 00:28:00 +0200 From: Martin Jambor To: Richard Guenther Cc: GCC Patches , Jan Hubicka Subject: Re: [PATCH 1/2] Pattern matching improvements in ipa-prop.c Message-ID: <20100624222800.GA2773@alvy.suse.cz> Mail-Followup-To: Richard Guenther , GCC Patches , Jan Hubicka References: <20100621095420.256789766@virgil.suse.cz> <20100621095442.714746858@virgil.suse.cz> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) 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 Hi, On Mon, Jun 21, 2010 at 01:06:46PM +0200, Richard Guenther wrote: > On Mon, 21 Jun 2010, Martin Jambor wrote: > > Instead of changing the existing testcase please add a new one. OK > > Also I'm a bit nervous about > > > if (!gimple_assign_single_p (stmt)) > > - return; > > + continue; > > what is 'arg' usually? Is it a non-SSA name? It must be a structure representing a member pointer. > Why can't it appear > on the lhs of a call? Why can't it appear as operand to an > asm output? Obviously you're right, I don't what I was thinking when writing that. > > Thus, the scanning in determine_cst_member_ptr looks non-conservative > (maybe the followup patch fixes that). I'd have used > > if (!stmt_may_clobber_ref (stmt, arg)) > continue; > > and then > > if (TREE_CODE (lhs) != COMPONENT_REF > || TREE_OPERAND (lhs, 0) != arg) > return; > > Ok with that changes. > I have bootstrapped and tested the following. If there are no objections, I will commit it tomorrow. Thanks for checkin it in detail, this was indeed a stupid mistake, Martin 2010-06-22 Martin Jambor * ipa-prop.c (determine_cst_member_ptr): Ignore non-clobbering statements instead of bailing out on them. Updated comments. (ipa_analyze_indirect_call_uses): Do not require that loads from the parameter are in the same BB as the condition. Update comments. * testsuite/g++.dg/ipa/iinline-2.C: New testcase. Index: icln/gcc/ipa-prop.c =================================================================== --- icln.orig/gcc/ipa-prop.c +++ icln/gcc/ipa-prop.c @@ -781,10 +781,11 @@ get_ssa_def_if_simple_copy (tree rhs) } /* Traverse statements from CALL backwards, scanning whether the argument ARG - which is a member pointer is filled in with constant values. If it is, fill - the jump function JFUNC in appropriately. METHOD_FIELD and DELTA_FIELD are - fields of the record type of the member pointer. To give an example, we - look for a pattern looking like the following: + which is a member pointer and is not addressable is filled in with constant + values. If it is, fill the jump function JFUNC in appropriately. + METHOD_FIELD and DELTA_FIELD are fields of the record type of the member + pointer. To give an example, we look for a pattern looking like the + following: D.2515.__pfn ={v} printStuff; D.2515.__delta ={v} 0; @@ -806,6 +807,8 @@ determine_cst_member_ptr (gimple call, t gimple stmt = gsi_stmt (gsi); tree lhs, rhs, fld; + if (!stmt_may_clobber_ref_p (stmt, arg)) + continue; if (!gimple_assign_single_p (stmt)) return; @@ -814,7 +817,7 @@ determine_cst_member_ptr (gimple call, t if (TREE_CODE (lhs) != COMPONENT_REF || TREE_OPERAND (lhs, 0) != arg) - continue; + return; fld = TREE_OPERAND (lhs, 1); if (!method && fld == method_field) @@ -1030,6 +1033,10 @@ ipa_note_param_call (struct cgraph_node : f$__delta_5 = f.__delta; f$__pfn_24 = f.__pfn; + + ... + + D.2496_3 = (int) f$__pfn_24; D.2497_4 = D.2496_3 & 1; if (D.2497_4 != 0) @@ -1037,7 +1044,7 @@ ipa_note_param_call (struct cgraph_node else goto ; - : + : D.2500_7 = (unsigned int) f$__delta_5; D.2501_8 = &S + D.2500_7; D.2502_9 = (int (*__vtbl_ptr_type) (void) * *) D.2501_8; @@ -1048,7 +1055,7 @@ ipa_note_param_call (struct cgraph_node D.2507_15 = *D.2506_14; iftmp.11_16 = (String:: *) D.2507_15; - : + : # iftmp.11_1 = PHI D.2500_19 = (unsigned int) f$__delta_5; D.2508_20 = &S + D.2500_19; @@ -1109,17 +1116,18 @@ ipa_analyze_indirect_call_uses (struct c d1 = SSA_NAME_DEF_STMT (n1); d2 = SSA_NAME_DEF_STMT (n2); + join = gimple_bb (def); if ((rec = ipa_get_stmt_member_ptr_load_param (d1, false))) { if (ipa_get_stmt_member_ptr_load_param (d2, false)) return; - bb = gimple_bb (d1); + bb = EDGE_PRED (join, 0)->src; virt_bb = gimple_bb (d2); } else if ((rec = ipa_get_stmt_member_ptr_load_param (d2, false))) { - bb = gimple_bb (d2); + bb = EDGE_PRED (join, 1)->src; virt_bb = gimple_bb (d1); } else @@ -1128,7 +1136,6 @@ ipa_analyze_indirect_call_uses (struct c /* Second, we need to check that the basic blocks are laid out in the way corresponding to the pattern. */ - join = gimple_bb (def); if (!single_pred_p (virt_bb) || !single_succ_p (virt_bb) || single_pred (virt_bb) != bb || single_succ (virt_bb) != join) @@ -1138,7 +1145,7 @@ ipa_analyze_indirect_call_uses (struct c significant bit of the pfn. */ branch = last_stmt (bb); - if (gimple_code (branch) != GIMPLE_COND) + if (!bb || gimple_code (branch) != GIMPLE_COND) return; if (gimple_cond_code (branch) != NE_EXPR Index: icln/gcc/testsuite/g++.dg/ipa/iinline-2.C =================================================================== --- /dev/null +++ icln/gcc/testsuite/g++.dg/ipa/iinline-2.C @@ -0,0 +1,61 @@ +/* Verify that simple indirect calls are inlined even without early + inlining.. */ +/* { dg-do compile } */ +/* { dg-options "-O3 -fdump-ipa-inline -fno-early-inlining" } */ +/* { dg-add-options bind_pic_locally } */ + +extern void non_existent (const char *, int); + +class String +{ +private: + const char *data; + +public: + String (const char *d) : data(d) + {} + + int funcOne (int delim) const; + int printStuffTwice (int delim) const; +}; + + +int String::funcOne (int delim) const +{ + int i; + for (i = 0; i < delim; i++) + non_existent(data, i); + + return 1; +} + +extern int global; + +int docalling (int c, int (String::* f)(int delim) const) +{ + String S ("muhehehe"); + + if (c > 2) + global = 3; + else + global = 5; + + return (S.*f)(4); +} + +int __attribute__ ((noinline,noclone)) get_input (void) +{ + return 1; +} + +int main (int argc, char *argv[]) +{ + int i = 0; + while (i < 1000) + i += docalling (get_input (), &String::funcOne); + non_existent ("done", i); + return 0; +} + +/* { dg-final { scan-ipa-dump "String::funcOne\[^\\n\]*inline copy in int main" "inline" } } */ +/* { dg-final { cleanup-ipa-dump "inline" } } */