[PATCH] D25854: [lto] Kill undefined extern_weak declarations before opt

[Peter Collingbourne via llvm-commits]

pcc added inline comments.


================
Comment at: lib/LTO/LTO.cpp:346
       continue;
     if (Res.Prevailing && GV) {
       Keep.push_back(GV);
----------------
mehdi_amini wrote:
> pcc wrote:
> > I would have thought that you could keep track of the undefined symbols in a local variable here and RAUW them in a loop below. That way, you can avoid the global tracking. 
> How do you address ThinLTO?
> 
> Which needs to be added to the patch BTW.
> How do you address ThinLTO?

The way I see this feature eventually being added to ThinLTO is:
- decouple LLVM linkages from ThinLTO summary linkages
- (orthogonally: represent "drop" as another type of linkage -- this would allow us to drop unchosen linkonce or weak (non-odr) definitions)
- add a new "linkage" (at this point we can probably call it a resolution) which represents an undefined extern_weak.
- copy all resolution information into the summary during addThinLTO while handling each input file (during mergeFrom or thereabouts)

> Which needs to be added to the patch BTW.

It doesn't seem necessary to do so in the first patch. This is just an optimisation, and the CFI feature that relies on this does not (currently) work with ThinLTO. But @eugenis please do add a FIXME.


================
Comment at: tools/llvm-lto2/llvm-lto2.cpp:68
              "     visible outside of the LTO unit\n"
              "A resolution for each symbol must be specified."),
     cl::ZeroOrMore);
----------------
Please update the documentation.


Repository:
  rL LLVM

https://reviews.llvm.org/D25854