>
> the leak is indeed a problem, thanks for spotting it. But apart from
> that, I really wanted to pass vNULL to intersect_aggregates_with_edge,
> and the patch below does it explicitely to make it clear, because while
> the function can do also intersecting its actual task here is to carry
> over aggregate constants from all types of callers (clones and
> non-clones) and all kinds of supported jump functions.
>
> That might be an overkill because the main goal of
> cgraph_edge_brings_all_agg_vals_for_node is to add the last edge within
> SCCs of nodes propagating the same constant to each other and I could
> not quickly come up with a testcase where the caller would be a
> non-clone (and the function something other than pass-through, but I
> suspect that could actually happen) but since the code is written we
> might as well use it. The function is written to bail out early before
> actual value comparing and that is why the code is rarely executed, in
> fact I found out that it is not covered by our testsuite (see
> https://users.suse.com/~mliska/lcov/gcc/ipa-cp.c.gcov.html) and so the
> patch also adds a testcase which does execute it.
>
> The way vectors are passed around by value rather than by reference is
> how I wrote this stuff shortly after conversion from our C VEC_ headers
> with which were used in the same way. I agree that a lot of code in
> ipa-cp would benefit from transitioning to auto_vecs but that is
> something for the next stage 1.
>
> The patch has been bootstrapped and LTO-profile-bootstrapped on
> x86-64-linux. OK for trunk?
>
> Thanks,
>
> Martin
>
>
> 2019-12-17 Martin Jambor <mjam...@suse.cz>
>
> PR ipa/92971
> * Ipa-cp.c (cgraph_edge_brings_all_agg_vals_for_node): Fix
> definition of values, release memory on exit.
>
> testsuite/
> * gcc.dg/ipa/ipcp-agg-12.c: New test.
> ---
> gcc/ipa-cp.c | 4 +-
> gcc/testsuite/gcc.dg/ipa/ipcp-agg-12.c | 53 ++++++++++++++++++++++++++
> 2 files changed, 55 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/ipa/ipcp-agg-12.c
>
> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> index 1a80ccbde2d..243b064ee2c 100644
> --- a/gcc/ipa-cp.c
> +++ b/gcc/ipa-cp.c
> @@ -5117,7 +5117,6 @@ cgraph_edge_brings_all_agg_vals_for_node (struct
> cgraph_edge *cs,
>
> for (i = 0; i < count; i++)
> {
> - static vec<ipa_agg_value> values = vNULL;
> class ipcp_param_lattices *plats;
> bool interesting = false;
> for (struct ipa_agg_replacement_value *av = aggval; av; av = av->next)
> @@ -5133,7 +5132,7 @@ cgraph_edge_brings_all_agg_vals_for_node (struct
> cgraph_edge *cs,
> if (plats->aggs_bottom)
> return false;
>
> - values = intersect_aggregates_with_edge (cs, i, values);
> + vec<ipa_agg_value> values = intersect_aggregates_with_edge (cs, i,
> vNULL);
> if (!values.exists ())
> return false;
>
> @@ -5157,6 +5156,7 @@ cgraph_edge_brings_all_agg_vals_for_node (struct
> cgraph_edge *cs,
> return false;
> }
> }
> + values.release ();
Generally it seems to me that things would be more readable/leak safe if
we used auto_vecs and passed them as function arguments to be filled in.
But since same constructs are used in ipa-cp/prop elsewhere the patch is
OK.
Honza
> }
> return true;
> }
> diff --git a/gcc/testsuite/gcc.dg/ipa/ipcp-agg-12.c
> b/gcc/testsuite/gcc.dg/ipa/ipcp-agg-12.c
> new file mode 100644
> index 00000000000..5c57913803e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ipa/ipcp-agg-12.c
> @@ -0,0 +1,53 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -fno-ipa-sra -fdump-ipa-cp-details
> --param=ipa-cp-eval-threshold=2" } */
> +
> +struct S
> +{
> + int a, b, c;
> +};
> +
> +int __attribute__((noinline)) foo (int i, struct S s);
> +int __attribute__((noinline)) bar (int i, struct S s);
> +int __attribute__((noinline)) baz (int i, struct S s);
> +
> +
> +int __attribute__((noinline))
> +bar (int i, struct S s)
> +{
> + return baz (i, s);
> +}
> +
> +int __attribute__((noinline))
> +baz (int i, struct S s)
> +{
> + return foo (i, s);
> +}
> +
> +int __attribute__((noinline))
> +foo (int i, struct S s)
> +{
> + if (i == 2)
> + return 0;
> + else
> + return s.b * s.b + bar (i - 1, s);
> +}
> +
> +volatile int g;
> +
> +void entry (void)
> +{
> + struct S s;
> + s.b = 4;
> + g = bar (g, s);
> +}
> +
> +
> +void entry2 (void)
> +{
> + struct S s;
> + s.b = 6;
> + g = baz (g, s);
> +}
> +
> +
> +/* { dg-final { scan-ipa-dump-times "adding an extra caller" 2 "cp" } } */
> --
> 2.24.0
>
