[lld] dd29597 - [LTO] Initialize canAutoHide() using canBeOmittedFromSymbolTable()
Jez Ng via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 3 16:04:23 PST 2022
Author: Jez Ng
Date: 2022-03-03T19:04:11-05:00
New Revision: dd29597e103cd13dd308fb68283fce9d3411f723
URL: https://github.com/llvm/llvm-project/commit/dd29597e103cd13dd308fb68283fce9d3411f723
DIFF: https://github.com/llvm/llvm-project/commit/dd29597e103cd13dd308fb68283fce9d3411f723.diff
LOG: [LTO] Initialize canAutoHide() using canBeOmittedFromSymbolTable()
Per discussion on
https://reviews.llvm.org/D59709#inline-1148734, this seems like the
right course of action. `canBeOmittedFromSymbolTable()` subsumes and
generalizes the previous logic. In addition to handling `linkonce_odr`
`unnamed_addr` globals, we now also internalize `linkonce_odr` +
`local_unnamed_addr` constants.
Reviewed By: tejohnson
Differential Revision: https://reviews.llvm.org/D120173
Added:
Modified:
lld/test/MachO/lto-internalize-unnamed-addr.ll
llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
llvm/lib/Transforms/IPO/FunctionImport.cpp
llvm/test/ThinLTO/X86/Inputs/linkonce_odr_unnamed_addr.ll
llvm/test/ThinLTO/X86/linkonce_odr_unnamed_addr.ll
Removed:
################################################################################
diff --git a/lld/test/MachO/lto-internalize-unnamed-addr.ll b/lld/test/MachO/lto-internalize-unnamed-addr.ll
index e0622d691cdc4..2b4f9821a3ea5 100644
--- a/lld/test/MachO/lto-internalize-unnamed-addr.ll
+++ b/lld/test/MachO/lto-internalize-unnamed-addr.ll
@@ -13,10 +13,10 @@
; RUN: %lld -lSystem -dylib %t/test.o %t/test2.o -o %t/test.dylib
; RUN: llvm-nm -m %t/test.dylib | FileCheck %s --check-prefix=LTO-DYLIB
-; RUN: %lld -lSystem %t/test.thinlto.o %t/test2.o -o %t/test.thinlto
+; RUN: %lld -lSystem %t/test.thinlto.o %t/test2.thinlto.o -o %t/test.thinlto
; RUN: llvm-nm -m %t/test.thinlto | FileCheck %s --check-prefix=THINLTO
-; RUN: %lld -lSystem -dylib %t/test.thinlto.o %t/test2.o -o %t/test.thinlto.dylib
+; RUN: %lld -lSystem -dylib %t/test.thinlto.o %t/test2.thinlto.o -o %t/test.thinlto.dylib
; RUN: llvm-nm -m %t/test.thinlto.dylib | FileCheck %s --check-prefix=THINLTO
; LTO-DAG: (__DATA,__data) non-external _global_unnamed
@@ -40,10 +40,8 @@
; THINLTO-DAG: (__DATA,__data) non-external (was a private external) _global_unnamed
; THINLTO-DAG: (__DATA,__data) weak external _local_unnamed
-;; The next two symbols are rendered as non-external (was a private external)
-;; by LD64. This is a missed optimization on LLD's end.
-; THINLTO-DAG: (__TEXT,__const) weak external _local_unnamed_always_const
-; THINLTO-DAG: (__TEXT,__const) weak external _local_unnamed_const
+; THINLTO-DAG: (__TEXT,__const) non-external (was a private external) _local_unnamed_always_const
+; THINLTO-DAG: (__TEXT,__const) non-external (was a private external) _local_unnamed_const
;; LD64 actually fails to link when the following symbol is included in the test
;; input, instead producing this error:
;; reference to bitcode symbol '_local_unnamed_sometimes_const' which LTO has not compiled in '_used' from /tmp/lto.o for architecture x86_64
diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
index 63c4aa38a7690..8bf70c513ab3f 100644
--- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -489,8 +489,7 @@ static void computeFunctionSummary(
HasIndirBranchToBlockAddress;
GlobalValueSummary::GVFlags Flags(
F.getLinkage(), F.getVisibility(), NotEligibleForImport,
- /* Live = */ false, F.isDSOLocal(),
- F.hasLinkOnceODRLinkage() && F.hasGlobalUnnamedAddr());
+ /* Live = */ false, F.isDSOLocal(), F.canBeOmittedFromSymbolTable());
FunctionSummary::FFlags FunFlags{
F.hasFnAttribute(Attribute::ReadNone),
F.hasFnAttribute(Attribute::ReadOnly),
@@ -611,8 +610,7 @@ static void computeVariableSummary(ModuleSummaryIndex &Index,
bool NonRenamableLocal = isNonRenamableLocal(V);
GlobalValueSummary::GVFlags Flags(
V.getLinkage(), V.getVisibility(), NonRenamableLocal,
- /* Live = */ false, V.isDSOLocal(),
- V.hasLinkOnceODRLinkage() && V.hasGlobalUnnamedAddr());
+ /* Live = */ false, V.isDSOLocal(), V.canBeOmittedFromSymbolTable());
VTableFuncList VTableFuncs;
// If splitting is not enabled, then we compute the summary information
@@ -654,8 +652,7 @@ computeAliasSummary(ModuleSummaryIndex &Index, const GlobalAlias &A,
bool NonRenamableLocal = isNonRenamableLocal(A);
GlobalValueSummary::GVFlags Flags(
A.getLinkage(), A.getVisibility(), NonRenamableLocal,
- /* Live = */ false, A.isDSOLocal(),
- A.hasLinkOnceODRLinkage() && A.hasGlobalUnnamedAddr());
+ /* Live = */ false, A.isDSOLocal(), A.canBeOmittedFromSymbolTable());
auto AS = std::make_unique<AliasSummary>(Flags);
auto *Aliasee = A.getAliaseeObject();
auto AliaseeVI = Index.getValueInfo(Aliasee->getGUID());
@@ -732,8 +729,7 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex(
GlobalValue::InternalLinkage, GlobalValue::DefaultVisibility,
/* NotEligibleToImport = */ true,
/* Live = */ true,
- /* Local */ GV->isDSOLocal(),
- GV->hasLinkOnceODRLinkage() && GV->hasGlobalUnnamedAddr());
+ /* Local */ GV->isDSOLocal(), GV->canBeOmittedFromSymbolTable());
CantBePromoted.insert(GV->getGUID());
// Create the appropriate summary type.
if (Function *F = dyn_cast<Function>(GV)) {
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index d9b43109f6295..7f90c2cec519e 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -1112,12 +1112,13 @@ void llvm::thinLTOFinalizeInModule(Module &TheModule,
llvm_unreachable("Expected GV to be converted");
} else {
// If all copies of the original symbol had global unnamed addr and
- // linkonce_odr linkage, it should be an auto hide symbol. In that case
- // the thin link would have marked it as CanAutoHide. Add hidden visibility
- // to the symbol to preserve the property.
+ // linkonce_odr linkage, or if all of them had local unnamed addr linkage
+ // and are constants, then it should be an auto hide symbol. In that case
+ // the thin link would have marked it as CanAutoHide. Add hidden
+ // visibility to the symbol to preserve the property.
if (NewLinkage == GlobalValue::WeakODRLinkage &&
GS->second->canAutoHide()) {
- assert(GV.hasLinkOnceODRLinkage() && GV.hasGlobalUnnamedAddr());
+ assert(GV.canBeOmittedFromSymbolTable());
GV.setVisibility(GlobalValue::HiddenVisibility);
}
diff --git a/llvm/test/ThinLTO/X86/Inputs/linkonce_odr_unnamed_addr.ll b/llvm/test/ThinLTO/X86/Inputs/linkonce_odr_unnamed_addr.ll
index e0bb06859e71a..99c031299b43d 100644
--- a/llvm/test/ThinLTO/X86/Inputs/linkonce_odr_unnamed_addr.ll
+++ b/llvm/test/ThinLTO/X86/Inputs/linkonce_odr_unnamed_addr.ll
@@ -1,5 +1,8 @@
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-grtev4-linux-gnu"
- at linkonceodrunnamed = linkonce_odr unnamed_addr constant i32 0
+ at linkonceodrunnamedconst = linkonce_odr unnamed_addr constant i32 0
+ at linkonceodrunnamed = linkonce_odr unnamed_addr global i32 0
+ at linkonceodrlocalunnamedconst = linkonce_odr local_unnamed_addr constant i32 0
+ at linkonceodrlocalunnamed = linkonce_odr local_unnamed_addr global i32 0
@odrunnamed = weak_odr unnamed_addr constant i32 0
diff --git a/llvm/test/ThinLTO/X86/linkonce_odr_unnamed_addr.ll b/llvm/test/ThinLTO/X86/linkonce_odr_unnamed_addr.ll
index 8b8e3677cbb98..46bafa0b5422a 100644
--- a/llvm/test/ThinLTO/X86/linkonce_odr_unnamed_addr.ll
+++ b/llvm/test/ThinLTO/X86/linkonce_odr_unnamed_addr.ll
@@ -1,5 +1,6 @@
-; This test ensures that when linkonce_odr + unnamed_addr symbols promoted to
-; weak symbols, it preserves the auto hide property when possible.
+; This test ensures that when linkonce_odr + unnamed_addr symbols & linkonce_odr
+; + local_unnamed_addr constants get promoted to weak symbols, we preserves the
+; auto hide property when possible.
; RUN: opt -module-summary %s -o %t.bc
; RUN: opt -module-summary %p/Inputs/linkonce_odr_unnamed_addr.ll -o %t2.bc
@@ -7,25 +8,56 @@
; RUN: llvm-lto -thinlto-action=thinlink -o %t3.bc %t.bc %t2.bc
; RUN: llvm-lto -thinlto-action=promote %t.bc -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s
; Check new LTO API
-; RUN: llvm-lto2 run -save-temps -o %t6.bc %t.bc %t2.bc -r=%t.bc,linkonceodrunnamed,p -r=%t.bc,odrunnamed,p -r=%t2.bc,linkonceodrunnamed, -r=%t2.bc,odrunnamed,
+; RUN: llvm-lto2 run -save-temps -o %t6.bc %t.bc %t2.bc \
+; RUN: -r=%t.bc,linkonceodrunnamedconst,p \
+; RUN: -r=%t.bc,linkonceodrunnamed,p \
+; RUN: -r=%t.bc,linkonceodrlocalunnamedconst,p \
+; RUN: -r=%t.bc,linkonceodrlocalunnamed,p \
+; RUN: -r=%t.bc,odrunnamed,p \
+; RUN: -r=%t2.bc,linkonceodrunnamedconst \
+; RUN: -r=%t2.bc,linkonceodrunnamed \
+; RUN: -r=%t2.bc,linkonceodrlocalunnamedconst \
+; RUN: -r=%t2.bc,linkonceodrlocalunnamed \
+; RUN: -r=%t2.bc,odrunnamed
; RUN: llvm-dis %t6.bc.1.1.promote.bc -o - | FileCheck %s
; Now test when one module does not have a summary. In that case we must be
; conservative and not auto hide.
; RUN: opt %p/Inputs/linkonce_odr_unnamed_addr.ll -o %t4.bc
; Check new LTO API (old LTO API does not detect this case).
-; RUN: llvm-lto2 run -save-temps -o %t6.bc %t.bc %t4.bc -r=%t.bc,linkonceodrunnamed,p -r=%t.bc,odrunnamed,p -r=%t4.bc,linkonceodrunnamed, -r=%t4.bc,odrunnamed,
+; RUN: llvm-lto2 run -save-temps -o %t6.bc %t.bc %t4.bc \
+; RUN: -r=%t.bc,linkonceodrunnamedconst,p \
+; RUN: -r=%t.bc,linkonceodrunnamed,p \
+; RUN: -r=%t.bc,linkonceodrlocalunnamedconst,p \
+; RUN: -r=%t.bc,linkonceodrlocalunnamed,p \
+; RUN: -r=%t.bc,odrunnamed,p \
+; RUN: -r=%t4.bc,linkonceodrunnamedconst \
+; RUN: -r=%t4.bc,linkonceodrunnamed \
+; RUN: -r=%t4.bc,linkonceodrlocalunnamedconst \
+; RUN: -r=%t4.bc,linkonceodrlocalunnamed \
+; RUN: -r=%t4.bc,odrunnamed
; RUN: llvm-dis %t6.bc.1.1.promote.bc -o - | FileCheck %s --check-prefix=NOSUMMARY
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-grtev4-linux-gnu"
; In this case all copies are linkonce_odr, so it may be hidden.
-; CHECK: @linkonceodrunnamed = weak_odr hidden unnamed_addr constant i32 0
-; NOSUMMARY: @linkonceodrunnamed = weak_odr unnamed_addr constant i32 0
- at linkonceodrunnamed = linkonce_odr unnamed_addr constant i32 0
+; CHECK: @linkonceodrunnamedconst = weak_odr hidden unnamed_addr constant i32 0
+; CHECK: @linkonceodrunnamed = weak_odr hidden unnamed_addr global i32 0
+; CHECK: @linkonceodrlocalunnamedconst = weak_odr hidden local_unnamed_addr constant i32 0
+; NOSUMMARY: @linkonceodrunnamedconst = weak_odr unnamed_addr constant i32 0
+; NOSUMMARY: @linkonceodrunnamed = weak_odr unnamed_addr global i32 0
+; NOSUMMARY: @linkonceodrlocalunnamedconst = weak_odr local_unnamed_addr constant i32 0
+ at linkonceodrunnamedconst = linkonce_odr unnamed_addr constant i32 0
+ at linkonceodrunnamed = linkonce_odr unnamed_addr global i32 0
+ at linkonceodrlocalunnamedconst = linkonce_odr local_unnamed_addr constant i32 0
; In this case, the other copy was weak_odr, so it may not be hidden.
; CHECK: @odrunnamed = weak_odr unnamed_addr constant i32 0
; NOSUMMARY: @odrunnamed = weak_odr unnamed_addr constant i32 0
@odrunnamed = linkonce_odr unnamed_addr constant i32 0
+
+; local_unnamed_addr globals cannot be hidden.
+; CHECK: @linkonceodrlocalunnamed = weak_odr local_unnamed_addr global i32 0
+; NOSUMMARY: @linkonceodrlocalunnamed = weak_odr local_unnamed_addr global i32 0
+ at linkonceodrlocalunnamed = linkonce_odr local_unnamed_addr global i32 0
More information about the llvm-commits
mailing list