[ThinLTO] Stop importing constant global vars as copies in the backend

Summary:
We were doing an optimization in the ThinLTO backends of importing
constant unnamed_addr globals unconditionally as a local copy (regardless
of whether the thin link decided to import them). This should be done in
the thin link instead, so that resulting exported references are marked
and promoted appropriately, but will need a summary enhancement to mark
these variables as constant unnamed_addr.

The function import logic during the thin link was trying to handle
this proactively, by conservatively marking all values referenced in
the initializer lists of exported global variables as also exported.
However, this only handled values referenced directly from the
initializer list of an exported global variable. If the value is itself
a constant unnamed_addr variable, we could end up exporting its
references as well. This caused multiple issues. The first is that the
transitively exported references weren't promoted. Secondly, some could
not be promoted/renamed (e.g. they had a section or other constraint).
recursively, instead of just adding the first level of initializer list
references to the ExportList directly.

Remove this optimization and the associated handling in the function
import backend. SPEC measurements indicate we weren't getting much
from it in any case.

Fixes PR31052.

Reviewers: mehdi_amini

Subscribers: krasin, llvm-commits

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

llvm-svn: 288446

Author: teresajohnson

llvm/lib/Transforms/IPO/FunctionImport.cpp

@@ -268,18 +268,6 @@ static void exportGlobalInModule(const ModuleSummaryIndex &Index,
   auto GVS = dyn_cast<GlobalVarSummary>(Summary);
   if (!GVS)
     return;
-  // FunctionImportGlobalProcessing::doPromoteLocalToGlobal() will always
-  // trigger importing  the initializer for `constant unnamed addr` globals that
-  // are referenced. We conservatively export all the referenced symbols for
-  // every global to workaround this, so that the ExportList is accurate.
-  // FIXME: with a "isConstant" flag in the summary we could be more targetted.
-  for (auto &Ref : GVS->refs()) {
-    auto GUID = Ref.getGUID();
-    auto *RefSummary = FindGlobalSummaryInModule(GUID);
-    if (RefSummary)
-      // Found a ref in the current module, mark it as exported
-      ExportList.insert(GUID);
-  }
 }
 
 using EdgeInfo = std::pair<const FunctionSummary *, unsigned /* Threshold */>;

llvm/lib/Transforms/Utils/FunctionImportUtils.cpp

@@ -56,17 +56,6 @@ bool FunctionImportGlobalProcessing::shouldPromoteLocalToGlobal(
   if (!isPerformingImport() && !isModuleExporting())
     return false;
 
-  // Local const variables never need to be promoted unless they are address
-  // taken. The imported uses can simply use the clone created in this module.
-  // For now we are conservative in determining which variables are not
-  // address taken by checking the unnamed addr flag. To be more aggressive,
-  // the address taken information must be checked earlier during parsing
-  // of the module and recorded in the summary index for use when importing
-  // from that module.
-  auto *GVar = dyn_cast<GlobalVariable>(SGV);
-  if (GVar && GVar->isConstant() && GVar->hasGlobalUnnamedAddr())
-    return false;
-
   // If we are exporting, we need to see whether this value is marked
   // as NoRename in the summary. If we are importing, we may not have
   // a summary in the distributed backend case (only summaries for values

llvm/test/Linker/funcimport.ll

@@ -14,7 +14,8 @@
 ; constant variable need promotion).
 ; RUN: llvm-link %t.bc -summary-index=%t3.thinlto.bc -S | FileCheck %s --check-prefix=EXPORTSTATIC
 ; EXPORTSTATIC-DAG: @staticvar.llvm.{{.*}} = hidden global
-; EXPORTSTATIC-DAG: @staticconstvar = internal unnamed_addr constant
+; Eventually @staticconstvar can be exported as a copy and not promoted
+; EXPORTSTATIC-DAG: @staticconstvar.llvm.0 = hidden unnamed_addr constant
 ; EXPORTSTATIC-DAG: @P.llvm.{{.*}} = hidden global void ()* null
 ; EXPORTSTATIC-DAG: define hidden i32 @staticfunc.llvm.
 ; EXPORTSTATIC-DAG: define hidden void @staticfunc2.llvm.
@@ -68,7 +69,8 @@
 ; promoted and renamed (including static constant variable).
 ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=referencestatics:%t.bc -S | FileCheck %s --check-prefix=IMPORTSTATIC
 ; IMPORTSTATIC-DAG: @staticvar.llvm.{{.*}} = external hidden global
-; IMPORTSTATIC-DAG: @staticconstvar.llvm.{{.*}} = internal unnamed_addr constant
+; Eventually @staticconstvar can be imported as a copy
+; IMPORTSTATIC-DAG: @staticconstvar.llvm.{{.*}} = external hidden unnamed_addr constant
 ; IMPORTSTATIC-DAG: define available_externally i32 @referencestatics
 ; IMPORTSTATIC-DAG: %call = call i32 @staticfunc.llvm.
 ; IMPORTSTATIC-DAG: %0 = load i32, i32* @staticvar.llvm.

