diff mbox series

[PR,88235] Relax cgraph_node::clone_of_p to also look through former clones

Message ID ri67edc9oqk.fsf@suse.cz
State New
Headers show
Series [PR,88235] Relax cgraph_node::clone_of_p to also look through former clones | expand

Commit Message

Martin Jambor March 6, 2019, 12:22 p.m. UTC
Hi,

PR 88235 is a cgraph verification failure which is a false positive.
The problem is that after thunk expansion which is done as a part of
thunk inlining the verifier is no longer able to detect that a call
graph edge callee points to a clone of the destination of the now
expanded thunk in the decl of the corresponding gimple statement.

Fixed in a way agreed on with Honza in bugzilla, we simply add a way to
detect former (expanded) thunks, I understand this is already done
by devirtualization in a similar way too, and use that information in
the verifier.

Passed bootstrap and testing on x86_64-linux, OK for trunk?  OK for
gcc-7-branch and gcc-8-branch too if a backport is straightforward (I
have not tried yet) and it passes testing there too?

Thanks,

Martin


2019-03-05  Martin Jambor  <mjambor@suse.cz>

	PR ipa/88235
	* cgraph.h (cgraph_node): New inline method former_thunk_p.
	* cgraph.c (cgraph_node::dump): Dump a note if node is a former thunk.
	(clone_of_p): Treat expanded thunks like thunks, be optimistic if they
	have multiple callees.  At the end check if declarations match as
	opposed to cgraph_nodes.

	testsuite/
	* g++.dg/ipa/pr88235.C: New test.
---
 gcc/cgraph.c                       | 15 ++++++--
 gcc/cgraph.h                       | 14 ++++++++
 gcc/testsuite/g++.dg/ipa/pr88235.C | 55 ++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/ipa/pr88235.C

Comments

Jan Hubicka March 7, 2019, 3:38 p.m. UTC | #1
Dne 2019-03-06 13:22, Martin Jambor napsal:
> Hi,
> 
> PR 88235 is a cgraph verification failure which is a false positive.
> The problem is that after thunk expansion which is done as a part of
> thunk inlining the verifier is no longer able to detect that a call
> graph edge callee points to a clone of the destination of the now
> expanded thunk in the decl of the corresponding gimple statement.
> 
> Fixed in a way agreed on with Honza in bugzilla, we simply add a way to
> detect former (expanded) thunks, I understand this is already done
> by devirtualization in a similar way too, and use that information in
> the verifier.
> 
> Passed bootstrap and testing on x86_64-linux, OK for trunk?  OK for
> gcc-7-branch and gcc-8-branch too if a backport is straightforward (I
> have not tried yet) and it passes testing there too?
> 
> Thanks,
> 
> Martin
> 
> 
> 2019-03-05  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR ipa/88235
> 	* cgraph.h (cgraph_node): New inline method former_thunk_p.
> 	* cgraph.c (cgraph_node::dump): Dump a note if node is a former thunk.
> 	(clone_of_p): Treat expanded thunks like thunks, be optimistic if they
> 	have multiple callees.  At the end check if declarations match as
> 	opposed to cgraph_nodes.
> 
> 	testsuite/
> 	* g++.dg/ipa/pr88235.C: New test.

OK,
thanks!
Honza
diff mbox series

Patch

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index de82316d4b1..e5f5a98a0c0 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -2109,6 +2109,8 @@  cgraph_node::dump (FILE *f)
 	       (int)thunk.indirect_offset,
 	       (int)thunk.virtual_offset_p);
     }
+  else if (former_thunk_p ())
+    fprintf (f, "  Former thunk");
   if (alias && thunk.alias
       && DECL_P (thunk.alias))
     {
@@ -2963,7 +2965,9 @@  cgraph_node::collect_callers (void)
   return redirect_callers;
 }
 
