[PATCH] D43690: [ThinLTO] Keep available_externally symbols live
Vlad Tsyrklevich via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 6 17:11:25 PST 2018
vlad.tsyrklevich added inline comments.
Comment at: lib/Transforms/IPO/FunctionImport.cpp:620
+ for (auto &S : VI.getSummaryList())
+ if (S->linkage() != GlobalValue::AvailableExternallyLinkage)
> 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?
More information about the llvm-commits