From patchwork Fri May 15 13:17:57 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Martin_Li=C5=A1ka?= X-Patchwork-Id: 472763 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3D77D140309 for ; Fri, 15 May 2015 23:18:11 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=pl4aMgB4; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=vMO6RYc6Y+sZgEYHI JEB4V2ibdjT5f5mQTS5BuGxoT+Z/gwSKocBfRDWuyFqtEjXtM37evnp+xuNmFO2g rIjLA5BZwhYt2x9eIgpEPTUYAcR+odb2lduNvBtxgaogPKi9QuwdJxDM0neVooXb Dvwyj6dgMvT4CBNY0kc9ToOAMs= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=d2i4rSHUb02IHnplHkQcWQQ HJIo=; b=pl4aMgB4Sl3sp637bnwUnNjwjmD3AF+JFbsAVyJDCZQryrooaGP4/CW YyanloncgENzSCipFwI5LzNca66Y2QGRFJzpAh51KJXCvYcBymCXcKi6qzChGBr2 KIzxWIJM1kY43opZHL7kOjRSzQ5nDXEsLBs6Pf/wBY2q/zUmbUUY= Received: (qmail 2147 invoked by alias); 15 May 2015 13:18:04 -0000 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 Received: (qmail 2138 invoked by uid 89); 15 May 2015 13:18:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.6 required=5.0 tests=AWL, BAYES_50, KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2 X-HELO: mx2.suse.de Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Fri, 15 May 2015 13:18:01 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id A1F5CADBD; Fri, 15 May 2015 13:17:57 +0000 (UTC) Message-ID: <5555F205.1060609@suse.cz> Date: Fri, 15 May 2015 15:17:57 +0200 From: =?UTF-8?B?TWFydGluIExpxaFrYQ==?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Jakub Jelinek CC: GCC Patches , Jan Hubicka Subject: Re: [PATCH] Fix PR ipa/65908. References: <5555B0A1.1060702@suse.cz> <20150515103832.GO1751@tucnak.redhat.com> In-Reply-To: <20150515103832.GO1751@tucnak.redhat.com> X-IsSubscribed: yes On 05/15/2015 12:38 PM, Jakub Jelinek wrote: > On Fri, May 15, 2015 at 10:38:57AM +0200, Martin Liška wrote: >> Following patch is fix for GCC-5 branch for PR ipa/65908, was tested on x86_64-linux-pc, as well as bootstrapped. >> As soon as the patch is applied, I'm going to send the similar patch for trunk. > > I'll leave the review to Honza or Richi, just a few nits: > 1) it should go to the trunk first, then to GCC-5 branch > >> --- a/gcc/ipa-icf.c >> +++ b/gcc/ipa-icf.c >> @@ -417,6 +417,33 @@ bool sem_function::compare_edge_flags (cgraph_edge *e1, cgraph_edge *e2) >> return true; >> } >> +/* Return true if DECL_ARGUMENT types are valid to be merged. */ > > Please add another newline between } and the comment. >> --- a/gcc/ipa-icf.h >> +++ b/gcc/ipa-icf.h >> @@ -364,6 +364,9 @@ private: >> bool equals_private (sem_item *item, >> hash_map &ignored_nodes); >> + /* Return true if DECL_ARGUMENT types are valid to be merged. */ >> + bool compatible_parm_types_p (); >> + >> /* Returns true if tree T can be compared as a handled component. */ >> static bool icf_handled_component_p (tree t); > > And here should be an empty line above the Return true if ... comment > too. Though, I wonder how this can apply cleanly to the current 5 branch, > because there is an empty newline in between equals_private and the comment > above icf_handled_component_p. > >> diff --git a/gcc/testsuite/g++.dg/ipa/pr65908.C b/gcc/testsuite/g++.dg/ipa/pr65908.C >> new file mode 100644 >> index 0000000..c1884ab >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/ipa/pr65908.C >> @@ -0,0 +1,26 @@ >> +// PR ipa/65908 >> +// { dg-options "-O2 -fPIC" } >> +// { dg-do compile { target { { fpic } } } } > > Perhaps > // PR ipa/65908 > // { dg-do compile } > // { dg-options "-O2" } > // { dg-additional-options "-fPIC" { target fpic } } > instead? > > Jakub > Hi. Thanks for all nits. I've just bootstrapped reapplied version of the patch, based on current trunk with respect to all changes observed by Jakub. Patch can also survive regression tests. What do you think about it Honza? Thanks, Martin From 22ee6b7aaae73db83663e8b5c12054b2f126a53e Mon Sep 17 00:00:00 2001 From: mliska Date: Fri, 15 May 2015 13:23:33 +0200 Subject: [PATCH] Fix PR ipa/65908. gcc/testsuite/ChangeLog: 2015-05-12 Martin Liska * g++.dg/ipa/pr65908.C: New test. gcc/ChangeLog: 2015-05-12 Martin Liska PR ipa/65908 * ipa-icf.c (sem_function::equals_private): Always compare argument type got from TYPE_ARG_TYPES. (sem_function::compatible_parm_types_p): New function. (sem_function::equals_wpa): Use the function. (sem_function::equals_private): Likewise. (sem_function::parse_tree_args): Handle case where we have a different number of arguments. * ipa-icf.h (sem_function::compatible_parm_types_p): Declare new function. --- gcc/ipa-icf.c | 78 +++++++++++++++++++++----------------- gcc/ipa-icf.h | 3 ++ gcc/testsuite/g++.dg/ipa/pr65908.C | 27 +++++++++++++ 3 files changed, 74 insertions(+), 34 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ipa/pr65908.C diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c index 3c4ac05..5d1881e 100644 --- a/gcc/ipa-icf.c +++ b/gcc/ipa-icf.c @@ -594,6 +594,38 @@ sem_function::param_used_p (unsigned int i) return ipa_is_param_used (IPA_NODE_REF (get_node ()), i); } +/* Return true if DECL_ARGUMENT types are valid to be merged. */ + +bool +sem_function::compatible_parm_types_p () +{ + tree parm1, parm2; + unsigned i = 0; + + for (parm1 = DECL_ARGUMENTS (decl), + parm2 = DECL_ARGUMENTS (m_compared_func->decl); + parm1 && parm2; + parm1 = DECL_CHAIN (parm1), parm2 = DECL_CHAIN (parm2), i++) + { + if (!param_used_p (i)) + continue; + + if (POINTER_TYPE_P (parm1) + && (TYPE_RESTRICT (parm1) != TYPE_RESTRICT (parm2))) + return return_false_with_msg ("argument restrict flag mismatch"); + /* nonnull_arg_p implies non-zero range to REFERENCE types. */ + if (POINTER_TYPE_P (parm1) + && TREE_CODE (parm1) != TREE_CODE (parm2) + && opt_for_fn (decl, flag_delete_null_pointer_checks)) + return return_false_with_msg ("pointer wrt reference mismatch"); + } + + if (parm1 || parm2) + return return_false (); + + return true; +} + /* Fast equality function based on knowledge known in WPA. */ bool @@ -713,22 +745,12 @@ sem_function::equals_wpa (sem_item *item, if (!func_checker::compatible_types_p (arg_types[i], m_compared_func->arg_types[i])) return return_false_with_msg ("argument type is different"); - - /* On used arguments we need to do a bit more of work. */ - if (!param_used_p (i)) - continue; - if (POINTER_TYPE_P (arg_types[i]) - && (TYPE_RESTRICT (arg_types[i]) - != TYPE_RESTRICT (m_compared_func->arg_types[i]))) - return return_false_with_msg ("argument restrict flag mismatch"); - /* nonnull_arg_p implies non-zero range to REFERENCE types. */ - if (POINTER_TYPE_P (arg_types[i]) - && TREE_CODE (arg_types[i]) - != TREE_CODE (m_compared_func->arg_types[i]) - && opt_for_fn (decl, flag_delete_null_pointer_checks)) - return return_false_with_msg ("pointer wrt reference mismatch"); } + /* For functions with !prototype_p, we have to compare DECL_ARGUMENTS. */ + if (!compatible_parm_types_p ()) + return return_false (); + if (node->num_references () != item->node->num_references ()) return return_false_with_msg ("different number of references"); @@ -938,9 +960,12 @@ sem_function::equals_private (sem_item *item) for (arg1 = DECL_ARGUMENTS (decl), arg2 = DECL_ARGUMENTS (m_compared_func->decl); arg1; arg1 = DECL_CHAIN (arg1), arg2 = DECL_CHAIN (arg2)) - if (!m_checker->compare_decl (arg1, arg2)) + if (!arg2 || !m_checker->compare_decl (arg1, arg2)) return return_false (); + if (!compatible_parm_types_p ()) + return return_false (); + if (!dyn_cast (node)->has_gimple_body_p ()) return true; @@ -1709,30 +1734,15 @@ sem_function::parse (cgraph_node *node, bitmap_obstack *stack) void sem_function::parse_tree_args (void) { - tree result; - if (arg_types.exists ()) arg_types.release (); arg_types.create (4); - tree fnargs = DECL_ARGUMENTS (decl); + tree type = TYPE_ARG_TYPES (TREE_TYPE (decl)); + for (tree parm = type; parm; parm = TREE_CHAIN (parm)) + arg_types.safe_push (TREE_VALUE (parm)); - for (tree parm = fnargs; parm; parm = DECL_CHAIN (parm)) - arg_types.safe_push (DECL_ARG_TYPE (parm)); - - /* Function result type. */ - result = DECL_RESULT (decl); - result_type = result ? TREE_TYPE (result) : NULL; - - /* During WPA, we can get arguments by following method. */ - if (!fnargs) - { - tree type = TYPE_ARG_TYPES (TREE_TYPE (decl)); - for (tree parm = type; parm; parm = TREE_CHAIN (parm)) - arg_types.safe_push (TYPE_CANONICAL (TREE_VALUE (parm))); - - result_type = TREE_TYPE (TREE_TYPE (decl)); - } + result_type = TREE_TYPE (TREE_TYPE (decl)); } /* For given basic blocks BB1 and BB2 (from functions FUNC1 and FUNC), diff --git a/gcc/ipa-icf.h b/gcc/ipa-icf.h index a3b9ab9..e6953a4 100644 --- a/gcc/ipa-icf.h +++ b/gcc/ipa-icf.h @@ -382,6 +382,9 @@ private: /* Processes function equality comparison. */ bool equals_private (sem_item *item); + /* Return true if DECL_ARGUMENT types are valid to be merged. */ + bool compatible_parm_types_p (); + /* Returns true if tree T can be compared as a handled component. */ static bool icf_handled_component_p (tree t); diff --git a/gcc/testsuite/g++.dg/ipa/pr65908.C b/gcc/testsuite/g++.dg/ipa/pr65908.C new file mode 100644 index 0000000..38730bd --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/pr65908.C @@ -0,0 +1,27 @@ +// PR ipa/65908 +// { dg-do compile } +// { dg-options "-O2" } +// { dg-additional-options "-fPIC" { target fpic } } + +class A +{ + A (A &); +}; +class B +{ + const A &m_fn1 () const; +}; +class C +{ + A m_fn2 () const; +}; +A +C::m_fn2 () const +{ + throw 0; +} +const A & +B::m_fn1 () const +{ + throw 0; +} -- 2.1.4