diff mbox

Fix PR ipa/65908.

Message ID 55782471.4020505@suse.cz
State New
Headers show

Commit Message

Martin Liška June 10, 2015, 11:50 a.m. UTC
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

Comments

Martin Liška June 17, 2015, 1:29 p.m. UTC | #1
On 06/10/2015 01:50 PM, Martin Liška wrote:
> 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
> 

As we're planning to release 5.2 soon, I ping the patch.

Martin
diff mbox

Patch

From c878090a910bafec5d1f85a58ec3c3dd5dfc9564 Mon Sep 17 00:00:00 2001
From: mliska <mliska@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  <mliska@suse.cz>

	* g++.dg/ipa/pr65908.C: New test.

gcc/ChangeLog:

2015-05-12  Martin Liska  <mliska@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