* [PATCH] Generalized value pass-through for self-recursive function (ipa/pr93203)
@ 2020-01-25 13:52 Feng Xue OS
2020-01-25 16:17 ` [PATCH V2] " Feng Xue OS
0 siblings, 1 reply; 17+ messages in thread
From: Feng Xue OS @ 2020-01-25 13:52 UTC (permalink / raw)
To: mjambor, Jan Hubicka, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1897 bytes --]
Besides simple pass-through (aggregate) jump function, arithmetic (aggregate)
jump function could also bring same (aggregate) value as parameter passed-in
for self-feeding recursive call. For example,
f1 (int i) /* normal jump function */
{
f1 (i & 1);
}
Suppose i is 0, recursive propagation via (i & 1) also gets 0, which
can be seen as a simple pass-through of i.
f2 (int *p) /* aggregate jump function */
{
int t = *p & 1;
f2 (&t);
}
Likewise, if *p is 0, (*p & 1) is also 0, and &t is an aggregate simple
pass-through of p.
This patch is to support these two kinds of value pass-through.
Bootstrapped/regtested on x86_64-linux and aarch64-linux.
Feng
---
2020-01-25 Feng Xue <fxue@os.amperecomputing.com>
PR ipa/93203
* ipa-cp.c (ipcp_lattice::add_value): Add source with same call
edge but different source value.
(adjust_callers_for_value_intersection): New function.
(gather_edges_for_value): Adjust order of callers to let a
non-self-recursive caller be the first element.
(self_recursive_pass_through_p): Add a new parameter simple, and
check generalized self-recursive pass-through jump function.
(self_recursive_agg_pass_through_p): Likewise.
(find_more_scalar_values_for_callers_subset): Compute value from
pass-through jump function for self-recursive.
(intersect_with_plats): Remove code of itersection with unknown
place holder value.
(intersect_with_agg_replacements): Likewise.
(intersect_aggregates_with_edge): Deduce with from pass-through
jump function for self-recursive.
(decide_whether_version_node): Remove dead callers and adjust
order to let a non-self-recursive caller be the first element.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Generalized-value-pass-through-for-self-recusive-fun.patch --]
[-- Type: text/x-patch; name="0001-Generalized-value-pass-through-for-self-recusive-fun.patch", Size: 14238 bytes --]
From 406c23711077c0df18d5e77270d0a82be098224b Mon Sep 17 00:00:00 2001
From: Feng Xue <fxue@os.amperecomputing.com>
Date: Tue, 21 Jan 2020 20:53:38 +0800
Subject: [PATCH] Generalized value pass-through for self-recusive function
---
gcc/ipa-cp.c | 196 ++++++++++++++++++-----------
gcc/testsuite/g++.dg/ipa/pr93203.C | 95 ++++++++++++++
gcc/testsuite/gcc.dg/ipa/ipcp-1.c | 2 +-
3 files changed, 217 insertions(+), 76 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/ipa/pr93203.C
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 17da1d8e8a7..533a429ba3b 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1850,7 +1850,7 @@ ipcp_lattice<valtype>::add_value (valtype newval, cgraph_edge *cs,
{
ipcp_value_source<valtype> *s;
for (s = val->sources; s; s = s->next)
- if (s->cs == cs)
+ if (s->cs == cs && s->val == src_val)
break;
if (s)
return false;
@@ -4207,6 +4207,33 @@ get_info_about_necessary_edges (ipcp_value<valtype> *val, cgraph_node *dest,
return hot;
}
+/* Given a NODE, and a set of its CALLERS, try to adjust order of the callers
+ to let a non-self-recursive caller be the first element. Thus, we can
+ simplify intersecting operations on values that arrive from all of these
+ callers, especially when there exists self-recursive call. Return true if
+ this kind of adjustment is possible. */
+
+static bool
+adjust_callers_for_value_intersection (vec<cgraph_edge *> callers,
+ cgraph_node *node)
+{
+ for (unsigned i = 0; i < callers.length (); i++)
+ {
+ cgraph_edge *cs = callers[i];
+
+ if (cs->caller != node)
+ {
+ if (i > 0)
+ {
+ callers[i] = callers[0];
+ callers[0] = cs;
+ }
+ return true;
+ }
+ }
+ return false;
+}
+
/* Return a vector of incoming edges that do bring value VAL to node DEST. It
is assumed their number is known and equal to CALLER_COUNT. */
@@ -4230,6 +4257,9 @@ gather_edges_for_value (ipcp_value<valtype> *val, cgraph_node *dest,
}
}
+ if (caller_count > 1)
+ adjust_callers_for_value_intersection (ret, dest);
+
return ret;
}
@@ -4241,7 +4271,6 @@ get_replacement_map (class ipa_node_params *info, tree value, int parm_num)
{
struct ipa_replace_map *replace_map;
-
replace_map = ggc_alloc<ipa_replace_map> ();
if (dump_file)
{
@@ -4592,36 +4621,40 @@ create_specialized_node (struct cgraph_node *node,
}
/* Return true, if JFUNC, which describes a i-th parameter of call CS, is a
- simple no-operation pass-through function to itself. */
+ pass-through function to itself. When SIMPLE is true, further check if
+ JFUNC is a simple no-operation pass-through. */
static bool
-self_recursive_pass_through_p (cgraph_edge *cs, ipa_jump_func *jfunc, int i)
+self_recursive_pass_through_p (cgraph_edge *cs, ipa_jump_func *jfunc, int i,
+ bool simple = true)
{
enum availability availability;
if (cs->caller == cs->callee->function_symbol (&availability)
&& availability > AVAIL_INTERPOSABLE
&& jfunc->type == IPA_JF_PASS_THROUGH
- && ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR
+ && (!simple || ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
&& ipa_get_jf_pass_through_formal_id (jfunc) == i)
return true;
return false;
}
/* Return true, if JFUNC, which describes a part of an aggregate represented
- or pointed to by the i-th parameter of call CS, is a simple no-operation
- pass-through function to itself. */
+ or pointed to by the i-th parameter of call CS, is a pass-through function
+ to itself. When SIMPLE is true, further check if JFUNC is a simple
+ no-operation pass-through. */
static bool
self_recursive_agg_pass_through_p (cgraph_edge *cs, ipa_agg_jf_item *jfunc,
- int i)
+ int i, bool simple = true)
{
enum availability availability;
if (cs->caller == cs->callee->function_symbol (&availability)
&& availability > AVAIL_INTERPOSABLE
&& jfunc->jftype == IPA_JF_LOAD_AGG
&& jfunc->offset == jfunc->value.load_agg.offset
- && jfunc->value.pass_through.operation == NOP_EXPR
- && jfunc->value.pass_through.formal_id == i)
+ && (!simple || jfunc->value.pass_through.operation == NOP_EXPR)
+ && jfunc->value.pass_through.formal_id == i
+ && useless_type_conversion_p (jfunc->value.load_agg.type, jfunc->type))
return true;
return false;
}
@@ -4653,9 +4686,6 @@ find_more_scalar_values_for_callers_subset (struct cgraph_node *node,
struct ipa_jump_func *jump_func;
tree t;
- if (IPA_NODE_REF (cs->caller) && IPA_NODE_REF (cs->caller)->node_dead)
- continue;
-
if (!IPA_EDGE_REF (cs)
|| i >= ipa_get_cs_argument_count (IPA_EDGE_REF (cs))
|| (i == 0
@@ -4665,10 +4695,30 @@ find_more_scalar_values_for_callers_subset (struct cgraph_node *node,
break;
}
jump_func = ipa_get_ith_jump_func (IPA_EDGE_REF (cs), i);
- if (self_recursive_pass_through_p (cs, jump_func, i))
- continue;
- t = ipa_value_from_jfunc (IPA_NODE_REF (cs->caller), jump_func, type);
+ /* Besides simple pass-through jump function, arithmetic jump
+ function could also introduce argument-direct-pass-through for
+ self-feeding recursive call. For example,
+
+ fn (int i)
+ {
+ fn (i & 1);
+ }
+
+ Given that i is 0, recursive propagation via (i & 1) also gets
+ 0. */
+ if (self_recursive_pass_through_p (cs, jump_func, i, false))
+ {
+ gcc_assert (newval);
+ t = ipa_get_jf_arith_result (
+ ipa_get_jf_pass_through_operation (jump_func),
+ newval,
+ ipa_get_jf_pass_through_operand (jump_func),
+ type);
+ }
+ else
+ t = ipa_value_from_jfunc (IPA_NODE_REF (cs->caller), jump_func,
+ type);
if (!t
|| (newval
&& !values_equal_for_ipcp_p (t, newval))
@@ -4817,19 +4867,12 @@ intersect_with_plats (class ipcp_param_lattices *plats,
break;
if (aglat->offset - offset == item->offset)
{
- gcc_checking_assert (item->value);
if (aglat->is_single_const ())
{
tree value = aglat->values->value;
if (values_equal_for_ipcp_p (item->value, value))
found = true;
- else if (item->value == error_mark_node)
- {
- /* Replace unknown place holder value with real one. */
- item->value = value;
- found = true;
- }
}
break;
}
@@ -4898,12 +4941,6 @@ intersect_with_agg_replacements (struct cgraph_node *node, int index,
{
if (values_equal_for_ipcp_p (item->value, av->value))
found = true;
- else if (item->value == error_mark_node)
- {
- /* Replace place holder value with real one. */
- item->value = av->value;
- found = true;
- }
break;
}
}
@@ -5008,31 +5045,16 @@ intersect_aggregates_with_edge (struct cgraph_edge *cs, int index,
for (unsigned i = 0; i < jfunc->agg.items->length (); i++)
{
struct ipa_agg_jf_item *agg_item = &(*jfunc->agg.items)[i];
- struct ipa_agg_value agg_value;
-
- if (self_recursive_agg_pass_through_p (cs, agg_item, index))
+ tree value = ipa_agg_value_from_node (caller_info, cs->caller,
+ agg_item);
+ if (value)
{
- /* For a self-recursive call, if aggregate jump function is a
- simple pass-through, the exact value that it stands for is
- not known at this point, which must comes from other call
- sites. But we still need to add a place holder in value
- sets to indicate it, here we use error_mark_node to
- represent the special unknown value, which will be replaced
- with real one during later intersecting operations. */
- agg_value.value = error_mark_node;
- }
- else
- {
- tree value = ipa_agg_value_from_node (caller_info, cs->caller,
- agg_item);
- if (!value)
- continue;
+ struct ipa_agg_value agg_value;
agg_value.value = value;
+ agg_value.offset = agg_item->offset;
+ inter.safe_push (agg_value);
}
-
- agg_value.offset = agg_item->offset;
- inter.safe_push (agg_value);
}
else
FOR_EACH_VEC_ELT (inter, k, item)
@@ -5053,25 +5075,32 @@ intersect_aggregates_with_edge (struct cgraph_edge *cs, int index,
{
tree value;
- if (self_recursive_agg_pass_through_p (cs, ti, index))
- {
- /* A simple aggregate pass-through in self-recursive
- call should lead to same value. */
- found = true;
- }
- else if ((value = ipa_agg_value_from_node (caller_info,
- cs->caller, ti)))
- {
- if (values_equal_for_ipcp_p (item->value, value))
- found = true;
- else if (item->value == error_mark_node)
- {
- /* Replace unknown place holder value with real
- one. */
- item->value = value;
- found = true;
- }
- }
+ /* Besides simple pass-through aggregate jump function,
+ arithmetic aggregate jump function could also bring
+ same aggregate value as parameter passed-in for
+ self-feeding recursive call. For example,
+
+ fn (int *i)
+ {
+ int j = *i & 1;
+ fn (&j);
+ }
+
+ Given that *i is 0, recursive propagation via (*i & 1)
+ also gets 0. */
+ if (self_recursive_agg_pass_through_p (cs, ti, index,
+ false))
+ value = ipa_get_jf_arith_result (
+ ti->value.pass_through.operation,
+ item->value,
+ ti->value.pass_through.operand,
+ ti->type);
+ else
+ value = ipa_agg_value_from_node (caller_info,
+ cs->caller, ti);
+
+ if (value && values_equal_for_ipcp_p (item->value, value))
+ found = true;
break;
}
l++;
@@ -5147,9 +5176,6 @@ find_aggregate_values_for_callers_subset (struct cgraph_node *node,
if (!item->value)
continue;
- /* All values must be real values, not unknown place holders. */
- gcc_assert (item->value != error_mark_node);
-
v = ggc_alloc<ipa_agg_replacement_value> ();
v->index = i;
v->offset = item->offset;
@@ -5488,6 +5514,7 @@ decide_whether_version_node (struct cgraph_node *node)
int i, count = ipa_get_param_count (info);
vec<tree> known_csts;
vec<ipa_polymorphic_call_context> known_contexts;
+ vec<cgraph_edge *> callers;
bool ret = false;
if (count == 0)
@@ -5542,16 +5569,36 @@ decide_whether_version_node (struct cgraph_node *node)
info = IPA_NODE_REF (node);
}
+ if (info->do_clone_for_all_contexts)
+ {
+ callers = node->collect_callers ();
+
+ for (unsigned i = 0; i < callers.length (); i++)
+ {
+ cgraph_edge *cs = callers[i];
+ class ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
+
+ if (caller_info && caller_info->node_dead)
+ callers.unordered_remove (i);
+ }
+
+ if (!adjust_callers_for_value_intersection (callers, node))
+ {
+ /* If all caller edges of node are self-recursive, the node
+ is not really be in use, no need to do cloning. */
+ callers.release ();
+ info->do_clone_for_all_contexts = false;
+ }
+ }
+
if (info->do_clone_for_all_contexts)
{
struct cgraph_node *clone;
- vec<cgraph_edge *> callers;
if (dump_file)
fprintf (dump_file, " - Creating a specialized node of %s "
"for all known contexts.\n", node->dump_name ());
- callers = node->collect_callers ();
find_more_scalar_values_for_callers_subset (node, known_csts, callers);
find_more_contexts_for_caller_subset (node, &known_contexts, callers);
ipa_agg_replacement_value *aggvals
@@ -5564,7 +5611,6 @@ decide_whether_version_node (struct cgraph_node *node)
}
clone = create_specialized_node (node, known_csts, known_contexts,
aggvals, callers);
- info = IPA_NODE_REF (node);
info->do_clone_for_all_contexts = false;
IPA_NODE_REF (clone)->is_all_contexts_clone = true;
ret = true;
diff --git a/gcc/testsuite/g++.dg/ipa/pr93203.C b/gcc/testsuite/g++.dg/ipa/pr93203.C
new file mode 100644
index 00000000000..b4cd69001b5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr93203.C
@@ -0,0 +1,95 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -w -std=gnu++11" } */
+
+class a {
+public:
+ a(char *);
+};
+class ad {
+public:
+ ad(a *);
+};
+class b {};
+using ah = class ak {
+ using al = char;
+
+public:
+ ak(b) : ak(0) {}
+ ak an() { return ap & 1; }
+ al ap;
+ ak(al af) : ap(af) {}
+};
+struct at {
+ ah au;
+ int av;
+ char aw;
+ char ax;
+};
+class az {
+public:
+ struct ba {
+ void bb(ak am) {
+ ak bc = am.an();
+ bb(bc);
+ }
+ };
+ void bd(ak am) { be.bb(am); }
+ ba be;
+};
+class bg {
+public:
+ int bh;
+ at bi;
+};
+int bj;
+int *bk;
+int c;
+class bl {
+ bool bm();
+ bg bn;
+ az bo;
+ int e;
+ int bq;
+};
+class bs {
+public:
+ bs(int *, ah *, char *, char *, int);
+};
+template < typename bt > class bu : bs {
+ using bv = typename bt::bv;
+
+public:
+ template < typename... bx >
+ bu(a *by, int *bz, at body, bx...)
+ : bs(bz, &body.au, &body.aw, &body.ax, body.av), ca(bx()...), cb(by),
+ cc(by), cd(by), ce(by) {}
+ void cf() {
+ auto cg = ch();
+ auto ci = *cj();
+ ca.ck(this, cg, &ci);
+ }
+ bt ca;
+ ad cb;
+ ad cc;
+ ad cd;
+ ad ce;
+ bv *cj();
+ bv ch();
+};
+class cl {
+public:
+ using bv = struct {};
+ cl(az *, int, int, int, int, a *, int, int **);
+ void ck(bs *, bv, bv *) {
+ b d;
+ ak ci(d);
+ bo.bd(ci);
+ }
+ az bo;
+};
+bool bl::bm() {
+ a by("");
+ bu< cl > cn(&by, &bj, bn.bi, &bo, c, bn.bh, e, 0, &by, bq, &bk);
+ cn.cf();
+}
+
diff --git a/gcc/testsuite/gcc.dg/ipa/ipcp-1.c b/gcc/testsuite/gcc.dg/ipa/ipcp-1.c
index 952694d302b..baa9c97ffb0 100644
--- a/gcc/testsuite/gcc.dg/ipa/ipcp-1.c
+++ b/gcc/testsuite/gcc.dg/ipa/ipcp-1.c
@@ -45,7 +45,7 @@ main (int argc, char *argv[])
}
-/* { dg-final { scan-ipa-dump "Creating a specialized node of f.*for all known contexts" "cp" } } */
+/* { dg-final { scan-ipa-dump "Creating a specialized node of f" "cp" } } */
/* { dg-final { scan-ipa-dump "replacing param .0 a with const 7" "cp" } } */
--
2.17.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH V2] Generalized value pass-through for self-recursive function (ipa/pr93203)
2020-01-25 13:52 [PATCH] Generalized value pass-through for self-recursive function (ipa/pr93203) Feng Xue OS
@ 2020-01-25 16:17 ` Feng Xue OS
2020-02-03 1:57 ` Ping* " Feng Xue OS
2020-02-05 17:27 ` Martin Jambor
0 siblings, 2 replies; 17+ messages in thread
From: Feng Xue OS @ 2020-01-25 16:17 UTC (permalink / raw)
To: mjambor, Jan Hubicka, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2195 bytes --]
Made some changes.
Feng
________________________________________
From: Feng Xue OS
Sent: Saturday, January 25, 2020 5:54 PM
To: mjambor@suse.cz; Jan Hubicka; gcc-patches@gcc.gnu.org
Subject: [PATCH] Generalized value pass-through for self-recursive function (ipa/pr93203)
Besides simple pass-through (aggregate) jump function, arithmetic (aggregate)
jump function could also bring same (aggregate) value as parameter passed-in
for self-feeding recursive call. For example,
f1 (int i) /* normal jump function */
{
f1 (i & 1);
}
Suppose i is 0, recursive propagation via (i & 1) also gets 0, which
can be seen as a simple pass-through of i.
f2 (int *p) /* aggregate jump function */
{
int t = *p & 1;
f2 (&t);
}
Likewise, if *p is 0, (*p & 1) is also 0, and &t is an aggregate simple
pass-through of p.
This patch is to support these two kinds of value pass-through.
Bootstrapped/regtested on x86_64-linux and aarch64-linux.
Feng
---
2020-01-25 Feng Xue <fxue@os.amperecomputing.com>
PR ipa/93203
* ipa-cp.c (ipcp_lattice::add_value): Add source with same call
edge but different source value.
(adjust_callers_for_value_intersection): New function.
(gather_edges_for_value): Adjust order of callers to let a
non-self-recursive caller be the first element.
(self_recursive_pass_through_p): Add a new parameter simple, and
check generalized self-recursive pass-through jump function.
(self_recursive_agg_pass_through_p): Likewise.
(find_more_scalar_values_for_callers_subset): Compute value from
pass-through jump function for self-recursive.
(intersect_with_plats): Remove code of itersection with unknown
place holder value.
(intersect_with_agg_replacements): Likewise.
(intersect_aggregates_with_edge): Deduce with from pass-through
jump function for self-recursive.
(decide_whether_version_node): Remove dead callers and adjust
order to let a non-self-recursive caller be the first element.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Generalized-value-pass-through-for-self-recusive-fun.patch --]
[-- Type: text/x-patch; name="0001-Generalized-value-pass-through-for-self-recusive-fun.patch", Size: 13998 bytes --]
From 74aef0cd2f40ff828a4b2abcbbdbbf4b1aea1fcf Mon Sep 17 00:00:00 2001
From: Feng Xue <fxue@os.amperecomputing.com>
Date: Tue, 21 Jan 2020 20:53:38 +0800
Subject: [PATCH] Generalized value pass-through for self-recusive function
---
gcc/ipa-cp.c | 195 ++++++++++++++++++-----------
gcc/testsuite/g++.dg/ipa/pr93203.C | 95 ++++++++++++++
gcc/testsuite/gcc.dg/ipa/ipcp-1.c | 2 +-
3 files changed, 216 insertions(+), 76 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/ipa/pr93203.C
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 17da1d8e8a7..64d23a34292 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1850,7 +1850,7 @@ ipcp_lattice<valtype>::add_value (valtype newval, cgraph_edge *cs,
{
ipcp_value_source<valtype> *s;
for (s = val->sources; s; s = s->next)
- if (s->cs == cs)
+ if (s->cs == cs && s->val == src_val)
break;
if (s)
return false;
@@ -4207,6 +4207,33 @@ get_info_about_necessary_edges (ipcp_value<valtype> *val, cgraph_node *dest,
return hot;
}
+/* Given a NODE, and a set of its CALLERS, try to adjust order of the callers
+ to let a non-self-recursive caller be the first element. Thus, we can
+ simplify intersecting operations on values that arrive from all of these
+ callers, especially when there exists self-recursive call. Return true if
+ this kind of adjustment is possible. */
+
+static bool
+adjust_callers_for_value_intersection (vec<cgraph_edge *> callers,
+ cgraph_node *node)
+{
+ for (unsigned i = 0; i < callers.length (); i++)
+ {
+ cgraph_edge *cs = callers[i];
+
+ if (cs->caller != node)
+ {
+ if (i > 0)
+ {
+ callers[i] = callers[0];
+ callers[0] = cs;
+ }
+ return true;
+ }
+ }
+ return false;
+}
+
/* Return a vector of incoming edges that do bring value VAL to node DEST. It
is assumed their number is known and equal to CALLER_COUNT. */
@@ -4230,6 +4257,9 @@ gather_edges_for_value (ipcp_value<valtype> *val, cgraph_node *dest,
}
}
+ if (caller_count > 1)
+ adjust_callers_for_value_intersection (ret, dest);
+
return ret;
}
@@ -4241,7 +4271,6 @@ get_replacement_map (class ipa_node_params *info, tree value, int parm_num)
{
struct ipa_replace_map *replace_map;
-
replace_map = ggc_alloc<ipa_replace_map> ();
if (dump_file)
{
@@ -4592,36 +4621,40 @@ create_specialized_node (struct cgraph_node *node,
}
/* Return true, if JFUNC, which describes a i-th parameter of call CS, is a
- simple no-operation pass-through function to itself. */
+ pass-through function to itself. When SIMPLE is true, further check if
+ JFUNC is a simple no-operation pass-through. */
static bool
-self_recursive_pass_through_p (cgraph_edge *cs, ipa_jump_func *jfunc, int i)
+self_recursive_pass_through_p (cgraph_edge *cs, ipa_jump_func *jfunc, int i,
+ bool simple = true)
{
enum availability availability;
if (cs->caller == cs->callee->function_symbol (&availability)
&& availability > AVAIL_INTERPOSABLE
&& jfunc->type == IPA_JF_PASS_THROUGH
- && ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR
+ && (!simple || ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
&& ipa_get_jf_pass_through_formal_id (jfunc) == i)
return true;
return false;
}
/* Return true, if JFUNC, which describes a part of an aggregate represented
- or pointed to by the i-th parameter of call CS, is a simple no-operation
- pass-through function to itself. */
+ or pointed to by the i-th parameter of call CS, is a pass-through function
+ to itself. When SIMPLE is true, further check if JFUNC is a simple
+ no-operation pass-through. */
static bool
self_recursive_agg_pass_through_p (cgraph_edge *cs, ipa_agg_jf_item *jfunc,
- int i)
+ int i, bool simple = true)
{
enum availability availability;
if (cs->caller == cs->callee->function_symbol (&availability)
&& availability > AVAIL_INTERPOSABLE
&& jfunc->jftype == IPA_JF_LOAD_AGG
&& jfunc->offset == jfunc->value.load_agg.offset
- && jfunc->value.pass_through.operation == NOP_EXPR
- && jfunc->value.pass_through.formal_id == i)
+ && (!simple || jfunc->value.pass_through.operation == NOP_EXPR)
+ && jfunc->value.pass_through.formal_id == i
+ && useless_type_conversion_p (jfunc->value.load_agg.type, jfunc->type))
return true;
return false;
}
@@ -4653,9 +4686,6 @@ find_more_scalar_values_for_callers_subset (struct cgraph_node *node,
struct ipa_jump_func *jump_func;
tree t;
- if (IPA_NODE_REF (cs->caller) && IPA_NODE_REF (cs->caller)->node_dead)
- continue;
-
if (!IPA_EDGE_REF (cs)
|| i >= ipa_get_cs_argument_count (IPA_EDGE_REF (cs))
|| (i == 0
@@ -4665,10 +4695,30 @@ find_more_scalar_values_for_callers_subset (struct cgraph_node *node,
break;
}
jump_func = ipa_get_ith_jump_func (IPA_EDGE_REF (cs), i);
- if (self_recursive_pass_through_p (cs, jump_func, i))
- continue;
- t = ipa_value_from_jfunc (IPA_NODE_REF (cs->caller), jump_func, type);
+ /* Besides simple pass-through jump function, arithmetic jump
+ function could also introduce argument-direct-pass-through for
+ self-feeding recursive call. For example,
+
+ fn (int i)
+ {
+ fn (i & 1);
+ }
+
+ Given that i is 0, recursive propagation via (i & 1) also gets
+ 0. */
+ if (self_recursive_pass_through_p (cs, jump_func, i, false))
+ {
+ gcc_assert (newval);
+ t = ipa_get_jf_arith_result (
+ ipa_get_jf_pass_through_operation (jump_func),
+ newval,
+ ipa_get_jf_pass_through_operand (jump_func),
+ type);
+ }
+ else
+ t = ipa_value_from_jfunc (IPA_NODE_REF (cs->caller), jump_func,
+ type);
if (!t
|| (newval
&& !values_equal_for_ipcp_p (t, newval))
@@ -4817,19 +4867,12 @@ intersect_with_plats (class ipcp_param_lattices *plats,
break;
if (aglat->offset - offset == item->offset)
{
- gcc_checking_assert (item->value);
if (aglat->is_single_const ())
{
tree value = aglat->values->value;
if (values_equal_for_ipcp_p (item->value, value))
found = true;
- else if (item->value == error_mark_node)
- {
- /* Replace unknown place holder value with real one. */
- item->value = value;
- found = true;
- }
}
break;
}
@@ -4898,12 +4941,6 @@ intersect_with_agg_replacements (struct cgraph_node *node, int index,
{
if (values_equal_for_ipcp_p (item->value, av->value))
found = true;
- else if (item->value == error_mark_node)
- {
- /* Replace place holder value with real one. */
- item->value = av->value;
- found = true;
- }
break;
}
}
@@ -5008,31 +5045,16 @@ intersect_aggregates_with_edge (struct cgraph_edge *cs, int index,
for (unsigned i = 0; i < jfunc->agg.items->length (); i++)
{
struct ipa_agg_jf_item *agg_item = &(*jfunc->agg.items)[i];
- struct ipa_agg_value agg_value;
-
- if (self_recursive_agg_pass_through_p (cs, agg_item, index))
- {
- /* For a self-recursive call, if aggregate jump function is a
- simple pass-through, the exact value that it stands for is
- not known at this point, which must comes from other call
- sites. But we still need to add a place holder in value
- sets to indicate it, here we use error_mark_node to
- represent the special unknown value, which will be replaced
- with real one during later intersecting operations. */
- agg_value.value = error_mark_node;
- }
- else
+ tree value = ipa_agg_value_from_node (caller_info, cs->caller,
+ agg_item);
+ if (value)
{
- tree value = ipa_agg_value_from_node (caller_info, cs->caller,
- agg_item);
- if (!value)
- continue;
+ struct ipa_agg_value agg_value;
agg_value.value = value;
+ agg_value.offset = agg_item->offset;
+ inter.safe_push (agg_value);
}
-
- agg_value.offset = agg_item->offset;
- inter.safe_push (agg_value);
}
else
FOR_EACH_VEC_ELT (inter, k, item)
@@ -5053,25 +5075,32 @@ intersect_aggregates_with_edge (struct cgraph_edge *cs, int index,
{
tree value;
- if (self_recursive_agg_pass_through_p (cs, ti, index))
- {
- /* A simple aggregate pass-through in self-recursive
- call should lead to same value. */
- found = true;
- }
- else if ((value = ipa_agg_value_from_node (caller_info,
- cs->caller, ti)))
- {
- if (values_equal_for_ipcp_p (item->value, value))
- found = true;
- else if (item->value == error_mark_node)
- {
- /* Replace unknown place holder value with real
- one. */
- item->value = value;
- found = true;
- }
- }
+ /* Besides simple pass-through aggregate jump function,
+ arithmetic aggregate jump function could also bring
+ same aggregate value as parameter passed-in for
+ self-feeding recursive call. For example,
+
+ fn (int *i)
+ {
+ int j = *i & 1;
+ fn (&j);
+ }
+
+ Given that *i is 0, recursive propagation via (*i & 1)
+ also gets 0. */
+ if (self_recursive_agg_pass_through_p (cs, ti, index,
+ false))
+ value = ipa_get_jf_arith_result (
+ ti->value.pass_through.operation,
+ item->value,
+ ti->value.pass_through.operand,
+ ti->type);
+ else
+ value = ipa_agg_value_from_node (caller_info,
+ cs->caller, ti);
+
+ if (value && values_equal_for_ipcp_p (item->value, value))
+ found = true;
break;
}
l++;
@@ -5147,9 +5176,6 @@ find_aggregate_values_for_callers_subset (struct cgraph_node *node,
if (!item->value)
continue;
- /* All values must be real values, not unknown place holders. */
- gcc_assert (item->value != error_mark_node);
-
v = ggc_alloc<ipa_agg_replacement_value> ();
v->index = i;
v->offset = item->offset;
@@ -5545,13 +5571,33 @@ decide_whether_version_node (struct cgraph_node *node)
if (info->do_clone_for_all_contexts)
{
struct cgraph_node *clone;
- vec<cgraph_edge *> callers;
+ vec<cgraph_edge *> callers = node->collect_callers ();
+
+ for (int i = callers.length () - 1; i >= 0; i--)
+ {
+ cgraph_edge *cs = callers[i];
+ class ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
+
+ if (caller_info && caller_info->node_dead)
+ callers.unordered_remove (i);
+ }
+
+ if (!adjust_callers_for_value_intersection (callers, node))
+ {
+ /* If node is not called by anyone, or all its caller edges are
+ self-recursive, the node is not really be in use, no need to
+ do cloning. */
+ callers.release ();
+ known_csts.release ();
+ known_contexts.release ();
+ info->do_clone_for_all_contexts = false;
+ return ret;
+ }
if (dump_file)
fprintf (dump_file, " - Creating a specialized node of %s "
"for all known contexts.\n", node->dump_name ());
- callers = node->collect_callers ();
find_more_scalar_values_for_callers_subset (node, known_csts, callers);
find_more_contexts_for_caller_subset (node, &known_contexts, callers);
ipa_agg_replacement_value *aggvals
@@ -5564,7 +5610,6 @@ decide_whether_version_node (struct cgraph_node *node)
}
clone = create_specialized_node (node, known_csts, known_contexts,
aggvals, callers);
- info = IPA_NODE_REF (node);
info->do_clone_for_all_contexts = false;
IPA_NODE_REF (clone)->is_all_contexts_clone = true;
ret = true;
diff --git a/gcc/testsuite/g++.dg/ipa/pr93203.C b/gcc/testsuite/g++.dg/ipa/pr93203.C
new file mode 100644
index 00000000000..b4cd69001b5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr93203.C
@@ -0,0 +1,95 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -w -std=gnu++11" } */
+
+class a {
+public:
+ a(char *);
+};
+class ad {
+public:
+ ad(a *);
+};
+class b {};
+using ah = class ak {
+ using al = char;
+
+public:
+ ak(b) : ak(0) {}
+ ak an() { return ap & 1; }
+ al ap;
+ ak(al af) : ap(af) {}
+};
+struct at {
+ ah au;
+ int av;
+ char aw;
+ char ax;
+};
+class az {
+public:
+ struct ba {
+ void bb(ak am) {
+ ak bc = am.an();
+ bb(bc);
+ }
+ };
+ void bd(ak am) { be.bb(am); }
+ ba be;
+};
+class bg {
+public:
+ int bh;
+ at bi;
+};
+int bj;
+int *bk;
+int c;
+class bl {
+ bool bm();
+ bg bn;
+ az bo;
+ int e;
+ int bq;
+};
+class bs {
+public:
+ bs(int *, ah *, char *, char *, int);
+};
+template < typename bt > class bu : bs {
+ using bv = typename bt::bv;
+
+public:
+ template < typename... bx >
+ bu(a *by, int *bz, at body, bx...)
+ : bs(bz, &body.au, &body.aw, &body.ax, body.av), ca(bx()...), cb(by),
+ cc(by), cd(by), ce(by) {}
+ void cf() {
+ auto cg = ch();
+ auto ci = *cj();
+ ca.ck(this, cg, &ci);
+ }
+ bt ca;
+ ad cb;
+ ad cc;
+ ad cd;
+ ad ce;
+ bv *cj();
+ bv ch();
+};
+class cl {
+public:
+ using bv = struct {};
+ cl(az *, int, int, int, int, a *, int, int **);
+ void ck(bs *, bv, bv *) {
+ b d;
+ ak ci(d);
+ bo.bd(ci);
+ }
+ az bo;
+};
+bool bl::bm() {
+ a by("");
+ bu< cl > cn(&by, &bj, bn.bi, &bo, c, bn.bh, e, 0, &by, bq, &bk);
+ cn.cf();
+}
+
diff --git a/gcc/testsuite/gcc.dg/ipa/ipcp-1.c b/gcc/testsuite/gcc.dg/ipa/ipcp-1.c
index 952694d302b..baa9c97ffb0 100644
--- a/gcc/testsuite/gcc.dg/ipa/ipcp-1.c
+++ b/gcc/testsuite/gcc.dg/ipa/ipcp-1.c
@@ -45,7 +45,7 @@ main (int argc, char *argv[])
}
-/* { dg-final { scan-ipa-dump "Creating a specialized node of f.*for all known contexts" "cp" } } */
+/* { dg-final { scan-ipa-dump "Creating a specialized node of f" "cp" } } */
/* { dg-final { scan-ipa-dump "replacing param .0 a with const 7" "cp" } } */
--
2.17.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* Ping* [PATCH V2] Generalized value pass-through for self-recursive function (ipa/pr93203)
2020-01-25 16:17 ` [PATCH V2] " Feng Xue OS
@ 2020-02-03 1:57 ` Feng Xue OS
2020-02-05 17:27 ` Martin Jambor
1 sibling, 0 replies; 17+ messages in thread
From: Feng Xue OS @ 2020-02-03 1:57 UTC (permalink / raw)
To: mjambor, Jan Hubicka, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2500 bytes --]
Thanks,
Feng
________________________________________
From: Feng Xue OS <fxue@os.amperecomputing.com>
Sent: Saturday, January 25, 2020 9:50 PM
To: mjambor@suse.cz; Jan Hubicka; gcc-patches@gcc.gnu.org
Subject: [PATCH V2] Generalized value pass-through for self-recursive function (ipa/pr93203)
Made some changes.
Feng
________________________________________
From: Feng Xue OS
Sent: Saturday, January 25, 2020 5:54 PM
To: mjambor@suse.cz; Jan Hubicka; gcc-patches@gcc.gnu.org
Subject: [PATCH] Generalized value pass-through for self-recursive function (ipa/pr93203)
Besides simple pass-through (aggregate) jump function, arithmetic (aggregate)
jump function could also bring same (aggregate) value as parameter passed-in
for self-feeding recursive call. For example,
f1 (int i) /* normal jump function */
{
f1 (i & 1);
}
Suppose i is 0, recursive propagation via (i & 1) also gets 0, which
can be seen as a simple pass-through of i.
f2 (int *p) /* aggregate jump function */
{
int t = *p & 1;
f2 (&t);
}
Likewise, if *p is 0, (*p & 1) is also 0, and &t is an aggregate simple
pass-through of p.
This patch is to support these two kinds of value pass-through.
Bootstrapped/regtested on x86_64-linux and aarch64-linux.
Feng
---
2020-01-25 Feng Xue <fxue@os.amperecomputing.com>
PR ipa/93203
* ipa-cp.c (ipcp_lattice::add_value): Add source with same call
edge but different source value.
(adjust_callers_for_value_intersection): New function.
(gather_edges_for_value): Adjust order of callers to let a
non-self-recursive caller be the first element.
(self_recursive_pass_through_p): Add a new parameter simple, and
check generalized self-recursive pass-through jump function.
(self_recursive_agg_pass_through_p): Likewise.
(find_more_scalar_values_for_callers_subset): Compute value from
pass-through jump function for self-recursive.
(intersect_with_plats): Remove code of itersection with unknown
place holder value.
(intersect_with_agg_replacements): Likewise.
(intersect_aggregates_with_edge): Deduce with from pass-through
jump function for self-recursive.
(decide_whether_version_node): Remove dead callers and adjust
order to let a non-self-recursive caller be the first element.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Generalized-value-pass-through-for-self-recusive-fun.patch --]
[-- Type: text/x-patch; name="0001-Generalized-value-pass-through-for-self-recusive-fun.patch", Size: 13998 bytes --]
From 74aef0cd2f40ff828a4b2abcbbdbbf4b1aea1fcf Mon Sep 17 00:00:00 2001
From: Feng Xue <fxue@os.amperecomputing.com>
Date: Tue, 21 Jan 2020 20:53:38 +0800
Subject: [PATCH] Generalized value pass-through for self-recusive function
---
gcc/ipa-cp.c | 195 ++++++++++++++++++-----------
gcc/testsuite/g++.dg/ipa/pr93203.C | 95 ++++++++++++++
gcc/testsuite/gcc.dg/ipa/ipcp-1.c | 2 +-
3 files changed, 216 insertions(+), 76 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/ipa/pr93203.C
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 17da1d8e8a7..64d23a34292 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1850,7 +1850,7 @@ ipcp_lattice<valtype>::add_value (valtype newval, cgraph_edge *cs,
{
ipcp_value_source<valtype> *s;
for (s = val->sources; s; s = s->next)
- if (s->cs == cs)
+ if (s->cs == cs && s->val == src_val)
break;
if (s)
return false;
@@ -4207,6 +4207,33 @@ get_info_about_necessary_edges (ipcp_value<valtype> *val, cgraph_node *dest,
return hot;
}
+/* Given a NODE, and a set of its CALLERS, try to adjust order of the callers
+ to let a non-self-recursive caller be the first element. Thus, we can
+ simplify intersecting operations on values that arrive from all of these
+ callers, especially when there exists self-recursive call. Return true if
+ this kind of adjustment is possible. */
+
+static bool
+adjust_callers_for_value_intersection (vec<cgraph_edge *> callers,
+ cgraph_node *node)
+{
+ for (unsigned i = 0; i < callers.length (); i++)
+ {
+ cgraph_edge *cs = callers[i];
+
+ if (cs->caller != node)
+ {
+ if (i > 0)
+ {
+ callers[i] = callers[0];
+ callers[0] = cs;
+ }
+ return true;
+ }
+ }
+ return false;
+}
+
/* Return a vector of incoming edges that do bring value VAL to node DEST. It
is assumed their number is known and equal to CALLER_COUNT. */
@@ -4230,6 +4257,9 @@ gather_edges_for_value (ipcp_value<valtype> *val, cgraph_node *dest,
}
}
+ if (caller_count > 1)
+ adjust_callers_for_value_intersection (ret, dest);
+
return ret;
}
@@ -4241,7 +4271,6 @@ get_replacement_map (class ipa_node_params *info, tree value, int parm_num)
{
struct ipa_replace_map *replace_map;
-
replace_map = ggc_alloc<ipa_replace_map> ();
if (dump_file)
{
@@ -4592,36 +4621,40 @@ create_specialized_node (struct cgraph_node *node,
}
/* Return true, if JFUNC, which describes a i-th parameter of call CS, is a
- simple no-operation pass-through function to itself. */
+ pass-through function to itself. When SIMPLE is true, further check if
+ JFUNC is a simple no-operation pass-through. */
static bool
-self_recursive_pass_through_p (cgraph_edge *cs, ipa_jump_func *jfunc, int i)
+self_recursive_pass_through_p (cgraph_edge *cs, ipa_jump_func *jfunc, int i,
+ bool simple = true)
{
enum availability availability;
if (cs->caller == cs->callee->function_symbol (&availability)
&& availability > AVAIL_INTERPOSABLE
&& jfunc->type == IPA_JF_PASS_THROUGH
- && ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR
+ && (!simple || ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
&& ipa_get_jf_pass_through_formal_id (jfunc) == i)
return true;
return false;
}
/* Return true, if JFUNC, which describes a part of an aggregate represented
- or pointed to by the i-th parameter of call CS, is a simple no-operation
- pass-through function to itself. */
+ or pointed to by the i-th parameter of call CS, is a pass-through function
+ to itself. When SIMPLE is true, further check if JFUNC is a simple
+ no-operation pass-through. */
static bool
self_recursive_agg_pass_through_p (cgraph_edge *cs, ipa_agg_jf_item *jfunc,
- int i)
+ int i, bool simple = true)
{
enum availability availability;
if (cs->caller == cs->callee->function_symbol (&availability)
&& availability > AVAIL_INTERPOSABLE
&& jfunc->jftype == IPA_JF_LOAD_AGG
&& jfunc->offset == jfunc->value.load_agg.offset
- && jfunc->value.pass_through.operation == NOP_EXPR
- && jfunc->value.pass_through.formal_id == i)
+ && (!simple || jfunc->value.pass_through.operation == NOP_EXPR)
+ && jfunc->value.pass_through.formal_id == i
+ && useless_type_conversion_p (jfunc->value.load_agg.type, jfunc->type))
return true;
return false;
}
@@ -4653,9 +4686,6 @@ find_more_scalar_values_for_callers_subset (struct cgraph_node *node,
struct ipa_jump_func *jump_func;
tree t;
- if (IPA_NODE_REF (cs->caller) && IPA_NODE_REF (cs->caller)->node_dead)
- continue;
-
if (!IPA_EDGE_REF (cs)
|| i >= ipa_get_cs_argument_count (IPA_EDGE_REF (cs))
|| (i == 0
@@ -4665,10 +4695,30 @@ find_more_scalar_values_for_callers_subset (struct cgraph_node *node,
break;
}
jump_func = ipa_get_ith_jump_func (IPA_EDGE_REF (cs), i);
- if (self_recursive_pass_through_p (cs, jump_func, i))
- continue;
- t = ipa_value_from_jfunc (IPA_NODE_REF (cs->caller), jump_func, type);
+ /* Besides simple pass-through jump function, arithmetic jump
+ function could also introduce argument-direct-pass-through for
+ self-feeding recursive call. For example,
+
+ fn (int i)
+ {
+ fn (i & 1);
+ }
+
+ Given that i is 0, recursive propagation via (i & 1) also gets
+ 0. */
+ if (self_recursive_pass_through_p (cs, jump_func, i, false))
+ {
+ gcc_assert (newval);
+ t = ipa_get_jf_arith_result (
+ ipa_get_jf_pass_through_operation (jump_func),
+ newval,
+ ipa_get_jf_pass_through_operand (jump_func),
+ type);
+ }
+ else
+ t = ipa_value_from_jfunc (IPA_NODE_REF (cs->caller), jump_func,
+ type);
if (!t
|| (newval
&& !values_equal_for_ipcp_p (t, newval))
@@ -4817,19 +4867,12 @@ intersect_with_plats (class ipcp_param_lattices *plats,
break;
if (aglat->offset - offset == item->offset)
{
- gcc_checking_assert (item->value);
if (aglat->is_single_const ())
{
tree value = aglat->values->value;
if (values_equal_for_ipcp_p (item->value, value))
found = true;
- else if (item->value == error_mark_node)
- {
- /* Replace unknown place holder value with real one. */
- item->value = value;
- found = true;
- }
}
break;
}
@@ -4898,12 +4941,6 @@ intersect_with_agg_replacements (struct cgraph_node *node, int index,
{
if (values_equal_for_ipcp_p (item->value, av->value))
found = true;
- else if (item->value == error_mark_node)
- {
- /* Replace place holder value with real one. */
- item->value = av->value;
- found = true;
- }
break;
}
}
@@ -5008,31 +5045,16 @@ intersect_aggregates_with_edge (struct cgraph_edge *cs, int index,
for (unsigned i = 0; i < jfunc->agg.items->length (); i++)
{
struct ipa_agg_jf_item *agg_item = &(*jfunc->agg.items)[i];
- struct ipa_agg_value agg_value;
-
- if (self_recursive_agg_pass_through_p (cs, agg_item, index))
- {
- /* For a self-recursive call, if aggregate jump function is a
- simple pass-through, the exact value that it stands for is
- not known at this point, which must comes from other call
- sites. But we still need to add a place holder in value
- sets to indicate it, here we use error_mark_node to
- represent the special unknown value, which will be replaced
- with real one during later intersecting operations. */
- agg_value.value = error_mark_node;
- }
- else
+ tree value = ipa_agg_value_from_node (caller_info, cs->caller,
+ agg_item);
+ if (value)
{
- tree value = ipa_agg_value_from_node (caller_info, cs->caller,
- agg_item);
- if (!value)
- continue;
+ struct ipa_agg_value agg_value;
agg_value.value = value;
+ agg_value.offset = agg_item->offset;
+ inter.safe_push (agg_value);
}
-
- agg_value.offset = agg_item->offset;
- inter.safe_push (agg_value);
}
else
FOR_EACH_VEC_ELT (inter, k, item)
@@ -5053,25 +5075,32 @@ intersect_aggregates_with_edge (struct cgraph_edge *cs, int index,
{
tree value;
- if (self_recursive_agg_pass_through_p (cs, ti, index))
- {
- /* A simple aggregate pass-through in self-recursive
- call should lead to same value. */
- found = true;
- }
- else if ((value = ipa_agg_value_from_node (caller_info,
- cs->caller, ti)))
- {
- if (values_equal_for_ipcp_p (item->value, value))
- found = true;
- else if (item->value == error_mark_node)
- {
- /* Replace unknown place holder value with real
- one. */
- item->value = value;
- found = true;
- }
- }
+ /* Besides simple pass-through aggregate jump function,
+ arithmetic aggregate jump function could also bring
+ same aggregate value as parameter passed-in for
+ self-feeding recursive call. For example,
+
+ fn (int *i)
+ {
+ int j = *i & 1;
+ fn (&j);
+ }
+
+ Given that *i is 0, recursive propagation via (*i & 1)
+ also gets 0. */
+ if (self_recursive_agg_pass_through_p (cs, ti, index,
+ false))
+ value = ipa_get_jf_arith_result (
+ ti->value.pass_through.operation,
+ item->value,
+ ti->value.pass_through.operand,
+ ti->type);
+ else
+ value = ipa_agg_value_from_node (caller_info,
+ cs->caller, ti);
+
+ if (value && values_equal_for_ipcp_p (item->value, value))
+ found = true;
break;
}
l++;
@@ -5147,9 +5176,6 @@ find_aggregate_values_for_callers_subset (struct cgraph_node *node,
if (!item->value)
continue;
- /* All values must be real values, not unknown place holders. */
- gcc_assert (item->value != error_mark_node);
-
v = ggc_alloc<ipa_agg_replacement_value> ();
v->index = i;
v->offset = item->offset;
@@ -5545,13 +5571,33 @@ decide_whether_version_node (struct cgraph_node *node)
if (info->do_clone_for_all_contexts)
{
struct cgraph_node *clone;
- vec<cgraph_edge *> callers;
+ vec<cgraph_edge *> callers = node->collect_callers ();
+
+ for (int i = callers.length () - 1; i >= 0; i--)
+ {
+ cgraph_edge *cs = callers[i];
+ class ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
+
+ if (caller_info && caller_info->node_dead)
+ callers.unordered_remove (i);
+ }
+
+ if (!adjust_callers_for_value_intersection (callers, node))
+ {
+ /* If node is not called by anyone, or all its caller edges are
+ self-recursive, the node is not really be in use, no need to
+ do cloning. */
+ callers.release ();
+ known_csts.release ();
+ known_contexts.release ();
+ info->do_clone_for_all_contexts = false;
+ return ret;
+ }
if (dump_file)
fprintf (dump_file, " - Creating a specialized node of %s "
"for all known contexts.\n", node->dump_name ());
- callers = node->collect_callers ();
find_more_scalar_values_for_callers_subset (node, known_csts, callers);
find_more_contexts_for_caller_subset (node, &known_contexts, callers);
ipa_agg_replacement_value *aggvals
@@ -5564,7 +5610,6 @@ decide_whether_version_node (struct cgraph_node *node)
}
clone = create_specialized_node (node, known_csts, known_contexts,
aggvals, callers);
- info = IPA_NODE_REF (node);
info->do_clone_for_all_contexts = false;
IPA_NODE_REF (clone)->is_all_contexts_clone = true;
ret = true;
diff --git a/gcc/testsuite/g++.dg/ipa/pr93203.C b/gcc/testsuite/g++.dg/ipa/pr93203.C
new file mode 100644
index 00000000000..b4cd69001b5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr93203.C
@@ -0,0 +1,95 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -w -std=gnu++11" } */
+
+class a {
+public:
+ a(char *);
+};
+class ad {
+public:
+ ad(a *);
+};
+class b {};
+using ah = class ak {
+ using al = char;
+
+public:
+ ak(b) : ak(0) {}
+ ak an() { return ap & 1; }
+ al ap;
+ ak(al af) : ap(af) {}
+};
+struct at {
+ ah au;
+ int av;
+ char aw;
+ char ax;
+};
+class az {
+public:
+ struct ba {
+ void bb(ak am) {
+ ak bc = am.an();
+ bb(bc);
+ }
+ };
+ void bd(ak am) { be.bb(am); }
+ ba be;
+};
+class bg {
+public:
+ int bh;
+ at bi;
+};
+int bj;
+int *bk;
+int c;
+class bl {
+ bool bm();
+ bg bn;
+ az bo;
+ int e;
+ int bq;
+};
+class bs {
+public:
+ bs(int *, ah *, char *, char *, int);
+};
+template < typename bt > class bu : bs {
+ using bv = typename bt::bv;
+
+public:
+ template < typename... bx >
+ bu(a *by, int *bz, at body, bx...)
+ : bs(bz, &body.au, &body.aw, &body.ax, body.av), ca(bx()...), cb(by),
+ cc(by), cd(by), ce(by) {}
+ void cf() {
+ auto cg = ch();
+ auto ci = *cj();
+ ca.ck(this, cg, &ci);
+ }
+ bt ca;
+ ad cb;
+ ad cc;
+ ad cd;
+ ad ce;
+ bv *cj();
+ bv ch();
+};
+class cl {
+public:
+ using bv = struct {};
+ cl(az *, int, int, int, int, a *, int, int **);
+ void ck(bs *, bv, bv *) {
+ b d;
+ ak ci(d);
+ bo.bd(ci);
+ }
+ az bo;
+};
+bool bl::bm() {
+ a by("");
+ bu< cl > cn(&by, &bj, bn.bi, &bo, c, bn.bh, e, 0, &by, bq, &bk);
+ cn.cf();
+}
+
diff --git a/gcc/testsuite/gcc.dg/ipa/ipcp-1.c b/gcc/testsuite/gcc.dg/ipa/ipcp-1.c
index 952694d302b..baa9c97ffb0 100644
--- a/gcc/testsuite/gcc.dg/ipa/ipcp-1.c
+++ b/gcc/testsuite/gcc.dg/ipa/ipcp-1.c
@@ -45,7 +45,7 @@ main (int argc, char *argv[])
}
-/* { dg-final { scan-ipa-dump "Creating a specialized node of f.*for all known contexts" "cp" } } */
+/* { dg-final { scan-ipa-dump "Creating a specialized node of f" "cp" } } */
/* { dg-final { scan-ipa-dump "replacing param .0 a with const 7" "cp" } } */
--
2.17.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH V2] Generalized value pass-through for self-recursive function (ipa/pr93203)
2020-01-25 16:17 ` [PATCH V2] " Feng Xue OS
2020-02-03 1:57 ` Ping* " Feng Xue OS
@ 2020-02-05 17:27 ` Martin Jambor
2020-02-10 3:28 ` Feng Xue OS
1 sibling, 1 reply; 17+ messages in thread
From: Martin Jambor @ 2020-02-05 17:27 UTC (permalink / raw)
To: Feng Xue OS, Jan Hubicka, gcc-patches
Hi,
On Sat, Jan 25 2020, Feng Xue OS wrote:
> Made some changes.
>
> Feng
>
> ________________________________________
> From: Feng Xue OS
> Sent: Saturday, January 25, 2020 5:54 PM
> To: mjambor@suse.cz; Jan Hubicka; gcc-patches@gcc.gnu.org
> Subject: [PATCH] Generalized value pass-through for self-recursive function (ipa/pr93203)
>
> Besides simple pass-through (aggregate) jump function, arithmetic (aggregate)
> jump function could also bring same (aggregate) value as parameter passed-in
> for self-feeding recursive call. For example,
>
> f1 (int i) /* normal jump function */
> {
> f1 (i & 1);
> }
>
> Suppose i is 0, recursive propagation via (i & 1) also gets 0, which
> can be seen as a simple pass-through of i.
>
> f2 (int *p) /* aggregate jump function */
> {
> int t = *p & 1;
> f2 (&t);
> }
> Likewise, if *p is 0, (*p & 1) is also 0, and &t is an aggregate simple
> pass-through of p.
>
> This patch is to support these two kinds of value pass-through.
> Bootstrapped/regtested on x86_64-linux and aarch64-linux.
sorry for the delay in the review. As far as I am concerned, I am OK
with the patch but please see the few comments below:
> ---
> 2020-01-25 Feng Xue <fxue@os.amperecomputing.com>
>
> PR ipa/93203
> * ipa-cp.c (ipcp_lattice::add_value): Add source with same call
> edge but different source value.
> (adjust_callers_for_value_intersection): New function.
> (gather_edges_for_value): Adjust order of callers to let a
> non-self-recursive caller be the first element.
> (self_recursive_pass_through_p): Add a new parameter simple, and
> check generalized self-recursive pass-through jump function.
> (self_recursive_agg_pass_through_p): Likewise.
> (find_more_scalar_values_for_callers_subset): Compute value from
> pass-through jump function for self-recursive.
> (intersect_with_plats): Remove code of itersection with unknown
> place holder value.
> (intersect_with_agg_replacements): Likewise.
> (intersect_aggregates_with_edge): Deduce with from pass-through
> jump function for self-recursive.
> (decide_whether_version_node): Remove dead callers and adjust
> order to let a non-self-recursive caller be the first element.
>
> From 74aef0cd2f40ff828a4b2abcbbdbbf4b1aea1fcf Mon Sep 17 00:00:00 2001
> From: Feng Xue <fxue@os.amperecomputing.com>
> Date: Tue, 21 Jan 2020 20:53:38 +0800
> Subject: [PATCH] Generalized value pass-through for self-recusive function
>
> ---
> gcc/ipa-cp.c | 195 ++++++++++++++++++-----------
> gcc/testsuite/g++.dg/ipa/pr93203.C | 95 ++++++++++++++
> gcc/testsuite/gcc.dg/ipa/ipcp-1.c | 2 +-
> 3 files changed, 216 insertions(+), 76 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/ipa/pr93203.C
>
> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> index 17da1d8e8a7..64d23a34292 100644
> --- a/gcc/ipa-cp.c
> +++ b/gcc/ipa-cp.c
...
> @@ -4817,19 +4867,12 @@ intersect_with_plats (class ipcp_param_lattices *plats,
> break;
> if (aglat->offset - offset == item->offset)
> {
> - gcc_checking_assert (item->value);
I've been staring at this for quite a while, trying to figure out how
your patch can put NULL here before I realized it was just a clean-up
:-) Sending such changes independently or pointing them out in the
email/ChangeLog makes review easier.
> if (aglat->is_single_const ())
> {
> tree value = aglat->values->value;
>
> if (values_equal_for_ipcp_p (item->value, value))
> found = true;
> - else if (item->value == error_mark_node)
> - {
> - /* Replace unknown place holder value with real one. */
> - item->value = value;
> - found = true;
> - }
> }
> break;
> }
...
> @@ -5564,7 +5610,6 @@ decide_whether_version_node (struct cgraph_node *node)
> }
> clone = create_specialized_node (node, known_csts, known_contexts,
> aggvals, callers);
> - info = IPA_NODE_REF (node);
please either drop this change or change it to:
gcc_checking_assert (info == IPA_NODE_REF (node));
this line of code was actually necessary when adding nodes possibly
invalidated addresses of all summaries - like fast_function_summary
classes still do. So if we ever decide to use fast summaries we need a
test to remind us that info address must be obtained again.
Thanks for working on this!
Martin
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH V2] Generalized value pass-through for self-recursive function (ipa/pr93203)
2020-02-05 17:27 ` Martin Jambor
@ 2020-02-10 3:28 ` Feng Xue OS
2020-02-11 10:05 ` Tamar Christina
0 siblings, 1 reply; 17+ messages in thread
From: Feng Xue OS @ 2020-02-10 3:28 UTC (permalink / raw)
To: Martin Jambor, Jan Hubicka, gcc-patches
>> - gcc_checking_assert (item->value);
> I've been staring at this for quite a while, trying to figure out how
> your patch can put NULL here before I realized it was just a clean-up
> :-) Sending such changes independently or pointing them out in the
> email/ChangeLog makes review easier.
Ok. I'll add some description on this cleanup on ChangeLog.
>> @@ -5564,7 +5610,6 @@ decide_whether_version_node (struct cgraph_node *node)
>> }
>> clone = create_specialized_node (node, known_csts, known_contexts,
>> aggvals, callers);
>> - info = IPA_NODE_REF (node);
> please either drop this change or change it to:
>
> gcc_checking_assert (info == IPA_NODE_REF (node));
> this line of code was actually necessary when adding nodes possibly
> invalidated addresses of all summaries - like fast_function_summary
> classes still do. So if we ever decide to use fast summaries we need a
> test to remind us that info address must be obtained again.
Ok. I'm not aware that, will keep the line as original.
Thanks,
Feng
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH V2] Generalized value pass-through for self-recursive function (ipa/pr93203)
2020-02-10 3:28 ` Feng Xue OS
@ 2020-02-11 10:05 ` Tamar Christina
2020-02-11 13:10 ` Feng Xue OS
0 siblings, 1 reply; 17+ messages in thread
From: Tamar Christina @ 2020-02-11 10:05 UTC (permalink / raw)
To: Feng Xue OS, Martin Jambor, Jan Hubicka, gcc-patches; +Cc: nd
Hi Feng,
This patch (commit a0f6a8cb414b687f22c9011a894d5e8e398c4be0) is causing ICEs in the GCC and perlbench benchmark in Spec2017.
during IPA pass: cp
lto1: internal compiler error: in find_more_scalar_values_for_callers_subset, at ipa-cp.c:4709
0x1698187 find_more_scalar_values_for_callers_subset
../.././gcc/ipa-cp.c:4709
0x169f7d3 decide_about_value<tree_node*>
../.././gcc/ipa-cp.c:5490
0x169fdc3 decide_whether_version_node
../.././gcc/ipa-cp.c:5537
0x169fdc3 ipcp_decision_stage
../.././gcc/ipa-cp.c:5718
0x169fdc3 ipcp_driver
../.././gcc/ipa-cp.c:5901
Please submit a full bug report,
Thanks,
Tamar
> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org>
> On Behalf Of Feng Xue OS
> Sent: Monday, February 10, 2020 03:29
> To: Martin Jambor <mjambor@suse.cz>; Jan Hubicka <hubicka@ucw.cz>;
> gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH V2] Generalized value pass-through for self-recursive
> function (ipa/pr93203)
>
> >> - gcc_checking_assert (item->value);
>
> > I've been staring at this for quite a while, trying to figure out how
> > your patch can put NULL here before I realized it was just a clean-up
> > :-) Sending such changes independently or pointing them out in the
> > email/ChangeLog makes review easier.
>
> Ok. I'll add some description on this cleanup on ChangeLog.
>
> >> @@ -5564,7 +5610,6 @@ decide_whether_version_node (struct
> cgraph_node *node)
> >> }
> >> clone = create_specialized_node (node, known_csts, known_contexts,
> >> aggvals, callers);
> >> - info = IPA_NODE_REF (node);
>
> > please either drop this change or change it to:
> >
> > gcc_checking_assert (info == IPA_NODE_REF (node));
>
> > this line of code was actually necessary when adding nodes possibly
> > invalidated addresses of all summaries - like fast_function_summary
> > classes still do. So if we ever decide to use fast summaries we need
> > a test to remind us that info address must be obtained again.
> Ok. I'm not aware that, will keep the line as original.
>
> Thanks,
> Feng
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH V2] Generalized value pass-through for self-recursive function (ipa/pr93203)
2020-02-11 10:05 ` Tamar Christina
@ 2020-02-11 13:10 ` Feng Xue OS
2020-02-11 14:31 ` Tamar Christina
0 siblings, 1 reply; 17+ messages in thread
From: Feng Xue OS @ 2020-02-11 13:10 UTC (permalink / raw)
To: Tamar Christina, Martin Jambor, Jan Hubicka, gcc-patches; +Cc: nd
Hi Tarmar,
Since I could not reproduce this ICE with my local testing on Spec2017, would you share your config, or command line options extracted from compilation of perlbench?
Thanks,
Feng
________________________________________
From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org> on behalf of Tamar Christina <Tamar.Christina@arm.com>
Sent: Tuesday, February 11, 2020 6:05 PM
To: Feng Xue OS; Martin Jambor; Jan Hubicka; gcc-patches@gcc.gnu.org
Cc: nd
Subject: RE: [PATCH V2] Generalized value pass-through for self-recursive function (ipa/pr93203)
Hi Feng,
This patch (commit a0f6a8cb414b687f22c9011a894d5e8e398c4be0) is causing ICEs in the GCC and perlbench benchmark in Spec2017.
during IPA pass: cp
lto1: internal compiler error: in find_more_scalar_values_for_callers_subset, at ipa-cp.c:4709
0x1698187 find_more_scalar_values_for_callers_subset
../.././gcc/ipa-cp.c:4709
0x169f7d3 decide_about_value<tree_node*>
../.././gcc/ipa-cp.c:5490
0x169fdc3 decide_whether_version_node
../.././gcc/ipa-cp.c:5537
0x169fdc3 ipcp_decision_stage
../.././gcc/ipa-cp.c:5718
0x169fdc3 ipcp_driver
../.././gcc/ipa-cp.c:5901
Please submit a full bug report,
Thanks,
Tamar
> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org>
> On Behalf Of Feng Xue OS
> Sent: Monday, February 10, 2020 03:29
> To: Martin Jambor <mjambor@suse.cz>; Jan Hubicka <hubicka@ucw.cz>;
> gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH V2] Generalized value pass-through for self-recursive
> function (ipa/pr93203)
>
> >> - gcc_checking_assert (item->value);
>
> > I've been staring at this for quite a while, trying to figure out how
> > your patch can put NULL here before I realized it was just a clean-up
> > :-) Sending such changes independently or pointing them out in the
> > email/ChangeLog makes review easier.
>
> Ok. I'll add some description on this cleanup on ChangeLog.
>
> >> @@ -5564,7 +5610,6 @@ decide_whether_version_node (struct
> cgraph_node *node)
> >> }
> >> clone = create_specialized_node (node, known_csts, known_contexts,
> >> aggvals, callers);
> >> - info = IPA_NODE_REF (node);
>
> > please either drop this change or change it to:
> >
> > gcc_checking_assert (info == IPA_NODE_REF (node));
>
> > this line of code was actually necessary when adding nodes possibly
> > invalidated addresses of all summaries - like fast_function_summary
> > classes still do. So if we ever decide to use fast summaries we need
> > a test to remind us that info address must be obtained again.
> Ok. I'm not aware that, will keep the line as original.
>
> Thanks,
> Feng
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH V2] Generalized value pass-through for self-recursive function (ipa/pr93203)
2020-02-11 13:10 ` Feng Xue OS
@ 2020-02-11 14:31 ` Tamar Christina
2020-02-13 5:39 ` [PATCH] Fix bug in recursiveness check for function to be cloned (ipa/pr93707) Feng Xue OS
0 siblings, 1 reply; 17+ messages in thread
From: Tamar Christina @ 2020-02-11 14:31 UTC (permalink / raw)
To: Feng Xue OS, Martin Jambor, Jan Hubicka, gcc-patches; +Cc: nd
Hi Feng,
It looks like the option that causes this to trigger is `--param ipa-cp-eval-threshold=1`
So on AArch64 I get it to trigger with -mcpu=native -Ofast -fomit-frame-pointer -flto --param ipa-cp-eval-threshold=1
Kind Regards,
Tamar
> -----Original Message-----
> From: Feng Xue OS <fxue@os.amperecomputing.com>
> Sent: Tuesday, February 11, 2020 13:11
> To: Tamar Christina <Tamar.Christina@arm.com>; Martin Jambor
> <mjambor@suse.cz>; Jan Hubicka <hubicka@ucw.cz>; gcc-
> patches@gcc.gnu.org
> Cc: nd <nd@arm.com>
> Subject: Re: [PATCH V2] Generalized value pass-through for self-recursive
> function (ipa/pr93203)
>
> Hi Tarmar,
>
> Since I could not reproduce this ICE with my local testing on Spec2017, would
> you share your config, or command line options extracted from compilation
> of perlbench?
>
> Thanks,
> Feng
>
> ________________________________________
> From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org>
> on behalf of Tamar Christina <Tamar.Christina@arm.com>
> Sent: Tuesday, February 11, 2020 6:05 PM
> To: Feng Xue OS; Martin Jambor; Jan Hubicka; gcc-patches@gcc.gnu.org
> Cc: nd
> Subject: RE: [PATCH V2] Generalized value pass-through for self-recursive
> function (ipa/pr93203)
>
> Hi Feng,
>
> This patch (commit a0f6a8cb414b687f22c9011a894d5e8e398c4be0) is causing
> ICEs in the GCC and perlbench benchmark in Spec2017.
>
> during IPA pass: cp
> lto1: internal compiler error: in find_more_scalar_values_for_callers_subset,
> at ipa-cp.c:4709
> 0x1698187 find_more_scalar_values_for_callers_subset
> ../.././gcc/ipa-cp.c:4709
> 0x169f7d3 decide_about_value<tree_node*>
> ../.././gcc/ipa-cp.c:5490
> 0x169fdc3 decide_whether_version_node
> ../.././gcc/ipa-cp.c:5537
> 0x169fdc3 ipcp_decision_stage
> ../.././gcc/ipa-cp.c:5718
> 0x169fdc3 ipcp_driver
> ../.././gcc/ipa-cp.c:5901
> Please submit a full bug report,
>
> Thanks,
> Tamar
>
> > -----Original Message-----
> > From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org>
> On
> > Behalf Of Feng Xue OS
> > Sent: Monday, February 10, 2020 03:29
> > To: Martin Jambor <mjambor@suse.cz>; Jan Hubicka <hubicka@ucw.cz>;
> > gcc-patches@gcc.gnu.org
> > Subject: Re: [PATCH V2] Generalized value pass-through for
> > self-recursive function (ipa/pr93203)
> >
> > >> - gcc_checking_assert (item->value);
> >
> > > I've been staring at this for quite a while, trying to figure out
> > > how your patch can put NULL here before I realized it was just a
> > > clean-up
> > > :-) Sending such changes independently or pointing them out in the
> > > email/ChangeLog makes review easier.
> >
> > Ok. I'll add some description on this cleanup on ChangeLog.
> >
> > >> @@ -5564,7 +5610,6 @@ decide_whether_version_node (struct
> > cgraph_node *node)
> > >> }
> > >> clone = create_specialized_node (node, known_csts,
> known_contexts,
> > >> aggvals, callers);
> > >> - info = IPA_NODE_REF (node);
> >
> > > please either drop this change or change it to:
> > >
> > > gcc_checking_assert (info == IPA_NODE_REF (node));
> >
> > > this line of code was actually necessary when adding nodes possibly
> > > invalidated addresses of all summaries - like fast_function_summary
> > > classes still do. So if we ever decide to use fast summaries we
> > > need a test to remind us that info address must be obtained again.
> > Ok. I'm not aware that, will keep the line as original.
> >
> > Thanks,
> > Feng
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] Fix bug in recursiveness check for function to be cloned (ipa/pr93707)
2020-02-11 14:31 ` Tamar Christina
@ 2020-02-13 5:39 ` Feng Xue OS
2020-02-17 8:44 ` Tamar Christina
2020-02-19 16:28 ` Martin Jambor
0 siblings, 2 replies; 17+ messages in thread
From: Feng Xue OS @ 2020-02-13 5:39 UTC (permalink / raw)
To: Tamar Christina, Martin Jambor, Jan Hubicka, gcc-patches; +Cc: nd
[-- Attachment #1: Type: text/plain, Size: 2483 bytes --]
I've submitted a bug tracker, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93707.
The root cause is that for a self-recursive function, a for-all-contexts clone could generate
an edge whose callee is not the function. Therefore, to check whether an edge stands for a
recursive call during cloning, we should not use ordinary way of comparing caller and callee
of the edge.
Bootstrapped/regtested on x86_64-linux and aarch64-linux, and also tested Spec 2017 with
option "--param ipa-cp-eval-threshold=1".
Feng
---
2020-02-13 Feng Xue <fxue@os.amperecomputing.com>
PR ipa/93707
* ipa-cp.c (self_recursive_pass_through_p): Add a new parameter "node",
and check recursiveness by comparing "node" and cs->caller.
(self_recursive_agg_pass_through_p): Likewise.
(find_more_scalar_values_for_callers_subset): Add "node" argument to
self_recursive_pass_through_p call.
(intersect_aggregates_with_edge): Add a new parameter "node", and add
"node" argument to self_cursive_agg_pass_through_p call.
(find_aggregate_values_for_callers_subset): Add "node" argument to
self_recursive_pass_through_p and intersect_aggregates_with_edge calls.
(cgraph_edge_brings_all_agg_vals_for_node): Add "node" argument to
intersect_aggregates_with_edge call.
> ________________________________________
> From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org>
> on behalf of Tamar Christina <Tamar.Christina@arm.com>
> Sent: Tuesday, February 11, 2020 6:05 PM
> To: Feng Xue OS; Martin Jambor; Jan Hubicka; gcc-patches@gcc.gnu.org
> Cc: nd
> Subject: RE: [PATCH V2] Generalized value pass-through for self-recursive
> function (ipa/pr93203)
>
> Hi Feng,
>
> This patch (commit a0f6a8cb414b687f22c9011a894d5e8e398c4be0) is causing
> ICEs in the GCC and perlbench benchmark in Spec2017.
>
> during IPA pass: cp
> lto1: internal compiler error: in find_more_scalar_values_for_callers_subset,
> at ipa-cp.c:4709
> 0x1698187 find_more_scalar_values_for_callers_subset
> ../.././gcc/ipa-cp.c:4709
> 0x169f7d3 decide_about_value<tree_node*>
> ../.././gcc/ipa-cp.c:5490
> 0x169fdc3 decide_whether_version_node
> ../.././gcc/ipa-cp.c:5537
> 0x169fdc3 ipcp_decision_stage
> ../.././gcc/ipa-cp.c:5718
> 0x169fdc3 ipcp_driver
> ../.././gcc/ipa-cp.c:5901
> Please submit a full bug report,
>
> Thanks,
> Tamar
>
[-- Attachment #2: 0001-Fix-bug-in-recursiveness-check-for-function-in-clone.patch --]
[-- Type: application/octet-stream, Size: 6301 bytes --]
From 40ff412bfef584461ddc16fbbaaad3275a294d64 Mon Sep 17 00:00:00 2001
From: Feng Xue <fxue@os.amperecomputing.com>
Date: Wed, 12 Feb 2020 13:04:33 -0500
Subject: [PATCH] Fix bug in recursiveness check for function in clone (PR
ipa/93707)
---
gcc/ipa-cp.c | 54 +++++++++++++++++-------------
gcc/testsuite/gcc.dg/ipa/pr93707.c | 29 ++++++++++++++++
2 files changed, 60 insertions(+), 23 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/ipa/pr93707.c
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 4f5b72e6994..9fbeeae9e77 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -4617,17 +4617,21 @@ create_specialized_node (struct cgraph_node *node,
return new_node;
}
-/* Return true, if JFUNC, which describes a i-th parameter of call CS, is a
- pass-through function to itself. When SIMPLE is true, further check if
- JFUNC is a simple no-operation pass-through. */
+/* Given an edge CS, along which we will do cloning for NODE, return true,
+ if CS represents a self-recursive call to NODE, and if JFUNC, which
+ describes the i-th argument of CS, is a pass-through jump function to the
+ i-th parameter of NODE. When SIMPLE is true, further check if JFUNC is a
+ simple no-operation pass-through. */
static bool
-self_recursive_pass_through_p (cgraph_edge *cs, ipa_jump_func *jfunc, int i,
- bool simple = true)
-{
- enum availability availability;
- if (cs->caller == cs->callee->function_symbol (&availability)
- && availability > AVAIL_INTERPOSABLE
+self_recursive_pass_through_p (cgraph_edge *cs, cgraph_node *node,
+ ipa_jump_func *jfunc,
+ int i, bool simple = true)
+{
+ /* CS could be created from a for-all-contexts clone, thus NODE might not
+ be callee of CS. Here we could not resort to ordinary way of comparing
+ caller and callee of CS to check recursiveness. */
+ if (cs->caller == node
&& jfunc->type == IPA_JF_PASS_THROUGH
&& (!simple || ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
&& ipa_get_jf_pass_through_formal_id (jfunc) == i)
@@ -4635,18 +4639,19 @@ self_recursive_pass_through_p (cgraph_edge *cs, ipa_jump_func *jfunc, int i,
return false;
}
-/* Return true, if JFUNC, which describes a part of an aggregate represented
- or pointed to by the i-th parameter of call CS, is a pass-through function
- to itself. When SIMPLE is true, further check if JFUNC is a simple
- no-operation pass-through. */
+/* Given an edge CS, along which we will do cloning for NODE, return true,
+ if CS represents a self-recursive call to NODE, and if JFUNC, which
+ describes a part of an aggregate represented or pointed to by the i-th
+ argument of CS, is a pass-through jump function to same aggregate part of
+ the i-th parameter of NODE. When SIMPLE is true, further check if JFUNC
+ is a simple no-operation pass-through. */
static bool
-self_recursive_agg_pass_through_p (cgraph_edge *cs, ipa_agg_jf_item *jfunc,
+self_recursive_agg_pass_through_p (cgraph_edge *cs, cgraph_node *node,
+ ipa_agg_jf_item *jfunc,
int i, bool simple = true)
{
- enum availability availability;
- if (cs->caller == cs->callee->function_symbol (&availability)
- && availability > AVAIL_INTERPOSABLE
+ if (cs->caller == node
&& jfunc->jftype == IPA_JF_LOAD_AGG
&& jfunc->offset == jfunc->value.load_agg.offset
&& (!simple || jfunc->value.pass_through.operation == NOP_EXPR)
@@ -4704,7 +4709,7 @@ find_more_scalar_values_for_callers_subset (struct cgraph_node *node,
Given that i is 0, recursive propagation via (i & 1) also gets
0. */
- if (self_recursive_pass_through_p (cs, jump_func, i, false))
+ if (self_recursive_pass_through_p (cs, node, jump_func, i, false))
{
gcc_assert (newval);
t = ipa_get_jf_arith_result (
@@ -4952,7 +4957,9 @@ intersect_with_agg_replacements (struct cgraph_node *node, int index,
whatsoever, return a released vector. */
static vec<ipa_agg_value>
-intersect_aggregates_with_edge (struct cgraph_edge *cs, int index,
+intersect_aggregates_with_edge (struct cgraph_edge *cs,
+ struct cgraph_node *node,
+ int index,
vec<ipa_agg_value> inter)
{
struct ipa_jump_func *jfunc;
@@ -5085,7 +5092,7 @@ intersect_aggregates_with_edge (struct cgraph_edge *cs, int index,
Given that *i is 0, recursive propagation via (*i & 1)
also gets 0. */
- if (self_recursive_agg_pass_through_p (cs, ti, index,
+ if (self_recursive_agg_pass_through_p (cs, node, ti, index,
false))
value = ipa_get_jf_arith_result (
ti->value.pass_through.operation,
@@ -5156,11 +5163,11 @@ find_aggregate_values_for_callers_subset (struct cgraph_node *node,
{
struct ipa_jump_func *jfunc
= ipa_get_ith_jump_func (IPA_EDGE_REF (cs), i);
- if (self_recursive_pass_through_p (cs, jfunc, i)
+ if (self_recursive_pass_through_p (cs, node, jfunc, i)
&& (!plats->aggs_by_ref
|| ipa_get_jf_pass_through_agg_preserved (jfunc)))
continue;
- inter = intersect_aggregates_with_edge (cs, i, inter);
+ inter = intersect_aggregates_with_edge (cs, node, i, inter);
if (!inter.exists ())
goto next_param;
@@ -5265,7 +5272,8 @@ cgraph_edge_brings_all_agg_vals_for_node (struct cgraph_edge *cs,
if (plats->aggs_bottom)
return false;
- vec<ipa_agg_value> values = intersect_aggregates_with_edge (cs, i, vNULL);
+ vec<ipa_agg_value> values
+ = intersect_aggregates_with_edge (cs, node, i, vNULL);
if (!values.exists ())
return false;
diff --git a/gcc/testsuite/gcc.dg/ipa/pr93707.c b/gcc/testsuite/gcc.dg/ipa/pr93707.c
new file mode 100644
index 00000000000..e200a3a432b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr93707.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 --param ipa-cp-eval-threshold=1" } */
+
+int foo();
+int data[100];
+
+__attribute__((noinline)) static int recur_fn (int i, int j, int depth)
+{
+ if (depth > 10)
+ return 1;
+
+ data[i + j]++;
+
+ if (depth & 3)
+ recur_fn (i, 1, depth + 1);
+ else
+ recur_fn (i, j & 1, depth + 1);
+
+ foo();
+
+ return i + j;
+}
+
+int caller (int v, int depth)
+{
+ recur_fn (1, v, depth);
+
+ return 0;
+}
--
2.17.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH] Fix bug in recursiveness check for function to be cloned (ipa/pr93707)
2020-02-13 5:39 ` [PATCH] Fix bug in recursiveness check for function to be cloned (ipa/pr93707) Feng Xue OS
@ 2020-02-17 8:44 ` Tamar Christina
2020-02-18 15:16 ` Ping: " Feng Xue OS
2020-02-19 16:28 ` Martin Jambor
1 sibling, 1 reply; 17+ messages in thread
From: Tamar Christina @ 2020-02-17 8:44 UTC (permalink / raw)
To: Feng Xue OS, Martin Jambor, Jan Hubicka, gcc-patches; +Cc: nd
Hi Feng,
Thanks! The patch seems to work.
Hopefully it gets reviewed soon so we can fix the two benchmarks 😊
Thanks,
Tamar
> -----Original Message-----
> From: Feng Xue OS <fxue@os.amperecomputing.com>
> Sent: Thursday, February 13, 2020 05:40
> To: Tamar Christina <Tamar.Christina@arm.com>; Martin Jambor
> <mjambor@suse.cz>; Jan Hubicka <hubicka@ucw.cz>; gcc-
> patches@gcc.gnu.org
> Cc: nd <nd@arm.com>
> Subject: [PATCH] Fix bug in recursiveness check for function to be cloned
> (ipa/pr93707)
>
> I've submitted a bug tracker,
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93707.
>
> The root cause is that for a self-recursive function, a for-all-contexts clone
> could generate an edge whose callee is not the function. Therefore, to check
> whether an edge stands for a recursive call during cloning, we should not use
> ordinary way of comparing caller and callee of the edge.
>
> Bootstrapped/regtested on x86_64-linux and aarch64-linux, and also tested
> Spec 2017 with option "--param ipa-cp-eval-threshold=1".
>
> Feng
> ---
> 2020-02-13 Feng Xue <fxue@os.amperecomputing.com>
>
> PR ipa/93707
> * ipa-cp.c (self_recursive_pass_through_p): Add a new parameter
> "node",
> and check recursiveness by comparing "node" and cs->caller.
> (self_recursive_agg_pass_through_p): Likewise.
> (find_more_scalar_values_for_callers_subset): Add "node" argument to
> self_recursive_pass_through_p call.
> (intersect_aggregates_with_edge): Add a new parameter "node", and
> add
> "node" argument to self_cursive_agg_pass_through_p call.
> (find_aggregate_values_for_callers_subset): Add "node" argument to
> self_recursive_pass_through_p and intersect_aggregates_with_edge
> calls.
> (cgraph_edge_brings_all_agg_vals_for_node): Add "node" argument to
> intersect_aggregates_with_edge call.
>
> > ________________________________________
> > From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org>
> on
> > behalf of Tamar Christina <Tamar.Christina@arm.com>
> > Sent: Tuesday, February 11, 2020 6:05 PM
> > To: Feng Xue OS; Martin Jambor; Jan Hubicka; gcc-patches@gcc.gnu.org
> > Cc: nd
> > Subject: RE: [PATCH V2] Generalized value pass-through for
> > self-recursive function (ipa/pr93203)
> >
> > Hi Feng,
> >
> > This patch (commit a0f6a8cb414b687f22c9011a894d5e8e398c4be0) is
> > causing ICEs in the GCC and perlbench benchmark in Spec2017.
> >
> > during IPA pass: cp
> > lto1: internal compiler error: in
> > find_more_scalar_values_for_callers_subset,
> > at ipa-cp.c:4709
> > 0x1698187 find_more_scalar_values_for_callers_subset
> > ../.././gcc/ipa-cp.c:4709
> > 0x169f7d3 decide_about_value<tree_node*>
> > ../.././gcc/ipa-cp.c:5490
> > 0x169fdc3 decide_whether_version_node
> > ../.././gcc/ipa-cp.c:5537
> > 0x169fdc3 ipcp_decision_stage
> > ../.././gcc/ipa-cp.c:5718
> > 0x169fdc3 ipcp_driver
> > ../.././gcc/ipa-cp.c:5901
> > Please submit a full bug report,
> >
> > Thanks,
> > Tamar
> >
^ permalink raw reply [flat|nested] 17+ messages in thread
* Ping: [PATCH] Fix bug in recursiveness check for function to be cloned (ipa/pr93707)
2020-02-17 8:44 ` Tamar Christina
@ 2020-02-18 15:16 ` Feng Xue OS
0 siblings, 0 replies; 17+ messages in thread
From: Feng Xue OS @ 2020-02-18 15:16 UTC (permalink / raw)
To: Tamar Christina, Martin Jambor, Jan Hubicka, gcc-patches; +Cc: nd
Thanks,
Feng
________________________________________
From: Tamar Christina <Tamar.Christina@arm.com>
Sent: Monday, February 17, 2020 4:44 PM
To: Feng Xue OS; Martin Jambor; Jan Hubicka; gcc-patches@gcc.gnu.org
Cc: nd
Subject: RE: [PATCH] Fix bug in recursiveness check for function to be cloned (ipa/pr93707)
Hi Feng,
Thanks! The patch seems to work.
Hopefully it gets reviewed soon so we can fix the two benchmarks 😊
Thanks,
Tamar
> -----Original Message-----
> From: Feng Xue OS <fxue@os.amperecomputing.com>
> Sent: Thursday, February 13, 2020 05:40
> To: Tamar Christina <Tamar.Christina@arm.com>; Martin Jambor
> <mjambor@suse.cz>; Jan Hubicka <hubicka@ucw.cz>; gcc-
> patches@gcc.gnu.org
> Cc: nd <nd@arm.com>
> Subject: [PATCH] Fix bug in recursiveness check for function to be cloned
> (ipa/pr93707)
>
> I've submitted a bug tracker,
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93707.
>
> The root cause is that for a self-recursive function, a for-all-contexts clone
> could generate an edge whose callee is not the function. Therefore, to check
> whether an edge stands for a recursive call during cloning, we should not use
> ordinary way of comparing caller and callee of the edge.
>
> Bootstrapped/regtested on x86_64-linux and aarch64-linux, and also tested
> Spec 2017 with option "--param ipa-cp-eval-threshold=1".
>
> Feng
> ---
> 2020-02-13 Feng Xue <fxue@os.amperecomputing.com>
>
> PR ipa/93707
> * ipa-cp.c (self_recursive_pass_through_p): Add a new parameter
> "node",
> and check recursiveness by comparing "node" and cs->caller.
> (self_recursive_agg_pass_through_p): Likewise.
> (find_more_scalar_values_for_callers_subset): Add "node" argument to
> self_recursive_pass_through_p call.
> (intersect_aggregates_with_edge): Add a new parameter "node", and
> add
> "node" argument to self_cursive_agg_pass_through_p call.
> (find_aggregate_values_for_callers_subset): Add "node" argument to
> self_recursive_pass_through_p and intersect_aggregates_with_edge
> calls.
> (cgraph_edge_brings_all_agg_vals_for_node): Add "node" argument to
> intersect_aggregates_with_edge call.
>
> > ________________________________________
> > From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org>
> on
> > behalf of Tamar Christina <Tamar.Christina@arm.com>
> > Sent: Tuesday, February 11, 2020 6:05 PM
> > To: Feng Xue OS; Martin Jambor; Jan Hubicka; gcc-patches@gcc.gnu.org
> > Cc: nd
> > Subject: RE: [PATCH V2] Generalized value pass-through for
> > self-recursive function (ipa/pr93203)
> >
> > Hi Feng,
> >
> > This patch (commit a0f6a8cb414b687f22c9011a894d5e8e398c4be0) is
> > causing ICEs in the GCC and perlbench benchmark in Spec2017.
> >
> > during IPA pass: cp
> > lto1: internal compiler error: in
> > find_more_scalar_values_for_callers_subset,
> > at ipa-cp.c:4709
> > 0x1698187 find_more_scalar_values_for_callers_subset
> > ../.././gcc/ipa-cp.c:4709
> > 0x169f7d3 decide_about_value<tree_node*>
> > ../.././gcc/ipa-cp.c:5490
> > 0x169fdc3 decide_whether_version_node
> > ../.././gcc/ipa-cp.c:5537
> > 0x169fdc3 ipcp_decision_stage
> > ../.././gcc/ipa-cp.c:5718
> > 0x169fdc3 ipcp_driver
> > ../.././gcc/ipa-cp.c:5901
> > Please submit a full bug report,
> >
> > Thanks,
> > Tamar
> >
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Fix bug in recursiveness check for function to be cloned (ipa/pr93707)
2020-02-13 5:39 ` [PATCH] Fix bug in recursiveness check for function to be cloned (ipa/pr93707) Feng Xue OS
2020-02-17 8:44 ` Tamar Christina
@ 2020-02-19 16:28 ` Martin Jambor
2020-02-20 3:36 ` Feng Xue OS
2020-02-20 12:57 ` Tamar Christina
1 sibling, 2 replies; 17+ messages in thread
From: Martin Jambor @ 2020-02-19 16:28 UTC (permalink / raw)
To: Feng Xue OS, Tamar Christina, Jan Hubicka, gcc-patches; +Cc: nd
Hi,
On Thu, Feb 13 2020, Feng Xue OS wrote:
> I've submitted a bug tracker, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93707.
>
> The root cause is that for a self-recursive function, a for-all-contexts clone could generate
> an edge whose callee is not the function. Therefore, to check whether an edge stands for a
> recursive call during cloning, we should not use ordinary way of comparing caller and callee
> of the edge.
>
> Bootstrapped/regtested on x86_64-linux and aarch64-linux, and also tested Spec 2017 with
> option "--param ipa-cp-eval-threshold=1".
>
> Feng
> ---
> 2020-02-13 Feng Xue <fxue@os.amperecomputing.com>
>
> PR ipa/93707
> * ipa-cp.c (self_recursive_pass_through_p): Add a new parameter "node",
> and check recursiveness by comparing "node" and cs->caller.
> (self_recursive_agg_pass_through_p): Likewise.
> (find_more_scalar_values_for_callers_subset): Add "node" argument to
> self_recursive_pass_through_p call.
> (intersect_aggregates_with_edge): Add a new parameter "node", and add
> "node" argument to self_cursive_agg_pass_through_p call.
> (find_aggregate_values_for_callers_subset): Add "node" argument to
> self_recursive_pass_through_p and intersect_aggregates_with_edge calls.
> (cgraph_edge_brings_all_agg_vals_for_node): Add "node" argument to
> intersect_aggregates_with_edge call.
first, thanks for a very nice testcase and for working on this.
However, I believe that his approach mostly papers over a bug that
happens earlier, specifically that cgraph_edge_brings_value_p returned
true for the self-recursive edge from all-context clone to itself, even
though when evaluating the second argument. We assume that all context
clones get at least as many constants as any other potential clone, but
that does not work for self-recursive edges with pass-through parameters
that that just pass along the received constant.
That is how we got a wrong edge in the vector of edges bringing the
constant and it is the primary reason why self_recursive_pass_through_p
and its users did not work correctly.
I would therefore like to fix the problem with the following patch. It
has passed bootstrap and testing on x86_64-linux, LTO bootstrap is
underway.
Honza, is it OK for trunk? Tamar, can you please double check it fixes
your problem with perlbench?
Thanks,
Martin
ipa-cp: Avoid wrongly gathering self-recursive edges (PR 93707)
2020-02-18 Martin Jambor <mjambor@suse.cz>
Feng Xue <fxue@os.amperecomputing.com>
PR ipa/93707
* ipa-cp.c (same_node_or_its_all_contexts_clone_p): Replaced with
new function calls_same_node_or_its_all_contexts_clone_p.
(cgraph_edge_brings_value_p): Use it. Fix comment.
(cgraph_edge_brings_value_p): Likewise.
testsuite/
* gcc.dg/ipa/pr93707.c: New test.
---
gcc/ChangeLog | 9 +++++++++
gcc/ipa-cp.c | 29 ++++++++++++++---------------
gcc/testsuite/ChangeLog | 6 ++++++
gcc/testsuite/gcc.dg/ipa/pr93707.c | 29 +++++++++++++++++++++++++++++
4 files changed, 58 insertions(+), 15 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/ipa/pr93707.c
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 4f5b72e6994..4609375bf8d 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -4033,15 +4033,23 @@ edge_clone_summary_t::duplicate (cgraph_edge *src_edge, cgraph_edge *dst_edge,
src_data->next_clone = dst_edge;
}
-/* Return true is NODE is DEST or its clone for all contexts. */
+/* Return true is CS calls DEST or its clone for all contexts, except for
+ self-recursive nodes in which it has to be DEST itself. */
static bool
-same_node_or_its_all_contexts_clone_p (cgraph_node *node, cgraph_node *dest)
+calls_same_node_or_its_all_contexts_clone_p (cgraph_edge *cs, cgraph_node *dest)
{
- if (node == dest)
+ enum availability availability;
+ cgraph_node *callee = cs->callee->function_symbol (&availability);
+
+ if (availability <= AVAIL_INTERPOSABLE)
+ return false;
+ if (callee == dest)
return true;
+ if (cs->caller == callee)
+ return false;
- class ipa_node_params *info = IPA_NODE_REF (node);
+ class ipa_node_params *info = IPA_NODE_REF (callee);
return info->is_all_contexts_clone && info->ipcp_orig_node == dest;
}
@@ -4053,11 +4061,8 @@ cgraph_edge_brings_value_p (cgraph_edge *cs, ipcp_value_source<tree> *src,
cgraph_node *dest, ipcp_value<tree> *dest_val)
{
class ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
- enum availability availability;
- cgraph_node *real_dest = cs->callee->function_symbol (&availability);
- if (availability <= AVAIL_INTERPOSABLE
- || !same_node_or_its_all_contexts_clone_p (real_dest, dest)
+ if (!calls_same_node_or_its_all_contexts_clone_p (cs, dest)
|| caller_info->node_dead)
return false;
@@ -4076,9 +4081,6 @@ cgraph_edge_brings_value_p (cgraph_edge *cs, ipcp_value_source<tree> *src,
}
else
{
- /* At the moment we do not propagate over arithmetic jump functions in
- SCCs, so it is safe to detect self-feeding recursive calls in this
- way. */
if (src->val == dest_val)
return true;
@@ -4113,11 +4115,8 @@ cgraph_edge_brings_value_p (cgraph_edge *cs,
ipcp_value<ipa_polymorphic_call_context> *)
{
class ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
- enum availability avail;
- cgraph_node *real_dest = cs->callee->function_symbol (&avail);
- if (avail <= AVAIL_INTERPOSABLE
- || !same_node_or_its_all_contexts_clone_p (real_dest, dest)
+ if (!calls_same_node_or_its_all_contexts_clone_p (cs, dest)
|| caller_info->node_dead)
return false;
if (!src->val)
diff --git a/gcc/testsuite/gcc.dg/ipa/pr93707.c b/gcc/testsuite/gcc.dg/ipa/pr93707.c
new file mode 100644
index 00000000000..e200a3a432b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr93707.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 --param ipa-cp-eval-threshold=1" } */
+
+int foo();
+int data[100];
+
+__attribute__((noinline)) static int recur_fn (int i, int j, int depth)
+{
+ if (depth > 10)
+ return 1;
+
+ data[i + j]++;
+
+ if (depth & 3)
+ recur_fn (i, 1, depth + 1);
+ else
+ recur_fn (i, j & 1, depth + 1);
+
+ foo();
+
+ return i + j;
+}
+
+int caller (int v, int depth)
+{
+ recur_fn (1, v, depth);
+
+ return 0;
+}
--
2.25.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Fix bug in recursiveness check for function to be cloned (ipa/pr93707)
2020-02-19 16:28 ` Martin Jambor
@ 2020-02-20 3:36 ` Feng Xue OS
2020-02-21 18:16 ` Martin Jambor
2020-02-20 12:57 ` Tamar Christina
1 sibling, 1 reply; 17+ messages in thread
From: Feng Xue OS @ 2020-02-20 3:36 UTC (permalink / raw)
To: Martin Jambor, Tamar Christina, Jan Hubicka, gcc-patches; +Cc: nd
This is a simpel and nice fix, but could suppress some CP opportunities for
self-recursive call. Using the test case as example, the first should be a
for-all-context clone, and the call "recur_fn (i, 1, depth + 1)" is replaced with
a newly created recursive node. Thus, in the next round of CP iteration, the
way to do CP for the 2nd arugment "1" is blocked, because its coming edge
can not pass check by cgraph_edge_brings_value_p().
+__attribute__((noinline)) static int recur_fn (int i, int j, int depth)
+{
+ if (depth > 10)
+ return 1;
+
+ data[i + j]++;
+
+ if (depth & 3)
+ recur_fn (i, 1, depth + 1);
+ else
+ recur_fn (i, j & 1, depth + 1);
+
+ foo();
+
+ return i + j;
+}
+
+int caller (int v, int depth)
+{
+ recur_fn (1, v, depth);
+
+ return 0;
+}
>However, I believe that his approach mostly papers over a bug that
>happens earlier, specifically that cgraph_edge_brings_value_p returned
>true for the self-recursive edge from all-context clone to itself, even
>though when evaluating the second argument. We assume that all context
>clones get at least as many constants as any other potential clone, but
>that does not work for self-recursive edges with pass-through parameters
>that that just pass along the received constant.
The following check on value in cgraph_edge_brings_value_p could ensure
whether the value can arrive the dest node or not. If the value is a constant
without source, as above example "1", this is allowed. Otherwise, code snippet
enclosed by "if (caller_info->ipcp_orig_node)" could capture for-all-context clone.
Thanks,
Feng
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH] Fix bug in recursiveness check for function to be cloned (ipa/pr93707)
2020-02-19 16:28 ` Martin Jambor
2020-02-20 3:36 ` Feng Xue OS
@ 2020-02-20 12:57 ` Tamar Christina
1 sibling, 0 replies; 17+ messages in thread
From: Tamar Christina @ 2020-02-20 12:57 UTC (permalink / raw)
To: Martin Jambor, Feng Xue OS, Jan Hubicka, gcc-patches; +Cc: nd
Hi Martin,
> Honza, is it OK for trunk? Tamar, can you please double check it fixes
> your problem with perlbench?
>
Thanks for working on this! Yes it does seem to fix the regression as well.
Cheers,
Tamar
> Thanks,
>
> Martin
>
>
> ipa-cp: Avoid wrongly gathering self-recursive edges (PR 93707)
>
> 2020-02-18 Martin Jambor <mjambor@suse.cz>
> Feng Xue <fxue@os.amperecomputing.com>
>
> PR ipa/93707
> * ipa-cp.c (same_node_or_its_all_contexts_clone_p): Replaced with
> new function calls_same_node_or_its_all_contexts_clone_p.
> (cgraph_edge_brings_value_p): Use it. Fix comment.
> (cgraph_edge_brings_value_p): Likewise.
>
> testsuite/
> * gcc.dg/ipa/pr93707.c: New test.
> ---
> gcc/ChangeLog | 9 +++++++++
> gcc/ipa-cp.c | 29 ++++++++++++++---------------
> gcc/testsuite/ChangeLog | 6 ++++++
> gcc/testsuite/gcc.dg/ipa/pr93707.c | 29
> +++++++++++++++++++++++++++++
> 4 files changed, 58 insertions(+), 15 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/ipa/pr93707.c
>
> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> index 4f5b72e6994..4609375bf8d 100644
> --- a/gcc/ipa-cp.c
> +++ b/gcc/ipa-cp.c
> @@ -4033,15 +4033,23 @@ edge_clone_summary_t::duplicate
> (cgraph_edge *src_edge, cgraph_edge *dst_edge,
> src_data->next_clone = dst_edge;
> }
>
> -/* Return true is NODE is DEST or its clone for all contexts. */
> +/* Return true is CS calls DEST or its clone for all contexts, except for
> + self-recursive nodes in which it has to be DEST itself. */
>
> static bool
> -same_node_or_its_all_contexts_clone_p (cgraph_node *node,
> cgraph_node *dest)
> +calls_same_node_or_its_all_contexts_clone_p (cgraph_edge *cs,
> cgraph_node *dest)
> {
> - if (node == dest)
> + enum availability availability;
> + cgraph_node *callee = cs->callee->function_symbol (&availability);
> +
> + if (availability <= AVAIL_INTERPOSABLE)
> + return false;
> + if (callee == dest)
> return true;
> + if (cs->caller == callee)
> + return false;
>
> - class ipa_node_params *info = IPA_NODE_REF (node);
> + class ipa_node_params *info = IPA_NODE_REF (callee);
> return info->is_all_contexts_clone && info->ipcp_orig_node == dest;
> }
>
> @@ -4053,11 +4061,8 @@ cgraph_edge_brings_value_p (cgraph_edge *cs,
> ipcp_value_source<tree> *src,
> cgraph_node *dest, ipcp_value<tree> *dest_val)
> {
> class ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
> - enum availability availability;
> - cgraph_node *real_dest = cs->callee->function_symbol (&availability);
>
> - if (availability <= AVAIL_INTERPOSABLE
> - || !same_node_or_its_all_contexts_clone_p (real_dest, dest)
> + if (!calls_same_node_or_its_all_contexts_clone_p (cs, dest)
> || caller_info->node_dead)
> return false;
>
> @@ -4076,9 +4081,6 @@ cgraph_edge_brings_value_p (cgraph_edge *cs,
> ipcp_value_source<tree> *src,
> }
> else
> {
> - /* At the moment we do not propagate over arithmetic jump functions in
> - SCCs, so it is safe to detect self-feeding recursive calls in this
> - way. */
> if (src->val == dest_val)
> return true;
>
> @@ -4113,11 +4115,8 @@ cgraph_edge_brings_value_p (cgraph_edge *cs,
> ipcp_value<ipa_polymorphic_call_context> *)
> {
> class ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
> - enum availability avail;
> - cgraph_node *real_dest = cs->callee->function_symbol (&avail);
>
> - if (avail <= AVAIL_INTERPOSABLE
> - || !same_node_or_its_all_contexts_clone_p (real_dest, dest)
> + if (!calls_same_node_or_its_all_contexts_clone_p (cs, dest)
> || caller_info->node_dead)
> return false;
> if (!src->val)
> diff --git a/gcc/testsuite/gcc.dg/ipa/pr93707.c
> b/gcc/testsuite/gcc.dg/ipa/pr93707.c
> new file mode 100644
> index 00000000000..e200a3a432b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ipa/pr93707.c
> @@ -0,0 +1,29 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3 --param ipa-cp-eval-threshold=1" } */
> +
> +int foo();
> +int data[100];
> +
> +__attribute__((noinline)) static int recur_fn (int i, int j, int depth)
> +{
> + if (depth > 10)
> + return 1;
> +
> + data[i + j]++;
> +
> + if (depth & 3)
> + recur_fn (i, 1, depth + 1);
> + else
> + recur_fn (i, j & 1, depth + 1);
> +
> + foo();
> +
> + return i + j;
> +}
> +
> +int caller (int v, int depth)
> +{
> + recur_fn (1, v, depth);
> +
> + return 0;
> +}
> --
> 2.25.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Fix bug in recursiveness check for function to be cloned (ipa/pr93707)
2020-02-20 3:36 ` Feng Xue OS
@ 2020-02-21 18:16 ` Martin Jambor
2020-02-22 3:32 ` Feng Xue OS
0 siblings, 1 reply; 17+ messages in thread
From: Martin Jambor @ 2020-02-21 18:16 UTC (permalink / raw)
To: Feng Xue OS, Tamar Christina, Jan Hubicka, gcc-patches; +Cc: nd
Hi,
On Thu, Feb 20 2020, Feng Xue OS wrote:
> This is a simpel and nice fix, but could suppress some CP opportunities for
> self-recursive call. Using the test case as example, the first should be a
> for-all-context clone, and the call "recur_fn (i, 1, depth + 1)" is replaced with
> a newly created recursive node. Thus, in the next round of CP iteration, the
> way to do CP for the 2nd arugment "1" is blocked, because its coming edge
> can not pass check by cgraph_edge_brings_value_p().
>
> +__attribute__((noinline)) static int recur_fn (int i, int j, int depth)
> +{
> + if (depth > 10)
> + return 1;
> +
> + data[i + j]++;
> +
> + if (depth & 3)
> + recur_fn (i, 1, depth + 1);
> + else
> + recur_fn (i, j & 1, depth + 1);
> +
> + foo();
> +
> + return i + j;
> +}
> +
> +int caller (int v, int depth)
> +{
> + recur_fn (1, v, depth);
> +
> + return 0;
> +}
>
>
>>However, I believe that his approach mostly papers over a bug that
>>happens earlier, specifically that cgraph_edge_brings_value_p returned
>>true for the self-recursive edge from all-context clone to itself, even
>>though when evaluating the second argument. We assume that all context
>>clones get at least as many constants as any other potential clone, but
>>that does not work for self-recursive edges with pass-through parameters
>>that that just pass along the received constant.
>
> The following check on value in cgraph_edge_brings_value_p could ensure
> whether the value can arrive the dest node or not. If the value is a constant
> without source, as above example "1", this is allowed. Otherwise, code snippet
> enclosed by "if (caller_info->ipcp_orig_node)" could capture for-all-context clone.
there has not been any "following check" in your email but I believe I
understand what you mean, and I added such check to my patch so that the
edge carrying the non-pass through jump function was accepted by the
cgraph_edge_brings_value_p predicate.
However, that lead to the same assert in
find_more_scalar_values_for_callers_subset because on that edge it tried
to compute the depth + 1 value before it had any value to calculate it
from.
So after staring at the problem for another while I realized that the
users self_recursive_pass_through_p and
self_recursive_agg_pass_through_p would be OK if it returned false for
self-recursive calls from/to a node which is already a clone - clones
have their known constant values set at the point of their creation -
and that doing so avoids this problem. So that is what the patch below
does. I have still kept the cgraph_edge_brings_value_p hunks too, so
that edges are collected reliably.
Bootstrapped and tested on an x86_64-linux, LTO bootstrap underway.
What do you think?
Martin
2020-02-21 Martin Jambor <mjambor@suse.cz>
Feng Xue <fxue@os.amperecomputing.com>
PR ipa/93707
* ipa-cp.c (same_node_or_its_all_contexts_clone_p): Replaced with
new function calls_same_node_or_its_all_contexts_clone_p.
(cgraph_edge_brings_value_p): Use it.
(cgraph_edge_brings_value_p): Likewise.
(self_recursive_pass_through_p): Return false if caller is a clone.
(self_recursive_agg_pass_through_p): Likewise.
testsuite/
* gcc.dg/ipa/pr93707.c: New test.
---
gcc/ChangeLog | 11 +++++++++
gcc/ipa-cp.c | 38 +++++++++++++++++-------------
gcc/testsuite/ChangeLog | 6 +++++
gcc/testsuite/gcc.dg/ipa/pr93707.c | 31 ++++++++++++++++++++++++
4 files changed, 69 insertions(+), 17 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/ipa/pr93707.c
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 7c481407de9..a965cae4f07 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,14 @@
+2020-02-21 Martin Jambor <mjambor@suse.cz>
+ Feng Xue <fxue@os.amperecomputing.com>
+
+ PR ipa/93707
+ * ipa-cp.c (same_node_or_its_all_contexts_clone_p): Replaced with
+ new function calls_same_node_or_its_all_contexts_clone_p.
+ (cgraph_edge_brings_value_p): Use it.
+ (cgraph_edge_brings_value_p): Likewise.
+ (self_recursive_pass_through_p): Return false if caller is a clone.
+ (self_recursive_agg_pass_through_p): Likewise.
+
2020-02-17 Richard Biener <rguenther@suse.de>
PR c/86134
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 4f5b72e6994..aa228df1204 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -4033,15 +4033,24 @@ edge_clone_summary_t::duplicate (cgraph_edge *src_edge, cgraph_edge *dst_edge,
src_data->next_clone = dst_edge;
}
-/* Return true is NODE is DEST or its clone for all contexts. */
+/* Return true is CS calls DEST or its clone for all contexts, except for
+ self-recursive nodes in which it has to be DEST itself. */
static bool
-same_node_or_its_all_contexts_clone_p (cgraph_node *node, cgraph_node *dest)
+calls_same_node_or_its_all_contexts_clone_p (cgraph_edge *cs, cgraph_node *dest,
+ bool allow_recursion_to_clone)
{
- if (node == dest)
+ enum availability availability;
+ cgraph_node *callee = cs->callee->function_symbol (&availability);
+
+ if (availability <= AVAIL_INTERPOSABLE)
+ return false;
+ if (callee == dest)
return true;
+ if (!allow_recursion_to_clone && cs->caller == callee)
+ return false;
- class ipa_node_params *info = IPA_NODE_REF (node);
+ class ipa_node_params *info = IPA_NODE_REF (callee);
return info->is_all_contexts_clone && info->ipcp_orig_node == dest;
}
@@ -4053,11 +4062,8 @@ cgraph_edge_brings_value_p (cgraph_edge *cs, ipcp_value_source<tree> *src,
cgraph_node *dest, ipcp_value<tree> *dest_val)
{
class ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
- enum availability availability;
- cgraph_node *real_dest = cs->callee->function_symbol (&availability);
- if (availability <= AVAIL_INTERPOSABLE
- || !same_node_or_its_all_contexts_clone_p (real_dest, dest)
+ if (!calls_same_node_or_its_all_contexts_clone_p (cs, dest, !src->val)
|| caller_info->node_dead)
return false;
@@ -4076,9 +4082,6 @@ cgraph_edge_brings_value_p (cgraph_edge *cs, ipcp_value_source<tree> *src,
}
else
{
- /* At the moment we do not propagate over arithmetic jump functions in
- SCCs, so it is safe to detect self-feeding recursive calls in this
- way. */
if (src->val == dest_val)
return true;
@@ -4113,11 +4116,8 @@ cgraph_edge_brings_value_p (cgraph_edge *cs,
ipcp_value<ipa_polymorphic_call_context> *)
{
class ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
- enum availability avail;
- cgraph_node *real_dest = cs->callee->function_symbol (&avail);
- if (avail <= AVAIL_INTERPOSABLE
- || !same_node_or_its_all_contexts_clone_p (real_dest, dest)
+ if (!calls_same_node_or_its_all_contexts_clone_p (cs, dest, true)
|| caller_info->node_dead)
return false;
if (!src->val)
@@ -4630,7 +4630,9 @@ self_recursive_pass_through_p (cgraph_edge *cs, ipa_jump_func *jfunc, int i,
&& availability > AVAIL_INTERPOSABLE
&& jfunc->type == IPA_JF_PASS_THROUGH
&& (!simple || ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
- && ipa_get_jf_pass_through_formal_id (jfunc) == i)
+ && ipa_get_jf_pass_through_formal_id (jfunc) == i
+ && IPA_NODE_REF (cs->caller)
+ && !IPA_NODE_REF (cs->caller)->ipcp_orig_node)
return true;
return false;
}
@@ -4651,7 +4653,9 @@ self_recursive_agg_pass_through_p (cgraph_edge *cs, ipa_agg_jf_item *jfunc,
&& jfunc->offset == jfunc->value.load_agg.offset
&& (!simple || jfunc->value.pass_through.operation == NOP_EXPR)
&& jfunc->value.pass_through.formal_id == i
- && useless_type_conversion_p (jfunc->value.load_agg.type, jfunc->type))
+ && useless_type_conversion_p (jfunc->value.load_agg.type, jfunc->type)
+ && IPA_NODE_REF (cs->caller)
+ && !IPA_NODE_REF (cs->caller)->ipcp_orig_node)
return true;
return false;
}
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index b326529ac75..e8c9f197e42 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2020-02-18 Martin Jambor <mjambor@suse.cz>
+ Feng Xue <fxue@os.amperecomputing.com>
+
+ PR ipa/93707
+ * gcc.dg/ipa/pr93707.c: New test.
+
2020-02-17 Richard Biener <rguenther@suse.de>
PR c/86134
diff --git a/gcc/testsuite/gcc.dg/ipa/pr93707.c b/gcc/testsuite/gcc.dg/ipa/pr93707.c
new file mode 100644
index 00000000000..685fae45020
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr93707.c
@@ -0,0 +1,31 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 --param ipa-cp-eval-threshold=1 -fdump-ipa-cp" } */
+
+int foo();
+int data[100];
+
+__attribute__((noinline)) static int recur_fn (int i, int j, int depth)
+{
+ if (depth > 10)
+ return 1;
+
+ data[i + j]++;
+
+ if (depth & 3)
+ recur_fn (i, 1, depth + 1);
+ else
+ recur_fn (i, j & 1, depth + 1);
+
+ foo();
+
+ return i + j;
+}
+
+int caller (int v, int depth)
+{
+ recur_fn (1, v, depth);
+
+ return 0;
+}
+
+/* { dg-final { scan-ipa-dump-times "Clone of recur_fn/" 2 "cp" } } */
--
2.25.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Fix bug in recursiveness check for function to be cloned (ipa/pr93707)
2020-02-21 18:16 ` Martin Jambor
@ 2020-02-22 3:32 ` Feng Xue OS
2020-02-24 15:41 ` Martin Jambor
0 siblings, 1 reply; 17+ messages in thread
From: Feng Xue OS @ 2020-02-22 3:32 UTC (permalink / raw)
To: Martin Jambor, Tamar Christina, Jan Hubicka, gcc-patches; +Cc: nd
It is a good solution.
Thanks,
Feng
________________________________________
From: Martin Jambor <mjambor@suse.cz>
Sent: Saturday, February 22, 2020 2:15 AM
To: Feng Xue OS; Tamar Christina; Jan Hubicka; gcc-patches@gcc.gnu.org
Cc: nd
Subject: Re: [PATCH] Fix bug in recursiveness check for function to be cloned (ipa/pr93707)
Hi,
On Thu, Feb 20 2020, Feng Xue OS wrote:
> This is a simpel and nice fix, but could suppress some CP opportunities for
> self-recursive call. Using the test case as example, the first should be a
> for-all-context clone, and the call "recur_fn (i, 1, depth + 1)" is replaced with
> a newly created recursive node. Thus, in the next round of CP iteration, the
> way to do CP for the 2nd arugment "1" is blocked, because its coming edge
> can not pass check by cgraph_edge_brings_value_p().
>
> +__attribute__((noinline)) static int recur_fn (int i, int j, int depth)
> +{
> + if (depth > 10)
> + return 1;
> +
> + data[i + j]++;
> +
> + if (depth & 3)
> + recur_fn (i, 1, depth + 1);
> + else
> + recur_fn (i, j & 1, depth + 1);
> +
> + foo();
> +
> + return i + j;
> +}
> +
> +int caller (int v, int depth)
> +{
> + recur_fn (1, v, depth);
> +
> + return 0;
> +}
>
>
>>However, I believe that his approach mostly papers over a bug that
>>happens earlier, specifically that cgraph_edge_brings_value_p returned
>>true for the self-recursive edge from all-context clone to itself, even
>>though when evaluating the second argument. We assume that all context
>>clones get at least as many constants as any other potential clone, but
>>that does not work for self-recursive edges with pass-through parameters
>>that that just pass along the received constant.
>
> The following check on value in cgraph_edge_brings_value_p could ensure
> whether the value can arrive the dest node or not. If the value is a constant
> without source, as above example "1", this is allowed. Otherwise, code snippet
> enclosed by "if (caller_info->ipcp_orig_node)" could capture for-all-context clone.
there has not been any "following check" in your email but I believe I
understand what you mean, and I added such check to my patch so that the
edge carrying the non-pass through jump function was accepted by the
cgraph_edge_brings_value_p predicate.
However, that lead to the same assert in
find_more_scalar_values_for_callers_subset because on that edge it tried
to compute the depth + 1 value before it had any value to calculate it
from.
So after staring at the problem for another while I realized that the
users self_recursive_pass_through_p and
self_recursive_agg_pass_through_p would be OK if it returned false for
self-recursive calls from/to a node which is already a clone - clones
have their known constant values set at the point of their creation -
and that doing so avoids this problem. So that is what the patch below
does. I have still kept the cgraph_edge_brings_value_p hunks too, so
that edges are collected reliably.
Bootstrapped and tested on an x86_64-linux, LTO bootstrap underway.
What do you think?
Martin
2020-02-21 Martin Jambor <mjambor@suse.cz>
Feng Xue <fxue@os.amperecomputing.com>
PR ipa/93707
* ipa-cp.c (same_node_or_its_all_contexts_clone_p): Replaced with
new function calls_same_node_or_its_all_contexts_clone_p.
(cgraph_edge_brings_value_p): Use it.
(cgraph_edge_brings_value_p): Likewise.
(self_recursive_pass_through_p): Return false if caller is a clone.
(self_recursive_agg_pass_through_p): Likewise.
testsuite/
* gcc.dg/ipa/pr93707.c: New test.
---
gcc/ChangeLog | 11 +++++++++
gcc/ipa-cp.c | 38 +++++++++++++++++-------------
gcc/testsuite/ChangeLog | 6 +++++
gcc/testsuite/gcc.dg/ipa/pr93707.c | 31 ++++++++++++++++++++++++
4 files changed, 69 insertions(+), 17 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/ipa/pr93707.c
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 7c481407de9..a965cae4f07 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,14 @@
+2020-02-21 Martin Jambor <mjambor@suse.cz>
+ Feng Xue <fxue@os.amperecomputing.com>
+
+ PR ipa/93707
+ * ipa-cp.c (same_node_or_its_all_contexts_clone_p): Replaced with
+ new function calls_same_node_or_its_all_contexts_clone_p.
+ (cgraph_edge_brings_value_p): Use it.
+ (cgraph_edge_brings_value_p): Likewise.
+ (self_recursive_pass_through_p): Return false if caller is a clone.
+ (self_recursive_agg_pass_through_p): Likewise.
+
2020-02-17 Richard Biener <rguenther@suse.de>
PR c/86134
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 4f5b72e6994..aa228df1204 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -4033,15 +4033,24 @@ edge_clone_summary_t::duplicate (cgraph_edge *src_edge, cgraph_edge *dst_edge,
src_data->next_clone = dst_edge;
}
-/* Return true is NODE is DEST or its clone for all contexts. */
+/* Return true is CS calls DEST or its clone for all contexts, except for
+ self-recursive nodes in which it has to be DEST itself. */
static bool
-same_node_or_its_all_contexts_clone_p (cgraph_node *node, cgraph_node *dest)
+calls_same_node_or_its_all_contexts_clone_p (cgraph_edge *cs, cgraph_node *dest,
+ bool allow_recursion_to_clone)
{
- if (node == dest)
+ enum availability availability;
+ cgraph_node *callee = cs->callee->function_symbol (&availability);
+
+ if (availability <= AVAIL_INTERPOSABLE)
+ return false;
+ if (callee == dest)
return true;
+ if (!allow_recursion_to_clone && cs->caller == callee)
+ return false;
- class ipa_node_params *info = IPA_NODE_REF (node);
+ class ipa_node_params *info = IPA_NODE_REF (callee);
return info->is_all_contexts_clone && info->ipcp_orig_node == dest;
}
@@ -4053,11 +4062,8 @@ cgraph_edge_brings_value_p (cgraph_edge *cs, ipcp_value_source<tree> *src,
cgraph_node *dest, ipcp_value<tree> *dest_val)
{
class ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
- enum availability availability;
- cgraph_node *real_dest = cs->callee->function_symbol (&availability);
- if (availability <= AVAIL_INTERPOSABLE
- || !same_node_or_its_all_contexts_clone_p (real_dest, dest)
+ if (!calls_same_node_or_its_all_contexts_clone_p (cs, dest, !src->val)
|| caller_info->node_dead)
return false;
@@ -4076,9 +4082,6 @@ cgraph_edge_brings_value_p (cgraph_edge *cs, ipcp_value_source<tree> *src,
}
else
{
- /* At the moment we do not propagate over arithmetic jump functions in
- SCCs, so it is safe to detect self-feeding recursive calls in this
- way. */
if (src->val == dest_val)
return true;
@@ -4113,11 +4116,8 @@ cgraph_edge_brings_value_p (cgraph_edge *cs,
ipcp_value<ipa_polymorphic_call_context> *)
{
class ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
- enum availability avail;
- cgraph_node *real_dest = cs->callee->function_symbol (&avail);
- if (avail <= AVAIL_INTERPOSABLE
- || !same_node_or_its_all_contexts_clone_p (real_dest, dest)
+ if (!calls_same_node_or_its_all_contexts_clone_p (cs, dest, true)
|| caller_info->node_dead)
return false;
if (!src->val)
@@ -4630,7 +4630,9 @@ self_recursive_pass_through_p (cgraph_edge *cs, ipa_jump_func *jfunc, int i,
&& availability > AVAIL_INTERPOSABLE
&& jfunc->type == IPA_JF_PASS_THROUGH
&& (!simple || ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
- && ipa_get_jf_pass_through_formal_id (jfunc) == i)
+ && ipa_get_jf_pass_through_formal_id (jfunc) == i
+ && IPA_NODE_REF (cs->caller)
+ && !IPA_NODE_REF (cs->caller)->ipcp_orig_node)
return true;
return false;
}
@@ -4651,7 +4653,9 @@ self_recursive_agg_pass_through_p (cgraph_edge *cs, ipa_agg_jf_item *jfunc,
&& jfunc->offset == jfunc->value.load_agg.offset
&& (!simple || jfunc->value.pass_through.operation == NOP_EXPR)
&& jfunc->value.pass_through.formal_id == i
- && useless_type_conversion_p (jfunc->value.load_agg.type, jfunc->type))
+ && useless_type_conversion_p (jfunc->value.load_agg.type, jfunc->type)
+ && IPA_NODE_REF (cs->caller)
+ && !IPA_NODE_REF (cs->caller)->ipcp_orig_node)
return true;
return false;
}
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index b326529ac75..e8c9f197e42 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2020-02-18 Martin Jambor <mjambor@suse.cz>
+ Feng Xue <fxue@os.amperecomputing.com>
+
+ PR ipa/93707
+ * gcc.dg/ipa/pr93707.c: New test.
+
2020-02-17 Richard Biener <rguenther@suse.de>
PR c/86134
diff --git a/gcc/testsuite/gcc.dg/ipa/pr93707.c b/gcc/testsuite/gcc.dg/ipa/pr93707.c
new file mode 100644
index 00000000000..685fae45020
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr93707.c
@@ -0,0 +1,31 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 --param ipa-cp-eval-threshold=1 -fdump-ipa-cp" } */
+
+int foo();
+int data[100];
+
+__attribute__((noinline)) static int recur_fn (int i, int j, int depth)
+{
+ if (depth > 10)
+ return 1;
+
+ data[i + j]++;
+
+ if (depth & 3)
+ recur_fn (i, 1, depth + 1);
+ else
+ recur_fn (i, j & 1, depth + 1);
+
+ foo();
+
+ return i + j;
+}
+
+int caller (int v, int depth)
+{
+ recur_fn (1, v, depth);
+
+ return 0;
+}
+
+/* { dg-final { scan-ipa-dump-times "Clone of recur_fn/" 2 "cp" } } */
--
2.25.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Fix bug in recursiveness check for function to be cloned (ipa/pr93707)
2020-02-22 3:32 ` Feng Xue OS
@ 2020-02-24 15:41 ` Martin Jambor
0 siblings, 0 replies; 17+ messages in thread
From: Martin Jambor @ 2020-02-24 15:41 UTC (permalink / raw)
To: Feng Xue OS, Tamar Christina, Jan Hubicka, gcc-patches; +Cc: nd
Hi,
On Sat, Feb 22 2020, Feng Xue OS wrote:
> It is a good solution.
>
OK, so I'd like to go with my patch as it hopefully keeps some of the
predicates a tiny bit simpler. I have re-based the patch on today's
trunk and amended function comments as necessary (which I forgot last
week). The result is below, it has passed bootstrap and testing and
LTO+PGO bootstrap on x86_64-linux.
Honza, is it OK for trunk?
Thanks,
Martin
ipa-cp: Avoid an ICE processing self-recursive cloned edges (PR 93707)
2020-02-24 Martin Jambor <mjambor@suse.cz>
Feng Xue <fxue@os.amperecomputing.com>
PR ipa/93707
* ipa-cp.c (same_node_or_its_all_contexts_clone_p): Replaced with
new function calls_same_node_or_its_all_contexts_clone_p.
(cgraph_edge_brings_value_p): Use it.
(cgraph_edge_brings_value_p): Likewise.
(self_recursive_pass_through_p): Return false if caller is a clone.
(self_recursive_agg_pass_through_p): Likewise.
testsuite/
* gcc.dg/ipa/pr93707.c: New test.
---
gcc/ChangeLog | 11 ++++++
gcc/ipa-cp.c | 55 +++++++++++++++++-------------
gcc/testsuite/ChangeLog | 6 ++++
gcc/testsuite/gcc.dg/ipa/pr93707.c | 31 +++++++++++++++++
4 files changed, 79 insertions(+), 24 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/ipa/pr93707.c
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 1d0c1ac0f35..27c020b8199 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -4035,15 +4035,25 @@ edge_clone_summary_t::duplicate (cgraph_edge *src_edge, cgraph_edge *dst_edge,
src_data->next_clone = dst_edge;
}
-/* Return true is NODE is DEST or its clone for all contexts. */
+/* Return true is CS calls DEST or its clone for all contexts. When
+ ALLOW_RECURSION_TO_CLONE is false, also return false for self-recursive
+ edges from/to an all-context clone. */
static bool
-same_node_or_its_all_contexts_clone_p (cgraph_node *node, cgraph_node *dest)
+calls_same_node_or_its_all_contexts_clone_p (cgraph_edge *cs, cgraph_node *dest,
+ bool allow_recursion_to_clone)
{
- if (node == dest)
+ enum availability availability;
+ cgraph_node *callee = cs->callee->function_symbol (&availability);
+
+ if (availability <= AVAIL_INTERPOSABLE)
+ return false;
+ if (callee == dest)
return true;
+ if (!allow_recursion_to_clone && cs->caller == callee)
+ return false;
- class ipa_node_params *info = IPA_NODE_REF (node);
+ class ipa_node_params *info = IPA_NODE_REF (callee);
return info->is_all_contexts_clone && info->ipcp_orig_node == dest;
}
@@ -4055,11 +4065,8 @@ cgraph_edge_brings_value_p (cgraph_edge *cs, ipcp_value_source<tree> *src,
cgraph_node *dest, ipcp_value<tree> *dest_val)
{
class ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
- enum availability availability;
- cgraph_node *real_dest = cs->callee->function_symbol (&availability);
- if (availability <= AVAIL_INTERPOSABLE
- || !same_node_or_its_all_contexts_clone_p (real_dest, dest)
+ if (!calls_same_node_or_its_all_contexts_clone_p (cs, dest, !src->val)
|| caller_info->node_dead)
return false;
@@ -4078,9 +4085,6 @@ cgraph_edge_brings_value_p (cgraph_edge *cs, ipcp_value_source<tree> *src,
}
else
{
- /* At the moment we do not propagate over arithmetic jump functions in
- SCCs, so it is safe to detect self-feeding recursive calls in this
- way. */
if (src->val == dest_val)
return true;
@@ -4115,11 +4119,8 @@ cgraph_edge_brings_value_p (cgraph_edge *cs,
ipcp_value<ipa_polymorphic_call_context> *)
{
class ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
- enum availability avail;
- cgraph_node *real_dest = cs->callee->function_symbol (&avail);
- if (avail <= AVAIL_INTERPOSABLE
- || !same_node_or_its_all_contexts_clone_p (real_dest, dest)
+ if (!calls_same_node_or_its_all_contexts_clone_p (cs, dest, true)
|| caller_info->node_dead)
return false;
if (!src->val)
@@ -4619,9 +4620,10 @@ create_specialized_node (struct cgraph_node *node,
return new_node;
}
-/* Return true, if JFUNC, which describes a i-th parameter of call CS, is a
- pass-through function to itself. When SIMPLE is true, further check if
- JFUNC is a simple no-operation pass-through. */
+/* Return true if JFUNC, which describes a i-th parameter of call CS, is a
+ pass-through function to itself when the cgraph_node involved is not an
+ IPA-CP clone. When SIMPLE is true, further check if JFUNC is a simple
+ no-operation pass-through. */
static bool
self_recursive_pass_through_p (cgraph_edge *cs, ipa_jump_func *jfunc, int i,
@@ -4632,15 +4634,18 @@ self_recursive_pass_through_p (cgraph_edge *cs, ipa_jump_func *jfunc, int i,
&& availability > AVAIL_INTERPOSABLE
&& jfunc->type == IPA_JF_PASS_THROUGH
&& (!simple || ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
- && ipa_get_jf_pass_through_formal_id (jfunc) == i)
+ && ipa_get_jf_pass_through_formal_id (jfunc) == i
+ && IPA_NODE_REF (cs->caller)
+ && !IPA_NODE_REF (cs->caller)->ipcp_orig_node)
return true;
return false;
}
-/* Return true, if JFUNC, which describes a part of an aggregate represented
- or pointed to by the i-th parameter of call CS, is a pass-through function
- to itself. When SIMPLE is true, further check if JFUNC is a simple
- no-operation pass-through. */
+/* Return true if JFUNC, which describes a part of an aggregate represented or
+ pointed to by the i-th parameter of call CS, is a pass-through function to
+ itself when the cgraph_node involved is not an IPA-CP clone.. When
+ SIMPLE is true, further check if JFUNC is a simple no-operation
+ pass-through. */
static bool
self_recursive_agg_pass_through_p (cgraph_edge *cs, ipa_agg_jf_item *jfunc,
@@ -4653,7 +4658,9 @@ self_recursive_agg_pass_through_p (cgraph_edge *cs, ipa_agg_jf_item *jfunc,
&& jfunc->offset == jfunc->value.load_agg.offset
&& (!simple || jfunc->value.pass_through.operation == NOP_EXPR)
&& jfunc->value.pass_through.formal_id == i
- && useless_type_conversion_p (jfunc->value.load_agg.type, jfunc->type))
+ && useless_type_conversion_p (jfunc->value.load_agg.type, jfunc->type)
+ && IPA_NODE_REF (cs->caller)
+ && !IPA_NODE_REF (cs->caller)->ipcp_orig_node)
return true;
return false;
}
diff --git a/gcc/testsuite/gcc.dg/ipa/pr93707.c b/gcc/testsuite/gcc.dg/ipa/pr93707.c
new file mode 100644
index 00000000000..685fae45020
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr93707.c
@@ -0,0 +1,31 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 --param ipa-cp-eval-threshold=1 -fdump-ipa-cp" } */
+
+int foo();
+int data[100];
+
+__attribute__((noinline)) static int recur_fn (int i, int j, int depth)
+{
+ if (depth > 10)
+ return 1;
+
+ data[i + j]++;
+
+ if (depth & 3)
+ recur_fn (i, 1, depth + 1);
+ else
+ recur_fn (i, j & 1, depth + 1);
+
+ foo();
+
+ return i + j;
+}
+
+int caller (int v, int depth)
+{
+ recur_fn (1, v, depth);
+
+ return 0;
+}
+
+/* { dg-final { scan-ipa-dump-times "Clone of recur_fn/" 2 "cp" } } */
--
2.25.0
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-02-24 15:41 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-25 13:52 [PATCH] Generalized value pass-through for self-recursive function (ipa/pr93203) Feng Xue OS
2020-01-25 16:17 ` [PATCH V2] " Feng Xue OS
2020-02-03 1:57 ` Ping* " Feng Xue OS
2020-02-05 17:27 ` Martin Jambor
2020-02-10 3:28 ` Feng Xue OS
2020-02-11 10:05 ` Tamar Christina
2020-02-11 13:10 ` Feng Xue OS
2020-02-11 14:31 ` Tamar Christina
2020-02-13 5:39 ` [PATCH] Fix bug in recursiveness check for function to be cloned (ipa/pr93707) Feng Xue OS
2020-02-17 8:44 ` Tamar Christina
2020-02-18 15:16 ` Ping: " Feng Xue OS
2020-02-19 16:28 ` Martin Jambor
2020-02-20 3:36 ` Feng Xue OS
2020-02-21 18:16 ` Martin Jambor
2020-02-22 3:32 ` Feng Xue OS
2020-02-24 15:41 ` Martin Jambor
2020-02-20 12:57 ` Tamar Christina
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).