[PATCH] D42713: LTO: Include dso-local bit in ThinLTO cache key.

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: llvm/lib/LTO/LTO.cpp:181
+    for (const ValueInfo &VI : GS->refs()) {
+      AddUnsigned(VI.isDSOLocal());
       AddUsedCfiGlobal(VI.getGUID());
----------------
pcc wrote:
> tejohnson wrote:
> > Is it necessary to do this for all refs, rather than just for each DefinedGlobals summary (like we do for the linkage below)?
> Yes because dso-local generally affects the code generated for references to globals.
Perhaps, but in FunctionImportGlobalProcessing::processGlobalForThinLTO we apply this flag to every GV defined in the module. Even if that generally only affects references to those GVs, it seems incomplete not to hash it for all GVs that might have the flag set in the IR. By updating the hash only for references, we will update the hash multiple times for some GVs (those that are in multiple ref lists) and not at all for others (those that don't have any references from within this module). It seems like it would be sufficient and more complete to update the hash for all GVs in the DefinedGlobals set.


https://reviews.llvm.org/D42713