[PATCH] D67579: [PGO] Use linkonce_odr linkage for __profd_ variables in comdat groups

Rong Xu via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 18 09:58:07 PDT 2019


On Tue, Sep 17, 2019 at 8:32 PM Xinliang David Li <davidxl at google.com>
wrote:

>
>
> On Tue, Sep 17, 2019 at 4:25 PM Rong Xu <xur at google.com> wrote:
>
>> +cc David
>>
>> On Tue, Sep 17, 2019 at 3:20 PM Reid Kleckner via Phabricator <
>> reviews at reviews.llvm.org> wrote:
>>
>>> rnk added a comment.
>>>
>>> From what I understand, the comdat group isn't necessary, so I disabled
>>> it on COFF in r372182. Can we simplify ELF to match?
>>>
>>
> Putting prof sections with the function in the same comdat group is
> probably not needed. There might be some weird interaction with GC and
> instrumentation binary size. If we don't see issues there, it is ok to
> simplify ELF to match.  Rong can help with some internal testing (instr
> build size, performance etc) on internal large app with linux.
>
> They are not in the same compdat group as the functions -- they have
their own group.
I agree that if this affects the data section size (which means duplicates
counter/variables), we should keep the current way. But I don't double this
is the case.
I will do some testing on removing comdat on ELF.

-Rong

David
>
>
>> It's probably OK without setting COMDAT for the variables in ELF as the
>> linkonce_odr  linkage can be used to remove the duplicates.
>> On the other hand, it's nature to set the COMDAT to sync the
>> declaration of the function.
>> I'm CCing david to see what is his thoughts.
>>
>> -Rong
>>
>>>
>>>
>>> Repository:
>>>   rL LLVM
>>>
>>> CHANGES SINCE LAST ACTION
>>>   https://reviews.llvm.org/D67579/new/
>>>
>>> https://reviews.llvm.org/D67579
>>>
>>>
>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190918/8c2bbc47/attachment.html>


More information about the llvm-commits mailing list