Re: Ping: [PATCH V6] Extend IPA-CP to support arithmetically-computed value-passing on by-ref argument (PR ipa/91682)

[Feng Xue OS <fxue at os dot amperecomputing dot com>]

Thanks. 

And for this issue, we can add a new tracker as a followup task.

Feng

________________________________________
From: Jan Hubicka <hubicka@ucw.cz>
Sent: Tuesday, November 12, 2019 8:34 PM
To: Feng Xue OS
Cc: Martin Jambor; gcc-patches@gcc.gnu.org
Subject: Re: Ping: [PATCH V6] Extend IPA-CP to support arithmetically-computed value-passing on by-ref argument (PR ipa/91682)

> > +2019-11-05  Feng Xue  <fxue@os.amperecomputing.com>
> > +
> > +   PR ipa/91682
> > +   * ipa-prop.h (jump_func_type): New value IPA_JF_LOAD_AGG.
> > +   (ipa_load_agg_data, ipa_agg_value, ipa_agg_value_set): New structs.
> > +   (ipa_agg_jf_item): Add new field jftype and type, redefine field value.
> > +   (ipa_agg_jump_function): Remove member function equal_to.
> > +   (ipa_agg_jump_function_p): Remove typedef.
> > +   (ipa_copy_agg_values, ipa_release_agg_values): New functions.
> > +   * ipa-prop.c (ipa_print_node_jump_functions_for_edge): Dump
> > +   information for aggregate jump function.
> > +   (get_ssa_def_if_simple_copy): Add new parameter rhs_stmt to
> > +   record last definition statement.
> > +   (load_from_unmodified_param_or_agg): New function.
> > +   (ipa_known_agg_contents_list): Add new field type and value, remove
> > +   field constant.
> > +   (build_agg_jump_func_from_list): Rename parameter const_count to
> > +   value_count, build aggregate jump function from ipa_load_agg_data.
> > +   (analyze_agg_content_value): New function.
> > +   (extract_mem_content): Analyze memory store assignment to prepare
> > +   information for aggregate jump function generation.
> > +   (determine_known_aggregate_parts): Add new parameter fbi, remove
> > +   parameter aa_walk_budeget_p.
> > +   (update_jump_functions_after_inlining): Update aggregate jump function.
> > +   (ipa_find_agg_cst_for_param): Change type of parameter agg.
> > +   (try_make_edge_direct_simple_call): Add new parameter new_root.
> > +   (try_make_edge_direct_virtual_call): Add new parameter new_root and
> > +   new_root_info.
> > +   (update_indirect_edges_after_inlining): Pass new argument to
> > +   try_make_edge_direct_simple_call and try_make_edge_direct_virtual_call.
> > +   (ipa_write_jump_function): Write aggregate jump function to file.
> > +   (ipa_read_jump_function): Read aggregate jump function from file.
> > +   (ipa_agg_value::equal_to): Migrate from ipa_agg_jf_item::equal_to.
> > +   * ipa-cp.c (ipa_get_jf_arith_result): New function.
> > +   (ipa_agg_value_from_node): Likewise.
> > +   (ipa_agg_value_set_from_jfunc): Likewise.
> > +   (propagate_vals_across_arith_jfunc): Likewise.
> > +   (propagate_aggregate_lattice): Likewise.
> > +   (ipa_get_jf_pass_through_result): Call ipa_get_jf_arith_result.
> > +   (propagate_vals_across_pass_through): Call
> > +   propagate_vals_across_arith_jfunc.
> > +   (get_clone_agg_value): Move forward.
> > +   (propagate_aggs_across_jump_function): Handle aggregate jump function
> > +   propagation.
> > +   (agg_jmp_p_vec_for_t_vec): Remove.
> > +   (context_independent_aggregate_values): Change use of
> > +   vec<ipa_agg_jf_item> to vec<ipa_agg_value>.
> > +   (copy_plats_to_inter, intersect_with_plats): Likewise.
> > +   (agg_replacements_to_vector, intersect_with_agg_replacements): Likewise.
> > +   (intersect_aggregate_with_edge): Likewise.
> > +   (find_aggregate_values_for_callers_subset): Likewise.
> > +   (cgraph_edge_brings_all_agg_vals_for_node): Likewise.
> > +   (estimate_local_effects): Change use of vec<ipa_agg_jump_function>
> > +   and vec<ipa_agg_jump_function_p> to vec<ipa_agg_value_set>.
> > +   (gather_context_independent_values): Likewise.
> > +   (perform_estimation_of_a_value, decide_whether_version_node): Likewise.
> > +   * ipa-fnsummary.c (evaluate_conditions_for_known_args): Change use of
> > +   vec<ipa_agg_jump_function_p> to vec<ipa_agg_value_set>.
> > +   (evaluate_properties_for_edge): Likewise.
> > +   (estimate_edge_devirt_benefit): Likewise.
> > +   (estimate_edge_size_and_time):  Likewise.
> > +   (estimate_calls_size_and_time): Likewise.
> > +   (ipa_call_context::ipa_call_context): Likewise.
> > +   (estimate_ipcp_clone_size_and_time):  Likewise.
> > +   * ipa-fnsummary.h (ipa_call_context): Change use of
> > +   vec<ipa_agg_jump_function_p> to vec<ipa_agg_value_set>.
> > +   * ipa-inline-analysis.c (do_estimate_edge_time): Change use of
> > +   vec<ipa_agg_jump_function_p> to vec<ipa_agg_value_set>.
> > +   (do_estimate_edge_size): Likewise.
> > +   (do_estimate_edge_hints): Likewise.
> > +
>
> OK, thanks - this looks like very nice ipa-prop improvement.
Also note that there is a long standing problem with inlining ipacp
clones.  This can be shown on the following example:

struct a {int a;};
static int foo (struct a a)
{
  return a.a;
}
__attribute__ ((noinline))
static int bar (struct a a)
{
  return foo(a);
}
main()
{
  struct a a={1};
  return bar (a);
}

Now if you compile it with -O2 -fno-early-inlining ipacp correctly
determines constants:

Estimating effects for bar/1.
   Estimating body: bar/1
   Known to be false:
   size:6 time:14.000000 nonspec time:14.000000
 - context independent values, size: 6, time_benefit: 0.000000
     Decided to specialize for all known contexts, code not going to grow.
Setting dest_lattice to bottom, because type of param 0 of foo is NULL or unsuitable for bits propagation

Estimating effects for foo/0.
   Estimating body: foo/0
   Known to be false: op0[offset: 0] changed
   size:3 time:2.000000 nonspec time:3.000000
 - context independent values, size: 3, time_benefit: 1.000000
     Decided to specialize for all known contexts, code not going to grow.


Yet the intended tranformation to "return 1" does not happen:

__attribute__((noinline))
bar.constprop (struct a a)
{
  int a$a;

  <bb 2> [local count: 1073741824]:
  a$a_5 = a.a;
  return a$a_5;

}



;; Function main (main, funcdef_no=2, decl_uid=1937, cgraph_uid=3, symbol_order=2) (executed once)

main ()
{
  struct a a;
  int _3;

  <bb 2> [local count: 1073741824]:
  a.a = 1;
  _3 = bar.constprop (a); [tail call]
  a ={v} {CLOBBER};
  return _3;

}

The problem here is that foo get inlined into bar and we never apply
ipcp transform on foo, so a.a never gets constant propagated.

For value ranges this works since late passes are able to propagate
constants from value ranges we attach to the default def SSA names.  I
think correct answer here is to do no subtitution in in ipa-prop.c
transform function.  Rather note the known values for late passes and
let FRE do its job.

Honza