[PATCH] D70327: [ThinLTO] Promotion handling cleanup (NFC)

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson created this revision.
tejohnson added reviewers: steven_wu, pcc, evgeny777.
Herald added subscribers: dexonsmith, hiraditya, inglorion, mehdi_amini.
Herald added a project: LLVM.

Clean up the code that does GV promotion in the ThinLTO backends.

Specifically, we don't need to check whether we are importing since that
is already checked and handled correctly in shouldPromoteLocalToGlobal.
Simply call shouldPromoteLocalToGlobal, and if it returns true we are
guaranteed that we are promoting, whether or not we are importing (or in
the exporting module). This also makes the handling in getName()
consistent with that in getLinkage(), which checks the DoPromote parameter
regardless of whether we are importing or exporting.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70327

Files:
  llvm/lib/Transforms/Utils/FunctionImportUtils.cpp


Index: llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
===================================================================
--- llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
+++ llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
@@ -96,10 +96,8 @@
                                                     bool DoPromote) {
   // For locals that must be promoted to global scope, ensure that
   // the promoted name uniquely identifies the copy in the original module,
-  // using the ID assigned during combined index creation. When importing,
-  // we rename all locals (not just those that are promoted) in order to
-  // avoid naming conflicts between locals imported from different modules.
-  if (SGV->hasLocalLinkage() && (DoPromote || isPerformingImport()))
+  // using the ID assigned during combined index creation.
+  if (SGV->hasLocalLinkage() && DoPromote)
     return ModuleSummaryIndex::getGlobalNameForLocal(
         SGV->getName(),
         ImportIndex.getModuleHash(SGV->getParent()->getModuleIdentifier()));
@@ -268,19 +266,13 @@
     }
   }
 
-  bool DoPromote = false;
-  if (GV.hasLocalLinkage() &&
-      ((DoPromote = shouldPromoteLocalToGlobal(&GV)) || isPerformingImport())) {
+  if (GV.hasLocalLinkage() && shouldPromoteLocalToGlobal(&GV)) {
     // Save the original name string before we rename GV below.
     auto Name = GV.getName().str();
-    // Once we change the name or linkage it is difficult to determine
-    // again whether we should promote since shouldPromoteLocalToGlobal needs
-    // to locate the summary (based on GUID from name and linkage). Therefore,
-    // use DoPromote result saved above.
-    GV.setName(getName(&GV, DoPromote));
-    GV.setLinkage(getLinkage(&GV, DoPromote));
-    if (!GV.hasLocalLinkage())
-      GV.setVisibility(GlobalValue::HiddenVisibility);
+    GV.setName(getName(&GV, /* DoPromote */ true));
+    GV.setLinkage(getLinkage(&GV, /* DoPromote */ true));
+    assert(!GV.hasLocalLinkage());
+    GV.setVisibility(GlobalValue::HiddenVisibility);
 
     // If we are renaming a COMDAT leader, ensure that we record the COMDAT
     // for later renaming as well. This is required for COFF.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70327.229597.patch
Type: text/x-patch
Size: 2166 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191115/36caac22/attachment-0001.bin>