[llvm] dde5893 - [ThinLTO] Import readonly vars with refs
via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 7 04:21:10 PST 2019
Author: evgeny
Date: 2019-11-07T15:13:35+03:00
New Revision: dde589389fcb8b5098f7a47f1b781b27d29a0cac
URL: https://github.com/llvm/llvm-project/commit/dde589389fcb8b5098f7a47f1b781b27d29a0cac
DIFF: https://github.com/llvm/llvm-project/commit/dde589389fcb8b5098f7a47f1b781b27d29a0cac.diff
LOG: [ThinLTO] Import readonly vars with refs
Patch allows importing declarations of functions and variables, referenced
by the initializer of some other readonly variable.
Differential revision: https://reviews.llvm.org/D69561
Added:
Modified:
llvm/include/llvm/IR/ModuleSummaryIndex.h
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/IR/ModuleSummaryIndex.cpp
llvm/lib/Transforms/IPO/FunctionImport.cpp
llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
llvm/test/Bitcode/summary_version.ll
llvm/test/Bitcode/thinlto-deadstrip-flag.ll
llvm/test/Bitcode/thinlto-synthetic-count-flag.ll
llvm/test/ThinLTO/X86/globals-import.ll
llvm/test/ThinLTO/X86/local_name_conflict.ll
llvm/test/tools/gold/X86/v1.12/thinlto_emit_linked_objects.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index be60447abd87..ae611b774478 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -941,6 +941,11 @@ class ModuleSummaryIndex {
/// considered live.
bool WithGlobalValueDeadStripping = false;
+ /// Indicates that summary-based attribute propagation has run and
+ /// GVarFlags::MaybeReadonly / GVarFlags::MaybeWriteonly are really
+ /// read/write only.
+ bool WithAttributePropagation = false;
+
/// Indicates that summary-based synthetic entry count propagation has run
bool HasSyntheticEntryCounts = false;
@@ -1065,6 +1070,18 @@ class ModuleSummaryIndex {
WithGlobalValueDeadStripping = true;
}
+ bool withAttributePropagation() const { return WithAttributePropagation; }
+ void setWithAttributePropagation() {
+ WithAttributePropagation = true;
+ }
+
+ bool isReadOnly(const GlobalVarSummary *GVS) const {
+ return WithAttributePropagation && GVS->maybeReadOnly();
+ }
+ bool isWriteOnly(const GlobalVarSummary *GVS) const {
+ return WithAttributePropagation && GVS->maybeWriteOnly();
+ }
+
bool hasSyntheticEntryCounts() const { return HasSyntheticEntryCounts; }
void setHasSyntheticEntryCounts() { HasSyntheticEntryCounts = true; }
@@ -1356,6 +1373,9 @@ class ModuleSummaryIndex {
/// Analyze index and detect unmodified globals
void propagateAttributes(const DenseSet<GlobalValue::GUID> &PreservedSymbols);
+
+ /// Checks if we can import global variable from another module.
+ bool canImportGlobalVar(GlobalValueSummary *S, bool AnalyzeRefs) const;
};
/// GraphTraits definition to build SCC for the index
@@ -1427,15 +1447,6 @@ struct GraphTraits<ModuleSummaryIndex *> : public GraphTraits<ValueInfo> {
return ValueInfo(I->haveGVs(), &P);
}
};
-
-static inline bool canImportGlobalVar(GlobalValueSummary *S) {
- assert(isa<GlobalVarSummary>(S->getBaseObject()));
-
- // We don't import GV with references, because it can result
- // in promotion of local variables in the source module.
- return !GlobalValue::isInterposableLinkage(S->linkage()) &&
- !S->notEligibleToImport() && S->refs().empty();
-}
} // end namespace llvm
#endif // LLVM_IR_MODULESUMMARYINDEX_H
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 71166ba337e6..b65c088ac92f 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -5761,7 +5761,7 @@ Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
}
const uint64_t Version = Record[0];
const bool IsOldProfileFormat = Version == 1;
- if (Version < 1 || Version > 7)
+ if (Version < 1 || Version > 8)
return error("Invalid summary version " + Twine(Version) +
". Version should be in the range [1-7].");
Record.clear();
@@ -5814,7 +5814,7 @@ Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
case bitc::FS_FLAGS: { // [flags]
uint64_t Flags = Record[0];
// Scan flags.
- assert(Flags <= 0x1f && "Unexpected bits in flag");
+ assert(Flags <= 0x3f && "Unexpected bits in flag");
// 1 bit: WithGlobalValueDeadStripping flag.
// Set on combined index only.
@@ -5837,6 +5837,10 @@ Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
// Set on combined index only.
if (Flags & 0x10)
TheIndex.setPartiallySplitLTOUnits();
+ // 1 bit: WithAttributePropagation flag.
+ // Set on combined index only.
+ if (Flags & 0x20)
+ TheIndex.setWithAttributePropagation();
break;
}
case bitc::FS_VALUE_GUID: { // [valueid, refguid]
@@ -6542,7 +6546,7 @@ static Expected<bool> getEnableSplitLTOUnitFlag(BitstreamCursor &Stream,
case bitc::FS_FLAGS: { // [flags]
uint64_t Flags = Record[0];
// Scan flags.
- assert(Flags <= 0x1f && "Unexpected bits in flag");
+ assert(Flags <= 0x3f && "Unexpected bits in flag");
return Flags & 0x8;
}
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 86d20e920fe2..97f5bc9ee45d 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -3723,7 +3723,7 @@ void ModuleBitcodeWriterBase::writeModuleLevelReferences(
// Current version for the summary.
// This is bumped whenever we introduce changes in the way some record are
// interpreted, like flags for instance.
-static const uint64_t INDEX_VERSION = 7;
+static const uint64_t INDEX_VERSION = 8;
/// Emit the per-module summary section alongside the rest of
/// the module's bitcode.
@@ -3899,6 +3899,8 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
Flags |= 0x8;
if (Index.partiallySplitLTOUnits())
Flags |= 0x10;
+ if (Index.withAttributePropagation())
+ Flags |= 0x20;
Stream.EmitRecord(bitc::FS_FLAGS, ArrayRef<uint64_t>{Flags});
for (const auto &GVI : valueIds()) {
diff --git a/llvm/lib/IR/ModuleSummaryIndex.cpp b/llvm/lib/IR/ModuleSummaryIndex.cpp
index 9f347d8da01d..d82d2e9cb733 100644
--- a/llvm/lib/IR/ModuleSummaryIndex.cpp
+++ b/llvm/lib/IR/ModuleSummaryIndex.cpp
@@ -172,8 +172,9 @@ void ModuleSummaryIndex::propagateAttributes(
// assembly leading it to be in the @llvm.*used).
if (auto *GVS = dyn_cast<GlobalVarSummary>(S->getBaseObject()))
// Here we intentionally pass S.get() not GVS, because S could be
- // an alias.
- if (!canImportGlobalVar(S.get()) ||
+ // an alias. We don't analyze references here, because we have to
+ // know exactly if GV is readonly to do so.
+ if (!canImportGlobalVar(S.get(), /* AnalyzeRefs */ false) ||
GUIDPreservedSymbols.count(P.first)) {
GVS->setReadOnly(false);
GVS->setWriteOnly(false);
@@ -193,6 +194,23 @@ void ModuleSummaryIndex::propagateAttributes(
}
}
+bool ModuleSummaryIndex::canImportGlobalVar(GlobalValueSummary *S,
+ bool AnalyzeRefs) const {
+ auto HasRefsPreventingImport = [this](const GlobalVarSummary *GVS) {
+ return !isReadOnly(GVS) && GVS->refs().size();
+ };
+ auto *GVS = cast<GlobalVarSummary>(S->getBaseObject());
+
+ // Global variable with non-trivial initializer can be imported
+ // if it's readonly. This gives us extra opportunities for constant
+ // folding and converting indirect calls to direct calls. We don't
+ // analyze GV references during attribute propagation, because we
+ // don't know yet if it is readonly or not.
+ return !GlobalValue::isInterposableLinkage(S->linkage()) &&
+ !S->notEligibleToImport() &&
+ (!AnalyzeRefs || !HasRefsPreventingImport(GVS));
+}
+
// TODO: write a graphviz dumper for SCCs (see ModuleSummaryIndex::exportToDot)
// then delete this function and update its tests
LLVM_DUMP_METHOD
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index 3f5cc078d75f..afc31bff3a60 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -280,7 +280,8 @@ updateValueInfoForIndirectCalls(const ModuleSummaryIndex &Index, ValueInfo VI) {
}
static void computeImportForReferencedGlobals(
- const FunctionSummary &Summary, const GVSummaryMapTy &DefinedGVSummaries,
+ const FunctionSummary &Summary, const ModuleSummaryIndex &Index,
+ const GVSummaryMapTy &DefinedGVSummaries,
FunctionImporter::ImportMapTy &ImportList,
StringMap<FunctionImporter::ExportSetTy> *ExportLists) {
for (auto &VI : Summary.refs()) {
@@ -303,16 +304,24 @@ static void computeImportForReferencedGlobals(
RefSummary->modulePath() != Summary.modulePath();
};
+ auto MarkExported = [&](const ValueInfo &VI, const GlobalValueSummary *S) {
+ if (ExportLists)
+ (*ExportLists)[S->modulePath()].insert(VI.getGUID());
+ };
+
for (auto &RefSummary : VI.getSummaryList())
if (isa<GlobalVarSummary>(RefSummary.get()) &&
- canImportGlobalVar(RefSummary.get()) &&
+ Index.canImportGlobalVar(RefSummary.get(), /* AnalyzeRefs */ true) &&
!LocalNotInModule(RefSummary.get())) {
auto ILI = ImportList[RefSummary->modulePath()].insert(VI.getGUID());
// Only update stat if we haven't already imported this variable.
if (ILI.second)
NumImportedGlobalVarsThinLink++;
- if (ExportLists)
- (*ExportLists)[RefSummary->modulePath()].insert(VI.getGUID());
+ MarkExported(VI, RefSummary.get());
+ // Promote referenced functions and variables
+ for (const auto &VI : RefSummary->refs())
+ for (const auto &RefFn : VI.getSummaryList())
+ MarkExported(VI, RefFn.get());
break;
}
}
@@ -351,8 +360,8 @@ static void computeImportForFunction(
FunctionImporter::ImportMapTy &ImportList,
StringMap<FunctionImporter::ExportSetTy> *ExportLists,
FunctionImporter::ImportThresholdsTy &ImportThresholds) {
- computeImportForReferencedGlobals(Summary, DefinedGVSummaries, ImportList,
- ExportLists);
+ computeImportForReferencedGlobals(Summary, Index, DefinedGVSummaries,
+ ImportList, ExportLists);
static int ImportCount = 0;
for (auto &Edge : Summary.calls()) {
ValueInfo VI = Edge.first;
@@ -864,6 +873,7 @@ void llvm::computeDeadSymbolsWithConstProp(
GVS->setWriteOnly(false);
}
}
+ Index.setWithAttributePropagation();
}
/// Compute the set of summaries needed for a ThinLTO backend compilation of
diff --git a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
index 5b68efbb6d8f..dc9859156c40 100644
--- a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
+++ b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
@@ -238,7 +238,7 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) {
// If global value dead stripping is not enabled in summary then
// propagateConstants hasn't been run. We can't internalize GV
// in such case.
- if (!GV.isDeclaration() && VI && ImportIndex.withGlobalValueDeadStripping()) {
+ if (!GV.isDeclaration() && VI && ImportIndex.withAttributePropagation()) {
if (GlobalVariable *V = dyn_cast<GlobalVariable>(&GV)) {
// We can have more than one local with the same GUID, in the case of
// same-named locals in
diff erent but same-named source files that were
@@ -252,7 +252,7 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) {
auto* GVS = dyn_cast_or_null<GlobalVarSummary>(
ImportIndex.findSummaryInModule(VI, M.getModuleIdentifier()));
// At this stage "maybe" is "definitely"
- if (GVS && (GVS->maybeReadOnly() || GVS->maybeWriteOnly()))
+ if (GVS && (ImportIndex.isReadOnly(GVS) || ImportIndex.isWriteOnly(GVS)))
V->addAttribute("thinlto-internalize");
}
}
diff --git a/llvm/test/Bitcode/summary_version.ll b/llvm/test/Bitcode/summary_version.ll
index e531a078d220..2a67073713c0 100644
--- a/llvm/test/Bitcode/summary_version.ll
+++ b/llvm/test/Bitcode/summary_version.ll
@@ -2,7 +2,7 @@
; RUN: opt -module-summary %s -o - | llvm-bcanalyzer -dump | FileCheck %s
; CHECK: <GLOBALVAL_SUMMARY_BLOCK
-; CHECK: <VERSION op0=7/>
+; CHECK: <VERSION op0=8/>
diff --git a/llvm/test/Bitcode/thinlto-deadstrip-flag.ll b/llvm/test/Bitcode/thinlto-deadstrip-flag.ll
index 5330a25dbf15..acde6e943d42 100644
--- a/llvm/test/Bitcode/thinlto-deadstrip-flag.ll
+++ b/llvm/test/Bitcode/thinlto-deadstrip-flag.ll
@@ -5,14 +5,14 @@
; RUN: llvm-lto2 run %t.o -o %t.out -thinlto-distributed-indexes \
; RUN: -r %t.o,glob,plx
; RUN: llvm-bcanalyzer -dump %t.o.thinlto.bc | FileCheck %s --check-prefix=WITHDEAD
-; WITHDEAD: <FLAGS op0=1/>
+; WITHDEAD: <FLAGS op0=33/>
; Ensure dead stripping performed flag is not set on distributed index
; when option used to disable dead stripping computation.
; RUN: llvm-lto2 run %t.o -o %t.out -thinlto-distributed-indexes \
; RUN: -r %t.o,glob,plx -compute-dead=false
; RUN: llvm-bcanalyzer -dump %t.o.thinlto.bc | FileCheck %s --check-prefix=NODEAD
-; NODEAD: <FLAGS op0=0/>
+; NODEAD: <FLAGS op0=32/>
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
diff --git a/llvm/test/Bitcode/thinlto-synthetic-count-flag.ll b/llvm/test/Bitcode/thinlto-synthetic-count-flag.ll
index eb18a025b94a..2174335f7bcb 100644
--- a/llvm/test/Bitcode/thinlto-synthetic-count-flag.ll
+++ b/llvm/test/Bitcode/thinlto-synthetic-count-flag.ll
@@ -5,7 +5,7 @@
; RUN: llvm-lto2 run %t.o -o %t.out -thinlto-distributed-indexes \
; RUN: -r %t.o,glob,plx -compute-dead=false
; RUN: llvm-bcanalyzer -dump %t.o.thinlto.bc | FileCheck %s --check-prefix=NOSYNTHETIC
-; NOSYNTHETIC: <FLAGS op0=0/>
+; NOSYNTHETIC: <FLAGS op0=32/>
; Ensure synthetic entry count flag is set on distributed index
; when option used to enable synthetic count propagation
@@ -13,7 +13,7 @@
; RUN: -r %t.o,glob,plx -thinlto-synthesize-entry-counts \
; RUN: -compute-dead=false
; RUN: llvm-bcanalyzer -dump %t.o.thinlto.bc | FileCheck %s --check-prefix=HASSYNTHETIC
-; HASSYNTHETIC: <FLAGS op0=4/>
+; HASSYNTHETIC: <FLAGS op0=36/>
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
diff --git a/llvm/test/ThinLTO/X86/globals-import.ll b/llvm/test/ThinLTO/X86/globals-import.ll
index 0837cafd06f8..d784398f792b 100644
--- a/llvm/test/ThinLTO/X86/globals-import.ll
+++ b/llvm/test/ThinLTO/X86/globals-import.ll
@@ -15,7 +15,7 @@
; RUN: llvm-dis %t2.bc.thinlto.promoted.bc -o - | FileCheck --check-prefix=PROMOTE1 %s
; RUN: llvm-dis %t2b.bc.thinlto.promoted.bc -o - | FileCheck --check-prefix=PROMOTE2 %s
-; IMPORT: @baz.llvm.0 = available_externally hidden constant i32 10, align 4
+; IMPORT: @baz.llvm.0 = internal constant i32 10, align 4
; PROMOTE1: @baz.llvm.0 = hidden constant i32 10, align 4
; PROMOTE1: define weak_odr i32 @foo() {
diff --git a/llvm/test/ThinLTO/X86/local_name_conflict.ll b/llvm/test/ThinLTO/X86/local_name_conflict.ll
index 9e5e79b6ae2b..d5497a469433 100644
--- a/llvm/test/ThinLTO/X86/local_name_conflict.ll
+++ b/llvm/test/ThinLTO/X86/local_name_conflict.ll
@@ -12,7 +12,7 @@
; that module (%t3.bc) to be imported. Check that the imported reference's
; promoted name matches the imported copy.
; RUN: llvm-lto -thinlto-action=import %t.bc -thinlto-index=%t4.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=IMPORT
-; IMPORT: @baz.llvm.[[HASH:[0-9]+]] = available_externally hidden constant i32 10, align 4
+; IMPORT: @baz.llvm.[[HASH:[0-9]+]] = internal constant i32 10, align 4
; IMPORT: call i32 @foo.llvm.[[HASH]]
; IMPORT: define available_externally hidden i32 @foo.llvm.[[HASH]]()
diff --git a/llvm/test/tools/gold/X86/v1.12/thinlto_emit_linked_objects.ll b/llvm/test/tools/gold/X86/v1.12/thinlto_emit_linked_objects.ll
index b110eb0de2e3..1d585762aa04 100644
--- a/llvm/test/tools/gold/X86/v1.12/thinlto_emit_linked_objects.ll
+++ b/llvm/test/tools/gold/X86/v1.12/thinlto_emit_linked_objects.ll
@@ -35,7 +35,7 @@
; should not be set.
; RUN: llvm-bcanalyzer --dump %t1.o.thinlto.bc | FileCheck %s -check-prefixes=CHECK-BC1
; CHECK-BC1: <GLOBALVAL_SUMMARY_BLOCK
-; CHECK-BC1: <FLAGS op0=1/>
+; CHECK-BC1: <FLAGS op0=33/>
; CHECK-BC1: </GLOBALVAL_SUMMARY_BLOCK
; Nothing interesting in the corresponding object file, so
More information about the llvm-commits
mailing list