diff mbox

Fix PR ipa/65263

Message ID 54F493A3.7020503@suse.cz
State New
Headers show

Commit Message

Martin Liška March 2, 2015, 4:45 p.m. UTC
Hello.

This is suggested patch for the PR. Tested on x86_64-unknown-linux-pc.

Ready for trunk?
Thanks,
Martin

Comments

Jan Hubicka March 2, 2015, 5:17 p.m. UTC | #1
> 
> 2015-03-01  Martin Liska  <mliska@suse.cz>
> 	    Jan Hubicka   <hubicka@ucw.cz>
> 
> 	PR ipa/65263
> 	* cgraph.c (cgraph_node::has_thunk_p): New function.
> 	* cgraph.h (cgraph_node::has_thunk_p: Likewise.
> 	* ipa-icf.c (redirect_all_callers): Do not redirect thunks.
> 	(sem_function::merge): Assert is changed.

OK, thanks! (assuming it fixes the original ICE reported)
Honza
> 
> gcc/testsuite/ChangeLog:
> 
> 2015-03-01  Martin Liska  <mliska@suse.cz>
> 	    Jan Hubicka   <hubicka@ucw.cz>
> 
> 	* g++.dg/ipa/pr65263.C: New test.
> ---
>  gcc/cgraph.c                       | 12 ++++++++++
>  gcc/cgraph.h                       |  3 +++
>  gcc/ipa-icf.c                      | 27 +++++++++++++++++----
>  gcc/testsuite/g++.dg/ipa/pr65263.C | 49 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 86 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/ipa/pr65263.C
> 
> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> index 5555439..9bae35e 100644
> --- a/gcc/cgraph.c
> +++ b/gcc/cgraph.c
> @@ -3325,4 +3325,16 @@ cgraph_node::call_for_symbol_and_aliases_1 (bool (*callback) (cgraph_node *,
>      }
>    return false;
>  }
> +
> +/* Return true if NODE has thunk.  */
> +
> +bool
> +cgraph_node::has_thunk_p (cgraph_node *node, void *)
> +{
> +  for (cgraph_edge *e = node->callers; e; e = e->next_caller)
> +    if (e->caller->thunk.thunk_p)
> +      return true;
> +  return false;
> +}
> +
>  #include "gt-cgraph.h"
> diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> index ff437cf..82519fa 100644
> --- a/gcc/cgraph.h
> +++ b/gcc/cgraph.h
> @@ -1204,6 +1204,9 @@ public:
>       with (not necessarily cgraph_node (DECL).  */
>    static cgraph_node *create_alias (tree alias, tree target);
>  
> +  /* Return true if NODE has thunk.  */
> +  static bool has_thunk_p (cgraph_node *node, void *);
> +
>    cgraph_edge *callees;
>    cgraph_edge *callers;
>    /* List of edges representing indirect calls with a yet undetermined
> diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
> index 31fcbec..155b96b 100644
> --- a/gcc/ipa-icf.c
> +++ b/gcc/ipa-icf.c
> @@ -697,12 +697,22 @@ redirect_all_callers (cgraph_node *n, cgraph_node *to)
>  {
>    int nredirected = 0;
>    ipa_ref *ref;
> +  cgraph_edge *e = n->callers;
>  
> -  while (n->callers)
> +  while (e)
>      {
> -      cgraph_edge *e = n->callers;
> -      e->redirect_callee (to);
> -      nredirected++;
> +      /* Redirecting thunks to interposable symbols or symbols in other sections
> +	 may not be supported by target output code.  Play safe for now and
> +	 punt on redirection.  */
> +      if (!e->caller->thunk.thunk_p)
> +	{
> +	  struct cgraph_edge *nexte = e->next_caller;
> +          e->redirect_callee (to);
> +	  e = nexte;
> +          nredirected++;
> +	}
> +      else
> +	e = e->next_callee;
>      }
>    for (unsigned i = 0; n->iterate_direct_aliases (i, ref);)
>      {
> @@ -717,6 +727,8 @@ redirect_all_callers (cgraph_node *n, cgraph_node *to)
>  	{
>  	  nredirected += redirect_all_callers (n_alias, to);
>  	  if (n_alias->can_remove_if_no_direct_calls_p ()
> +	      && !n_alias->call_for_symbol_and_aliases (cgraph_node::has_thunk_p,
> +							NULL, true)
>  	      && !n_alias->has_aliases_p ())
>  	    n_alias->remove ();
>  	}
> @@ -907,6 +919,8 @@ sem_function::merge (sem_item *alias_item)
>  	  return false;
>  	}
>        if (!create_wrapper
> +	  && !alias->call_for_symbol_and_aliases (cgraph_node::has_thunk_p,
> +						  NULL, true)
>  	  && !alias->can_remove_if_no_direct_calls_p ())
>  	{
>  	  if (dump_file)
> @@ -975,7 +989,10 @@ sem_function::merge (sem_item *alias_item)
>        if (dump_file)
>  	fprintf (dump_file, "Unified; Wrapper has been created.\n\n");
>      }
> -  gcc_assert (alias->icf_merged || remove);
> +
> +  /* It's possible that redirection can hit thunks that block
> +     redirection opportunities.  */
> +  gcc_assert (alias->icf_merged || remove || redirect_callers);
>    original->icf_merged = true;
>  
>    /* Inform the inliner about cross-module merging.  */
> diff --git a/gcc/testsuite/g++.dg/ipa/pr65263.C b/gcc/testsuite/g++.dg/ipa/pr65263.C
> new file mode 100644
> index 0000000..34459a2
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ipa/pr65263.C
> @@ -0,0 +1,49 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -c -w" } */
> +
> +template <class> class A;
> +template <class R> struct VirtualMatrice {
> +  virtual bool m_fn1(int) const { return true; }
> +  struct B {
> +    A<R> x;
> +    B(VirtualMatrice *p1, A<R> p2) : x(p2) { p1->m_fn1(0) ?: throw; }
> +  };
> +  void operator*(A<R> p1) { B(this, p1); }
> +  ~VirtualMatrice();
> +}
> +;
> +template <class> class A {
> +public:
> +  operator int *();
> +  A(int *, long);
> +};
> +
> +class G : public A<int> {
> +public:
> +  G(long);
> +};
> +int typedef Complex;
> +template <class> class H : VirtualMatrice<int> {};
> +template <class> class C;
> +template <> class C<int> : H<Complex>, VirtualMatrice<Complex> {
> +  bool m_fn1(int) const { return true; }
> +};
> +template <class K, class Mat>
> +void DoIdoAction(int, int, A<K> p3, A<K>, A<K>, A<K>, Mat, Mat &p8) {
> +  p8 *p3;
> +}
> +
> +class D {
> +  typedef int K;
> +  class F {
> +    int operator()() const;
> +  };
> +};
> +int D::F::operator()() const {
> +  VirtualMatrice<K> *a;
> +  VirtualMatrice<K> b, &B = *a;
> +  G c(0), g(1);
> +  int d, e, f;
> +  A<K> h(&g[f], 0), i(&g[e], 0), j(&g[d], 0);
> +  DoIdoAction(0, 3, h, i, j, c, b, B);
> +}
> -- 
> 2.1.2
>
diff mbox

Patch

From c0f86505df4519b555f110d8c9d320c17b9fcfd9 Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Mon, 2 Mar 2015 14:15:30 +0100
Subject: [PATCH] Fix PR ipa/65263.

gcc/ChangeLog:

2015-03-01  Martin Liska  <mliska@suse.cz>
	    Jan Hubicka   <hubicka@ucw.cz>

	PR ipa/65263
	* cgraph.c (cgraph_node::has_thunk_p): New function.
	* cgraph.h (cgraph_node::has_thunk_p: Likewise.
	* ipa-icf.c (redirect_all_callers): Do not redirect thunks.
	(sem_function::merge): Assert is changed.

gcc/testsuite/ChangeLog:

2015-03-01  Martin Liska  <mliska@suse.cz>
	    Jan Hubicka   <hubicka@ucw.cz>

	* g++.dg/ipa/pr65263.C: New test.
---
 gcc/cgraph.c                       | 12 ++++++++++
 gcc/cgraph.h                       |  3 +++
 gcc/ipa-icf.c                      | 27 +++++++++++++++++----
 gcc/testsuite/g++.dg/ipa/pr65263.C | 49 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 86 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/ipa/pr65263.C

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 5555439..9bae35e 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -3325,4 +3325,16 @@  cgraph_node::call_for_symbol_and_aliases_1 (bool (*callback) (cgraph_node *,
     }
   return false;
 }
+
+/* Return true if NODE has thunk.  */
+
+bool
+cgraph_node::has_thunk_p (cgraph_node *node, void *)
+{
+  for (cgraph_edge *e = node->callers; e; e = e->next_caller)
+    if (e->caller->thunk.thunk_p)
+      return true;
+  return false;
+}
+
 #include "gt-cgraph.h"
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index ff437cf..82519fa 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -1204,6 +1204,9 @@  public:
      with (not necessarily cgraph_node (DECL).  */
   static cgraph_node *create_alias (tree alias, tree target);
 
+  /* Return true if NODE has thunk.  */
+  static bool has_thunk_p (cgraph_node *node, void *);
+
   cgraph_edge *callees;
   cgraph_edge *callers;
   /* List of edges representing indirect calls with a yet undetermined
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 31fcbec..155b96b 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -697,12 +697,22 @@  redirect_all_callers (cgraph_node *n, cgraph_node *to)
 {
   int nredirected = 0;
   ipa_ref *ref;
+  cgraph_edge *e = n->callers;
 
-  while (n->callers)
+  while (e)
     {
-      cgraph_edge *e = n->callers;
-      e->redirect_callee (to);
-      nredirected++;
+      /* Redirecting thunks to interposable symbols or symbols in other sections
+	 may not be supported by target output code.  Play safe for now and
+	 punt on redirection.  */
+      if (!e->caller->thunk.thunk_p)
+	{
+	  struct cgraph_edge *nexte = e->next_caller;
+          e->redirect_callee (to);
+	  e = nexte;
+          nredirected++;
+	}
+      else
+	e = e->next_callee;
     }
   for (unsigned i = 0; n->iterate_direct_aliases (i, ref);)
     {
@@ -717,6 +727,8 @@  redirect_all_callers (cgraph_node *n, cgraph_node *to)
 	{
 	  nredirected += redirect_all_callers (n_alias, to);
 	  if (n_alias->can_remove_if_no_direct_calls_p ()
+	      && !n_alias->call_for_symbol_and_aliases (cgraph_node::has_thunk_p,
+							NULL, true)
 	      && !n_alias->has_aliases_p ())
 	    n_alias->remove ();
 	}
@@ -907,6 +919,8 @@  sem_function::merge (sem_item *alias_item)
 	  return false;
 	}
       if (!create_wrapper
+	  && !alias->call_for_symbol_and_aliases (cgraph_node::has_thunk_p,
+						  NULL, true)
 	  && !alias->can_remove_if_no_direct_calls_p ())
 	{
 	  if (dump_file)
@@ -975,7 +989,10 @@  sem_function::merge (sem_item *alias_item)
       if (dump_file)
 	fprintf (dump_file, "Unified; Wrapper has been created.\n\n");
     }
-  gcc_assert (alias->icf_merged || remove);
+
+  /* It's possible that redirection can hit thunks that block
+     redirection opportunities.  */
+  gcc_assert (alias->icf_merged || remove || redirect_callers);
   original->icf_merged = true;
 
   /* Inform the inliner about cross-module merging.  */
diff --git a/gcc/testsuite/g++.dg/ipa/pr65263.C b/gcc/testsuite/g++.dg/ipa/pr65263.C
new file mode 100644
index 0000000..34459a2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr65263.C
@@ -0,0 +1,49 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O3 -c -w" } */
+
+template <class> class A;
+template <class R> struct VirtualMatrice {
+  virtual bool m_fn1(int) const { return true; }
+  struct B {
+    A<R> x;
+    B(VirtualMatrice *p1, A<R> p2) : x(p2) { p1->m_fn1(0) ?: throw; }
+  };
+  void operator*(A<R> p1) { B(this, p1); }
+  ~VirtualMatrice();
+}
+;
+template <class> class A {
+public:
+  operator int *();
+  A(int *, long);
+};
+
+class G : public A<int> {
+public:
+  G(long);
+};
+int typedef Complex;
+template <class> class H : VirtualMatrice<int> {};
+template <class> class C;
+template <> class C<int> : H<Complex>, VirtualMatrice<Complex> {
+  bool m_fn1(int) const { return true; }
+};
+template <class K, class Mat>
+void DoIdoAction(int, int, A<K> p3, A<K>, A<K>, A<K>, Mat, Mat &p8) {
+  p8 *p3;
+}
+
+class D {
+  typedef int K;
+  class F {
+    int operator()() const;
+  };
+};
+int D::F::operator()() const {
+  VirtualMatrice<K> *a;
+  VirtualMatrice<K> b, &B = *a;
+  G c(0), g(1);
+  int d, e, f;
+  A<K> h(&g[f], 0), i(&g[e], 0), j(&g[d], 0);
+  DoIdoAction(0, 3, h, i, j, c, b, B);
+}
-- 
2.1.2