Skip to content

Commit

Permalink
Revert "[LTO][ThinLTO] Use the linker resolutions to mark global valu…
Browse files Browse the repository at this point in the history
…es ..."

Changes more tests then expected on one of the build bots.
reverting to investigate.

This reverts https://llvm.org/svn/llvm-project/llvm/trunk@317374

llvm-svn: 317395
  • Loading branch information
Sean Fertile committed Nov 4, 2017
1 parent 73cf924 commit 39770ca
Show file tree
Hide file tree
Showing 19 changed files with 37 additions and 115 deletions.
12 changes: 2 additions & 10 deletions llvm/include/llvm/IR/ModuleSummaryIndex.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,15 +148,11 @@ class GlobalValueSummary {
/// In combined summary, indicate that the global value is live.
unsigned Live : 1;

/// Indicates that the linker resolved the symbol to a definition from
/// within the same linkage unit.
unsigned DSOLocal : 1;

/// Convenience Constructors
explicit GVFlags(GlobalValue::LinkageTypes Linkage,
bool NotEligibleToImport, bool Live, bool IsLocal)
bool NotEligibleToImport, bool Live)
: Linkage(Linkage), NotEligibleToImport(NotEligibleToImport),
Live(Live), DSOLocal(IsLocal) {}
Live(Live) {}
};

