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 <symtab_node *, sem_item *> &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 <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::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 <cgraph_node *> (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

Reply via email to