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

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson 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
 
----------------
MaskRay wrote:
> 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.
> 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.

Is this a case we want to detect and assert on in this code? Should it be detected and flagged elsewhere if it is erroneous?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135427