On 05/15/2015 08:52 PM, Jan Hubicka wrote: >> +/* Return true if DECL_ARGUMENT types are valid to be merged. */ > Perhaps bettter as > > Perform additional check needed to match types function parameters that are > used. Unlike for normal parameters it matters if type is TYPE_RESTRICT and we > make an assumption that REFERENCE_TYPE parameters are always non-NULL. > >> + >> +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++) > > I think this is still not right. What you wan to to do is to have > > 1) comparible_parm_types_p (t1,t2, index) that returns true if T1 and T2 are > matching with checks bellow: >> + { >> + 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"); >> + } > withtout actually walking the chain. > > 2) make equals_wpa to walk TYPE_ARG_TYPES of the function type and match them. > This is because DECL_ARGUMENTS are part of function body and before you > read it into memory, these are NULL > > Walking DECL_ARGUMENTS here may cause ipa-icf to give up in case one body > is > read (and thus have some arguments) and other is not. > > 3) make equals_private walk DECL_ARGUMENTS and match them > (well at the time you populate the map.) > You probalby can skip matching PARM_DECLS that are !parm_used_p (i) > for anything else than types_compatible_p. > > We only care they are passed the same way by ABI. Everything else is not > relevant. > > Honza >
Hi Honza. I forgot a bit about the patch. Please check updated version of the patch which can boostrap and survives regression tests. Martin
>From c878090a910bafec5d1f85a58ec3c3dd5dfc9564 Mon Sep 17 00:00:00 2001 From: mliska <mli...@suse.cz> Date: Fri, 15 May 2015 13:23:33 +0200 Subject: [PATCH] Fix PR ipa/65908. gcc/testsuite/ChangeLog: 2015-05-12 Martin Liska <mli...@suse.cz> * g++.dg/ipa/pr65908.C: New test. gcc/ChangeLog: 2015-05-12 Martin Liska <mli...@suse.cz> PR ipa/65908 * ipa-icf.c (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 | 73 ++++++++++++++++++++------------------ gcc/ipa-icf.h | 5 +++ gcc/testsuite/g++.dg/ipa/pr65908.C | 27 ++++++++++++++ 3 files changed, 70 insertions(+), 35 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ipa/pr65908.C diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c index d800e1e..8a00428 100644 --- a/gcc/ipa-icf.c +++ b/gcc/ipa-icf.c @@ -583,6 +583,28 @@ sem_function::param_used_p (unsigned int i) return ipa_is_param_used (IPA_NODE_REF (get_node ()), i); } +/* Perform additional check needed to match types function parameters that are +used. Unlike for normal parameters it matters if type is TYPE_RESTRICT and we +make an assumption that REFERENCE_TYPE parameters are always non-NULL. */ + +bool +sem_function::compatible_parm_types_p (tree parm1, tree parm2, unsigned index) +{ + if (!param_used_p (index)) + return true; + + 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"); + + return true; +} + /* Fast equality function based on knowledge known in WPA. */ bool @@ -703,19 +725,9 @@ sem_function::equals_wpa (sem_item *item, 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"); + if (!compatible_parm_types_p (arg_types[i], m_compared_func->arg_types[i], + i)) + return return_false (); } if (node->num_references () != item->node->num_references ()) @@ -924,11 +936,17 @@ sem_function::equals_private (sem_item *item) false, &refs_set, &m_compared_func->refs_set); + unsigned index = 0; 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)) - return return_false (); + arg1; arg1 = DECL_CHAIN (arg1), arg2 = DECL_CHAIN (arg2), index++) + { + if (!arg2 || !m_checker->compare_decl (arg1, arg2)) + return return_false (); + + if (!compatible_parm_types_p (arg1, arg2, index)) + return return_false (); + } if (!dyn_cast <cgraph_node *> (node)->has_gimple_body_p ()) return true; @@ -1698,30 +1716,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..2d78797 100644 --- a/gcc/ipa-icf.h +++ b/gcc/ipa-icf.h @@ -382,6 +382,11 @@ private: /* Processes function equality comparison. */ bool equals_private (sem_item *item); + /* Perform additional check needed to match types function parameters that are + used. Unlike for normal parameters it matters if type is TYPE_RESTRICT and we + make an assumption that REFERENCE_TYPE parameters are always non-NULL. */ + bool compatible_parm_types_p (tree parm1, tree parm2, unsigned index); + /* 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