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: mjam...@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 <f...@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 <f...@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
