[llvm] [ThinLTO] Properly support targets that require importing all external functions (PR #133588)
via llvm-commits
llvm-commits at lists.llvm.org
Sat Mar 29 08:56:26 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lto
Author: Shilei Tian (shiltian)
<details>
<summary>Changes</summary>
`ForceImportAll` mostly works, but it uses `available_externally` linkage. For
targets that don't support object linking, like AMDGPU, this approach doesn't
work.
With this PR, all imported functions use "default" linkage. I'm not entirely
sure if that's the best approach, but for AMDGPU it's not really a problem since
we internalize everything at a later stage.
---
Full diff: https://github.com/llvm/llvm-project/pull/133588.diff
5 Files Affected:
- (modified) llvm/include/llvm/Transforms/Utils/FunctionImportUtils.h (+3)
- (modified) llvm/lib/Transforms/IPO/FunctionImport.cpp (+8-10)
- (modified) llvm/lib/Transforms/Utils/FunctionImportUtils.cpp (+12-4)
- (modified) llvm/test/Transforms/FunctionImport/adjustable_threshold.ll (+5-5)
- (modified) llvm/test/Transforms/FunctionImport/noinline.ll (+1-1)
``````````diff
diff --git a/llvm/include/llvm/Transforms/Utils/FunctionImportUtils.h b/llvm/include/llvm/Transforms/Utils/FunctionImportUtils.h
index 6d83b615d5f13..2be0a20c8afa8 100644
--- a/llvm/include/llvm/Transforms/Utils/FunctionImportUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/FunctionImportUtils.h
@@ -16,10 +16,13 @@
#include "llvm/ADT/SetVector.h"
#include "llvm/IR/ModuleSummaryIndex.h"
+#include "llvm/Support/CommandLine.h"
namespace llvm {
class Module;
+extern cl::opt<bool> ForceImportAll;
+
/// Class to handle necessary GlobalValue changes required by ThinLTO
/// function importing, including linkage changes and any necessary renaming.
class FunctionImportGlobalProcessing {
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index f0daf1a558316..41b2b53959224 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -79,10 +79,6 @@ static cl::opt<int> ImportCutoff(
"import-cutoff", cl::init(-1), cl::Hidden, cl::value_desc("N"),
cl::desc("Only import first N functions if N>=0 (default -1)"));
-static cl::opt<bool>
- ForceImportAll("force-import-all", cl::init(false), cl::Hidden,
- cl::desc("Import functions with noinline attribute"));
-
static cl::opt<float>
ImportInstrFactor("import-instr-evolution-factor", cl::init(0.7),
cl::Hidden, cl::value_desc("x"),
@@ -1663,7 +1659,8 @@ void llvm::thinLTOFinalizeInModule(Module &TheModule,
// interposable property and possibly get inlined. Simply drop
// the definition in that case.
if (GlobalValue::isAvailableExternallyLinkage(NewLinkage) &&
- GlobalValue::isInterposableLinkage(GV.getLinkage())) {
+ GlobalValue::isInterposableLinkage(GV.getLinkage()) &&
+ !ForceImportAll) {
if (!convertToDeclaration(GV))
// FIXME: Change this to collect replaced GVs and later erase
// them from the parent module once thinLTOResolvePrevailingGUID is
@@ -1680,11 +1677,12 @@ void llvm::thinLTOFinalizeInModule(Module &TheModule,
assert(GV.canBeOmittedFromSymbolTable());
GV.setVisibility(GlobalValue::HiddenVisibility);
}
-
- LLVM_DEBUG(dbgs() << "ODR fixing up linkage for `" << GV.getName()
- << "` from " << GV.getLinkage() << " to " << NewLinkage
- << "\n");
- GV.setLinkage(NewLinkage);
+ if (!ForceImportAll) {
+ LLVM_DEBUG(dbgs() << "ODR fixing up linkage for `" << GV.getName()
+ << "` from " << GV.getLinkage() << " to "
+ << NewLinkage << "\n");
+ GV.setLinkage(NewLinkage);
+ }
}
// Remove declarations from comdats, including available_externally
// as this is a declaration for the linker, and will be dropped eventually.
diff --git a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
index ae1af943bc11c..c93fa6f7fbfd8 100644
--- a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
+++ b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
@@ -12,7 +12,7 @@
//===----------------------------------------------------------------------===//
#include "llvm/Transforms/Utils/FunctionImportUtils.h"
-#include "llvm/Support/CommandLine.h"
+
using namespace llvm;
/// Uses the "source_filename" instead of a Module hash ID for the suffix of
@@ -24,6 +24,12 @@ static cl::opt<bool> UseSourceFilenameForPromotedLocals(
"This requires that the source filename has a unique name / "
"path to avoid name collisions."));
+namespace llvm {
+cl::opt<bool>
+ ForceImportAll("force-import-all", cl::init(false), cl::Hidden,
+ cl::desc("Import functions with noinline attribute"));
+}
+
/// Checks if we should import SGV as a definition, otherwise import as a
/// declaration.
bool FunctionImportGlobalProcessing::doImportAsDefinition(
@@ -147,7 +153,8 @@ FunctionImportGlobalProcessing::getLinkage(const GlobalValue *SGV,
// and/or optimization, but are turned into declarations later
// during the EliminateAvailableExternally pass.
if (doImportAsDefinition(SGV) && !isa<GlobalAlias>(SGV))
- return GlobalValue::AvailableExternallyLinkage;
+ return ForceImportAll ? GlobalValue::ExternalLinkage
+ : GlobalValue::AvailableExternallyLinkage;
// An imported external declaration stays external.
return SGV->getLinkage();
@@ -175,7 +182,7 @@ FunctionImportGlobalProcessing::getLinkage(const GlobalValue *SGV,
// equivalent, so the issue described above for weak_any does not exist,
// and the definition can be imported. It can be treated similarly
// to an imported externally visible global value.
- if (doImportAsDefinition(SGV) && !isa<GlobalAlias>(SGV))
+ if (doImportAsDefinition(SGV) && !isa<GlobalAlias>(SGV) && !ForceImportAll)
return GlobalValue::AvailableExternallyLinkage;
else
return GlobalValue::ExternalLinkage;
@@ -193,7 +200,8 @@ FunctionImportGlobalProcessing::getLinkage(const GlobalValue *SGV,
// If we are promoting the local to global scope, it is handled
// similarly to a normal externally visible global.
if (DoPromote) {
- if (doImportAsDefinition(SGV) && !isa<GlobalAlias>(SGV))
+ if (doImportAsDefinition(SGV) && !isa<GlobalAlias>(SGV) &&
+ !ForceImportAll)
return GlobalValue::AvailableExternallyLinkage;
else
return GlobalValue::ExternalLinkage;
diff --git a/llvm/test/Transforms/FunctionImport/adjustable_threshold.ll b/llvm/test/Transforms/FunctionImport/adjustable_threshold.ll
index f60758c3d3bf4..5a4a275f449d1 100644
--- a/llvm/test/Transforms/FunctionImport/adjustable_threshold.ll
+++ b/llvm/test/Transforms/FunctionImport/adjustable_threshold.ll
@@ -15,11 +15,11 @@
; RUN: opt -passes=function-import -summary-file %t3.thinlto.bc %t.bc \
; RUN: -import-instr-limit=1 -force-import-all -S \
; RUN: | FileCheck %s --check-prefix=IMPORTALL
-; IMPORTALL-DAG: define available_externally void @globalfunc1()
-; IMPORTALL-DAG: define available_externally void @trampoline()
-; IMPORTALL-DAG: define available_externally void @largefunction()
-; IMPORTALL-DAG: define available_externally hidden void @staticfunc2.llvm.0()
-; IMPORTALL-DAG: define available_externally void @globalfunc2()
+; IMPORTALL-DAG: define void @globalfunc1()
+; IMPORTALL-DAG: define void @trampoline()
+; IMPORTALL-DAG: define void @largefunction()
+; IMPORTALL-DAG: define hidden void @staticfunc2.llvm.0()
+; IMPORTALL-DAG: define void @globalfunc2()
declare void @globalfunc1()
declare void @globalfunc2()
diff --git a/llvm/test/Transforms/FunctionImport/noinline.ll b/llvm/test/Transforms/FunctionImport/noinline.ll
index 8687f1b3b5f7c..eef4d6b5c25c8 100644
--- a/llvm/test/Transforms/FunctionImport/noinline.ll
+++ b/llvm/test/Transforms/FunctionImport/noinline.ll
@@ -19,5 +19,5 @@ entry:
}
; NOIMPORT: declare void @foo(ptr)
-; IMPORT: define available_externally void @foo
+; IMPORT: define void @foo
declare void @foo(ptr) #1
``````````
</details>
https://github.com/llvm/llvm-project/pull/133588
More information about the llvm-commits
mailing list