[PATCH 3/n] ipa: Simplify interface of ipa_call_context::estimate_size_and_time

[Jan Hubicka]

> Hi,
> 
> On Sat, Aug 29 2020, Jan Hubicka wrote:
> >> Hi,
> >> 
> >> this patch changes ipa_call_context::estimate_size_and_time to store
> >> its results into member fields of the ipa_call_context class instead
> >> into pointers it receives as parameters so that it can compute ore
> >> stuff without cluttering the interface even further.
> >> 
> >> Bootstrapped and tested on x86_64-linux.  OK for master on top of the
> >> previous two patches?
> >
> > ipa_call_context is intended to be structure holding all parameters that
> > are needed to produce the estimates (size/time/hints).
> 
> even today it only "holds" the data when it resides in the RCU cache,
> otherwise it points to data "owned" by the caller.  Admittedly, my first
> patch makes the cache data structure separate, making ipa_class_context
> only a utility for calculating the estimates - but given how all the
> code is structured, it does not really work as the grand encapsulator of
> all context data when passing it from a function to function.
> 
> > Adding the actual estimates there would duplicate it with cache.
> 
> The first patch in the series makes the cache items not contain
> ipa_call_context directly, so in my patch series at least, the estimates
> are not duplicated.

ipa_call_context defines the context estimates depends on.  This puts
all the info to one place and makes the cache well defined - it assigns
contexts to estimates. From this point of view I do not quite like
duplicating this logic (copying things into different datastructure) and
making contxt to also contain the esitmates since these are, well, not
context of the call.

I am happy with merging the analysis results into something like
class function_body_estimate holding all the values.

Games with the ownerhip you mention above was not original plan.
While perfing inliner I noticed that we spend measurable time in malloc
and that mostly goes to alocaitng the vectors (which we did for long
time).  Perhaps cleaner solution is to have
ipa_context_base which is derived by ipa_context and ipa_cached_context
where first preallocats on stack while second allocates on heap?

Honza
> 
> > What
> > about keeping them separate and inventing ipa_call_estimates structure
> > to hold the reults?
> 
> I can but unless you do not like the first patch and want me to re-write
> it or just not do anything like it, I don't think it matters because the
> structures will almost always lie next to each other on the user's
> stack.
> 
> Martin
> 
> >> 
> >> 
> >> gcc/ChangeLog:
> >> 
> >> 2020-08-28  Martin Jambor  <mjambor@suse.cz>
> >> 
> >> 	* ipa-fnsummary.h (class ipa_call_context): Changed declaration of
> >> 	estimate_size_and_time to accept two booleans.  Added an overload
> >> 	of the method without any parameters.  New fields m_size,
> >> 	m_min_size, m_time, m_nonspecialized_time and m_hints.
> >> 	* ipa-cp.c (hint_time_bonus): Changed the second parameter from
> >> 	just hints to a const reference to ipa_call_context.
> >> 	(perform_estimation_of_a_value): Adjusted to the new interface of
> >> 	ipa_call_context::estimate_size_and_time.
> >> 	* ipa-fnsummary.c (ipa_call_context::estimate_size_and_time):
> >> 	Modified to store results into member fields of the class.
> >> 	* ipa-inline-analysis.c (do_estimate_edge_time): Adjusted to the
> >> 	new interface of ipa_call_context::estimate_size_and_time.
> >> 	(do_estimate_edge_size): Likewise.
> >> 	(do_estimate_edge_hints): Likewise.