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

[Yuanfang Chen via Phabricator via llvm-commits]

ychen 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
+
----------------
tejohnson wrote:
> 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?
Thanks for taking a look, Teresa. I reduced PR49009 to a test case like this. `D2` was dropped due to non-prevailing `D0 at a.ll`, hence the `D1` to `D2` aliasing caused the assertion failure.

```
---- a.ll  (D2,px D0,)
$D5 = comdat any

@D1 = weak_odr unnamed_addr alias void (%"X"*), void (%"Y"*)* @D2

define weak_odr void @D2(%"Y"* %this) unnamed_addr #0 comdat($D5) align 2 {
entry:
  ret void
}

define weak_odr void @D0(%"Y"* %this) unnamed_addr #0 comdat($D5) align 2 {
entry:
  tail call void @llvm.trap()
  unreachable
}


---- b.ll  (D0,px)
$D0 = comdat any

define linkonce_odr void @D0(%"Y"* %this) unnamed_addr #0 comdat align 2 {
entry:
  tail call void @llvm.trap()
  unreachable
}
```




Repository:
  rL LLVM

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

https://reviews.llvm.org/D34803