ipa-cp: Do not be too optimistic about self-recursive edges (PR 107661)

  Hi,

PR 107661 shows that function push_agg_values_for_index_from_edge
should not attempt to optimize self-recursive call graph edges when
called from cgraph_edge_brings_all_agg_vals_for_node.  Unlike when
being called from find_aggregate_values_for_callers_subset, we cannot
expect that any cloning for constants would lead to the edge leading
from a new clone to the same new clone, in this case it would only be
redirected to a new callee.

Fixed by adding a parameter to push_agg_values_from_edge whether being
optimistic about self-recursive edges is possible.

Bootstrapped, LTO-bootstrapped and tested on x86_64-linux.  OK for
trunk?

Thanks,

Martin


gcc/ChangeLog:

2022-11-22  Martin Jambor  <mjambor@suse.cz>

	PR ipa/107661
	* ipa-cp.cc (push_agg_values_from_edge): New parameter
	optimize_self_recursion, use it to decide whether to pass interim to
	the helper function.
	(find_aggregate_values_for_callers_subset): Pass true in the new
	parameter of push_agg_values_from_edge.
	(cgraph_edge_brings_all_agg_vals_for_node): Pass false in the new
	parameter of push_agg_values_from_edge.

gcc/testsuite/ChangeLog:

2022-11-22  Martin Jambor  <mjambor@suse.cz>

	PR ipa/107661
	* g++.dg/ipa/pr107661.C: New test.
---
 gcc/ipa-cp.cc                       | 18 +++++++-----
 gcc/testsuite/g++.dg/ipa/pr107661.C | 45 +++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/ipa/pr107661.C
  

