Re: [PATCH 1/2, PR 63814] Strengthen cgraph_edge_brings_value_p
On Fri, Nov 21, 2014 at 10:59 AM, Martin Jambor mjam...@suse.cz wrote:
Hi,
PR 63814 is caused by cgraph_edge_brings_value_p misidentifying an
edge to an expanded artificial thunk as an edge to the original node,
which then leads to crazy double-cloning and doubling the thunks along
the call.
This patch fixes the bug by strengthening the predicate so that it
knows where the value is supposed to go and can check that it goes
there and not anywhere else. It also adds an extra availability check
that was probably missing in it.
Bootstrapped and tested on x86_64-linux, and i686-linux. OK for
trunk?
Thanks,
Martin
2014-11-20 Martin Jambor mjam...@suse.cz
PR ipa/63814
* ipa-cp.c (same_node_or_its_all_contexts_clone_p): New function.
(cgraph_edge_brings_value_p): New parameter dest, use
same_node_or_its_all_contexts_clone_p and check availability.
(cgraph_edge_brings_value_p): Likewise.
(get_info_about_necessary_edges): New parameter dest, pass it to
cgraph_edge_brings_value_p. Update caller.
(gather_edges_for_value): Likewise.
(perhaps_add_new_callers): Use cgraph_edge_brings_value_p to check
both the destination and availability.
I checked in this testcase:
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index a17cc6c..1410f10 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2014-12-02 H.J. Lu hongjiu...@intel.com
+
+ PR ipa/63814
+ * g++.dg/ipa/pr63814.C: New test.
+
2014-12-02 Wilco Dijkstra wilco.dijks...@arm.com
* gcc.target/aarch64/remat1.c: New testcase.
diff --git a/gcc/testsuite/g++.dg/ipa/pr63814.C
b/gcc/testsuite/g++.dg/ipa/pr63814.C
new file mode 100644
index 000..15a7dda
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr63814.C
@@ -0,0 +1,29 @@
+// { dg-do run { target fpic } }
+// { dg-options -O3 -fpic }
+
+struct CBase {
+ virtual void BaseFunc () {}
+};
+
+struct MMixin {
+ virtual void * MixinFunc (int, int) = 0;
+};
+
+struct CExample: CBase, public MMixin
+{
+ void *MixinFunc (int arg, int arg2)
+ {
+return this;
+ }
+};
+
+void *test (MMixin anExample)
+{
+ return anExample.MixinFunc (0, 0);
+}
+
+int main ()
+{
+ CExample c;
+ return (test (c) != c);
+}
H.J.
Re: [PATCH 1/2, PR 63814] Strengthen cgraph_edge_brings_value_p
Ping.
Thx,
Martin
On Fri, Nov 21, 2014 at 07:59:11PM +0100, Martin Jambor wrote:
Hi,
PR 63814 is caused by cgraph_edge_brings_value_p misidentifying an
edge to an expanded artificial thunk as an edge to the original node,
which then leads to crazy double-cloning and doubling the thunks along
the call.
This patch fixes the bug by strengthening the predicate so that it
knows where the value is supposed to go and can check that it goes
there and not anywhere else. It also adds an extra availability check
that was probably missing in it.
Bootstrapped and tested on x86_64-linux, and i686-linux. OK for
trunk?
Thanks,
Martin
2014-11-20 Martin Jambor mjam...@suse.cz
PR ipa/63814
* ipa-cp.c (same_node_or_its_all_contexts_clone_p): New function.
(cgraph_edge_brings_value_p): New parameter dest, use
same_node_or_its_all_contexts_clone_p and check availability.
(cgraph_edge_brings_value_p): Likewise.
(get_info_about_necessary_edges): New parameter dest, pass it to
cgraph_edge_brings_value_p. Update caller.
(gather_edges_for_value): Likewise.
(perhaps_add_new_callers): Use cgraph_edge_brings_value_p to check
both the destination and availability.
Index: src/gcc/ipa-cp.c
===
--- src.orig/gcc/ipa-cp.c
+++ src/gcc/ipa-cp.c
@@ -2785,17 +2785,31 @@ get_clone_agg_value (struct cgraph_node
return NULL_TREE;
}
-/* Return true if edge CS does bring about the value described by SRC. */
+/* Return true is NODE is DEST or its clone for all contexts. */
static bool
-cgraph_edge_brings_value_p (struct cgraph_edge *cs,
- ipcp_value_sourcetree *src)
+same_node_or_its_all_contexts_clone_p (cgraph_node *node, cgraph_node *dest)
+{
+ if (node == dest)
+return true;
+
+ struct ipa_node_params *info = IPA_NODE_REF (node);
+ return info-is_all_contexts_clone info-ipcp_orig_node == dest;
+}
+
+/* Return true if edge CS does bring about the value described by SRC to node
+ DEST or its clone for all contexts. */
+
+static bool
+cgraph_edge_brings_value_p (cgraph_edge *cs, ipcp_value_sourcetree *src,
+ cgraph_node *dest)
{
struct ipa_node_params *caller_info = IPA_NODE_REF (cs-caller);
- cgraph_node *real_dest = cs-callee-function_symbol ();
- struct ipa_node_params *dst_info = IPA_NODE_REF (real_dest);
+ enum availability availability;
+ cgraph_node *real_dest = cs-callee-function_symbol (availability);
- if ((dst_info-ipcp_orig_node !dst_info-is_all_contexts_clone)
+ if (!same_node_or_its_all_contexts_clone_p (real_dest, dest)
+ || availability = AVAIL_INTERPOSABLE
|| caller_info-node_dead)
return false;
if (!src-val)
@@ -2834,18 +2848,18 @@ cgraph_edge_brings_value_p (struct cgrap
}
}
-/* Return true if edge CS does bring about the value described by SRC. */
+/* Return true if edge CS does bring about the value described by SRC to node
+ DEST or its clone for all contexts. */
static bool
-cgraph_edge_brings_value_p (struct cgraph_edge *cs,
- ipcp_value_sourceipa_polymorphic_call_context
- *src)
+cgraph_edge_brings_value_p (cgraph_edge *cs,
+ ipcp_value_sourceipa_polymorphic_call_context
*src,
+ cgraph_node *dest)
{
struct ipa_node_params *caller_info = IPA_NODE_REF (cs-caller);
cgraph_node *real_dest = cs-callee-function_symbol ();
- struct ipa_node_params *dst_info = IPA_NODE_REF (real_dest);
- if ((dst_info-ipcp_orig_node !dst_info-is_all_contexts_clone)
+ if (!same_node_or_its_all_contexts_clone_p (real_dest, dest)
|| caller_info-node_dead)
return false;
if (!src-val)
@@ -2871,13 +2885,14 @@ get_next_cgraph_edge_clone (struct cgrap
return next_edge_clone[cs-uid];
}
-/* Given VAL, iterate over all its sources and if they still hold, add their
- edge frequency and their number into *FREQUENCY and *CALLER_COUNT
- respectively. */
+/* Given VAL that is intended for DEST, iterate over all its sources and if
+ they still hold, add their edge frequency and their number into *FREQUENCY
+ and *CALLER_COUNT respectively. */
template typename valtype
static bool
-get_info_about_necessary_edges (ipcp_valuevaltype *val, int *freq_sum,
+get_info_about_necessary_edges (ipcp_valuevaltype *val, cgraph_node *dest,
+ int *freq_sum,
gcov_type *count_sum, int *caller_count)
{
ipcp_value_sourcevaltype *src;
@@ -2890,7 +2905,7 @@ get_info_about_necessary_edges (ipcp_val
struct cgraph_edge *cs = src-cs;
while (cs)
{
- if (cgraph_edge_brings_value_p (cs, src))
+ if (cgraph_edge_brings_value_p (cs, src, dest))
{
count++;
Re: [PATCH 1/2, PR 63814] Strengthen cgraph_edge_brings_value_p
Hi, PR 63814 is caused by cgraph_edge_brings_value_p misidentifying an edge to an expanded artificial thunk as an edge to the original node, which then leads to crazy double-cloning and doubling the thunks along the call. This patch fixes the bug by strengthening the predicate so that it knows where the value is supposed to go and can check that it goes there and not anywhere else. It also adds an extra availability check that was probably missing in it. Bootstrapped and tested on x86_64-linux, and i686-linux. OK for trunk? Thanks, Martin 2014-11-20 Martin Jambor mjam...@suse.cz PR ipa/63814 * ipa-cp.c (same_node_or_its_all_contexts_clone_p): New function. (cgraph_edge_brings_value_p): New parameter dest, use same_node_or_its_all_contexts_clone_p and check availability. (cgraph_edge_brings_value_p): Likewise. (get_info_about_necessary_edges): New parameter dest, pass it to cgraph_edge_brings_value_p. Update caller. (gather_edges_for_value): Likewise. (perhaps_add_new_callers): Use cgraph_edge_brings_value_p to check both the destination and availability. OK
[PATCH 1/2, PR 63814] Strengthen cgraph_edge_brings_value_p
Hi,
PR 63814 is caused by cgraph_edge_brings_value_p misidentifying an
edge to an expanded artificial thunk as an edge to the original node,
which then leads to crazy double-cloning and doubling the thunks along
the call.
This patch fixes the bug by strengthening the predicate so that it
knows where the value is supposed to go and can check that it goes
there and not anywhere else. It also adds an extra availability check
that was probably missing in it.
Bootstrapped and tested on x86_64-linux, and i686-linux. OK for
trunk?
Thanks,
Martin
2014-11-20 Martin Jambor mjam...@suse.cz
PR ipa/63814
* ipa-cp.c (same_node_or_its_all_contexts_clone_p): New function.
(cgraph_edge_brings_value_p): New parameter dest, use
same_node_or_its_all_contexts_clone_p and check availability.
(cgraph_edge_brings_value_p): Likewise.
(get_info_about_necessary_edges): New parameter dest, pass it to
cgraph_edge_brings_value_p. Update caller.
(gather_edges_for_value): Likewise.
(perhaps_add_new_callers): Use cgraph_edge_brings_value_p to check
both the destination and availability.
Index: src/gcc/ipa-cp.c
===
--- src.orig/gcc/ipa-cp.c
+++ src/gcc/ipa-cp.c
@@ -2785,17 +2785,31 @@ get_clone_agg_value (struct cgraph_node
return NULL_TREE;
}
-/* Return true if edge CS does bring about the value described by SRC. */
+/* Return true is NODE is DEST or its clone for all contexts. */
static bool
-cgraph_edge_brings_value_p (struct cgraph_edge *cs,
- ipcp_value_sourcetree *src)
+same_node_or_its_all_contexts_clone_p (cgraph_node *node, cgraph_node *dest)
+{
+ if (node == dest)
+return true;
+
+ struct ipa_node_params *info = IPA_NODE_REF (node);
+ return info-is_all_contexts_clone info-ipcp_orig_node == dest;
+}
+
+/* Return true if edge CS does bring about the value described by SRC to node
+ DEST or its clone for all contexts. */
+
+static bool
+cgraph_edge_brings_value_p (cgraph_edge *cs, ipcp_value_sourcetree *src,
+ cgraph_node *dest)
{
struct ipa_node_params *caller_info = IPA_NODE_REF (cs-caller);
- cgraph_node *real_dest = cs-callee-function_symbol ();
- struct ipa_node_params *dst_info = IPA_NODE_REF (real_dest);
+ enum availability availability;
+ cgraph_node *real_dest = cs-callee-function_symbol (availability);
- if ((dst_info-ipcp_orig_node !dst_info-is_all_contexts_clone)
+ if (!same_node_or_its_all_contexts_clone_p (real_dest, dest)
+ || availability = AVAIL_INTERPOSABLE
|| caller_info-node_dead)
return false;
if (!src-val)
@@ -2834,18 +2848,18 @@ cgraph_edge_brings_value_p (struct cgrap
}
}
-/* Return true if edge CS does bring about the value described by SRC. */
+/* Return true if edge CS does bring about the value described by SRC to node
+ DEST or its clone for all contexts. */
static bool
-cgraph_edge_brings_value_p (struct cgraph_edge *cs,
- ipcp_value_sourceipa_polymorphic_call_context
- *src)
+cgraph_edge_brings_value_p (cgraph_edge *cs,
+ ipcp_value_sourceipa_polymorphic_call_context
*src,
+ cgraph_node *dest)
{
struct ipa_node_params *caller_info = IPA_NODE_REF (cs-caller);
cgraph_node *real_dest = cs-callee-function_symbol ();
- struct ipa_node_params *dst_info = IPA_NODE_REF (real_dest);
- if ((dst_info-ipcp_orig_node !dst_info-is_all_contexts_clone)
+ if (!same_node_or_its_all_contexts_clone_p (real_dest, dest)
|| caller_info-node_dead)
return false;
if (!src-val)
@@ -2871,13 +2885,14 @@ get_next_cgraph_edge_clone (struct cgrap
return next_edge_clone[cs-uid];
}
-/* Given VAL, iterate over all its sources and if they still hold, add their
- edge frequency and their number into *FREQUENCY and *CALLER_COUNT
- respectively. */
+/* Given VAL that is intended for DEST, iterate over all its sources and if
+ they still hold, add their edge frequency and their number into *FREQUENCY
+ and *CALLER_COUNT respectively. */
template typename valtype
static bool
-get_info_about_necessary_edges (ipcp_valuevaltype *val, int *freq_sum,
+get_info_about_necessary_edges (ipcp_valuevaltype *val, cgraph_node *dest,
+ int *freq_sum,
gcov_type *count_sum, int *caller_count)
{
ipcp_value_sourcevaltype *src;
@@ -2890,7 +2905,7 @@ get_info_about_necessary_edges (ipcp_val
struct cgraph_edge *cs = src-cs;
while (cs)
{
- if (cgraph_edge_brings_value_p (cs, src))
+ if (cgraph_edge_brings_value_p (cs, src, dest))
{
count++;
freq += cs-frequency;
@@ -2907,12 +2922,13 @@ get_info_about_necessary_edges (ipcp_val
return hot;
}
-/* Return a vector of incoming edges that do
