* [PR 88235] Relax cgraph_node::clone_of_p to also look through former clones
@ 2019-03-06 12:29 Martin Jambor
2019-03-07 16:07 ` Jan Hubicka
0 siblings, 1 reply; 2+ messages in thread
From: Martin Jambor @ 2019-03-06 12:29 UTC (permalink / raw)
To: GCC Patches; +Cc: Jan Hubicka
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
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;
+}
--
2.21.0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PR 88235] Relax cgraph_node::clone_of_p to also look through former clones
2019-03-06 12:29 [PR 88235] Relax cgraph_node::clone_of_p to also look through former clones Martin Jambor
@ 2019-03-07 16:07 ` Jan Hubicka
0 siblings, 0 replies; 2+ messages in thread
From: Jan Hubicka @ 2019-03-07 16:07 UTC (permalink / raw)
To: Martin Jambor; +Cc: GCC Patches, Jan Hubicka
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-03-07 15:38 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-06 12:29 [PR 88235] Relax cgraph_node::clone_of_p to also look through former clones Martin Jambor
2019-03-07 16:07 ` Jan Hubicka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).