[ThinLTO] Revert r325320: Import global variables
This caused some links to fail with ThinLTO due to missing symbols as
well as causing some binaries to have failures at runtime. We're working
with the author to get a test case, but want to get the tree green
again.
Further, it appears to introduce a data race. While the test usage of
threads was disabled in r325361 & r325362, that isn't an acceptable fix.
I've reverted both of these as well. This code needs to be thread safe.
Test cases for this are already on the original commit thread.
llvm-svn: 326638
diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index 42081442..ec15bbf 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -1051,10 +1051,14 @@
ValueMap.MD()[CU->getRawEnumTypes()].reset(nullptr);
ValueMap.MD()[CU->getRawMacros()].reset(nullptr);
ValueMap.MD()[CU->getRawRetainedTypes()].reset(nullptr);
- // We import global variables only temporarily in order for instcombine
- // and globalopt to perform constant folding and static constructor
- // evaluation. After that elim-avail-extern will covert imported globals
- // back to declarations, so we don't need debug info for them.
+ // If we ever start importing global variable defs, we'll need to
+ // add their DIGlobalVariable to the globals list on the imported
+ // DICompileUnit. Confirm none are imported, and then we can
+ // map the list of global variables to nullptr.
+ assert(none_of(
+ ValuesToLink,
+ [](const GlobalValue *GV) { return isa<GlobalVariable>(GV); }) &&
+ "Unexpected importing of a GlobalVariable definition");
ValueMap.MD()[CU->getRawGlobalVariables()].reset(nullptr);
// Imported entities only need to be mapped in if they have local
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index aade726..b68058c 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -61,7 +61,6 @@
#define DEBUG_TYPE "function-import"
STATISTIC(NumImportedFunctions, "Number of functions imported");
-STATISTIC(NumImportedGlobalVars, "Number of global variables imported");
STATISTIC(NumImportedModules, "Number of modules imported from");
STATISTIC(NumDeadSymbols, "Number of dead stripped symbols in index");
STATISTIC(NumLiveSymbols, "Number of live symbols in index");
@@ -239,33 +238,6 @@
return Index.getValueInfo(GUID);
}
-static void computeImportForReferencedGlobals(
- const FunctionSummary &Summary, const GVSummaryMapTy &DefinedGVSummaries,
- FunctionImporter::ImportMapTy &ImportList,
- StringMap<FunctionImporter::ExportSetTy> *ExportLists) {
- for (auto &VI : Summary.refs()) {
- if (DefinedGVSummaries.count(VI.getGUID())) {
- DEBUG(dbgs() << "Ref ignored! Target already in destination module.\n");
- continue;
- }
-
- DEBUG(dbgs() << " ref -> " << VI.getGUID() << "\n");
-
- for (auto &RefSummary : VI.getSummaryList())
- if (RefSummary->getSummaryKind() == GlobalValueSummary::GlobalVarKind &&
- // Don't try to import regular LTO summaries added to dummy module.
- !RefSummary->modulePath().empty() &&
- !GlobalValue::isInterposableLinkage(RefSummary->linkage()) &&
- // For now we don't import global variables which have outgoing
- // refs. Otherwise we have to promote referenced vars/functions.
- RefSummary->refs().empty()) {
- ImportList[RefSummary->modulePath()][VI.getGUID()] = 1;
- if (ExportLists)
- (*ExportLists)[RefSummary->modulePath()].insert(VI.getGUID());
- }
- }
-}
-
/// Compute the list of functions to import for a given caller. Mark these
/// imported functions and the symbols they reference in their source module as
/// exported from their source module.
@@ -275,8 +247,6 @@
SmallVectorImpl<EdgeInfo> &Worklist,
FunctionImporter::ImportMapTy &ImportList,
StringMap<FunctionImporter::ExportSetTy> *ExportLists = nullptr) {
- computeImportForReferencedGlobals(Summary, DefinedGVSummaries, ImportList,
- ExportLists);
for (auto &Edge : Summary.calls()) {
ValueInfo VI = Edge.first;
DEBUG(dbgs() << " edge -> " << VI.getGUID() << " Threshold:" << Threshold
@@ -419,34 +389,6 @@
}
}
-#ifndef NDEBUG
-static bool isGlobalVarSummary(const ModuleSummaryIndex &Index,
- GlobalValue::GUID G) {
- if (const auto &VI = Index.getValueInfo(G)) {
- auto SL = VI.getSummaryList();
- if (!SL.empty())
- return SL[0]->getSummaryKind() == GlobalValueSummary::GlobalVarKind;
- }
- return false;
-}
-
-static GlobalValue::GUID getGUID(GlobalValue::GUID G) { return G; }
-
-static GlobalValue::GUID
-getGUID(const std::pair<const GlobalValue::GUID, unsigned> &P) {
- return P.first;
-}
-
-template <class T>
-unsigned numGlobalVarSummaries(const ModuleSummaryIndex &Index, T &Cont) {
- unsigned NumGVS = 0;
- for (auto &V : Cont)
- if (isGlobalVarSummary(Index, getGUID(V)))
- ++NumGVS;
- return NumGVS;
-}
-#endif
-
/// Compute all the import and export for every module using the Index.
void llvm::ComputeCrossModuleImport(
const ModuleSummaryIndex &Index,
@@ -484,17 +426,12 @@
for (auto &ModuleImports : ImportLists) {
auto ModName = ModuleImports.first();
auto &Exports = ExportLists[ModName];
- unsigned NumGVS = numGlobalVarSummaries(Index, Exports);
- DEBUG(dbgs() << "* Module " << ModName << " exports "
- << Exports.size() - NumGVS << " functions and " << NumGVS
- << " vars. Imports from "
- << ModuleImports.second.size() << " modules.\n");
+ DEBUG(dbgs() << "* Module " << ModName << " exports " << Exports.size()
+ << " functions. Imports from " << ModuleImports.second.size()
+ << " modules.\n");
for (auto &Src : ModuleImports.second) {
auto SrcModName = Src.first();
- unsigned NumGVSPerMod = numGlobalVarSummaries(Index, Src.second);
- DEBUG(dbgs() << " - " << Src.second.size() - NumGVSPerMod
- << " functions imported from " << SrcModName << "\n");
- DEBUG(dbgs() << " - " << NumGVSPerMod << " global vars imported from "
+ DEBUG(dbgs() << " - " << Src.second.size() << " functions imported from "
<< SrcModName << "\n");
}
}
@@ -502,17 +439,13 @@
}
#ifndef NDEBUG
-static void dumpImportListForModule(const ModuleSummaryIndex &Index,
- StringRef ModulePath,
+static void dumpImportListForModule(StringRef ModulePath,
FunctionImporter::ImportMapTy &ImportList) {
DEBUG(dbgs() << "* Module " << ModulePath << " imports from "
<< ImportList.size() << " modules.\n");
for (auto &Src : ImportList) {
auto SrcModName = Src.first();
- unsigned NumGVSPerMod = numGlobalVarSummaries(Index, Src.second);
- DEBUG(dbgs() << " - " << Src.second.size() - NumGVSPerMod
- << " functions imported from " << SrcModName << "\n");
- DEBUG(dbgs() << " - " << NumGVSPerMod << " vars imported from "
+ DEBUG(dbgs() << " - " << Src.second.size() << " functions imported from "
<< SrcModName << "\n");
}
}
@@ -532,7 +465,7 @@
ComputeImportForModule(FunctionSummaryMap, Index, ImportList);
#ifndef NDEBUG
- dumpImportListForModule(Index, ModulePath, ImportList);
+ dumpImportListForModule(ModulePath, ImportList);
#endif
}
@@ -559,7 +492,7 @@
ImportList[Summary->modulePath()][GUID] = 1;
}
#ifndef NDEBUG
- dumpImportListForModule(Index, ModulePath, ImportList);
+ dumpImportListForModule(ModulePath, ImportList);
#endif
}
@@ -837,7 +770,7 @@
Module &DestModule, const FunctionImporter::ImportMapTy &ImportList) {
DEBUG(dbgs() << "Starting import for Module "
<< DestModule.getModuleIdentifier() << "\n");
- unsigned ImportedCount = 0, ImportedGVCount = 0;
+ unsigned ImportedCount = 0;
IRMover Mover(DestModule);
// Do the actual import of functions now, one Module at a time
@@ -897,7 +830,7 @@
if (Import) {
if (Error Err = GV.materialize())
return std::move(Err);
- ImportedGVCount += GlobalsToImport.insert(&GV);
+ GlobalsToImport.insert(&GV);
}
}
for (GlobalAlias &GA : SrcModule->aliases()) {
@@ -954,14 +887,9 @@
NumImportedModules++;
}
- NumImportedFunctions += (ImportedCount - ImportedGVCount);
- NumImportedGlobalVars += ImportedGVCount;
+ NumImportedFunctions += ImportedCount;
- DEBUG(dbgs() << "Imported " << ImportedCount - ImportedGVCount
- << " functions for Module " << DestModule.getModuleIdentifier()
- << "\n");
- DEBUG(dbgs() << "Imported " << ImportedGVCount
- << " global variables for Module "
+ DEBUG(dbgs() << "Imported " << ImportedCount << " functions for Module "
<< DestModule.getModuleIdentifier() << "\n");
return ImportedCount;
}
diff --git a/llvm/test/ThinLTO/X86/Inputs/globals-import-cf-baz.ll b/llvm/test/ThinLTO/X86/Inputs/globals-import-cf-baz.ll
deleted file mode 100644
index e411630..0000000
--- a/llvm/test/ThinLTO/X86/Inputs/globals-import-cf-baz.ll
+++ /dev/null
@@ -1,4 +0,0 @@
-target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-pc-linux-gnu"
-
-@baz = local_unnamed_addr constant i32 10, align 4
diff --git a/llvm/test/ThinLTO/X86/globals-import-const-fold.ll b/llvm/test/ThinLTO/X86/globals-import-const-fold.ll
deleted file mode 100644
index 49e31b7..0000000
--- a/llvm/test/ThinLTO/X86/globals-import-const-fold.ll
+++ /dev/null
@@ -1,23 +0,0 @@
-; RUN: opt -module-summary %s -o %t1.bc
-; RUN: opt -module-summary %p/Inputs/globals-import-cf-baz.ll -o %t2.bc
-; RUN: llvm-lto -thinlto-action=thinlink %t1.bc %t2.bc -o %t3.index.bc
-
-; RUN: llvm-lto -thinlto-action=import %t1.bc %t2.bc -thinlto-index=%t3.index.bc
-; RUN: llvm-dis %t1.bc.thinlto.imported.bc -o - | FileCheck --check-prefix=IMPORT %s
-; RUN: llvm-lto -thinlto-action=optimize %t1.bc.thinlto.imported.bc -o %t1.bc.thinlto.opt.bc
-; RUN: llvm-dis %t1.bc.thinlto.opt.bc -o - | FileCheck --check-prefix=OPTIMIZE %s
-
-; IMPORT: @baz = available_externally local_unnamed_addr constant i32 10
-
-; OPTIMIZE: define i32 @main()
-; OPTIMIZE-NEXT: ret i32 10
-
-target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-pc-linux-gnu"
-
-@baz = external local_unnamed_addr constant i32, align 4
-
-define i32 @main() local_unnamed_addr {
- %1 = load i32, i32* @baz, align 4
- ret i32 %1
-}
diff --git a/llvm/test/Transforms/FunctionImport/funcimport.ll b/llvm/test/Transforms/FunctionImport/funcimport.ll
index bb974b5..4ff51a3 100644
--- a/llvm/test/Transforms/FunctionImport/funcimport.ll
+++ b/llvm/test/Transforms/FunctionImport/funcimport.ll
@@ -7,8 +7,7 @@
; RUN: opt -function-import -stats -print-imports -enable-import-metadata -summary-file %t3.thinlto.bc %t.bc -S 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=INSTLIMDEF
; Try again with new pass manager
; RUN: opt -passes='function-import' -stats -print-imports -enable-import-metadata -summary-file %t3.thinlto.bc %t.bc -S 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=INSTLIMDEF
-; RUN: opt -passes='function-import' -debug-only=function-import -enable-import-metadata -summary-file %t3.thinlto.bc %t.bc -S 2>&1 | FileCheck %s --check-prefix=DUMP
-; "-stats" and "-debug-only" require +Asserts.
+; "-stats" requires +Asserts.
; REQUIRES: asserts
; Test import with smaller instruction limit
@@ -81,7 +80,7 @@
; Ensure that all uses of local variable @P which has used in setfuncptr
; and callfuncptr are to the same promoted/renamed global.
-; CHECK-DAG: @P.llvm.{{.*}} = available_externally hidden global void ()* null
+; CHECK-DAG: @P.llvm.{{.*}} = external hidden global void ()*
; CHECK-DAG: %0 = load void ()*, void ()** @P.llvm.
; CHECK-DAG: store void ()* @staticfunc2.llvm.{{.*}}, void ()** @P.llvm.
@@ -108,8 +107,6 @@
; INSTLIMDEF-DAG: Import globalfunc2
; INSTLIMDEF-DAG: 13 function-import - Number of functions imported
-; INSTLIMDEF-DAG: 4 function-import - Number of global variables imported
-
; CHECK-DAG: !0 = !{!"{{.*}}/Inputs/funcimport.ll"}
; The actual GUID values will depend on path to test.
@@ -145,9 +142,3 @@
; GUID-DAG: GUID {{.*}} is linkoncealias
; GUID-DAG: GUID {{.*}} is callfuncptr
; GUID-DAG: GUID {{.*}} is linkoncefunc
-
-; DUMP: Module [[M1:.*]] imports from 1 module
-; DUMP-NEXT: 13 functions imported from [[M2:.*]]
-; DUMP-NEXT: 4 vars imported from [[M2]]
-; DUMP: Imported 13 functions for Module [[M1]]
-; DUMP-NEXT: Imported 4 global variables for Module [[M1]]
diff --git a/llvm/test/Transforms/PGOProfile/thinlto_samplepgo_icp3.ll b/llvm/test/Transforms/PGOProfile/thinlto_samplepgo_icp3.ll
index 3044964..d6a94ca 100644
--- a/llvm/test/Transforms/PGOProfile/thinlto_samplepgo_icp3.ll
+++ b/llvm/test/Transforms/PGOProfile/thinlto_samplepgo_icp3.ll
@@ -6,7 +6,7 @@
; Test to make sure importing and dead stripping works in the
; case where the target is a local function that also indirectly calls itself.
-; RUN: llvm-lto2 run -thinlto-threads=1 -save-temps -o %t3 %t.bc %t2.bc -r %t.bc,fptr,plx -r %t.bc,main,plx -r %t2.bc,_Z6updatei,pl -r %t2.bc,fptr,l -print-imports 2>&1 | FileCheck %s --check-prefix=IMPORTS
+; RUN: llvm-lto2 run -save-temps -o %t3 %t.bc %t2.bc -r %t.bc,fptr,plx -r %t.bc,main,plx -r %t2.bc,_Z6updatei,pl -r %t2.bc,fptr,l -print-imports 2>&1 | FileCheck %s --check-prefix=IMPORTS
; Make sure we import the promted indirectly called target
; IMPORTS: Import _ZL3foov.llvm.0