[PATCH] D135427: [LTO] Make local linkage GlobalValue in non-prevailing COMDAT available_externally

Sat Oct 8 13:27:48 PDT 2022

MaskRay added inline comments.

================
Comment at: llvm/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:
> MaskRay wrote:
> > The test has an issue. If C is prevailing in %t1.o, its member testglobfunc must be prevailing as well.
> > 
> > I'll swap the two `-r` options for `testglobfunc`: `-r=%t1.o,testglobfunc,lxp -r=%t2.o,testglobfunc,lx`
> This was by design. It was trying to simulate an issue that occurred with mixed Thin and regular LTO modules (per some discussion on original patch and bug). I'm not sure if we need to keep it that way, but I noticed you removed the check of testglobfunc being made available_externally - what happened to that check?
`assert(GO.hasLocalLinkage() && "GO's comdat should have been dropped");` fails with `-r=%t2.o,testglobfunc,lxp -r=%t1.o,testglobfunc,lx`. 

Restore the original RUN line and remove the assert then? Note that the assert can fire in other erroneous cases where a user uses an `external` linkage definition to prevail a linkonce_odr definition in a prevailing comdat.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135427