[PATCH 3/n] ipa: Simplify interface of ipa_call_context::estimate_size_and_time
Sat Aug 29 21:00:12 GMT 2020
> 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?
> > 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
> >> gcc/ChangeLog:
> >> 2020-08-28 Martin Jambor <email@example.com>
> >> * 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.
More information about the Gcc-patches