[llvm] r285507 - [ThinLTO] Use NoPromote flag in summary during promotion

[Teresa Johnson via llvm-commits]

Author: tejohnson
Date: Sat Oct 29 16:31:48 2016
New Revision: 285507

URL: http://llvm.org/viewvc/llvm-project?rev=285507&view=rev
Log:
[ThinLTO] Use NoPromote flag in summary during promotion

Summary:
Replace the check of whether a GV has a section with the flag check
in the summary. This is in preparation for using the NoPromote flag
to convey other situations when we can't promote (e.g. locals used in
inline asm).

Reviewers: mehdi_amini

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D26063

Modified:
    llvm/trunk/include/llvm/Transforms/Utils/FunctionImportUtils.h
    llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp

Modified: llvm/trunk/include/llvm/Transforms/Utils/FunctionImportUtils.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/FunctionImportUtils.h?rev=285507&r1=285506&r2=285507&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Transforms/Utils/FunctionImportUtils.h (original)
+++ llvm/trunk/include/llvm/Transforms/Utils/FunctionImportUtils.h Sat Oct 29 16:31:48 2016
@@ -54,8 +54,9 @@ class FunctionImportGlobalProcessing {
 
   /// Get the name for SGV that should be used in the linked destination
   /// module. Specifically, this handles the case where we need to rename
-  /// a local that is being promoted to global scope.
-  std::string getName(const GlobalValue *SGV);
+  /// a local that is being promoted to global scope, which it will always
+  /// do when \p DoPromote is true (or when importing a local).
+  std::string getName(const GlobalValue *SGV, bool DoPromote);
 
   /// Process globals so that they can be used in ThinLTO. This includes
   /// promoting local variables so that they can be reference externally by
@@ -66,8 +67,9 @@ class FunctionImportGlobalProcessing {
 
   /// Get the new linkage for SGV that should be used in the linked destination
   /// module. Specifically, for ThinLTO importing or exporting it may need
-  /// to be adjusted.
-  GlobalValue::LinkageTypes getLinkage(const GlobalValue *SGV);
+  /// to be adjusted. When \p DoPromote is true then we must adjust the
+  /// linkage for a required promotion of a local to global scope.
+  GlobalValue::LinkageTypes getLinkage(const GlobalValue *SGV, bool DoPromote);
 
 public:
   FunctionImportGlobalProcessing(

Modified: llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp?rev=285507&r1=285506&r2=285507&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp Sat Oct 29 16:31:48 2016
@@ -67,9 +67,9 @@ bool FunctionImportGlobalProcessing::doP
   if (GVar && GVar->isConstant() && GVar->hasGlobalUnnamedAddr())
     return false;
 
-  if (GVar && GVar->hasSection())
-    // Some sections like "__DATA,__cfstring" are "magic" and promotion is not
-    // allowed. Just disable promotion on any GVar with sections right now.
+  auto *Summary = ImportIndex.getGlobalValueSummary(SGV->getGUID());
+  assert(Summary && "Missing summary for global value");
+  if (Summary->noRename())
     return false;
 
   // Eventually we only need to promote functions in the exporting module that
@@ -78,14 +78,14 @@ bool FunctionImportGlobalProcessing::doP
   return true;
 }
 
-std::string FunctionImportGlobalProcessing::getName(const GlobalValue *SGV) {
+std::string FunctionImportGlobalProcessing::getName(const GlobalValue *SGV,
+                                                    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() &&
-      (doPromoteLocalToGlobal(SGV) || isPerformingImport()))
+  if (SGV->hasLocalLinkage() && (DoPromote || isPerformingImport()))
     return ModuleSummaryIndex::getGlobalNameForLocal(
         SGV->getName(),
         ImportIndex.getModuleHash(SGV->getParent()->getModuleIdentifier()));
@@ -93,13 +93,14 @@ std::string FunctionImportGlobalProcessi
 }
 
 GlobalValue::LinkageTypes
-FunctionImportGlobalProcessing::getLinkage(const GlobalValue *SGV) {
+FunctionImportGlobalProcessing::getLinkage(const GlobalValue *SGV,
+                                           bool DoPromote) {
   // Any local variable that is referenced by an exported function needs
   // to be promoted to global scope. Since we don't currently know which
   // functions reference which local variables/functions, we must treat
   // all as potentially exported if this module is exporting anything.
   if (isModuleExporting()) {
-    if (SGV->hasLocalLinkage() && doPromoteLocalToGlobal(SGV))
+    if (SGV->hasLocalLinkage() && DoPromote)
       return GlobalValue::ExternalLinkage;
     return SGV->getLinkage();
   }
@@ -164,7 +165,7 @@ FunctionImportGlobalProcessing::getLinka
   case GlobalValue::PrivateLinkage:
     // If we are promoting the local to global scope, it is handled
     // similarly to a normal externally visible global.
-    if (doPromoteLocalToGlobal(SGV)) {
+    if (DoPromote) {
       if (doImportAsDefinition(SGV) && !dyn_cast<GlobalAlias>(SGV))
         return GlobalValue::AvailableExternallyLinkage;
       else
@@ -190,14 +191,19 @@ FunctionImportGlobalProcessing::getLinka
 }
 
 void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) {
+  bool DoPromote = false;
   if (GV.hasLocalLinkage() &&
-      (doPromoteLocalToGlobal(&GV) || isPerformingImport())) {
-    GV.setName(getName(&GV));
-    GV.setLinkage(getLinkage(&GV));
+      ((DoPromote = doPromoteLocalToGlobal(&GV)) || isPerformingImport())) {
+    // Once we change the name or linkage it is difficult to determine
+    // again whether we should promote since doPromoteLocalToGlobal 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);
   } else
-    GV.setLinkage(getLinkage(&GV));
+    GV.setLinkage(getLinkage(&GV, /* DoPromote */ false));
 
   // Remove functions imported as available externally defs from comdats,
   // as this is a declaration for the linker, and will be dropped eventually.