[llvm] r326638 - [ThinLTO] Revert r325320: Import global variables
Chandler Carruth via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 2 15:40:08 PST 2018
Author: chandlerc
Date: Fri Mar 2 15:40:08 2018
New Revision: 326638
URL: http://llvm.org/viewvc/llvm-project?rev=326638&view=rev
Log:
[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.
Removed:
llvm/trunk/test/ThinLTO/X86/Inputs/globals-import-cf-baz.ll
llvm/trunk/test/ThinLTO/X86/globals-import-const-fold.ll
Modified:
llvm/trunk/lib/Linker/IRMover.cpp
llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
llvm/trunk/test/Transforms/FunctionImport/funcimport.ll
llvm/trunk/test/Transforms/PGOProfile/thinlto_samplepgo_icp3.ll
Modified: llvm/trunk/lib/Linker/IRMover.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/IRMover.cpp?rev=326638&r1=326637&r2=326638&view=diff
==============================================================================
--- llvm/trunk/lib/Linker/IRMover.cpp (original)
+++ llvm/trunk/lib/Linker/IRMover.cpp Fri Mar 2 15:40:08 2018
@@ -1051,10 +1051,14 @@ void IRLinker::prepareCompileUnitsForImp
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
Modified: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp?rev=326638&r1=326637&r2=326638&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp Fri Mar 2 15:40:08 2018
@@ -61,7 +61,6 @@ using namespace llvm;
#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 @@ updateValueInfoForIndirectCalls(const Mo
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 @@ static void computeImportForFunction(
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 @@ static void ComputeImportForModule(
}
}
-#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 @@ void llvm::ComputeCrossModuleImport(
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 @@ void llvm::ComputeCrossModuleImport(
}
#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 @@ void llvm::ComputeCrossModuleImportForMo
ComputeImportForModule(FunctionSummaryMap, Index, ImportList);
#ifndef NDEBUG
- dumpImportListForModule(Index, ModulePath, ImportList);
+ dumpImportListForModule(ModulePath, ImportList);
#endif
}
@@ -559,7 +492,7 @@ void llvm::ComputeCrossModuleImportForMo
ImportList[Summary->modulePath()][GUID] = 1;
}
#ifndef NDEBUG
- dumpImportListForModule(Index, ModulePath, ImportList);
+ dumpImportListForModule(ModulePath, ImportList);
#endif
}
@@ -837,7 +770,7 @@ Expected<bool> FunctionImporter::importF
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 @@ Expected<bool> FunctionImporter::importF
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 @@ Expected<bool> FunctionImporter::importF
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;
}
Removed: llvm/trunk/test/ThinLTO/X86/Inputs/globals-import-cf-baz.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/Inputs/globals-import-cf-baz.ll?rev=326637&view=auto
==============================================================================
--- llvm/trunk/test/ThinLTO/X86/Inputs/globals-import-cf-baz.ll (original)
+++ llvm/trunk/test/ThinLTO/X86/Inputs/globals-import-cf-baz.ll (removed)
@@ -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"
-
- at baz = local_unnamed_addr constant i32 10, align 4
Removed: llvm/trunk/test/ThinLTO/X86/globals-import-const-fold.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/globals-import-const-fold.ll?rev=326637&view=auto
==============================================================================
--- llvm/trunk/test/ThinLTO/X86/globals-import-const-fold.ll (original)
+++ llvm/trunk/test/ThinLTO/X86/globals-import-const-fold.ll (removed)
@@ -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"
-
- at 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
-}
Modified: llvm/trunk/test/Transforms/FunctionImport/funcimport.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionImport/funcimport.ll?rev=326638&r1=326637&r2=326638&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/FunctionImport/funcimport.ll (original)
+++ llvm/trunk/test/Transforms/FunctionImport/funcimport.ll Fri Mar 2 15:40:08 2018
@@ -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 @@ declare void @callfuncptr(...) #1
; 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 @@ declare void @variadic(...)
; 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 @@ declare void @variadic(...)
; 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]]
Modified: llvm/trunk/test/Transforms/PGOProfile/thinlto_samplepgo_icp3.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/PGOProfile/thinlto_samplepgo_icp3.ll?rev=326638&r1=326637&r2=326638&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/PGOProfile/thinlto_samplepgo_icp3.ll (original)
+++ llvm/trunk/test/Transforms/PGOProfile/thinlto_samplepgo_icp3.ll Fri Mar 2 15:40:08 2018
@@ -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
More information about the llvm-commits
mailing list