[llvm] r325320 - [ThinLTO] Import global variables
David Green via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 16 08:04:03 PST 2018
Hello Eugene,
After this patch, about 1 time in 20 I'm seeing failures in Transforms/PGOProfile/thinlto_samplepgo_icp3.ll. When it passes it prints something like:
/work/llvm-git/test/Transforms/PGOProfile/Inputs/thinlto_samplepgo_icp3.ll: Import fptr from /work/llvm-git/test/Transforms/PGOProfile/thinlto_samplepgo_icp3.ll
/work/llvm-git/test/Transforms/PGOProfile/thinlto_samplepgo_icp3.ll: Import _ZL3foov.llvm.0 from /work/llvm-git/test/Transforms/PGOProfile/Inputs/thinlto_samplepgo_icp3.ll
When it fails it prints:
/work/llvm-git/test/Transforms/PGOProfile/Inputs/thinlto_samplepgo_icp3.ll/work/llvm-git/test/Transforms/PGOProfile/thinlto_samplepgo_icp3.ll: Import : Import fptr_ZL3foov.llvm.0 from /work/llvm-git/test/Transforms/PGOProfile/thinlto_samplepgo_icp3.ll from
/work/llvm-git/test/Transforms/PGOProfile/Inputs/thinlto_samplepgo_icp3.ll
I cant reproduce before this patch, but don't see anything very thread-y in here that may be causing this. Perhaps some knock-on affect? Perhaps it's printing more output now?
Any ideas?
Thanks,
Dave
From: llvm-commits <llvm-commits-bounces at lists.llvm.org> on behalf of Eugene Leviant via llvm-commits <llvm-commits at lists.llvm.org>
Sent: 16 February 2018 08:11
To: llvm-commits at lists.llvm.org
Subject: [llvm] r325320 - [ThinLTO] Import global variables
Author: evgeny777
Date: Fri Feb 16 00:11:04 2018
New Revision: 325320
URL: http://llvm.org/viewvc/llvm-project?rev=325320&view=rev
Log:
[ThinLTO] Import global variables
Differential revision: https://reviews.llvm.org/D43077
Added:
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
Modified: llvm/trunk/lib/Linker/IRMover.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/IRMover.cpp?rev=325320&r1=325319&r2=325320&view=diff
==============================================================================
--- llvm/trunk/lib/Linker/IRMover.cpp (original)
+++ llvm/trunk/lib/Linker/IRMover.cpp Fri Feb 16 00:11:04 2018
@@ -1051,14 +1051,10 @@ void IRLinker::prepareCompileUnitsForImp
ValueMap.MD()[CU->getRawEnumTypes()].reset(nullptr);
ValueMap.MD()[CU->getRawMacros()].reset(nullptr);
ValueMap.MD()[CU->getRawRetainedTypes()].reset(nullptr);
- // 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");
+ // 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.
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=325320&r1=325319&r2=325320&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp Fri Feb 16 00:11:04 2018
@@ -61,6 +61,7 @@ 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");
@@ -238,6 +239,33 @@ 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.
@@ -247,6 +275,8 @@ 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
@@ -389,6 +419,34 @@ 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,
@@ -426,12 +484,17 @@ void llvm::ComputeCrossModuleImport(
for (auto &ModuleImports : ImportLists) {
auto ModName = ModuleImports.first();
auto &Exports = ExportLists[ModName];
- DEBUG(dbgs() << "* Module " << ModName << " exports " << Exports.size()
- << " functions. Imports from " << ModuleImports.second.size()
- << " modules.\n");
+ unsigned NumGVS = numGlobalVarSummaries(Index, Exports);
+ DEBUG(dbgs() << "* Module " << ModName << " exports "
+ << Exports.size() - NumGVS << " functions and " << NumGVS
+ << " vars. Imports from "
+ << ModuleImports.second.size() << " modules.\n");
for (auto &Src : ModuleImports.second) {
auto SrcModName = Src.first();
- DEBUG(dbgs() << " - " << Src.second.size() << " functions imported from "
+ unsigned NumGVSPerMod = numGlobalVarSummaries(Index, Src.second);
+ DEBUG(dbgs() << " - " << Src.second.size() - NumGVSPerMod
+ << " functions imported from " << SrcModName << "\n");
+ DEBUG(dbgs() << " - " << NumGVSPerMod << " global vars imported from "
<< SrcModName << "\n");
}
}
@@ -439,13 +502,17 @@ void llvm::ComputeCrossModuleImport(
}
#ifndef NDEBUG
-static void dumpImportListForModule(StringRef ModulePath,
+static void dumpImportListForModule(const ModuleSummaryIndex &Index,
+ StringRef ModulePath,
FunctionImporter::ImportMapTy &ImportList) {
DEBUG(dbgs() << "* Module " << ModulePath << " imports from "
<< ImportList.size() << " modules.\n");
for (auto &Src : ImportList) {
auto SrcModName = Src.first();
- DEBUG(dbgs() << " - " << Src.second.size() << " functions imported from "
+ unsigned NumGVSPerMod = numGlobalVarSummaries(Index, Src.second);
+ DEBUG(dbgs() << " - " << Src.second.size() - NumGVSPerMod
+ << " functions imported from " << SrcModName << "\n");
+ DEBUG(dbgs() << " - " << NumGVSPerMod << " vars imported from "
<< SrcModName << "\n");
}
}
@@ -465,7 +532,7 @@ void llvm::ComputeCrossModuleImportForMo
ComputeImportForModule(FunctionSummaryMap, Index, ImportList);
#ifndef NDEBUG
- dumpImportListForModule(ModulePath, ImportList);
+ dumpImportListForModule(Index, ModulePath, ImportList);
#endif
}
@@ -492,7 +559,7 @@ void llvm::ComputeCrossModuleImportForMo
ImportList[Summary->modulePath()][GUID] = 1;
}
#ifndef NDEBUG
- dumpImportListForModule(ModulePath, ImportList);
+ dumpImportListForModule(Index, ModulePath, ImportList);
#endif
}
@@ -770,7 +837,7 @@ Expected<bool> FunctionImporter::importF
Module &DestModule, const FunctionImporter::ImportMapTy &ImportList) {
DEBUG(dbgs() << "Starting import for Module "
<< DestModule.getModuleIdentifier() << "\n");
- unsigned ImportedCount = 0;
+ unsigned ImportedCount = 0, ImportedGVCount = 0;
IRMover Mover(DestModule);
// Do the actual import of functions now, one Module at a time
@@ -830,7 +897,7 @@ Expected<bool> FunctionImporter::importF
if (Import) {
if (Error Err = GV.materialize())
return std::move(Err);
- GlobalsToImport.insert(&GV);
+ ImportedGVCount += GlobalsToImport.insert(&GV);
}
}
for (GlobalAlias &GA : SrcModule->aliases()) {
@@ -887,9 +954,14 @@ Expected<bool> FunctionImporter::importF
NumImportedModules++;
}
- NumImportedFunctions += ImportedCount;
+ NumImportedFunctions += (ImportedCount - ImportedGVCount);
+ NumImportedGlobalVars += ImportedGVCount;
- DEBUG(dbgs() << "Imported " << ImportedCount << " functions for Module "
+ DEBUG(dbgs() << "Imported " << ImportedCount - ImportedGVCount
+ << " functions for Module " << DestModule.getModuleIdentifier()
+ << "\n");
+ DEBUG(dbgs() << "Imported " << ImportedGVCount
+ << " global variables for Module "
<< DestModule.getModuleIdentifier() << "\n");
return ImportedCount;
}
Added: 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=325320&view=auto
==============================================================================
--- llvm/trunk/test/ThinLTO/X86/Inputs/globals-import-cf-baz.ll (added)
+++ llvm/trunk/test/ThinLTO/X86/Inputs/globals-import-cf-baz.ll Fri Feb 16 00:11:04 2018
@@ -0,0 +1,4 @@
+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
Added: 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=325320&view=auto
==============================================================================
--- llvm/trunk/test/ThinLTO/X86/globals-import-const-fold.ll (added)
+++ llvm/trunk/test/ThinLTO/X86/globals-import-const-fold.ll Fri Feb 16 00:11:04 2018
@@ -0,0 +1,23 @@
+; 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=325320&r1=325319&r2=325320&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/FunctionImport/funcimport.ll (original)
+++ llvm/trunk/test/Transforms/FunctionImport/funcimport.ll Fri Feb 16 00:11:04 2018
@@ -7,7 +7,8 @@
; 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
-; "-stats" requires +Asserts.
+; 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.
; REQUIRES: asserts
; Test import with smaller instruction limit
@@ -80,7 +81,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.{{.*}} = external hidden global void ()*
+; CHECK-DAG: @P.llvm.{{.*}} = available_externally hidden global void ()* null
; CHECK-DAG: %0 = load void ()*, void ()** @P.llvm.
; CHECK-DAG: store void ()* @staticfunc2.llvm.{{.*}}, void ()** @P.llvm.
@@ -107,6 +108,8 @@ 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.
@@ -142,3 +145,9 @@ 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]]
_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list