-/* Return TRUE if NODE2 a clone of NODE or is equivalent to it.  */
+
+/* Return TRUE if NODE2 a clone of NODE or is equivalent to it.  Return
+   optimistically true if this cannot be determined.  */
 
 static bool
 clone_of_p (cgraph_node *node, cgraph_node *node2)
@@ -2975,12 +2979,17 @@  clone_of_p (cgraph_node *node, cgraph_node *node2)
   /* There are no virtual clones of thunks so check former_clone_of or if we
      might have skipped thunks because this adjustments are no longer
      necessary.  */
-  while (node->thunk.thunk_p)
+  while (node->thunk.thunk_p || node->former_thunk_p ())
     {
       if (node2->former_clone_of == node->decl)
 	return true;
       if (!node->thunk.this_adjusting)
 	return false;
+      /* In case of instrumented expanded thunks, which can have multiple calls
+	 in them, we do not know how to continue and just have to be
+	 optimistic.  */
+      if (node->callees->next_callee)
+	return true;
       node = node->callees->callee->ultimate_alias_target ();
       skipped_thunk = true;
     }
@@ -2996,7 +3005,7 @@  clone_of_p (cgraph_node *node, cgraph_node *node2)
 	return false;
     }
 
-  while (node != node2 && node2)
+  while (node2 && node->decl != node2->decl)
     node2 = node2->clone_of;
   return node2 != NULL;
 }
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index c294602d762..9a19d83fffb 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -1283,6 +1283,9 @@  public:
      Note that at WPA stage, the function body may not be present in memory.  */
   inline bool has_gimple_body_p (void);
 
+  /* Return true if this node represents a former, i.e. an expanded, thunk.  */
+  inline bool former_thunk_p (void);
+
   /* Return true if function should be optimized for size.  */
   bool optimize_for_size_p (void);
 
@@ -2921,6 +2924,17 @@  cgraph_node::has_gimple_body_p (void)
   return definition && !thunk.thunk_p && !alias;
 }
 
+/* Return true if this node represents a former, i.e. an expanded, thunk.  */
+
+inline bool
+cgraph_node::former_thunk_p (void)
+{
+  return (!thunk.thunk_p
+	  && (thunk.fixed_offset
+	      || thunk.virtual_offset_p
+	      || thunk.indirect_offset));
+}
+
 /* Walk all functions with body defined.  */
 #define FOR_EACH_FUNCTION_WITH_GIMPLE_BODY(node) \
    for ((node) = symtab->first_function_with_gimple_body (); (node); \
diff --git a/gcc/testsuite/g++.dg/ipa/pr88235.C b/gcc/testsuite/g++.dg/ipa/pr88235.C
new file mode 100644
index 00000000000..29f3252b828
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr88235.C
@@ -0,0 +1,55 @@ 
+// { dg-do compile }
+// { dg-options "-O1 -fdevirtualize -finline-small-functions -fipa-cp -fipa-cp-clone --param ipa-cp-eval-threshold=125 --param max-inline-insns-single=4" }
+
+extern "C" int printf (const char *, ...);
+enum E { vf_request, vf_event } want;
+
+int errs = 0;
+
+class ivResource {
+public:
+  virtual ~ivResource () { }
+};
+
+class ivHandler   : public ivResource   {
+public:
+  virtual void event() { }
+};
+
+class ivGlyph   : public ivResource   {
+public:
+  virtual ~ivGlyph  () { }
+  virtual void request () {
+    if (want!=vf_request)
+      ++errs;
+  }
+};
+
+class ItemView : public ivGlyph, public ivHandler {
+public:
+  virtual void event () {
+    if (want!=vf_event)
+      ++errs;
+  }
+} a;
+
+ivGlyph *bar() {
+  return &a;
+}
+
+ivHandler *bar2() {
+  return &a;
+}
+
+int main() {
+  want=vf_request;
+  bar()->request();
+  want=vf_event;
+  bar2()->event();
+  if (errs) {
+    printf("FAIL\n");
+    return 1;
+  }
+  printf("PASS\n");
+  return 0;
+}