[PATCH] D80765: [ELF] Handle bitcode comdat groups separately to deduplicate thinlto comdat sections

[Hongtao Yu via Phabricator via llvm-commits]

hoyFB added a comment.

In D80765#2099946 <https://reviews.llvm.org/D80765#2099946>, @MaskRay wrote:

> Apologies for my slowness getting to this patch.
>
> `symtab->ltoOutputComdatGroups` does work:
>
>   % readelf -g a3.o.lto.o a3.o1.lto.o 
>  
>   File: a3.o.lto.o
>  
>   COMDAT group section [    4] `.group' [4068369915778327548] contains 2 sections:
>      [Index]    Name
>      [    5]   .debug_types
>      [    6]   .rela.debug_types
>  
>   File: a3.o1.lto.o
>  
>   COMDAT group section [    4] `.group' [4068369915778327548] contains 2 sections:
>      [Index]    Name
>      [    5]   .debug_types
>      [    6]   .rela.debug_types
>
>
> The  .debug_types from a3.o1.lto.o can be discarded by the new COMDAT logic. However, I am concerned that making it general as this patch does can miss some codegen bugs. See @pcc's argument in 
>  in https://reviews.llvm.org/D56015#1339411 . Since .debug_types (notably, a non-SHF_ALLOC section) is the only COMDAT rule this patch will discard, how about special casing .debug_types (i.e. if isLTOOutput && the group is related to .debug_types)?


Thanks for taking time reviewing this change. It is an interesting argument whether LTO should introduce new COMDAT symbols. Do we have a conclusion on this? So far the .debug_types are the only COMDAT we've seen introduced by LTO, but in theory any LTO-exclusive pass may introduce a new COMDAT symbol and it's up to the pass how to set the symbol's linkage type.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80765/new/

https://reviews.llvm.org/D80765