[PATCH] D31444: LTO: call getRealLinkageName on IRNames before feeding to getGUID

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

This seems reasonable for now, with a test case.

In https://reviews.llvm.org/D31444#712714, @mehdi_amini wrote:

> Can we get consistently `GlobalValue::getGUID` taking an IRName and taking care of calling `getRealLinkageName`? (assuming it is what's expected)


Something like that seems reasonable to me as a followup.
Here are all the places where we are passing a string to getGUID: http://llvm-cs.pcc.me.uk/include/llvm/IR/GlobalValue.h/rgetGUID
Some are wrong (passing a value name) and some are passing the result of getGlobalIdentifier.

So maybe we should have just one static function:

  static GUID getGUID(StringRef Name, GlobalValue::LinkageTypes Linkage, StringRef FileName);

That should hopefully make the API harder to misuse.

(Although that would leave us with a somewhat awkward API for type identifier GUIDs, so maybe we should move the GUID generation out of GlobalValue as I think we discussed at one point.)



================
Comment at: lib/LTO/LTO.cpp:1016
           !Res.second.IRName.empty())
-        GUIDPreservedSymbols.insert(GlobalValue::getGUID(Res.second.IRName));
+        GUIDPreservedSymbols.insert(GlobalValue::getGUID(GlobalValue::getRealLinkageName(Res.second.IRName)));
     }
----------------
80 cols


================
Comment at: lib/LTO/LTO.cpp:1035
         continue;
-      auto GUID = GlobalValue::getGUID(Res.second.IRName);
+      auto GUID = GlobalValue::getGUID(GlobalValue::getRealLinkageName(Res.second.IRName));
       // Mark exported unless index-based analysis determined it to be dead.
----------------
80 cols


https://reviews.llvm.org/D31444