>
> All right, but let's start from the basic objective of the patch. Do
> we want to have something like ipa_call_arg_values?
OK, i suppose you patch was motivated by two things:
1) combine results of the estimates to one place (reducing number
of parameters in estimate_size_and_time) which can be done by
inventing ipa_estimates datastructure
2) combine the different things we track on values into one
datastructure (ipa_call_agg_values).
Which essentially will reduce API for
estimate_ipcp_clone_size_and_time
and evaluate_properties_for_edge
and the construtor of ipa_call_context
ipa_call_context is meant to do the following
1) hold precisely the info on which estimates depends on. "unnecesary"
info is thrown away to make comparsions quick and memory use down.
However the notion of unnecesariness is closely tied into what
summary contains. I.e. the info still can be useful for propagation.
2) be able to produce estimates from that info
3) be able to compare itself with other context to handle the context
cache.
In longer term I want to keep ability to have multiple contextes per
function (i.e. add hash method) and update corresponding estimates in
cache incrementally/recompute if function changes. I implemented that
but it did not seem to pay back in compile time tests enough to be
merged. However I epxpect this to change in future as we track more
useful info and units keep growing.
>
> I hope that the answer to this question is yes. I certainly hope that
> we want to get rid of passing around each vector as an individual
> parameter. The one alternative I can think of would be to make each
> function that now receives the three or four vectors either a method of
> ipa_call_context or receive ipa_call_context as the parameter. That
> however does not fit naturally to uses in 1) ipa_fn_summary_t::duplicate,
> 2) ipa_merge_fn_summary_after_inlining and 3) anywhere in IPA-CP where
> I'd like the pass to continue with the pass owning the vectors and
> constructing contexts for each change.
Yep, because these functions compute info context is build from :)
I understnad that you want to have a common datastructure to handle
known information about parameters. Have way to construct it using the
two ways we use (in inliner and in ipa-cp) and feed it into context.
If you merge the four vectors and clauses into ipa_call_agg_values which
has auto_vecs of prealocated size, so if it is homed on stack, it will
mostly avoid calling malloc, then you can pass it to the constructor of
context and turn evaluate_properties_for_edge and
estimate_ipcp_clone_size_and_time to perhaps member function used to
fill them up.
We can still play the games about ownership of the vectors needed to
keep malloc calls down.
>
> If the answer to the above question is yes, then we can have
> ipa_call_arg_values containing pointers to the vectors - which would be
> part of ipa_call_context or it can be even derived from it - and which
> would be constructible from ipa_auto_call_arg_values, which would
> contain the vectors as autovecs. This would also help me unify the
> information for IPA-CP clones.
>
> Then ipa_cached_call_context would not need to be a different type, it
> would only allocate the vectors almost like before although I'd prefer
> it to be, just so that the release function is specific to this kind of
> context. Just the vec structures themselves would be on the heap.
> Would that work?
>
> Note that, however, I still think that the most context-consumer
> friendly interface would be to have static function
> ipa_call_context::for_inlined_edge (see my 2nd patch) which would fetch
> a context either from a cache or create it without the user knowing
> anything about the cache at all. But then in order to cache the
> estimates, they would have to be part of the context... but I do not
> insist, we can leave the caching explicit.
It seems to me that keeping the context separate from info about
arguments is doable and would be cleaner (since it is not meant to be
holder for info about arguments).
Does that seem to make sense?
Honza
>
> Anyway, please let me know what you think about the above plan.
>
> Martin
>
>
>
> >
> > 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 <mjam...@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.
