[llvm] 657eef6 - [ThinLTO] Distinguish symbols that are promoted (#181946)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 19 10:26:02 PST 2026
Author: Mircea Trofin
Date: 2026-02-19T10:25:57-08:00
New Revision: 657eef69e4b418ed4a0326056582f512d851d18e
URL: https://github.com/llvm/llvm-project/commit/657eef69e4b418ed4a0326056582f512d851d18e
DIFF: https://github.com/llvm/llvm-project/commit/657eef69e4b418ed4a0326056582f512d851d18e.diff
LOG: [ThinLTO] Distinguish symbols that are promoted (#181946)
Thinlink may decide some symbols with internal linkage should get promoted to external. Such a symbol, when being imported, would have its name changed by appending a suffix (`.llvm.<a hash>`) to avoid collisions - since internal linkage symbols have non-unique names.
Later, still during Thinlink, in `thinLTOResolvePrevailingGUID`, the fact that this symbol was promoted is not considered and we set its linkage to `AvailableExternally`(when reading `thinLTOResolvePrevailingGUID`, note that "prevailing-ness" is not a concept that the original symbol would have participated in)
This should result in a final (native) link error, because the symbol's definition may be elided. But we get lucky: in the post-thinlink backend, during import, in `llvm::thinLTOFinalizeInModule`, after this symbol's name was changed and its linkage also changed to `External` (see `FunctionImportGlobalProcessing::processGlobalForThinLTO`), we try to find it in the `DefinedGlobals`, fail (because its guid is computed from its changed name)) and leave its linkage as-is. Which happens to be correct.
This patch makes this outcome intentional rather than accidental. It becomes critical once we land [this RFC](https://discourse.llvm.org/t/rfc-keep-globalvalue-guids-stable/84801).
As a side-benefit, the extra attribute propagations that weren't happening in `llvm::thinLTOFinalizeInModule` now do.
Added:
llvm/test/ThinLTO/X86/Inputs/export2.ll
llvm/test/ThinLTO/X86/export2.ll
Modified:
llvm/include/llvm/IR/ModuleSummaryIndex.h
llvm/lib/LTO/LTO.cpp
llvm/lib/Transforms/IPO/FunctionAttrs.cpp
llvm/lib/Transforms/IPO/FunctionImport.cpp
llvm/test/ThinLTO/X86/funcattrs-prop-exported-internal.ll
llvm/test/ThinLTO/X86/module_asm2.ll
llvm/tools/llvm-link/llvm-link.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 31c2f13a63e5f..5ede26cc7923c 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -509,6 +509,10 @@ class GlobalValueSummary {
/// summary. The value is interpreted as 'ImportKind' enum defined above.
unsigned ImportType : 1;
+ /// This symbol was promoted. Thinlink stages need to be aware of this
+ /// transition
+ unsigned Promoted : 1;
+
/// Convenience Constructors
explicit GVFlags(GlobalValue::LinkageTypes Linkage,
GlobalValue::VisibilityTypes Visibility,
@@ -517,7 +521,7 @@ class GlobalValueSummary {
: Linkage(Linkage), Visibility(Visibility),
NotEligibleToImport(NotEligibleToImport), Live(Live),
DSOLocal(IsLocal), CanAutoHide(CanAutoHide),
- ImportType(static_cast<unsigned>(ImportType)) {}
+ ImportType(static_cast<unsigned>(ImportType)), Promoted(false) {}
};
private:
@@ -584,12 +588,28 @@ class GlobalValueSummary {
return static_cast<GlobalValue::LinkageTypes>(Flags.Linkage);
}
+ bool wasPromoted() const { return Flags.Promoted; }
+
+ void promote() {
+ assert(GlobalValue::isLocalLinkage(linkage()) &&
+ "unexpected (re-)promotion of non-local symbol");
+ assert(!Flags.Promoted);
+ Flags.Promoted = true;
+ Flags.Linkage = GlobalValue::LinkageTypes::ExternalLinkage;
+ }
+
/// Sets the linkage to the value determined by global summary-based
/// optimization. Will be applied in the ThinLTO backends.
void setLinkage(GlobalValue::LinkageTypes Linkage) {
+ assert(!wasPromoted());
+ assert(!GlobalValue::isExternalLinkage(Linkage) && "use `promote` instead");
Flags.Linkage = Linkage;
}
+ void setExternalLinkageForTest() {
+ Flags.Linkage = GlobalValue::LinkageTypes::ExternalLinkage;
+ }
+
/// Return true if this global value can't be imported.
bool notEligibleToImport() const { return Flags.NotEligibleToImport; }
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index 749af6eda3fdb..c88ec5e09396a 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -393,6 +393,9 @@ static void thinLTOResolvePrevailingGUID(
// FIXME: We may want to split the compile time and correctness
// aspects into separate routines.
if (isPrevailing(VI.getGUID(), S.get())) {
+ assert(!S->wasPromoted() &&
+ "promoted symbols used to be internal linkage and shouldn't have "
+ "a prevailing variant");
if (GlobalValue::isLinkOnceLinkage(OriginalLinkage)) {
S->setLinkage(GlobalValue::getWeakLinkage(
GlobalValue::isLinkOnceODRLinkage(OriginalLinkage)));
@@ -415,8 +418,11 @@ static void thinLTOResolvePrevailingGUID(
// When force-import-all is used, it indicates that object linking is not
// supported by the target. In this case, we can't change the linkage as
// well in case the global is converted to declaration.
+ // Also, if the symbol was promoted, it wouldn't have a prevailing variant,
+ // but also its linkage is set correctly (to External) already.
else if (!isa<AliasSummary>(S.get()) &&
- !GlobalInvolvedWithAlias.count(S.get()) && !ForceImportAll)
+ !GlobalInvolvedWithAlias.count(S.get()) && !ForceImportAll &&
+ !S->wasPromoted())
S->setLinkage(GlobalValue::AvailableExternallyLinkage);
// For ELF, set visibility to the computed visibility from summaries. We
@@ -485,7 +491,7 @@ static void thinLTOInternalizeAndPromoteGUID(
// exported.
if (isExported(S->modulePath(), VI)) {
if (GlobalValue::isLocalLinkage(S->linkage()))
- S->setLinkage(GlobalValue::ExternalLinkage);
+ S->promote();
continue;
}
diff --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
index 855692db006f9..626d28d4c50d8 100644
--- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -387,7 +387,7 @@ static FunctionSummary *calculatePrevailingSummary(
}
Local = FS;
} else if (GlobalValue::isExternalLinkage(Linkage)) {
- assert(IsPrevailing(VI.getGUID(), GVS.get()));
+ assert(IsPrevailing(VI.getGUID(), GVS.get()) || GVS->wasPromoted());
Prevailing = FS;
break;
} else if (GlobalValue::isWeakODRLinkage(Linkage) ||
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index 077ab72c15e10..4d4b32749a045 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -2106,7 +2106,7 @@ static bool doImportingForModuleForTest(
for (auto &I : *Index) {
for (auto &S : I.second.getSummaryList()) {
if (GlobalValue::isLocalLinkage(S->linkage()))
- S->setLinkage(GlobalValue::ExternalLinkage);
+ S->setExternalLinkageForTest();
}
}
diff --git a/llvm/test/ThinLTO/X86/Inputs/export2.ll b/llvm/test/ThinLTO/X86/Inputs/export2.ll
new file mode 100644
index 0000000000000..f70b21b2a5678
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/Inputs/export2.ll
@@ -0,0 +1,9 @@
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @main() {
+entry:
+ call void @callstaticfunc()
+ ret i32 0
+}
+
+declare void @callstaticfunc()
diff --git a/llvm/test/ThinLTO/X86/export2.ll b/llvm/test/ThinLTO/X86/export2.ll
new file mode 100644
index 0000000000000..7909543e68e6f
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/export2.ll
@@ -0,0 +1,27 @@
+; Do setup work for all below tests: generate bitcode and combined index
+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/export2.ll -o %t2.bc
+;
+; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t3 \
+; RUN: -thinlto-distributed-indexes \
+; RUN: -r=%t1.bc,callstaticfunc,px \
+; RUN: -r=%t2.bc,main,px \
+; RUN: -r=%t2.bc,callstaticfunc,
+
+; both functions must appear as external linkage in the index.
+; RUN: llvm-dis %t1.bc.thinlto.bc -o - | FileCheck %s
+; CHECK: linkage: external
+; CHECK: linkage: external
+
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @callstaticfunc() {
+entry:
+ call void @staticfunc()
+ ret void
+}
+
+define internal void @staticfunc() {
+entry:
+ ret void
+}
diff --git a/llvm/test/ThinLTO/X86/funcattrs-prop-exported-internal.ll b/llvm/test/ThinLTO/X86/funcattrs-prop-exported-internal.ll
index 2b2228f1158dd..3080842c40107 100644
--- a/llvm/test/ThinLTO/X86/funcattrs-prop-exported-internal.ll
+++ b/llvm/test/ThinLTO/X86/funcattrs-prop-exported-internal.ll
@@ -27,7 +27,7 @@ define void @importer() {
; If somehow the caller doesn't get the attributes, we
; shouldn't propagate from the internal callee.
-; CHECK: define void @importer_noattr() {
+; CHECK: define void @importer_noattr() #0 {
define void @importer_noattr() {
call void @caller_noattr()
ret void
diff --git a/llvm/test/ThinLTO/X86/module_asm2.ll b/llvm/test/ThinLTO/X86/module_asm2.ll
index 7ab7847f79439..8d4d191105419 100644
--- a/llvm/test/ThinLTO/X86/module_asm2.ll
+++ b/llvm/test/ThinLTO/X86/module_asm2.ll
@@ -9,7 +9,7 @@
; RUN: llvm-nm %t2.bc.thinlto.o | FileCheck %s --check-prefix=NM1
; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t.o -save-temps \
-; RUN: -r=%t1.bc,foo,plx \
+; RUN: -r=%t1.bc,foo,lx \
; RUN: -r=%t1.bc,globalfunc,plx \
; RUN: -r=%t1.bc,globalfunc,lx \
; RUN: -r=%t1.bc,weakfunc,plx \
diff --git a/llvm/tools/llvm-link/llvm-link.cpp b/llvm/tools/llvm-link/llvm-link.cpp
index 33c3e6fc350fb..f6fc41d750b50 100644
--- a/llvm/tools/llvm-link/llvm-link.cpp
+++ b/llvm/tools/llvm-link/llvm-link.cpp
@@ -422,7 +422,7 @@ static bool linkFiles(const char *argv0, LLVMContext &Context, Linker &L,
for (auto &I : *Index) {
for (auto &S : I.second.getSummaryList()) {
if (GlobalValue::isLocalLinkage(S->linkage()))
- S->setLinkage(GlobalValue::ExternalLinkage);
+ S->setExternalLinkageForTest();
}
}
More information about the llvm-commits
mailing list