Skip to content

Commit 38b001c

Browse files
committed
[ThinLTO] Properly support targets that require importing all external functions
`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.
1 parent 847cdd4 commit 38b001c

File tree

5 files changed

+29
-20
lines changed

5 files changed

+29
-20
lines changed

llvm/include/llvm/Transforms/Utils/FunctionImportUtils.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,13 @@
1616

1717
#include "llvm/ADT/SetVector.h"
1818
#include "llvm/IR/ModuleSummaryIndex.h"
19+
#include "llvm/Support/CommandLine.h"
1920

2021
namespace llvm {
2122
class Module;
2223

24+
extern cl::opt<bool> ForceImportAll;
25+
2326
/// Class to handle necessary GlobalValue changes required by ThinLTO
2427
/// function importing, including linkage changes and any necessary renaming.
2528
class FunctionImportGlobalProcessing {

llvm/lib/Transforms/IPO/FunctionImport.cpp

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,6 @@ static cl::opt<int> ImportCutoff(
7979
"import-cutoff", cl::init(-1), cl::Hidden, cl::value_desc("N"),
8080
cl::desc("Only import first N functions if N>=0 (default -1)"));
8181

82-
static cl::opt<bool>
83-
ForceImportAll("force-import-all", cl::init(false), cl::Hidden,
84-
cl::desc("Import functions with noinline attribute"));
85-
8682
static cl::opt<float>
8783
ImportInstrFactor("import-instr-evolution-factor", cl::init(0.7),
8884
cl::Hidden, cl::value_desc("x"),
@@ -1663,7 +1659,8 @@ void llvm::thinLTOFinalizeInModule(Module &TheModule,
16631659
// interposable property and possibly get inlined. Simply drop
16641660
// the definition in that case.
16651661
if (GlobalValue::isAvailableExternallyLinkage(NewLinkage) &&
1666-
GlobalValue::isInterposableLinkage(GV.getLinkage())) {
1662+
GlobalValue::isInterposableLinkage(GV.getLinkage()) &&
1663+
!ForceImportAll) {
16671664
if (!convertToDeclaration(GV))
16681665
// FIXME: Change this to collect replaced GVs and later erase
16691666
// them from the parent module once thinLTOResolvePrevailingGUID is
@@ -1680,11 +1677,12 @@ void llvm::thinLTOFinalizeInModule(Module &TheModule,
16801677
assert(GV.canBeOmittedFromSymbolTable());
16811678
GV.setVisibility(GlobalValue::HiddenVisibility);
16821679
}
1683-
1684-
LLVM_DEBUG(dbgs() << "ODR fixing up linkage for `" << GV.getName()
1685-
<< "` from " << GV.getLinkage() << " to " << NewLinkage
1686-
<< "\n");
1687-
GV.setLinkage(NewLinkage);
1680+
if (!ForceImportAll) {
1681+
LLVM_DEBUG(dbgs() << "ODR fixing up linkage for `" << GV.getName()
1682+
<< "` from " << GV.getLinkage() << " to "
1683+
<< NewLinkage << "\n");
1684+
GV.setLinkage(NewLinkage);
1685+
}
16881686
}
16891687
// Remove declarations from comdats, including available_externally
16901688
// as this is a declaration for the linker, and will be dropped eventually.

llvm/lib/Transforms/Utils/FunctionImportUtils.cpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
//===----------------------------------------------------------------------===//
1313

1414
#include "llvm/Transforms/Utils/FunctionImportUtils.h"
15-
#include "llvm/Support/CommandLine.h"
15+
1616
using namespace llvm;
1717

1818
/// Uses the "source_filename" instead of a Module hash ID for the suffix of
@@ -24,6 +24,12 @@ static cl::opt<bool> UseSourceFilenameForPromotedLocals(
2424
"This requires that the source filename has a unique name / "
2525
"path to avoid name collisions."));
2626

27+
namespace llvm {
28+
cl::opt<bool>
29+
ForceImportAll("force-import-all", cl::init(false), cl::Hidden,
30+
cl::desc("Import functions with noinline attribute"));
31+
}
32+
2733
/// Checks if we should import SGV as a definition, otherwise import as a
2834
/// declaration.
2935
bool FunctionImportGlobalProcessing::doImportAsDefinition(
@@ -147,7 +153,8 @@ FunctionImportGlobalProcessing::getLinkage(const GlobalValue *SGV,
147153
// and/or optimization, but are turned into declarations later
148154
// during the EliminateAvailableExternally pass.
149155
if (doImportAsDefinition(SGV) && !isa<GlobalAlias>(SGV))
150-
return GlobalValue::AvailableExternallyLinkage;
156+
return ForceImportAll ? GlobalValue::ExternalLinkage
157+
: GlobalValue::AvailableExternallyLinkage;
151158
// An imported external declaration stays external.
152159
return SGV->getLinkage();
153160

@@ -175,7 +182,7 @@ FunctionImportGlobalProcessing::getLinkage(const GlobalValue *SGV,
175182
// equivalent, so the issue described above for weak_any does not exist,
176183
// and the definition can be imported. It can be treated similarly
177184
// to an imported externally visible global value.
178-
if (doImportAsDefinition(SGV) && !isa<GlobalAlias>(SGV))
185+
if (doImportAsDefinition(SGV) && !isa<GlobalAlias>(SGV) && !ForceImportAll)
179186
return GlobalValue::AvailableExternallyLinkage;
180187
else
181188
return GlobalValue::ExternalLinkage;
@@ -193,7 +200,8 @@ FunctionImportGlobalProcessing::getLinkage(const GlobalValue *SGV,
193200
// If we are promoting the local to global scope, it is handled
194201
// similarly to a normal externally visible global.
195202
if (DoPromote) {
196-
if (doImportAsDefinition(SGV) && !isa<GlobalAlias>(SGV))
203+
if (doImportAsDefinition(SGV) && !isa<GlobalAlias>(SGV) &&
204+
!ForceImportAll)
197205
return GlobalValue::AvailableExternallyLinkage;
198206
else
199207
return GlobalValue::ExternalLinkage;

llvm/test/Transforms/FunctionImport/adjustable_threshold.ll

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@
1515
; RUN: opt -passes=function-import -summary-file %t3.thinlto.bc %t.bc \
1616
; RUN: -import-instr-limit=1 -force-import-all -S \
1717
; RUN: | FileCheck %s --check-prefix=IMPORTALL
18-
; IMPORTALL-DAG: define available_externally void @globalfunc1()
19-
; IMPORTALL-DAG: define available_externally void @trampoline()
20-
; IMPORTALL-DAG: define available_externally void @largefunction()
21-
; IMPORTALL-DAG: define available_externally hidden void @staticfunc2.llvm.0()
22-
; IMPORTALL-DAG: define available_externally void @globalfunc2()
18+
; IMPORTALL-DAG: define void @globalfunc1()
19+
; IMPORTALL-DAG: define void @trampoline()
20+
; IMPORTALL-DAG: define void @largefunction()
21+
; IMPORTALL-DAG: define hidden void @staticfunc2.llvm.0()
22+
; IMPORTALL-DAG: define void @globalfunc2()
2323

2424
declare void @globalfunc1()
2525
declare void @globalfunc2()

llvm/test/Transforms/FunctionImport/noinline.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,5 @@ entry:
1919
}
2020

2121
; NOIMPORT: declare void @foo(ptr)
22-
; IMPORT: define available_externally void @foo
22+
; IMPORT: define void @foo
2323
declare void @foo(ptr) #1

0 commit comments

Comments
 (0)