private:
Expand Down Expand Up @@ -233,10 +229,6 @@ class GlobalValueSummary {

void setLive(bool Live) { Flags.Live = Live; }

void setDSOLocal(bool Local) { Flags.DSOLocal = Local; }

bool isDSOLocal() const { return Flags.DSOLocal; }

/// Flag that this global value cannot be imported.
void setNotEligibleToImport() { Flags.NotEligibleToImport = true; }

Expand Down
8 changes: 3 additions & 5 deletions llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ template <> struct MappingTraits<TypeIdSummary> {

struct FunctionSummaryYaml {
unsigned Linkage;
bool NotEligibleToImport, Live, IsLocal;
bool NotEligibleToImport, Live;
std::vector<uint64_t> TypeTests;
std::vector<FunctionSummary::VFuncId> TypeTestAssumeVCalls,
TypeCheckedLoadVCalls;
Expand Down Expand Up @@ -177,7 +177,6 @@ template <> struct MappingTraits<FunctionSummaryYaml> {
io.mapOptional("Linkage", summary.Linkage);
io.mapOptional("NotEligibleToImport", summary.NotEligibleToImport);
io.mapOptional("Live", summary.Live);
io.mapOptional("Local", summary.IsLocal);
io.mapOptional("TypeTests", summary.TypeTests);
io.mapOptional("TypeTestAssumeVCalls", summary.TypeTestAssumeVCalls);
io.mapOptional("TypeCheckedLoadVCalls", summary.TypeCheckedLoadVCalls);
Expand Down Expand Up @@ -212,7 +211,7 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
Elem.SummaryList.push_back(llvm::make_unique<FunctionSummary>(
GlobalValueSummary::GVFlags(
static_cast<GlobalValue::LinkageTypes>(FSum.Linkage),
FSum.NotEligibleToImport, FSum.Live, FSum.IsLocal),
FSum.NotEligibleToImport, FSum.Live),
0, FunctionSummary::FFlags{}, ArrayRef<ValueInfo>{},
ArrayRef<FunctionSummary::EdgeTy>{}, std::move(FSum.TypeTests),
std::move(FSum.TypeTestAssumeVCalls),
Expand All @@ -229,8 +228,7 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
FSums.push_back(FunctionSummaryYaml{
FSum->flags().Linkage,
static_cast<bool>(FSum->flags().NotEligibleToImport),
static_cast<bool>(FSum->flags().Live),
static_cast<bool>(FSum->flags().DSOLocal), FSum->type_tests(),
static_cast<bool>(FSum->flags().Live), FSum->type_tests(),
FSum->type_test_assume_vcalls(), FSum->type_checked_load_vcalls(),
FSum->type_test_assume_const_vcalls(),
FSum->type_checked_load_const_vcalls()});
Expand Down
9 changes: 4 additions & 5 deletions llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
// FIXME: refactor this to use the same code that inliner is using.
F.isVarArg();
GlobalValueSummary::GVFlags Flags(F.getLinkage(), NotEligibleForImport,
/* Live = */ false, F.isDSOLocal());
/* Live = */ false);
FunctionSummary::FFlags FunFlags{
F.hasFnAttribute(Attribute::ReadNone),
F.hasFnAttribute(Attribute::ReadOnly),
Expand All @@ -329,7 +329,7 @@ computeVariableSummary(ModuleSummaryIndex &Index, const GlobalVariable &V,
findRefEdges(Index, &V, RefEdges, Visited);
bool NonRenamableLocal = isNonRenamableLocal(V);
GlobalValueSummary::GVFlags Flags(V.getLinkage(), NonRenamableLocal,
/* Live = */ false, V.isDSOLocal());
/* Live = */ false);
auto GVarSummary =
llvm::make_unique<GlobalVarSummary>(Flags, RefEdges.takeVector());
if (NonRenamableLocal)
Expand All @@ -342,7 +342,7 @@ computeAliasSummary(ModuleSummaryIndex &Index, const GlobalAlias &A,
DenseSet<GlobalValue::GUID> &CantBePromoted) {
bool NonRenamableLocal = isNonRenamableLocal(A);
GlobalValueSummary::GVFlags Flags(A.getLinkage(), NonRenamableLocal,
/* Live = */ false, A.isDSOLocal());
/* Live = */ false);
auto AS = llvm::make_unique<AliasSummary>(Flags);
auto *Aliasee = A.getBaseObject();
auto *AliaseeSummary = Index.getGlobalValueSummary(*Aliasee);
Expand Down Expand Up @@ -410,8 +410,7 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex(
assert(GV->isDeclaration() && "Def in module asm already has definition");
GlobalValueSummary::GVFlags GVFlags(GlobalValue::InternalLinkage,
/* NotEligibleToImport = */ true,
/* Live = */ true,
/* Local */ GV->isDSOLocal());
/* Live = */ true);
CantBePromoted.insert(GlobalValue::getGUID(Name));
// Create the appropriate summary type.
if (Function *F = dyn_cast<Function>(GV)) {
Expand Down
4 changes: 1 addition & 3 deletions llvm/lib/Bitcode/Reader/BitcodeReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -889,9 +889,7 @@ static GlobalValueSummary::GVFlags getDecodedGVSummaryFlags(uint64_t RawFlags,
// to work correctly on earlier versions, we must conservatively treat all
// values as live.
bool Live = (RawFlags & 0x2) || Version < 3;
bool Local = (RawFlags & 0x4);

return GlobalValueSummary::GVFlags(Linkage, NotEligibleToImport, Live, Local);
return GlobalValueSummary::GVFlags(Linkage, NotEligibleToImport, Live);
}

static GlobalValue::VisibilityTypes getDecodedVisibility(unsigned Val) {
Expand Down
2 changes: 0 additions & 2 deletions llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -955,8 +955,6 @@ static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags) {

RawFlags |= Flags.NotEligibleToImport; // bool
RawFlags |= (Flags.Live << 1);
RawFlags |= (Flags.DSOLocal << 2);

// Linkage don't need to be remapped at that time for the summary. Any future
// change to the getEncodedLinkage() function will need to be taken into
// account here as well.
Expand Down
21 changes: 5 additions & 16 deletions llvm/lib/LTO/LTO.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -630,9 +630,6 @@ LTO::addRegularLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
NonPrevailingComdats.insert(GV->getComdat());
cast<GlobalObject>(GV)->setComdat(nullptr);
}

// Set the 'local' flag based on the linker resolution for this symbol.
GV->setDSOLocal(Res.FinalDefinitionInLinkageUnit);
}
// Common resolution: collect the maximum size/alignment over all commons.
// We also record if we see an instance of a common as prevailing, so that
Expand All @@ -646,6 +643,7 @@ LTO::addRegularLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
CommonRes.Prevailing |= Res.Prevailing;
}

// FIXME: use proposed local attribute for FinalDefinitionInLinkageUnit.
}
if (!M.getComdatSymbolTable().empty())
for (GlobalValue &GV : M.global_values())
Expand Down Expand Up @@ -700,10 +698,10 @@ Error LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
assert(ResI != ResE);
SymbolResolution Res = *ResI++;

if (!Sym.getIRName().empty()) {
auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier(
Sym.getIRName(), GlobalValue::ExternalLinkage, ""));
if (Res.Prevailing) {
if (Res.Prevailing) {
if (!Sym.getIRName().empty()) {
auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier(
Sym.getIRName(), GlobalValue::ExternalLinkage, ""));
ThinLTO.PrevailingModuleForGUID[GUID] = BM.getModuleIdentifier();

// For linker redefined symbols (via --wrap or --defsym) we want to
Expand All @@ -715,15 +713,6 @@ Error LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
GUID, BM.getModuleIdentifier()))
S->setLinkage(GlobalValue::WeakAnyLinkage);
}

// If the linker resolved the symbol to a local definition then mark it
// as local in the summary for the module we are adding.
if (Res.FinalDefinitionInLinkageUnit) {
if (auto S = ThinLTO.CombinedIndex.findSummaryInModule(
GUID, BM.getModuleIdentifier())) {
S->setDSOLocal(true);
}
}
}
}

