[PATCH] D59709: [ThinLTO] Auto-hide prevailing linkonce_odr only when all copies eligible

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp:423
+                                    /* Live = */ false, V.isDSOLocal(),
+                                    V.hasLinkOnceODRLinkage() && V.hasGlobalUnnamedAddr());
 
----------------
int3 wrote:
> @tejohnson should we account for `local_unnamed_addr` constants here too? `canBeOmittedFromSymbolTable()` checks for both: https://github.com/llvm/llvm-project/blob/main/llvm/lib/IR/Globals.cpp#L354
Yes it looks like it, at least for const variables (as well as for all functions since they are const, and aliases that meet the same criteria). In fact, reading through the old comments, I see pcc asked about this and I wanted to do that as a follow up (since the previous version already only handled that case - see the change in FunctionImport.cpp), which apparently never happened. Can you send a patch to change this? Note that you will need to adjust the assert in FunctionImport.cpp. I would just change all of these places that check for LinkOnceODR && GlobalUnnamedAddr to simply call canBeOmittedFromSymbolTable().


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59709