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 <[email protected]>
Date: Fri, 15 May 2015 13:23:33 +0200
Subject: [PATCH] Fix PR ipa/65908.
gcc/testsuite/ChangeLog:
2015-05-12 Martin Liska <[email protected]>
* g++.dg/ipa/pr65908.C: New test.
gcc/ChangeLog:
2015-05-12 Martin Liska <[email protected]>
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
