ThinLTO: Don't import aliases of any kind (even linkonce_odr)
Summary:
Until a more advanced version of importing can be implemented for
aliases (one that imports an alias as an available_externally definition
of the aliasee), skip the narrow subset of cases that was possible but
came at a cost: aliases of linkonce_odr functions could be imported
because the linkonce_odr function could be safely duplicated from the
source module. This came/comes at the cost of not being able to 'home'
imported linkonce functions (they had to be emitted linkonce_odr in all
the destination modules (even if they weren't used by an alias) rather
than as available_externally - causing extra object size).
Tangentially, this also was the only reason ThinLTO would emit multiple
CUs in to the resulting DWARF - which happens to be a problem for
Fission (there's a fix for this in GDB but not released yet, etc).
(actually it's not the only reason - but I'm sending a patch to fix the
other reason shortly)
There's no reason to believe this particularly narrow alias importing
was especially/meaningfully important, only that it was /possible/ to
implement in this way. When a more general solution is done, it should
still satisfy the DWARF concerns above, since the import will still be
available_externally, and thus not create extra CUs.
Since now all aliases are treated the same, I removed/simplified some
test cases since they were testing corner cases where there are no
longer any corners.
Reviewers: tejohnson, mehdi_amini
Differential Revision: https://reviews.llvm.org/D35875
llvm-svn: 309278
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index 233a36d..a36314c 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -132,16 +132,11 @@
if (GlobalValue::isInterposableLinkage(GVSummary->linkage()))
// There is no point in importing these, we can't inline them
return false;
- if (auto *AS = dyn_cast<AliasSummary>(GVSummary)) {
- GVSummary = &AS->getAliasee();
- // Alias can't point to "available_externally". However when we import
- // linkOnceODR the linkage does not change. So we import the alias
- // and aliasee only in this case.
+ if (auto *AS = dyn_cast<AliasSummary>(GVSummary))
+ // Aliases can't point to "available_externally".
// FIXME: we should import alias as available_externally *function*,
- // the destination module does need to know it is an alias.
- if (!GlobalValue::isLinkOnceODRLinkage(GVSummary->linkage()))
- return false;
- }
+ // the destination module does not need to know it is an alias.
+ return false;
auto *Summary = cast<FunctionSummary>(GVSummary);
@@ -227,16 +222,11 @@
DEBUG(dbgs() << "ignored! No qualifying callee with summary found.\n");
continue;
}
- // "Resolve" the summary, traversing alias,
- const FunctionSummary *ResolvedCalleeSummary;
- if (isa<AliasSummary>(CalleeSummary)) {
- ResolvedCalleeSummary = cast<FunctionSummary>(
- &cast<AliasSummary>(CalleeSummary)->getAliasee());
- assert(
- GlobalValue::isLinkOnceODRLinkage(ResolvedCalleeSummary->linkage()) &&
- "Unexpected alias to a non-linkonceODR in import list");
- } else
- ResolvedCalleeSummary = cast<FunctionSummary>(CalleeSummary);
+
+ // "Resolve" the summary
+ assert(!isa<AliasSummary>(CalleeSummary) &&
+ "Unexpected alias in import list");
+ const auto *ResolvedCalleeSummary = cast<FunctionSummary>(CalleeSummary);
assert(ResolvedCalleeSummary->instCount() <= NewThreshold &&
"selectCallee() didn't honor the threshold");
diff --git a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
index a98d072..36cbd98 100644
--- a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
+++ b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
@@ -23,19 +23,13 @@
bool FunctionImportGlobalProcessing::doImportAsDefinition(
const GlobalValue *SGV, SetVector<GlobalValue *> *GlobalsToImport) {
- // For alias, we tie the definition to the base object. Extract it and recurse
- if (auto *GA = dyn_cast<GlobalAlias>(SGV)) {
- if (GA->isInterposable())
- return false;
- const GlobalObject *GO = GA->getBaseObject();
- if (!GO->hasLinkOnceODRLinkage())
- return false;
- return FunctionImportGlobalProcessing::doImportAsDefinition(
- GO, GlobalsToImport);
- }
// Only import the globals requested for importing.
if (GlobalsToImport->count(const_cast<GlobalValue *>(SGV)))
return true;
+
+ assert(!isa<GlobalAlias>(SGV) &&
+ "Unexpected global alias in the import list.");
+
// Otherwise no.
return false;
}
@@ -132,8 +126,10 @@
return SGV->getLinkage();
switch (SGV->getLinkage()) {
+ case GlobalValue::LinkOnceAnyLinkage:
+ case GlobalValue::LinkOnceODRLinkage:
case GlobalValue::ExternalLinkage:
- // External defnitions are converted to available_externally
+ // External and linkonce definitions are converted to available_externally
// definitions upon import, so that they are available for inlining
// and/or optimization, but are turned into declarations later
// during the EliminateAvailableExternally pass.
@@ -150,12 +146,6 @@
// An imported available_externally declaration stays that way.
return SGV->getLinkage();
- case GlobalValue::LinkOnceAnyLinkage:
- case GlobalValue::LinkOnceODRLinkage:
- // These both stay the same when importing the definition.
- // The ThinLTO pass will eventually force-import their definitions.
- return SGV->getLinkage();
-
case GlobalValue::WeakAnyLinkage:
// Can't import weak_any definitions correctly, or we might change the
// program semantics, since the linker will pick the first weak_any