> Hi,
> 
> PR 107661 shows that function push_agg_values_for_index_from_edge
> should not attempt to optimize self-recursive call graph edges when
> called from cgraph_edge_brings_all_agg_vals_for_node.  Unlike when
> being called from find_aggregate_values_for_callers_subset, we cannot
> expect that any cloning for constants would lead to the edge leading
> from a new clone to the same new clone, in this case it would only be
> redirected to a new callee.
> 
> Fixed by adding a parameter to push_agg_values_from_edge whether being
> optimistic about self-recursive edges is possible.
> 
> Bootstrapped, LTO-bootstrapped and tested on x86_64-linux.  OK for
> trunk?
OK,
thanks!
Honya
> 
> Thanks,
> 
> Martin
> 
> 
> gcc/ChangeLog:
> 
> 2022-11-22  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR ipa/107661
> 	* ipa-cp.cc (push_agg_values_from_edge): New parameter
> 	optimize_self_recursion, use it to decide whether to pass interim to
> 	the helper function.
> 	(find_aggregate_values_for_callers_subset): Pass true in the new
> 	parameter of push_agg_values_from_edge.
> 	(cgraph_edge_brings_all_agg_vals_for_node): Pass false in the new
> 	parameter of push_agg_values_from_edge.
> 
> gcc/testsuite/ChangeLog:
> 
> 2022-11-22  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR ipa/107661
> 	* g++.dg/ipa/pr107661.C: New test.
> ---
>  gcc/ipa-cp.cc                       | 18 +++++++-----
>  gcc/testsuite/g++.dg/ipa/pr107661.C | 45 +++++++++++++++++++++++++++++
>  2 files changed, 56 insertions(+), 7 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/ipa/pr107661.C
> 
> diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc
> index d2bcd5e5e69..f0feb4beb8f 100644
> --- a/gcc/ipa-cp.cc
> +++ b/gcc/ipa-cp.cc
> @@ -5752,14 +5752,16 @@ push_agg_values_for_index_from_edge (struct cgraph_edge *cs, int index,
>     description of ultimate callee of CS or the one it was cloned from (the
>     summary where lattices are).  If INTERIM is non-NULL, it contains the
>     current interim state of collected aggregate values which can be used to
> -   compute values passed over self-recursive edges and to skip values which
> -   clearly will not be part of intersection with INTERIM.  */
> +   compute values passed over self-recursive edges (if OPTIMIZE_SELF_RECURSION
> +   is true) and to skip values which clearly will not be part of intersection
> +   with INTERIM.  */
>  
>  static void
>  push_agg_values_from_edge (struct cgraph_edge *cs,
>  			   ipa_node_params *dest_info,
>  			   vec<ipa_argagg_value> *res,
> -			   const ipa_argagg_value_list *interim)
> +			   const ipa_argagg_value_list *interim,
> +			   bool optimize_self_recursion)
>  {
>    ipa_edge_args *args = ipa_edge_args_sum->get (cs);
>    if (!args)
> @@ -5785,7 +5787,9 @@ push_agg_values_from_edge (struct cgraph_edge *cs,
>        ipcp_param_lattices *plats = ipa_get_parm_lattices (dest_info, index);
>        if (plats->aggs_bottom)
>  	continue;
> -      push_agg_values_for_index_from_edge (cs, index, res, interim);
> +      push_agg_values_for_index_from_edge (cs, index, res,
> +					   optimize_self_recursion ? interim
> +					   : NULL);
>      }
>  }
>  
> @@ -5804,7 +5808,7 @@ find_aggregate_values_for_callers_subset (struct cgraph_node *node,
>    /* gather_edges_for_value puts a non-recursive call into the first element of
>       callers if it can.  */
>    auto_vec<ipa_argagg_value, 32> interim;
> -  push_agg_values_from_edge (callers[0], dest_info, &interim, NULL);
> +  push_agg_values_from_edge (callers[0], dest_info, &interim, NULL, true);
>  
>    unsigned valid_entries = interim.length ();
>    if (!valid_entries)
> @@ -5815,7 +5819,7 @@ find_aggregate_values_for_callers_subset (struct cgraph_node *node,
>      {
>        auto_vec<ipa_argagg_value, 32> last;
>        ipa_argagg_value_list avs (&interim);
> -      push_agg_values_from_edge (callers[i], dest_info, &last, &avs);
> +      push_agg_values_from_edge (callers[i], dest_info, &last, &avs, true);
>  
>        valid_entries = intersect_argaggs_with (interim, last);
>        if (!valid_entries)
> @@ -5882,7 +5886,7 @@ cgraph_edge_brings_all_agg_vals_for_node (struct cgraph_edge *cs,
>    ipa_node_params *dest_info = ipa_node_params_sum->get (node);
>    gcc_checking_assert (dest_info->ipcp_orig_node);
>    dest_info = ipa_node_params_sum->get (dest_info->ipcp_orig_node);
> -  push_agg_values_from_edge (cs, dest_info, &edge_values, &existing);
> +  push_agg_values_from_edge (cs, dest_info, &edge_values, &existing, false);
>    const ipa_argagg_value_list avl (&edge_values);
>    return avl.superset_of_p (existing);
>  }
> diff --git a/gcc/testsuite/g++.dg/ipa/pr107661.C b/gcc/testsuite/g++.dg/ipa/pr107661.C
> new file mode 100644
> index 00000000000..cc6f8538dbf
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ipa/pr107661.C
> @@ -0,0 +1,45 @@
> +/* { dg-do run  { target c++11 } } */
> +/* { dg-options "-O1 -fipa-cp -fipa-cp-clone" } */
> +
> +struct R {} RGood;
> +struct L {} LBad;
> +
> +volatile int vi;
> +static void __attribute__((noipa)) L_run(void) { vi = 0; __builtin_abort (); }
> +static void callback_fn_L(void) { vi = 1; L_run(); }
> +static void callback_fn_R(void) { vi = 2; }
> +
> +struct function_ref {
> +  void (*callback)(void) = nullptr;
> +
> +  function_ref(L * pl) { callback = callback_fn_L; }
> +  function_ref(R * pr) { callback = callback_fn_R; }
> +};
> +
> +// allow one level of recursion to call callback twice
> +static int is_recur(void) {
> +    static int n = 0;
> +    switch (n++) {
> +      case 0: return 1;
> +      default: return 0;
> +    }
> +}
> +
> +static void do3(volatile int * punused, function_ref Expired) {
> +  Expired.callback();
> +
> +  if (is_recur())
> +      do3(punused, Expired);
> +}
> +
> +static void do1(function_ref Expired) {
> +  volatile int unused = 42;
> +
> +  do3(&unused, Expired);
> +}
> +
> +int main(int, const char **) { do1(&RGood); return 0; }
> +
> +void seemingly_unused_foo(void) { do1(&LBad); }
> +
> +void (*fnptr)(void) = seemingly_unused_foo;
> -- 
> 2.38.1
>