[llvm] [ThinLTO] Reduce the number of renaming due to promotions in distribu… (PR #188074)

[Teresa Johnson via llvm-commits]

================
@@ -1607,6 +1607,25 @@ void llvm::gatherImportedSummariesForModule(
 
     SummariesForIndex[GUID] = DS->second;
   }
+
+  // For each source module we import from, also include summaries for local
+  // functions that have NoRenameOnPromotion set. This is needed for distributed
+  // ThinLTO. Otherwise, the local function of the source module will keep its
+  // origin name, e.g., foo() while the function in destination module will have
+  // name foo.llvm.<...>() and this will cause a link failure.
+  for (auto &[ModPath, SummariesForIndex] : ModuleToSummariesForIndex) {
----------------
teresajohnson wrote:

Unfortunately, the handling here is going to add the summary declaration for every local in each of these modules that is marked no rename on promotion, regardless of whether they are referenced from the importing module. However, detecting this during function importing is too early, as we don't know which will be no rename. For this I would suggest adding a comment, noting that it is going to import a superset of the necessary declarations, but that detecting which are needed exactly is a phase ordering problem (or more expensive to do here).

Also, while AlwaysRenamePromotedLocals is disabled by default for the LTO backends, it doesn't block setting the flag checked here during LTO, meaning this is going to affect builds with that off by default. For that, I would suggest moving the AlwaysRenamePromotedLocals flag definition into a common file, probably ModuleSummaryIndex.cpp, and using it to guard this handling.

https://github.com/llvm/llvm-project/pull/188074