ThinLTO: do not promote GlobalVariable that have a specific section.

Differential Revision: http://reviews.llvm.org/D18298

From: Mehdi Amini <mehdi.amini@apple.com>
llvm-svn: 267646

include/llvm/IR/ModuleSummaryIndex.h

@@ -168,6 +168,10 @@ public:
     return static_cast<GlobalValue::LinkageTypes>(Flags.Linkage);
   }
 
+  /// Return true if this summary is for a GlobalValue that needs promotion
+  /// to be referenced from another module.
+  bool needsRenaming() const { return GlobalValue::isLocalLinkage(linkage()); }
+
   /// Return true if this global value is located in a specific section.
   bool hasSection() const { return Flags.HasSection; }
 

lib/Transforms/IPO/FunctionImport.cpp

@@ -75,6 +75,69 @@ static std::unique_ptr<Module> loadFile(const std::string &FileName,
 
 namespace {
 
+// Return true if the Summary describes a GlobalValue that can be externally
+// referenced, i.e. it does not need renaming (linkage is not local) or renaming
+// is possible (does not have a section for instance).
+static bool canBeExternallyReferenced(const GlobalValueSummary &Summary) {
+  if (!Summary.needsRenaming())
+    return true;
+
+  if (Summary.hasSection())
+    // Can't rename a global that needs renaming if has a section.
+    return false;
+
+  return true;
+}
+
+// Return true if \p GUID describes a GlobalValue that can be externally
+// referenced, i.e. it does not need renaming (linkage is not local) or
+// renaming is possible (does not have a section for instance).
+static bool canBeExternallyReferenced(const ModuleSummaryIndex &Index,
+                                      GlobalValue::GUID GUID) {
+  auto Summaries = Index.findGlobalValueSummaryList(GUID);
+  if (Summaries == Index.end())
+    return true;
+  if (Summaries->second.size() != 1)
+    // If there are multiple globals with this GUID, then we know it is
+    // not a local symbol, and it is necessarily externally referenced.
+    return true;
+
+  // We don't need to check for the module path, because if it can't be
+  // externally referenced and we call it, it is necessarilly in the same
+  // module
+  return canBeExternallyReferenced(**Summaries->second.begin());
+}
+
+// Return true if the global described by \p Summary can be imported in another
+// module.
+static bool eligibleForImport(const ModuleSummaryIndex &Index,
+                              const GlobalValueSummary &Summary) {
+  if (!canBeExternallyReferenced(Summary))
+    // Can't import a global that needs renaming if has a section for instance.
+    // FIXME: we may be able to import it by copying it without promotion.
+    return false;
+
+  // Check references (and potential calls) in the same module. If the current
+  // value references a global that can't be externally referenced it is not
+  // eligible for import.
+  bool AllRefsCanBeExternallyReferenced =
+      llvm::all_of(Summary.refs(), [&](const ValueInfo &VI) {
+        return canBeExternallyReferenced(Index, VI.getGUID());
+      });
+  if (!AllRefsCanBeExternallyReferenced)
+    return false;
+
+  if (auto *FuncSummary = dyn_cast<FunctionSummary>(&Summary)) {
+    bool AllCallsCanBeExternallyReferenced = llvm::all_of(
+        FuncSummary->calls(), [&](const FunctionSummary::EdgeTy &Edge) {
+          return canBeExternallyReferenced(Index, Edge.first.getGUID());
+        });
+    if (!AllCallsCanBeExternallyReferenced)
+      return false;
+  }
+  return true;
+}
+
 /// Given a list of possible callee implementation for a call site, select one
 /// that fits the \p Threshold.
 ///
@@ -86,7 +149,8 @@ namespace {
 /// - One that has PGO data attached.
 /// - [insert you fancy metric here]
 static const GlobalValueSummary *
-selectCallee(const GlobalValueSummaryList &CalleeSummaryList,
+selectCallee(const ModuleSummaryIndex &Index,
+             const GlobalValueSummaryList &CalleeSummaryList,
              unsigned Threshold) {
   auto It = llvm::find_if(
       CalleeSummaryList,
@@ -111,6 +175,9 @@ selectCallee(const GlobalValueSummaryList &CalleeSummaryList,
         if (Summary->instCount() > Threshold)
           return false;
 
+        if (!eligibleForImport(Index, *Summary))
+          return false;
+
         return true;
       });
   if (It == CalleeSummaryList.end())
@@ -125,10 +192,9 @@ static const GlobalValueSummary *selectCallee(GlobalValue::GUID GUID,
                                               unsigned Threshold,
                                               const ModuleSummaryIndex &Index) {
   auto CalleeSummaryList = Index.findGlobalValueSummaryList(GUID);
-  if (CalleeSummaryList == Index.end()) {
+  if (CalleeSummaryList == Index.end())
     return nullptr; // This function does not have a summary
-  }
-  return selectCallee(CalleeSummaryList->second, Threshold);
+  return selectCallee(Index, CalleeSummaryList->second, Threshold);
 }
 
 /// Mark the global \p GUID as export by module \p ExportModulePath if found in

lib/Transforms/Utils/FunctionImportUtils.cpp

@@ -64,6 +64,11 @@ bool FunctionImportGlobalProcessing::doPromoteLocalToGlobal(
   if (GVar && GVar->isConstant() && GVar->hasUnnamedAddr())
     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.
+    return false;
+
   // Eventually we only need to promote functions in the exporting module that
   // are referenced by a potentially exported function (i.e. one that is in the
   // summary index).

test/ThinLTO/X86/Inputs/section.ll

@@ -0,0 +1,13 @@
+; An internal global variable that can't be renamed because it has a section
+@var_with_section = internal global i32 0, section "some_section"
+
+; @reference_gv_with_section() can't be imported
+define i32 @reference_gv_with_section() {
+    %res = load i32, i32* @var_with_section
+    ret i32 %res
+}
+
+; canary
+define void @foo() {
+    ret void
+}

test/ThinLTO/X86/section.ll

@@ -0,0 +1,25 @@
+; Do setup work for all below tests: generate bitcode and combined index
+; RUN: opt -module-summary %s -o %t.bc
+; RUN: opt -module-summary %p/Inputs/section.ll -o %t2.bc
+; RUN: llvm-lto -thinlto-action=thinlink -o %t3.bc %t.bc %t2.bc
+
+; Check that we don't promote 'var_with_section'
+; RUN: llvm-lto -thinlto-action=promote %t2.bc -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=PROMOTE
+; PROMOTE: @var_with_section = internal global i32 0, section "some_section"
+
+; RUN: llvm-lto -thinlto-action=import %t.bc -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=IMPORT
+; Check that section prevent import of @reference_gv_with_section.
+; IMPORT: declare void @reference_gv_with_section()
+; Canary to check that importing is correctly set up.
+; IMPORT: define available_externally void @foo()
+
+
+define i32 @main() {
+    call void @reference_gv_with_section()
+    call void @foo()
+    ret i32 42
+}
+
+
+declare void @reference_gv_with_section()
+declare void @foo()