Re: [PATCH] Handle aggregate pass-through for self-recursive call (PR ipa/92794)
>> +static bool
>> +self_recursive_agg_pass_through_p (cgraph_edge *cs, ipa_agg_jf_item *jfunc,
>> +int i)
>> +{
>> + if (cs->caller == cs->callee->function_symbol ()
> I don't know if self-recursive calls can be interposed at all, if yes
> you need to add the availability check like we have in
> self_recursive_pass_through_p (if not, we should probably remove it
> there).
Added.
Feng
Re: [PATCH] Handle aggregate pass-through for self-recursive call (PR ipa/92794)
> Hi,
>
> On Tue, Dec 17 2019, Feng Xue OS wrote:
> > If argument for a self-recursive call is a simple pass-through, the call
> > edge is also considered as source of any value originated from
> > non-recursive call to the function. Scalar pass-through and full aggregate
> > pass-through due to pointer pass-through have also been handled.
> > But we missed another kind of pass-through like below case, partial
> > aggregate pass-through. This patch is meant to fix the problem which
> > caused ICE as in 92794.
> >
> > void foo(struct T *val_ptr)
> > {
> > struct T new_val;
> > new_val.field = val_ptr->field;
> > foo ();
> > ...
> > }
> >
> > Bootstrapped/regtested on x86_64-linux and aarch64-linux.
> >
> > 2019-12-17 Feng Xue
> >
> > PR ipa/92794
> > * ipa-cp.c (self_recursive_agg_pass_through_p): New function.
> > (intersect_with_plats): Use error_mark_node as place holder
> > when aggregate jump function is simple pass-through for
> > self-recursive call.
> > (intersect_with_agg_replacements): Likewise.
> > (intersect_aggregates_with_edge): Likewise.
> > (find_aggregate_values_for_callers_subset): Likewise.
> >
> > Thanks,
> > Feng
> > From 42ba553ebf80eadb62619c5570a4b406f8c90c49 Mon Sep 17 00:00:00 2001
> > From: Feng Xue
> > Date: Mon, 16 Dec 2019 20:33:36 +0800
> > Subject: [PATCH] Handle aggregate simple pass-through for self-recursive
> > call
> >
> > ---
> > gcc/ipa-cp.c | 97 +-
> > gcc/testsuite/gcc.dg/ipa/pr92794.c | 30 +
> > 2 files changed, 111 insertions(+), 16 deletions(-)
> > create mode 100644 gcc/testsuite/gcc.dg/ipa/pr92794.c
> >
> > diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> > index 1a80ccbde2d..0e17fedd649 100644
> > --- a/gcc/ipa-cp.c
> > +++ b/gcc/ipa-cp.c
> > @@ -4564,6 +4564,23 @@ 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 simple no-operation
> > + pass-through function to itself. */
> > +
> > +static bool
> > +self_recursive_agg_pass_through_p (cgraph_edge *cs, ipa_agg_jf_item *jfunc,
> > + int i)
> > +{
> > + if (cs->caller == cs->callee->function_symbol ()
>
> I don't know if self-recursive calls can be interposed at all, if yes
> you need to add the availability check like we have in
> self_recursive_pass_through_p (if not, we should probably remove it
> there).
Yes, self recursion can interpose if you have alias and enter the
recursive loop by different symbol name then recurse.
We have optional ref argument in ultimate_alias_target and friends where
you can specify symbol in which the reference appears and then the
predicate knows how to verify this (odd) condition.
There is cgraph_edge::recursive_p that can make mistake in positive
direction in the case of interposition. We probably want to distinguish
these cases and have parameter for that...
Honza
>
> Apart from that, I believe the patch is probably the best thing we can
> do to deal with these interesting situations.
>
> Thanks for looking into the bug,
>
> Martin
>
>
> > + && 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)
> > +return true;
> > + return false;
> > +}
Re: [PATCH] Handle aggregate pass-through for self-recursive call (PR ipa/92794)
Hi,
On Tue, Dec 17 2019, Feng Xue OS wrote:
> If argument for a self-recursive call is a simple pass-through, the call
> edge is also considered as source of any value originated from
> non-recursive call to the function. Scalar pass-through and full aggregate
> pass-through due to pointer pass-through have also been handled.
> But we missed another kind of pass-through like below case, partial
> aggregate pass-through. This patch is meant to fix the problem which
> caused ICE as in 92794.
>
> void foo(struct T *val_ptr)
> {
> struct T new_val;
> new_val.field = val_ptr->field;
> foo ();
> ...
> }
>
> Bootstrapped/regtested on x86_64-linux and aarch64-linux.
>
> 2019-12-17 Feng Xue
>
> PR ipa/92794
> * ipa-cp.c (self_recursive_agg_pass_through_p): New function.
> (intersect_with_plats): Use error_mark_node as place holder
> when aggregate jump function is simple pass-through for
> self-recursive call.
> (intersect_with_agg_replacements): Likewise.
> (intersect_aggregates_with_edge): Likewise.
> (find_aggregate_values_for_callers_subset): Likewise.
>
> Thanks,
> Feng
> From 42ba553ebf80eadb62619c5570a4b406f8c90c49 Mon Sep 17 00:00:00 2001
> From: Feng Xue
> Date: Mon, 16 Dec 2019 20:33:36 +0800
> Subject: [PATCH] Handle aggregate simple pass-through for self-recursive call
>
> ---
> gcc/ipa-cp.c | 97 +-
> gcc/testsuite/gcc.dg/ipa/pr92794.c | 30 +
> 2 files changed, 111 insertions(+), 16 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/ipa/pr92794.c
>
> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> index 1a80ccbde2d..0e17fedd649 100644
> --- a/gcc/ipa-cp.c
> +++ b/gcc/ipa-cp.c
> @@ -4564,6 +4564,23 @@ 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 simple no-operation
> + pass-through function to itself. */
> +
> +static bool
> +self_recursive_agg_pass_through_p (cgraph_edge *cs, ipa_agg_jf_item *jfunc,
> +int i)
> +{
> + if (cs->caller == cs->callee->function_symbol ()
I don't know if self-recursive calls can be interposed at all, if yes
you need to add the availability check like we have in
self_recursive_pass_through_p (if not, we should probably remove it
there).
Apart from that, I believe the patch is probably the best thing we can
do to deal with these interesting situations.
Thanks for looking into the bug,
Martin
> + && 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)
> +return true;
> + return false;
> +}
[PATCH] Handle aggregate pass-through for self-recursive call (PR ipa/92794)
If argument for a self-recursive call is a simple pass-through, the call
edge is also considered as source of any value originated from
non-recursive call to the function. Scalar pass-through and full aggregate
pass-through due to pointer pass-through have also been handled.
But we missed another kind of pass-through like below case, partial
aggregate pass-through. This patch is meant to fix the problem which
caused ICE as in 92794.
void foo(struct T *val_ptr)
{
struct T new_val;
new_val.field = val_ptr->field;
foo ();
...
}
Bootstrapped/regtested on x86_64-linux and aarch64-linux.
2019-12-17 Feng Xue
PR ipa/92794
* ipa-cp.c (self_recursive_agg_pass_through_p): New function.
(intersect_with_plats): Use error_mark_node as place holder
when aggregate jump function is simple pass-through for
self-recursive call.
(intersect_with_agg_replacements): Likewise.
(intersect_aggregates_with_edge): Likewise.
(find_aggregate_values_for_callers_subset): Likewise.
Thanks,
FengFrom 42ba553ebf80eadb62619c5570a4b406f8c90c49 Mon Sep 17 00:00:00 2001
From: Feng Xue
Date: Mon, 16 Dec 2019 20:33:36 +0800
Subject: [PATCH] Handle aggregate simple pass-through for self-recursive call
---
gcc/ipa-cp.c | 97 +-
gcc/testsuite/gcc.dg/ipa/pr92794.c | 30 +
2 files changed, 111 insertions(+), 16 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/ipa/pr92794.c
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 1a80ccbde2d..0e17fedd649 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -4564,6 +4564,23 @@ 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 simple no-operation
+ pass-through function to itself. */
+
+static bool
+self_recursive_agg_pass_through_p (cgraph_edge *cs, ipa_agg_jf_item *jfunc,
+ int i)
+{
+ if (cs->caller == cs->callee->function_symbol ()
+ && 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)
+return true;
+ return false;
+}
+
/* Given a NODE, and a subset of its CALLERS, try to populate blanks slots in
KNOWN_CSTS with constants that are also known for all of the CALLERS. */
@@ -4756,10 +4773,19 @@ intersect_with_plats (class ipcp_param_lattices *plats,
if (aglat->offset - offset == item->offset)
{
gcc_checking_assert (item->value);
- if (aglat->is_single_const ()
- && values_equal_for_ipcp_p (item->value,
- aglat->values->value))
- found = true;
+ 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;
}
aglat = aglat->next;
@@ -4827,6 +4853,12 @@ 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;
}
}
@@ -4931,17 +4963,31 @@ 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];
- tree value = ipa_agg_value_from_node (caller_info, cs->caller,
- agg_item);
- if (value)
+ 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 it stands for is not
+ known, 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
{
- struct ipa_agg_value agg_value;
+ tree value = ipa_agg_value_from_node (caller_info, cs->caller,
+ agg_item);
+ if (!value)
+ continue;
- agg_value.offset = agg_item->offset;
agg_value.value = value;
-
- inter.safe_push (agg_value);
}
+
+ agg_value.offset = agg_item->offset;
+ inter.safe_push (agg_value);
}
else
FOR_EACH_VEC_ELT (inter, k, item)
@@ -4960,11 +5006,27 @@ intersect_aggregates_with_edge
