Re: [PATCH] define auto_vec copy ctor and assignment (PR 90904)

[Martin Jambor <mjambor@suse.cz>]

Hi,

On Wed, Jun 30 2021, Richard Biener wrote:
> On Wed, Jun 30, 2021 at 3:46 AM Martin Sebor <msebor@gmail.com> wrote:
>> On 6/29/21 4:58 AM, Richard Biener wrote:

[...]

>> >
>> > But I wonder if since you now identified them they could be massaged
>> > prior to doing the change.
>> >
>> > I do hope we end up not needing .to_vec () after all, if no users remain ;)
>>
>> I'd be happy to if none remained.  I see how to eliminate those in
>> calls to functions like c_parser_declaration_or_fndef() (done in
>> the attached revision of the patch), but no easy way to get rid
>> of those that replace other implicit conversions, like all those
>> assignments to the vec members of the ipa_call_arg_values ctor.
>> If it's appropriate to std::move those then that would get rid
>> of the .to_vec () call.  I'm not familiar with the code but I
>> have the impression it might be meant more as a reference to
>> some "remote" object (an instance of ipa_auto_call_arg_values?)
>> If that's right then making the vec members auto_vec references
>> (or pointers) would be one way to "fix" this.
>
> I think ipa_call_arg_values is just a temporary container used to
> pass a collection of vec<>s along API boundaries.  I'm not sure
> whether it's default CTOR is ever used but it's definitely an
> optimization avoiding extra indirection (when changing the vec<>
> members to vec<> * or references, in case the default CTOR is
> not used).  It _might_ be that the vecs are all just read and never
> written to in the APIs using this abstract type

No, IPA-CP does add and then remove extra context-specific values in the
auto version of the container, ipa_auto_call_arg_values, but I do not
think that consumers of ipa_call_arg_values do.  The caching mechanism
can make a (partial) copy.

> but then it's
> likely the vector are always appropriately pre-allocated.

They are, they should never be reallocated.

> Maybe using array_slice instead of vec<> members would work,
> but they'd pack less efficient (but I guess not an issue for this
> aggregate which should be only used temporarily for argument
> passing).

I need to educate myself more about array_slice to to comment on that.
But note that apart from reducing the number of parameters, there is
also an ipa_call_arg_values field in ipa_call_context and especially
ipa_cached_call_context.

Martin