Hi, I think the patch generally looks reasonable +2019-11-13 Feng Xue <f...@os.amperecomputing.com> + + PR ipa/92133 + * doc/invoke.texi (ipa-cp-max-recursion-depth): Document new option. + * params.opt (ipa-cp-max-recursion-depth): New. + * ipa-cp.c (ipcp_lattice<valtype>::add_value): Add two new parameters + val_pos_p and unlimited. + (self_recursively_generated_p): New function. + (get_val_across_arith_op): Likewise. + (propagate_vals_across_arith_jfunc): Add constant propagation for + self-recursive function. + (incorporate_penalties): Do not penalize pure self-recursive function. + (good_cloning_opportunity_p): Dump node_is_self_scc flag. + (propagate_constants_topo): Set node_is_self_scc flag for cgraph node. + (get_info_about_necessary_edges): Relax hotness check for edge to + self-recursive function. + * ipa-prop.h (ipa_node_params): Add new field node_is_self_scc. +
In general the patch looks good to me, but I would like Martin Jambor to comment on the ipa-prop/cp interfaces. However... +@item ipa-cp-max-recursion-depth +Maximum depth of recursive cloning for self-recursive function. + ... I believe we will need more careful cost model for this. I think we want to limit the overall growth for all the clones and also probably enable this only when ipa-predicates things the individual clones will actualy be faster by some non-trivial percentage. For recursive inliner we have: --param max-inline-recursive-depth which has similar meaning to your parameter (so perhaps similar name would be good) --param min-inline-recursive-probability which requires the inlining to happen only across edges which are known to be taken with reasonable chance --param max-inline-insns-recursive which specifies overall size after all the recursive inlining Those parameters are not parituclarly well tought out or tested, but they may be good start. Do you have some data on code size/performance effects of this change? Honza
