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

Sat Jun 13 09:06:30 PDT 2020

tejohnson added a comment.

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

> In D80765#2090905 <https://reviews.llvm.org/D80765#2090905>, @christylee wrote:
>
> > @grimar  Do you know who might be a good reviewer?
>
>
> @tejohnson is a good one.

Since this is very specific to lld, and not in the LLVM LTO handling, I'm not sure I'm the best person to review. I looked at the change but don't have enough understanding of how lld is otherwise handling comdat groups to do a really informed review.

> I'll also get to this patch soon. I tried an earlier version of this patch and noticed issues testing internally. I'll retest since the patch has changed a lot.

Sounds good.

================
Comment at: lld/test/ELF/lto/debug-types-deduplication.ll:4
+; RUN: llvm-as %p/Inputs/debug-types-deduplication.ll -o %t2.o
+; RUN: ld.lld %t.o %t2.o -mllvm --generate-type-units --entry=main -o %t3.o
+; RUN: llvm-dwarfdump --debug-types %t3.o | FileCheck %s
----------------
IMO it is better to test ThinLTO by using "opt -module-summary" to create the bitcode object, and not have the serialized summary. It makes it clearer that it is a ThinLTO test.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80765