<div dir="ltr">I tested the idea using llvm bootstrap.<div>Without comdat group, the object file sizes as well as the library archive sizes are smaller.</div><div>But the section sizes in the final binary (like clang-10) increase significantly.</div><div>This is using lld linker. I think we need more investigation here.</div><div><br></div><div>Here is the section sizes for clang-10.</div><div>=========== with comdat grouping ===============</div><div> 26 __llvm_prf_cnts 008a9a08 00000000058a29c8 00000000058a29c8 0569f9c8 2**3<br> CONTENTS, ALLOC, LOAD, DATA<br> 27 __llvm_prf_data 004842c0 000000000614c3d0 000000000614c3d0 05f493d0 2**3<br> CONTENTS, ALLOC, LOAD, DATA<br> 28 __llvm_prf_vals 00047d28 00000000065d0690 00000000065d0690 063cd690 2**3<br> CONTENTS, ALLOC, LOAD, DATA<br> 29 __llvm_prf_vnds 00142c60 00000000066183c0 00000000066183c0 064153c0 2**5<br> CONTENTS, ALLOC, LOAD, DATA<br> 30 __llvm_prf_names 0028f48c 000000000675b020 000000000675b020 06558020 2**0<br> CONTENTS, ALLOC, LOAD, DATA<br> 31 __llvm_orderfile 00000000 00000000069ea4ac 00000000069ea4ac 067e74ac 2**2<br> CONTENTS, ALLOC, LOAD, DATA<br></div><div><br></div><div>============= without comdat grouping ===============</div><div> 26 __llvm_prf_cnts 00a896a8 00000000058a2908 00000000058a2908 0569f908 2**3<br> CONTENTS, ALLOC, LOAD, DATA<br> 27 __llvm_prf_data 006b4cc0 000000000632bfb0 000000000632bfb0 06128fb0 2**3<br> CONTENTS, ALLOC, LOAD, DATA<br> 28 __llvm_prf_vals 00061e88 00000000069e0c70 00000000069e0c70 067ddc70 2**3<br> CONTENTS, ALLOC, LOAD, DATA<br> 29 __llvm_prf_vnds 00142c60 0000000006a42b00 0000000006a42b00 0683fb00 2**5<br> CONTENTS, ALLOC, LOAD, DATA<br> 30 __llvm_prf_names 0028f48c 0000000006b85760 0000000006b85760 06982760 2**0<br> CONTENTS, ALLOC, LOAD, DATA<br> 31 __llvm_orderfile 00000000 0000000006e14bec 0000000006e14bec 06c11bec 2**2<br> CONTENTS, ALLOC, LOAD, DATA<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Sep 18, 2019 at 9:58 AM Rong Xu <<a href="mailto:xur@google.com">xur@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Sep 17, 2019 at 8:32 PM Xinliang David Li <<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Sep 17, 2019 at 4:25 PM Rong Xu <<a href="mailto:xur@google.com" target="_blank">xur@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">+cc David<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Sep 17, 2019 at 3:20 PM Reid Kleckner via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">rnk added a comment.<br>
<br>
>From what I understand, the comdat group isn't necessary, so I disabled it on COFF in r372182. Can we simplify ELF to match?<br></blockquote></div></div></blockquote><div><br></div><div>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. </div><div><br></div></div></div></blockquote><div>They are not in the same compdat group as the functions -- they have their own group.</div><div>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.</div><div>I will do some testing on removing comdat on ELF.</div><div><br></div><div>-Rong</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div></div><div>David</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"></blockquote><div>It's probably OK without setting COMDAT for the variables in ELF as the linkonce_odr linkage can be used to remove the duplicates. </div><div>On the other hand, it's nature to set the COMDAT to sync the declaration of the function. </div><div>I'm CCing david to see what is his thoughts.</div><div><br></div><div>-Rong</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
Repository:<br>
rL LLVM<br>
<br>
CHANGES SINCE LAST ACTION<br>
<a href="https://reviews.llvm.org/D67579/new/" rel="noreferrer" target="_blank">https://reviews.llvm.org/D67579/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D67579" rel="noreferrer" target="_blank">https://reviews.llvm.org/D67579</a><br>
<br>
<br>
<br>
</blockquote></div></div>
</blockquote></div></div>
</blockquote></div></div>
</blockquote></div>