Expand Down
17 changes: 0 additions & 17 deletions llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -203,23 +203,6 @@ FunctionImportGlobalProcessing::getLinkage(const GlobalValue *SGV,
}

void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) {

// Check the summaries to see if the symbol gets resolved to a known local
// definition.
if (GV.hasName()) {
ValueInfo VI = ImportIndex.getValueInfo(GV.getGUID());
if (VI) {
// Need to check all summaries are local in case of hash collisions.
bool IsLocal = VI.getSummaryList().size() &&
llvm::all_of(VI.getSummaryList(),
[](const std::unique_ptr<GlobalValueSummary> &Summary) {
return Summary->isDSOLocal();
});
if (IsLocal)
GV.setDSOLocal(true);
}
}

bool DoPromote = false;
if (GV.hasLocalLinkage() &&
((DoPromote = shouldPromoteLocalToGlobal(&GV)) || isPerformingImport())) {
Expand Down
22 changes: 0 additions & 22 deletions llvm/test/Bitcode/thinlto-summary-local-5.0.ll

This file was deleted.

Binary file not shown.
2 changes: 1 addition & 1 deletion llvm/test/LTO/Resolution/X86/comdat-mixed-lto.ll
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
; would clash with the copy from this module.
; RUN: llvm-dis %t3.0.0.preopt.bc -o - | FileCheck %s
; CHECK: define internal void @__cxx_global_var_init() section ".text.startup" {
; CHECK: define available_externally dso_local void @testglobfunc() section ".text.startup" {
; CHECK: define available_externally void @testglobfunc() section ".text.startup" {

; ModuleID = 'comdat-mixed-lto.o'
source_filename = "comdat-mixed-lto.cpp"
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/LTO/Resolution/X86/comdat.ll
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,14 @@ bb11:
; CHECK-DAG: @a23 = alias i32 (i8*), i32 (i8*)* @f1.2{{$}}
; CHECK-DAG: @a24 = alias i16, bitcast (i32 (i8*)* @f1.2 to i16*)

; CHECK: define weak_odr dso_local i32 @f1(i8*) comdat($c1) {
; CHECK: define weak_odr i32 @f1(i8*) comdat($c1) {
; CHECK-NEXT: bb10:
; CHECK-NEXT: br label %bb11{{$}}
; CHECK: bb11:
; CHECK-NEXT: ret i32 42
; CHECK-NEXT: }

; CHECK: define internal dso_local i32 @f1.2(i8* %this) comdat($c2) {
; CHECK: define internal i32 @f1.2(i8* %this) comdat($c2) {
; CHECK-NEXT: bb20:
; CHECK-NEXT: store i8* %this, i8** null
; CHECK-NEXT: br label %bb21
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/LTO/Resolution/X86/commons.ll
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
; RUN: llvm-dis -o - %t.out.0.0.preopt.bc | FileCheck %s

; A strong definition should override the common
; CHECK: @x = dso_local global i32 42, align 4
; CHECK: @x = global i32 42, align 4

target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
Expand Down
30 changes: 11 additions & 19 deletions llvm/test/ThinLTO/X86/deadstrip.ll
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,23 @@
; RUN: -r %t2.bc,_boo,pl \
; RUN: -r %t2.bc,_dead_func,pl \
; RUN: -r %t2.bc,_another_dead_func,pl
; RUN: llvm-dis < %t.out.0.3.import.bc | FileCheck %s --check-prefix=LTO2
; RUN: llvm-dis < %t.out.1.3.import.bc | FileCheck %s --check-prefix=LTO2-CHECK2
; RUN: llvm-dis < %t.out.0.3.import.bc | FileCheck %s
; RUN: llvm-dis < %t.out.1.3.import.bc | FileCheck %s --check-prefix=CHECK2
; RUN: llvm-nm %t.out.1 | FileCheck %s --check-prefix=CHECK2-NM

; RUN: llvm-bcanalyzer -dump %t.out.index.bc | FileCheck %s --check-prefix=COMBINED
; Live, NotEligibleForImport, Internal
; COMBINED-DAG: <COMBINED {{.*}} op2=55
; Live, Internal
; COMBINED-DAG: <COMBINED {{.*}} op2=39
; Live, Local, External
; COMBINED-DAG: <COMBINED {{.*}} op2=96
; COMBINED-DAG: <COMBINED {{.*}} op2=96
; COMBINED-DAG: <COMBINED {{.*}} op2=96
; Local, (Dead)
; COMBINED-DAG: <COMBINED {{.*}} op2=64
; COMBINED-DAG: <COMBINED {{.*}} op2=64
; COMBINED-DAG: <COMBINED {{.*}} op2=64
; Live, External
; COMBINED-DAG: <COMBINED {{.*}} op2=32
; COMBINED-DAG: <COMBINED {{.*}} op2=32
; COMBINED-DAG: <COMBINED {{.*}} op2=32
; (Dead)
; COMBINED-DAG: <COMBINED {{.*}} op2=0
; COMBINED-DAG: <COMBINED {{.*}} op2=0
; COMBINED-DAG: <COMBINED {{.*}} op2=0

; Dead-stripping on the index allows to internalize these,
; and limit the import of @baz thanks to early pruning.
Expand All @@ -45,18 +45,10 @@
; CHECK: define internal void @bar_internal()
; CHECK: define internal void @dead_func() {
; CHECK-NOT: available_externally {{.*}} @baz()
; LTO2-NOT: available_externally {{.*}} @baz()
; LTO2: @llvm.global_ctors =
; LTO2: define internal void @_GLOBAL__I_a()
; LTO2: define internal dso_local void @bar() {
; LTO2: define internal void @bar_internal()
; LTO2: define internal dso_local void @dead_func() {
; LTO2-NOT: available_externally {{.*}} @baz()

; Make sure we didn't internalize @boo, which is reachable via
; llvm.global_ctors
; CHECK2: define void @boo()
; LTO2-CHECK2: define dso_local void @boo()
; We should have eventually removed @baz since it was internalized and unused
; CHECK2-NM-NOT: _baz

Expand Down Expand Up @@ -88,7 +80,7 @@

; We can't internalize @dead_func because of the use in the regular LTO
; partition.
; CHECK-NOTDEAD: define dso_local void @dead_func()
; CHECK-NOTDEAD: define void @dead_func()
; We also can't eliminate @baz because it is in the regular LTO partition
; and called from @dead_func.
; CHECK-NM-NOTDEAD: T _baz
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/ThinLTO/X86/funcimport2.ll
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
; RUN: -r=%t2.bc,_main,plx \
; RUN: -r=%t2.bc,_foo,l
; RUN: llvm-dis %t.o.1.3.import.bc -o - | FileCheck %s
; CHECK: define available_externally dso_local void @foo()
; CHECK: define available_externally void @foo()

; We shouldn't do any importing at -O0
; rm -f %t.o.1.3.import.bc
Expand All @@ -17,7 +17,7 @@
; RUN: -r=%t2.bc,_main,plx \
; RUN: -r=%t2.bc,_foo,l
; RUN: llvm-dis %t.o.1.3.import.bc -o - | FileCheck %s --check-prefix=CHECKO0
; CHECKO0: declare dso_local void @foo(...)
; CHECKO0: declare void @foo(...)

target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-apple-macosx10.11.0"
Expand Down
9 changes: 3 additions & 6 deletions llvm/test/ThinLTO/X86/internalize.ll
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
; RUN: opt -module-summary %s -o %t1.bc
;; RUN: opt -module-summary %s -o %t1.bc
; RUN: llvm-lto -thinlto-action=thinlink -o %t.index.bc %t1.bc
; RUN: llvm-lto -thinlto-action=internalize -thinlto-index %t.index.bc %t1.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=REGULAR
; RUN: llvm-lto -thinlto-action=internalize -thinlto-index %t.index.bc %t1.bc -o - --exported-symbol=foo | llvm-dis -o - | FileCheck %s --check-prefix=INTERNALIZE
Expand All @@ -7,7 +7,7 @@
; RUN: -r=%t1.bc,_foo,pxl \
; RUN: -r=%t1.bc,_bar,pl \
; RUN: -r=%t1.bc,_linkonce_func,pl
; RUN: llvm-dis < %t.o.0.2.internalize.bc | FileCheck %s --check-prefix=INTERNALIZE2
; RUN: llvm-dis < %t.o.0.2.internalize.bc | FileCheck %s --check-prefix=INTERNALIZE


; REGULAR: define void @foo
Expand All @@ -16,9 +16,6 @@
; INTERNALIZE: define void @foo
; INTERNALIZE: define internal void @bar
; INTERNALIZE: define internal void @linkonce_func()
; INTERNALIZE2: define dso_local void @foo
; INTERNALIZE2: define internal dso_local void @bar
; INTERNALIZE2: define internal dso_local void @linkonce_func()

target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-apple-macosx10.11.0"
Expand All @@ -32,4 +29,4 @@ define void @bar() {
}
define linkonce void @linkonce_func() {
ret void
}
}
2 changes: 1 addition & 1 deletion llvm/test/ThinLTO/X86/reference_non_importable.ll
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ target triple = "x86_64-apple-macosx10.11.0"

; 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 dso_local i8** @foo()
; IMPORT: define available_externally i8** @foo()
define i8 **@foo() {
ret i8 **@b
}
1 change: 0 additions & 1 deletion llvm/test/Transforms/LowerTypeTests/import-unsat.ll
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
; SUMMARY-NEXT: - Linkage: 0
; SUMMARY-NEXT: NotEligibleToImport: false
; SUMMARY-NEXT: Live: true
; SUMMARY-NEXT: Local: false
; SUMMARY-NEXT: TypeTests: [ 123 ]
; SUMMARY-NEXT: TypeIdMap:
; SUMMARY-NEXT: typeid1:
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Transforms/PGOProfile/thinlto_samplepgo_icp2.ll
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
; RUN: llvm-nm %t3.2 | FileCheck %s --check-prefix=NM
; NM: _ZL3barv
; RUN: llvm-dis < %t3.2.2.internalize.bc | FileCheck %s --check-prefix=INTERNALIZE
; INTERNALIZE: define dso_local void @_ZL3barv
; INTERNALIZE: define void @_ZL3barv

target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
Expand Down
1 change: 0 additions & 1 deletion llvm/test/Transforms/WholeProgramDevirt/import-indir.ll
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
; SUMMARY-NEXT: - Linkage: 0
; SUMMARY-NEXT: NotEligibleToImport: false
; SUMMARY-NEXT: Live: true
; SUMMARY-NEXT: Local: false
; SUMMARY-NEXT: TypeTestAssumeVCalls:
; SUMMARY-NEXT: - GUID: 123
; SUMMARY-NEXT: Offset: 0
Expand Down

0 comments on commit 39770ca

Please sign in to comment.