[llvm] [ThinLTO] Distinguish symbols that are promoted (PR #181946)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 17 16:54:53 PST 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lto
Author: Mircea Trofin (mtrofin)
<details>
<summary>Changes</summary>
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).
---
Full diff: https://github.com/llvm/llvm-project/pull/181946.diff
8 Files Affected:
- (modified) llvm/include/llvm/IR/ModuleSummaryIndex.h (+21-1)
- (modified) llvm/lib/LTO/LTO.cpp (+3-2)
- (modified) llvm/lib/Transforms/IPO/FunctionAttrs.cpp (+1-1)
- (modified) llvm/lib/Transforms/IPO/FunctionImport.cpp (+1-1)
- (added) llvm/test/ThinLTO/X86/Inputs/export2.ll (+9)
- (added) llvm/test/ThinLTO/X86/export2.ll (+27)
- (modified) llvm/test/ThinLTO/X86/funcattrs-prop-exported-internal.ll (+1-1)
- (modified) llvm/tools/llvm-link/llvm-link.cpp (+1-1)
``````````diff
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 31c2f13a63e5f..98988c317496b 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::isExternalLinkage(linkage()) &&
+ "unexpected (re-)promotion of external 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..a1926843be284 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -416,7 +416,8 @@ static void thinLTOResolvePrevailingGUID(
// supported by the target. In this case, we can't change the linkage as
// well in case the global is converted to declaration.
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 +486,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/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();
}
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/181946
More information about the llvm-commits
mailing list