On Tue, Mar 14, 2017 at 12:04 PM, Martin Liška <mli...@suse.cz> wrote:
> On 03/14/2017 11:29 AM, Richard Biener wrote:
>> On Tue, Mar 14, 2017 at 11:09 AM, Martin Liška <mli...@suse.cz> wrote:
>>> On 03/14/2017 10:46 AM, Richard Biener wrote:
>>>> On Mon, Mar 13, 2017 at 4:19 PM, Martin Liška <mli...@suse.cz> wrote:
>>>>> Hello.
>>>>>
>>>>> This is a small follow-up patch, where local.local flag should be also
>>>>> dropped
>>>>> for a default implementation.
>>>>>
>>>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>>>
>>>>> Ready to be installed?
>>>>
>>>> I see we have clered the flag on the clones this way. But isn't it
>>>> bogus to have
>>>> it set in the first place? That is, isn't analysis sofar given bogus info?
>>>
>>> Yes, I did it for clones. Reason why we mark it is that local flag is set
>>> by pass_ipa_function_and_variable_visibility pass, which runs before MV
>>> pass.
>>>
>>> I can imagine MV can bail out for a non-trivial reason and the visibility
>>> pass
>>> should somehow simulate and predict what happens in the MV pass?
>>
>> Setting .local to true is an optimization, isn't it? So just not doing it
>> when
>> the attribute is present should be safe. Just "undoing" .local = true later
>> is asking for trouble if some intermediate pass decides to do some transform
>> based on .local = true, no?
>
> Ok, there's more detail analysis. When a MV function is set local (either
> it's a static symbol),
> or IPA split (or IPA inline) makes a clone. This is all fine because we
> define local as:
>
> struct GTY(()) cgraph_local_info {
> /* Set when function is visible in current compilation unit only and
> its address is never taken. */
> unsigned local : 1;
>
> All IPA passes can happily makes optimization based on that. However, in
> moment where we
> introduce a dispatcher (that obviously takes an address), then the function
> starts to not
> be local.
>
> Thus I believe my original patch (and the part which is already in trunk)
> should work
> in the right way.
Ah, ok. I thought it was also supposed to preserve the ABI but if
only the ABI of the
dispatcher (the original fndecl) matters then this is moot.
Patch is ok then.
Thanks,
Richard.
> Martin
>
>>
>> Richard.
>>
>>> Martin
>>>
>>>>
>>>> Shouldn't we instead fix local_p to not mark functions with MV attribute
>>>> local
>>>> in the first place?
>>>>
>>>> Richard.
>>>>
>>>>> Martin
>>>
>
