[PATCH] D34803: [LTO] Remove values from non-prevailing comdats

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: llvm/trunk/test/LTO/Resolution/X86/comdat-mixed-lto.ll:11
+; available_externally.
+; RUN: llvm-lto2 run -r=%t1.o,C,pl -r=%t2.o,C,l -r=%t2.o,testglobfunc,lxp -r=%t1.o,testglobfunc,lx -o %t3 %t1.o %t2.o -save-temps
+
----------------
ychen wrote:
> It seems PR49009 failed due to this patch.
> 
> Here both `C at t1.o` and `testglobfunc at t2.o` prevail however they are from COMDATs of the same key, I think this resolution is not possible from the linker's point of view?
> 
Reading through the bug this was fixing (PR32980) again, I think this is trying to simulate the scenario that occurred there. It sounds like it was a ThinLTO build where LTO splitting was in effect, so we also had a regular LTO module. When we are done with the LTO backends the regular LTO module is handed back to the linker first. The problem occurred because the comdat added to the regular LTO module was initially not prevailing, but since the regular LTO native object was handed back to the linker for the final native linker first, its (then incomplete) comdat and symbols were subsequently selected as prevailing, leading to the incomplete comdat issue. We avoided this by removing the comdat from the non-prevailing copies. I think the below test is trying to simulate that effect since here we only have a single link. I can add a comment to the test.

I looked at PR49009 but it isn't clear from what is written in the bug how this patch caused that failure. Can you elaborate as to what is happening to that symbol with and without this patch?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D34803