[llvm] 901183b - [IPO] Opt-in local clones for thinlto imports
Mircea Trofin via llvm-commits
llvm-commits at lists.llvm.org
Thu May 11 15:56:55 PDT 2023
Author: Mircea Trofin
Date: 2023-05-11T15:56:43-07:00
New Revision: 901183b5964c84d3ab6a3896b2c0ced7ffbd00f5
URL: https://github.com/llvm/llvm-project/commit/901183b5964c84d3ab6a3896b2c0ced7ffbd00f5
DIFF: https://github.com/llvm/llvm-project/commit/901183b5964c84d3ab6a3896b2c0ced7ffbd00f5.diff
LOG: [IPO] Opt-in local clones for thinlto imports
ThinLTO imports (which appear as `available_externally`) that survive
inlining get deleted. With today's inliner that's reasonable, because
the way the function would be inlined into in other modules would be the
same - because of the bottom-up traversal assumption, and the fact that
the inliner doesn't take into account surrounding context [*]. The
ModuleInliner invalidates the first assumption, and the ML inliner the
second.
This patch adds a way to opt-in a module to keep its variant of an
imported function, even if it survived past inlining.
[*] Almost. Deferred inlining is an exception which can lead to
(empirically) infrequent discrepancies.
Differential Revision: https://reviews.llvm.org/D150148
Added:
llvm/test/Transforms/EliminateAvailableExternally/transform-to-local.ll
Modified:
llvm/lib/Transforms/IPO/ElimAvailExtern.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/IPO/ElimAvailExtern.cpp b/llvm/lib/Transforms/IPO/ElimAvailExtern.cpp
index 92dbeb9daf88c..2b34d3b5a56ea 100644
--- a/llvm/lib/Transforms/IPO/ElimAvailExtern.cpp
+++ b/llvm/lib/Transforms/IPO/ElimAvailExtern.cpp
@@ -12,22 +12,82 @@
//===----------------------------------------------------------------------===//
#include "llvm/Transforms/IPO/ElimAvailExtern.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/Statistic.h"
#include "llvm/IR/Constant.h"
+#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/GlobalValue.h"
#include "llvm/IR/GlobalVariable.h"
+#include "llvm/IR/MDBuilder.h"
#include "llvm/IR/Module.h"
+#include "llvm/Support/CommandLine.h"
#include "llvm/Transforms/IPO.h"
#include "llvm/Transforms/Utils/GlobalStatus.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
using namespace llvm;
#define DEBUG_TYPE "elim-avail-extern"
-STATISTIC(NumFunctions, "Number of functions removed");
+cl::opt<bool> ConvertToLocal(
+ "avail-extern-to-local", cl::Hidden,
+ cl::desc("Convert available_externally into locals, renaming them "
+ "to avoid link-time clashes."));
+
+STATISTIC(NumRemovals, "Number of functions removed");
+STATISTIC(NumConversions, "Number of functions converted");
STATISTIC(NumVariables, "Number of global variables removed");
+void deleteFunction(Function &F) {
+ // This will set the linkage to external
+ F.deleteBody();
+ ++NumRemovals;
+}
+
+/// Create a copy of the thinlto import, mark it local, and redirect direct
+/// calls to the copy. Only direct calls are replaced, so that e.g. indirect
+/// call function pointer tests would use the global identity of the function.
+///
+/// Currently, Value Profiling ("VP") MD_prof data isn't updated to refer to the
+/// clone's GUID (which will be
diff erent, because the name and linkage is
+///
diff erent), under the assumption that the last consumer of this data is
+/// upstream the pipeline (e.g. ICP).
+static void convertToLocalCopy(Module &M, Function &F) {
+ assert(F.hasAvailableExternallyLinkage());
+ assert(!F.isDeclaration());
+ // If we can't find a single use that's a call, just delete the function.
+ if (F.uses().end() == llvm::find_if(F.uses(), [&](Use &U) {
+ return isa<CallBase>(U.getUser());
+ }))
+ return deleteFunction(F);
+
+ auto OrigName = F.getName().str();
+ // Build a new name. We still need the old name (see below).
+ // We could just rely on internal linking allowing 2 modules have internal
+ // functions with the same name, but that just creates more trouble than
+ // necessary e.g. distinguishing profiles or debugging. Instead, we append the
+ // module identifier.
+ auto NewName = OrigName + ".__uniq" + getUniqueModuleId(&M);
+ F.setName(NewName);
+ if (auto *SP = F.getSubprogram())
+ SP->replaceLinkageName(MDString::get(F.getParent()->getContext(), NewName));
+
+ F.setLinkage(GlobalValue::InternalLinkage);
+ // Now make a declaration for the old name. We'll use it if there are non-call
+ // uses. For those, it would be incorrect to replace them with the local copy:
+ // for example, one such use could be taking the address of the function and
+ // passing it to an external function, which, in turn, might compare the
+ // function pointer to the original (non-local) function pointer, e.g. as part
+ // of indirect call promotion.
+ auto *Decl =
+ Function::Create(F.getFunctionType(), GlobalValue::ExternalLinkage,
+ F.getAddressSpace(), OrigName, F.getParent());
+ F.replaceUsesWithIf(Decl,
+ [&](Use &U) { return !isa<CallBase>(U.getUser()); });
+ ++NumConversions;
+}
+
static bool eliminateAvailableExternally(Module &M) {
bool Changed = false;
@@ -43,19 +103,21 @@ static bool eliminateAvailableExternally(Module &M) {
}
GV.removeDeadConstantUsers();
GV.setLinkage(GlobalValue::ExternalLinkage);
- NumVariables++;
+ ++NumVariables;
Changed = true;
}
// Drop the bodies of available externally functions.
- for (Function &F : M) {
- if (!F.hasAvailableExternallyLinkage())
+ for (Function &F : llvm::make_early_inc_range(M)) {
+ if (F.isDeclaration() || !F.hasAvailableExternallyLinkage())
continue;
- if (!F.isDeclaration())
- // This will set the linkage to external
- F.deleteBody();
+
+ if (ConvertToLocal)
+ convertToLocalCopy(M, F);
+ else
+ deleteFunction(F);
+
F.removeDeadConstantUsers();
- NumFunctions++;
Changed = true;
}
diff --git a/llvm/test/Transforms/EliminateAvailableExternally/transform-to-local.ll b/llvm/test/Transforms/EliminateAvailableExternally/transform-to-local.ll
new file mode 100644
index 0000000000000..786cc260d331c
--- /dev/null
+++ b/llvm/test/Transforms/EliminateAvailableExternally/transform-to-local.ll
@@ -0,0 +1,29 @@
+; REQUIRES: asserts
+; RUN: opt -passes=elim-avail-extern -avail-extern-to-local -stats -S 2>&1 < %s | FileCheck %s
+
+
+declare void @call_out(ptr %fct)
+
+define available_externally hidden void @f() {
+ ret void
+}
+
+define available_externally hidden void @g() {
+ ret void
+}
+
+define void @hello(ptr %g) {
+ call void @f()
+ %f = load ptr, ptr @f
+ call void @call_out(ptr %f)
+ ret void
+}
+
+; CHECK: define internal void @f.__uniq.{{[0-9|a-f]*}}()
+; CHECK: declare hidden void @g()
+; CHECK: call void @f.__uniq.{{[0-9|a-f]*}}()
+; CHECK-NEXT: load ptr, ptr @f
+; CHECK-NEXT: call void @call_out(ptr %f)
+; CHECK: Statistics Collected
+; CHECK: 1 elim-avail-extern - Number of functions converted
+; CHECK: 1 elim-avail-extern - Number of functions removed
\ No newline at end of file
More information about the llvm-commits
mailing list