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

[Xinliang David Li via llvm-commits]

difference in performance? How about CSPGO?

On Thu, Sep 19, 2019 at 12:25 PM Rong Xu <xur at google.com> wrote:

> I tested the idea using llvm bootstrap.
> Without comdat group, the object file sizes as well as the library archive
> sizes are smaller.
> But the section sizes in the final binary (like clang-10) increase
> significantly.
> This is using lld linker. I think we need more investigation here.
>
> Here is the section sizes for clang-10.
> =========== with comdat grouping ===============
>  26 __llvm_prf_cnts 008a9a08  00000000058a29c8  00000000058a29c8  0569f9c8
>  2**3
>                   CONTENTS, ALLOC, LOAD, DATA
>  27 __llvm_prf_data 004842c0  000000000614c3d0  000000000614c3d0  05f493d0
>  2**3
>                   CONTENTS, ALLOC, LOAD, DATA
>  28 __llvm_prf_vals 00047d28  00000000065d0690  00000000065d0690  063cd690
>  2**3
>                   CONTENTS, ALLOC, LOAD, DATA
>  29 __llvm_prf_vnds 00142c60  00000000066183c0  00000000066183c0  064153c0
>  2**5
>                   CONTENTS, ALLOC, LOAD, DATA
>  30 __llvm_prf_names 0028f48c  000000000675b020  000000000675b020
>  06558020  2**0
>                   CONTENTS, ALLOC, LOAD, DATA
>  31 __llvm_orderfile 00000000  00000000069ea4ac  00000000069ea4ac
>  067e74ac  2**2
>                   CONTENTS, ALLOC, LOAD, DATA
>
> ============= without comdat grouping ===============
>  26 __llvm_prf_cnts 00a896a8  00000000058a2908  00000000058a2908  0569f908
>  2**3
>                   CONTENTS, ALLOC, LOAD, DATA
>  27 __llvm_prf_data 006b4cc0  000000000632bfb0  000000000632bfb0  06128fb0
>  2**3
>                   CONTENTS, ALLOC, LOAD, DATA
>  28 __llvm_prf_vals 00061e88  00000000069e0c70  00000000069e0c70  067ddc70
>  2**3
>                   CONTENTS, ALLOC, LOAD, DATA
>  29 __llvm_prf_vnds 00142c60  0000000006a42b00  0000000006a42b00  0683fb00
>  2**5
>                   CONTENTS, ALLOC, LOAD, DATA
>  30 __llvm_prf_names 0028f48c  0000000006b85760  0000000006b85760
>  06982760  2**0
>                   CONTENTS, ALLOC, LOAD, DATA
>  31 __llvm_orderfile 00000000  0000000006e14bec  0000000006e14bec
>  06c11bec  2**2
>                   CONTENTS, ALLOC, LOAD, DATA
>
> On Wed, Sep 18, 2019 at 9:58 AM Rong Xu <xur at google.com> wrote:
>
>>
>>
>> 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/20190919/43eab3e7/attachment.html>