From patchwork Mon Jun 21 09:54:22 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 56298 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 6F94BB7D48 for ; Mon, 21 Jun 2010 20:12:44 +1000 (EST) Received: (qmail 5765 invoked by alias); 21 Jun 2010 10:12:41 -0000 Received: (qmail 5668 invoked by uid 22791); 21 Jun 2010 10:12:39 -0000 X-SWARE-Spam-Status: No, hits=-3.8 required=5.0 tests=AWL,BAYES_00,TW_SV X-Spam-Check-By: sourceware.org Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 21 Jun 2010 10:12:27 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.221.2]) by mx2.suse.de (Postfix) with ESMTP id A3EDB867E2 for ; Mon, 21 Jun 2010 12:12:24 +0200 (CEST) Resent-From: mjambor@suse.cz Resent-Date: Mon, 21 Jun 2010 12:12:24 +0200 Resent-Message-ID: <20100621101224.GC7456@virgil.arch.suse.de> Resent-To: GCC Patches Message-Id: <20100621095442.846731128@virgil.suse.cz> User-Agent: quilt/0.47-14.9 Date: Mon, 21 Jun 2010 11:54:22 +0200 From: Martin Jambor To: GCC Patches Cc: Richard Guenther , Jan Hubicka Subject: [PATCH 2/2] Replace the modified flag with AA-queries References: <20100621095420.256789766@virgil.suse.cz> Content-Disposition: inline; filename=remove_modified_flag.diff 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, indirect inlining was written before we had AA-info at our disposal in the "early" passes and so when confronted with the necessity to guarantee that a member pointer parameter was not changed before it was used I have resorted to requiring it is never modified in the whole function. With alias analysis information this is no longer necessary. This code therefore does away with the flag and uses AA instead. To big extent I have copied the clobbering identification code from IPA-SRA and hope I got it correct. I added a new testcase checking that we so not indirect-inline when we shouldn't because a member-pointer parameter is modified before being passed on. I am aware that there are no hunks removing testcase gcc.dg/ipa/modif-1.c in the patch, that is because I generate my patches with quilt rather than svn. I will remember to svn-delete the file. I have bootstrapped and tested this patch (on top of the previous one) on x86_64-linux without any issues, OK for trunk? Thanks, Martin 2010-06-18 Martin Jambor * ipa-prop.h (struct ipa_param_descriptor): Remove the modified flag. (ipa_is_param_modified): Removed. * ipa-prop.c (visit_store_addr_for_mod_analysis): Do not set the modified flag. (mark_modified): New function. (is_parm_modified_at_call): Likewise. (compute_pass_through_member_ptrs): Use is_parm_modified_at_call instead of ipa_is_param_modified. (ipa_analyze_indirect_call_uses): Likewise. (ipa_print_node_params): Do not dump the modified flag. (ipa_write_node_info): Do not stream the modified flag. (ipa_read_node_info): Likewise. * testsuite/g++.dg/ipa/iinline-2.C: New test. * testsuite/gcc.dg/ipa/modif-1.c: Removed. Index: icln/gcc/ipa-prop.c =================================================================== --- icln.orig/gcc/ipa-prop.c 2010-06-20 18:23:13.000000000 +0200 +++ icln/gcc/ipa-prop.c 2010-06-20 18:24:32.000000000 +0200 @@ -212,7 +212,6 @@ visit_store_addr_for_mod_analysis (gimpl { int index = ipa_get_param_decl_index (info, op); gcc_assert (index >= 0); - info->params[index].modified = true; info->params[index].used = true; } @@ -707,6 +706,34 @@ type_like_member_ptr_p (tree type, tree return true; } +/* Callback of walk_aliased_vdefs. Flags that it has been invoked to the + boolean variable pointed to by DATA. */ + +static bool +mark_modified (ao_ref *ao ATTRIBUTE_UNUSED, tree vdef ATTRIBUTE_UNUSED, + void *data) +{ + bool *b = (bool *) data; + *b = true; + return true; +} + +/* Return true if the formal parameter PARM might have been modified in this + function before reaching the statement CALL. */ + +static bool +is_parm_modified_at_call (gimple call, tree parm) +{ + bool modified = false; + ao_ref refd; + + ao_ref_init (&refd, parm); + walk_aliased_vdefs (&refd, gimple_vuse (call), mark_modified, + &modified, NULL); + + return modified; +} + /* Go through arguments of the CALL and for every one that looks like a member pointer, check whether it can be safely declared pass-through and if so, mark that to the corresponding item of jump FUNCTIONS. Return true iff @@ -733,7 +760,7 @@ compute_pass_through_member_ptrs (struct int index = ipa_get_param_decl_index (info, arg); gcc_assert (index >=0); - if (!ipa_is_param_modified (info, index)) + if (!is_parm_modified_at_call (call, arg)) { functions[num].type = IPA_JF_PASS_THROUGH; functions[num].value.pass_through.formal_id = index; @@ -1184,7 +1211,7 @@ ipa_analyze_indirect_call_uses (struct c return; index = ipa_get_param_decl_index (info, rec); - if (index >= 0 && !ipa_is_param_modified (info, index)) + if (index >= 0 && !is_parm_modified_at_call (call, rec)) ipa_note_param_call (node, index, call, false); return; @@ -1854,8 +1881,6 @@ ipa_print_node_params (FILE * f, struct (DECL_NAME (temp) ? (*lang_hooks.decl_printable_name) (temp, 2) : "(unnamed)")); - if (ipa_is_param_modified (info, i)) - fprintf (f, " modified"); if (ipa_is_param_used (info, i)) fprintf (f, " used"); fprintf (f, "\n"); @@ -2470,10 +2495,7 @@ ipa_write_node_info (struct output_block gcc_assert (!info->node_enqueued); gcc_assert (!info->ipcp_orig_node); for (j = 0; j < ipa_get_param_count (info); j++) - { - bp_pack_value (&bp, info->params[j].modified, 1); - bp_pack_value (&bp, info->params[j].used, 1); - } + bp_pack_value (&bp, info->params[j].used, 1); lto_output_bitpack (&bp); for (e = node->callees; e; e = e->next_callee) { @@ -2511,10 +2533,7 @@ ipa_read_node_info (struct lto_input_blo } info->node_enqueued = false; for (k = 0; k < ipa_get_param_count (info); k++) - { - info->params[k].modified = bp_unpack_value (&bp, 1); - info->params[k].used = bp_unpack_value (&bp, 1); - } + info->params[k].used = bp_unpack_value (&bp, 1); for (e = node->callees; e; e = e->next_callee) { struct ipa_edge_args *args = IPA_EDGE_REF (e); Index: icln/gcc/ipa-prop.h =================================================================== --- icln.orig/gcc/ipa-prop.h 2010-06-20 17:56:03.000000000 +0200 +++ icln/gcc/ipa-prop.h 2010-06-20 18:23:16.000000000 +0200 @@ -161,8 +161,6 @@ struct ipa_param_descriptor struct ipcp_lattice ipcp_lattice; /* PARAM_DECL of this parameter. */ tree decl; - /* Whether the value parameter has been modified within the function. */ - unsigned modified : 1; /* The parameter is used. */ unsigned used : 1; }; @@ -228,17 +226,6 @@ ipa_get_param (struct ipa_node_params *i return info->params[i].decl; } -/* Return the modification flag corresponding to the Ith formal parameter of - the function associated with INFO. Note that there is no setter method as - the goal is to set all flags when building the array in - ipa_detect_param_modifications. */ - -static inline bool -ipa_is_param_modified (struct ipa_node_params *info, int i) -{ - return info->params[i].modified; -} - /* Return the used flag corresponding to the Ith formal parameter of the function associated with INFO. */ Index: icln/gcc/testsuite/g++.dg/ipa/iinline-2.C =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ icln/gcc/testsuite/g++.dg/ipa/iinline-2.C 2010-06-20 18:23:16.000000000 +0200 @@ -0,0 +1,64 @@ +/* Verify that we do not indirect-inline using member pointer + parameters which have been modified. */ +/* { dg-do run } */ +/* { dg-options "-O3 -fno-early-inlining" } */ +/* { dg-add-options bind_pic_locally } */ + +extern "C" void abort (void); + +class String +{ +private: + const char *data; + +public: + String (const char *d) : data(d) + {} + + int funcOne (int stuff) const; + int funcTwo (int stuff) const; +}; + + +int String::funcOne (int stuff) const +{ + return stuff + 1; +} + +int String::funcTwo (int stuff) const +{ + return stuff + 100; +} + +int (String::* gmp)(int stuff) const = &String::funcTwo; + +int docalling_1 (int (String::* f)(int stuff) const) +{ + String S ("muhehehe"); + + return (S.*f)(4); +} + +int docalling (int a, int (String::* f)(int stuff) const) +{ + if (a < 200) + f = gmp; + + return docalling_1 (f); +} + +int __attribute__ ((noinline,noclone)) get_input (void) +{ + return 1; +} + +int main (int argc, char *argv[]) +{ + int i = 0; + while (i < 10) + i += docalling (get_input (), &String::funcOne); + + if (i != 104) + abort(); + return 0; +}