[PATCH] D43690: [ThinLTO] Keep available_externally symbols live

[Vlad Tsyrklevich via Phabricator via llvm-commits]

vlad.tsyrklevich added inline comments.


================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:620
+      for (auto &S : VI.getSummaryList())
+        if (S->linkage() != GlobalValue::AvailableExternallyLinkage)
+          return;
----------------
pcc wrote:
> tejohnson wrote:
> > Is it possible to have a symbol that is weak in one module and available_externally in another? If so, it would be more correct to return only if none of the copies is available_externally.
> I suppose so. In the situation where we have at least one available-externally we could end up needing a symbol referenced by the available-externally as a result of a backend inlining it into a caller. So we would need to keep them live anyway.
> 
> Now that I think about it, this applies to linkonce-odr as well (i.e. we could convert them to available-externally as a result of non-prevailing, and then inline them in the backend and thus end up needing a symbol that was referenced by the available-externally but not necessarily by the prevailing copy of the global). But that's somewhat of an orthogonal problem to this.
As I mentioned to @pcc, his example would not be true for ThinLTO as of D42107 because it deletes dead symbols in lto::thinBackend. I'm not sure if it's possible to reach that case with regular LTO.

@tejohnson I think that case is technically possible; however, it would re-introduce the same bug that D42107 originally tried to fix: https://bugs.llvm.org/show_bug.cgi?id=35938

Give that, I'm not sure if there's a way to reconcile that change with this one. I'm not sure if marking individual symbols live/not-live is feasible?


Repository:
  rL LLVM

https://reviews.llvm.org/D43690