llvm/test/ThinLTO/X86/Inputs/reference_non_importable.ll

@@ -0,0 +1,8 @@
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.11.0"
+
+declare i8 **@foo()
+define i32 @main() {
+    call i8 **@foo()
+	ret i32 0
+}

llvm/test/ThinLTO/X86/Inputs/referenced_by_constant.ll

@@ -6,11 +6,17 @@ define void @referencedbyglobal() {
     ret void
 }
 
+define internal void @localreferencedbyglobal() {
+    ret void
+}
+
 @someglobal = internal unnamed_addr constant i8* bitcast (void ()* @referencedbyglobal to i8*)
+@someglobal2 = internal unnamed_addr constant i8* bitcast (void ()* @localreferencedbyglobal to i8*)
 @ptr = global i8** null
+@ptr2 = global i8** null
 
 define  void @bar() #0 align 2 {
   store i8** getelementptr inbounds (i8*, i8** @someglobal, i64 0) , i8*** @ptr, align 8
+  store i8** getelementptr inbounds (i8*, i8** @someglobal2, i64 0) , i8*** @ptr2, align 8
   ret void
 }
-

llvm/test/ThinLTO/X86/funcimport.ll

@@ -10,7 +10,8 @@
 ; constant variable need promotion).
 ; RUN: llvm-lto -thinlto-action=promote %t.bc -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=EXPORTSTATIC
 ; EXPORTSTATIC-DAG: @staticvar.llvm.0 = hidden global
-; EXPORTSTATIC-DAG: @staticconstvar = internal unnamed_addr constant
+; Eventually @staticconstvar can be exported as a copy and not promoted
+; EXPORTSTATIC-DAG: @staticconstvar.llvm.0 = hidden unnamed_addr constant
 ; EXPORTSTATIC-DAG: @P.llvm.0 = hidden global void ()* null
 ; EXPORTSTATIC-DAG: define hidden i32 @staticfunc.llvm.0
 ; EXPORTSTATIC-DAG: define hidden void @staticfunc2.llvm.0

llvm/test/ThinLTO/X86/reference_non_importable.ll

@@ -0,0 +1,28 @@
+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/reference_non_importable.ll -o %t2.bc
+
+; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o -save-temps \
+; RUN:     -r=%t1.bc,_foo,pxl \
+; RUN:     -r=%t1.bc,_b,pxl \
+; RUN:     -r=%t2.bc,_main,pxl \
+; RUN:     -r=%t2.bc,_foo,xl
+
+
+
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.11.0"
+
+; We shouldn't promote the private because it has a section
+; RUN: llvm-dis < %t.o.0.2.internalize.bc | FileCheck  %s --check-prefix=PROMOTE
+; PROMOTE: @a = private global i8 0, section "__TEXT,__cstring,cstring_literals"
+@a = private global i8 0, section "__TEXT,__cstring,cstring_literals"
+@b = global i8 *@a
+
+
+; We want foo to be imported in the main module!
+; RUN: llvm-dis < %t.o.1.3.import.bc  | FileCheck  %s --check-prefix=IMPORT
+; IMPORT: define available_externally i8** @foo()
+define i8 **@foo() {
+	ret i8 **@b
+}

llvm/test/ThinLTO/X86/referenced_by_constant.ll

@@ -3,11 +3,23 @@
 ; RUN: opt -module-summary %p/Inputs/referenced_by_constant.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto-action=thinlink -o %t3.bc %t.bc %t2.bc
 
-; Check the import side: we import bar() and @someglobal, but not @referencedbyglobal()
+; Check the import side: we currently only import bar() (with a future
+; enhancement to identify constants in the summary, we should mark
+; @someglobal/@someglobal2 for import as a local copy, which would
+; cause @referencedbyglobal and @localreferencedbyglobal to be exported
+; and promoted).
 ; RUN: llvm-lto -thinlto-action=import %t.bc -thinlto-index=%t3.bc -o - | llvm-dis -o -   | FileCheck %s --check-prefix=IMPORT
-; IMPORT: @someglobal.llvm.0 =
+; IMPORT: @someglobal.llvm.0 = external hidden unnamed_addr constant
+; IMPORT: @someglobal2.llvm.0 = external hidden unnamed_addr constant
 ; IMPORT: define available_externally void @bar()
-; IMPORT: declare void @referencedbyglobal()
+
+; Check the export side: we currently only export bar(), which causes
+; @someglobal and @someglobal2 to be promoted (see above).
+; RUN: llvm-lto -thinlto-action=promote %t2.bc -thinlto-index=%t3.bc -o - | llvm-dis -o -   | FileCheck %s --check-prefix=EXPORT
+; EXPORT: @someglobal.llvm.0 = hidden unnamed_addr constant
+; EXPORT: @someglobal2.llvm.0 = hidden unnamed_addr constant
+; EXPORT: define void @referencedbyglobal()
+; EXPORT: define internal void @localreferencedbyglobal()
 
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.11.0"
@@ -18,4 +30,3 @@ define void @foo() {
     call void @bar()
     ret void
 }
-