[llvm] [ThinLTO]Mark referencers of local ifunc not eligible for import (PR #92431)
Mingming Liu via llvm-commits
llvm-commits at lists.llvm.org
Thu May 16 10:19:43 PDT 2024
https://github.com/minglotus-6 updated https://github.com/llvm/llvm-project/pull/92431
>From 00dee25c342efd559b41baa5486dedf46ce680d1 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Thu, 16 May 2024 00:05:39 -0700
Subject: [PATCH 1/3] [ThinLTO]Mark referencers of local ifunc not eligible for
import
---
llvm/lib/Analysis/ModuleSummaryAnalysis.cpp | 48 +++++++++++++-------
llvm/test/ThinLTO/X86/ref-ifunc.ll | 49 +++++++++++++++++++++
2 files changed, 81 insertions(+), 16 deletions(-)
create mode 100644 llvm/test/ThinLTO/X86/ref-ifunc.ll
diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
index c3d15afe6d9cb..57930d288638a 100644
--- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -88,17 +88,23 @@ extern cl::opt<unsigned> MaxNumVTableAnnotations;
// Walk through the operands of a given User via worklist iteration and populate
// the set of GlobalValue references encountered. Invoked either on an
// Instruction or a GlobalVariable (which walks its initializer).
-// Return true if any of the operands contains blockaddress. This is important
-// to know when computing summary for global var, because if global variable
-// references basic block address we can't import it separately from function
-// containing that basic block. For simplicity we currently don't import such
-// global vars at all. When importing function we aren't interested if any
-// instruction in it takes an address of any basic block, because instruction
-// can only take an address of basic block located in the same function.
+//
+// Return true if any user makes the analyzed instruction or global variable not
+// eligible to import.
+// - If a local-linkage ifunc is referenced, mark the analyzed value as not
+// eligible for import.
+// - Additionally, global vars are marked as not eligible to import if they
+// references references basic block address, because we can't import it
+// separately from function containing that basic block. For simplicity we
+// currently don't import such global vars at all. When importing functions we
+// aren't interested if any instruction in it takes an address of any basic
+// block, because instruction can only take an address of basic block located in
+// the same function.
static bool findRefEdges(ModuleSummaryIndex &Index, const User *CurUser,
SetVector<ValueInfo, std::vector<ValueInfo>> &RefEdges,
SmallPtrSet<const User *, 8> &Visited) {
bool HasBlockAddress = false;
+ bool RefLocalLinkageIFunc = false;
SmallVector<const User *, 32> Worklist;
if (Visited.insert(CurUser).second)
Worklist.push_back(CurUser);
@@ -119,8 +125,18 @@ static bool findRefEdges(ModuleSummaryIndex &Index, const User *CurUser,
// We have a reference to a global value. This should be added to
// the reference set unless it is a callee. Callees are handled
// specially by WriteFunction and are added to a separate list.
- if (!(CB && CB->isCallee(&OI)))
+ if (!(CB && CB->isCallee(&OI))) {
+ // If an ifunc has local linkage, do not add it into ref edges and
+ // mark its referencer not eligible for import. An ifunc doesn't have
+ // summary and ThinLTO cannot promote it; importing the referencer may
+ // cause linkage errors.
+ if (auto *GI = dyn_cast_if_present<GlobalIFunc>(GV);
+ GI && GI->hasLocalLinkage()) {
+ RefLocalLinkageIFunc = true;
+ continue;
+ }
RefEdges.insert(Index.getOrInsertValueInfo(GV));
+ }
continue;
}
if (Visited.insert(Operand).second)
@@ -145,7 +161,7 @@ static bool findRefEdges(ModuleSummaryIndex &Index, const User *CurUser,
}
}
}
- return HasBlockAddress;
+ return HasBlockAddress || RefLocalLinkageIFunc;
}
static CalleeInfo::HotnessType getHotness(uint64_t ProfileCount,
@@ -313,7 +329,7 @@ static void computeFunctionSummary(
// Add personality function, prefix data and prologue data to function's ref
// list.
- findRefEdges(Index, &F, RefEdges, Visited);
+ bool HasIFunc = findRefEdges(Index, &F, RefEdges, Visited);
std::vector<const Instruction *> NonVolatileLoads;
std::vector<const Instruction *> NonVolatileStores;
@@ -326,7 +342,7 @@ static void computeFunctionSummary(
bool HasInlineAsmMaybeReferencingInternal = false;
bool HasIndirBranchToBlockAddress = false;
- bool HasIFuncCall = false;
+
bool HasUnknownCall = false;
bool MayThrow = false;
for (const BasicBlock &BB : F) {
@@ -372,11 +388,11 @@ static void computeFunctionSummary(
// of calling it we should add GV to RefEdges directly.
RefEdges.insert(Index.getOrInsertValueInfo(GV));
else if (auto *U = dyn_cast<User>(Stored))
- findRefEdges(Index, U, RefEdges, Visited);
+ HasIFunc |= findRefEdges(Index, U, RefEdges, Visited);
continue;
}
}
- findRefEdges(Index, &I, RefEdges, Visited);
+ HasIFunc |= findRefEdges(Index, &I, RefEdges, Visited);
const auto *CB = dyn_cast<CallBase>(&I);
if (!CB) {
if (I.mayThrow())
@@ -450,7 +466,7 @@ static void computeFunctionSummary(
// Non-local ifunc is not cloned and does not have the issue.
if (auto *GI = dyn_cast_if_present<GlobalIFunc>(CalledValue))
if (GI->hasLocalLinkage())
- HasIFuncCall = true;
+ HasIFunc = true;
// Skip inline assembly calls.
if (CI && CI->isInlineAsm())
continue;
@@ -555,7 +571,7 @@ static void computeFunctionSummary(
SmallPtrSet<const User *, 8> &Cache) {
for (const auto *I : Instrs) {
Cache.erase(I);
- findRefEdges(Index, I, Edges, Cache);
+ HasIFunc = findRefEdges(Index, I, Edges, Cache);
}
};
@@ -633,7 +649,7 @@ static void computeFunctionSummary(
bool NonRenamableLocal = isNonRenamableLocal(F);
bool NotEligibleForImport = NonRenamableLocal ||
HasInlineAsmMaybeReferencingInternal ||
- HasIndirBranchToBlockAddress || HasIFuncCall;
+ HasIndirBranchToBlockAddress || HasIFunc;
GlobalValueSummary::GVFlags Flags(
F.getLinkage(), F.getVisibility(), NotEligibleForImport,
/* Live = */ false, F.isDSOLocal(), F.canBeOmittedFromSymbolTable(),
diff --git a/llvm/test/ThinLTO/X86/ref-ifunc.ll b/llvm/test/ThinLTO/X86/ref-ifunc.ll
new file mode 100644
index 0000000000000..2447c1396a765
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/ref-ifunc.ll
@@ -0,0 +1,49 @@
+; RUN: opt -module-summary %s -o %t.bc
+
+; RUN: llvm-dis %t.bc -o - | FileCheck %s
+
+; Tests that caller is not eligible to import and it doesn't have refs to ifunc 'callee'
+
+; CHECK: gv: (name: "caller", summaries: (function: ({{.*}}, flags: ({{.*}}notEligibleToImport: 1
+; CHECK-NOT: refs
+; CHECK-SAME: guid = 16677772384402303968
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+ at __cpu_model = external global { i32, i32, i32, [1 x i32] }
+
+ at callee = internal ifunc void(), ptr @callee.resolver
+
+define void @dispatch(ptr %func) {
+ tail call void %func()
+ ret void
+}
+
+
+define void @caller() {
+entry:
+ tail call void @dispatch(ptr @callee)
+ ret void
+}
+
+define internal ptr @callee.resolver() {
+resolver_entry:
+ tail call void @__cpu_indicator_init()
+ %0 = load i32, ptr getelementptr inbounds ({ i32, i32, i32, [1 x i32] }, ptr @__cpu_model, i64 0, i32 3, i64 0)
+ %1 = and i32 %0, 1024
+ %.not = icmp eq i32 %1, 0
+ %func_sel = select i1 %.not, ptr @callee.default.1, ptr @callee.avx2.0
+ ret ptr %func_sel
+}
+
+define internal void @callee.default.1(i32 %a) {
+ ret void
+}
+
+define internal void @callee.avx2.0(i32 %a) {
+ ret void
+}
+
+declare i32 @rand()
+declare void @__cpu_indicator_init()
>From d2e2814064774eb4fc4678276ce424bee18dd9d7 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Thu, 16 May 2024 10:15:35 -0700
Subject: [PATCH 2/3] clean up test
---
llvm/test/ThinLTO/X86/ref-ifunc.ll | 3 ---
1 file changed, 3 deletions(-)
diff --git a/llvm/test/ThinLTO/X86/ref-ifunc.ll b/llvm/test/ThinLTO/X86/ref-ifunc.ll
index 2447c1396a765..714ae83cb1abb 100644
--- a/llvm/test/ThinLTO/X86/ref-ifunc.ll
+++ b/llvm/test/ThinLTO/X86/ref-ifunc.ll
@@ -20,9 +20,7 @@ define void @dispatch(ptr %func) {
ret void
}
-
define void @caller() {
-entry:
tail call void @dispatch(ptr @callee)
ret void
}
@@ -45,5 +43,4 @@ define internal void @callee.avx2.0(i32 %a) {
ret void
}
-declare i32 @rand()
declare void @__cpu_indicator_init()
>From 1cb811a7b1daf92754d59f94167e5c6d38763cda Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Thu, 16 May 2024 10:19:11 -0700
Subject: [PATCH 3/3] separate 'HasBlockAddress' and 'HasIFunc'
---
llvm/lib/Analysis/ModuleSummaryAnalysis.cpp | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
index 57930d288638a..36cff096f33aa 100644
--- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -102,9 +102,9 @@ extern cl::opt<unsigned> MaxNumVTableAnnotations;
// the same function.
static bool findRefEdges(ModuleSummaryIndex &Index, const User *CurUser,
SetVector<ValueInfo, std::vector<ValueInfo>> &RefEdges,
- SmallPtrSet<const User *, 8> &Visited) {
+ SmallPtrSet<const User *, 8> &Visited,
+ bool &RefLocalLinkageIFunc) {
bool HasBlockAddress = false;
- bool RefLocalLinkageIFunc = false;
SmallVector<const User *, 32> Worklist;
if (Visited.insert(CurUser).second)
Worklist.push_back(CurUser);
@@ -161,7 +161,7 @@ static bool findRefEdges(ModuleSummaryIndex &Index, const User *CurUser,
}
}
}
- return HasBlockAddress || RefLocalLinkageIFunc;
+ return HasBlockAddress;
}
static CalleeInfo::HotnessType getHotness(uint64_t ProfileCount,
@@ -329,7 +329,8 @@ static void computeFunctionSummary(
// Add personality function, prefix data and prologue data to function's ref
// list.
- bool HasIFunc = findRefEdges(Index, &F, RefEdges, Visited);
+ bool HasIFunc = false;
+ findRefEdges(Index, &F, RefEdges, Visited, HasIFunc);
std::vector<const Instruction *> NonVolatileLoads;
std::vector<const Instruction *> NonVolatileStores;
@@ -388,11 +389,11 @@ static void computeFunctionSummary(
// of calling it we should add GV to RefEdges directly.
RefEdges.insert(Index.getOrInsertValueInfo(GV));
else if (auto *U = dyn_cast<User>(Stored))
- HasIFunc |= findRefEdges(Index, U, RefEdges, Visited);
+ findRefEdges(Index, U, RefEdges, Visited, HasIFunc);
continue;
}
}
- HasIFunc |= findRefEdges(Index, &I, RefEdges, Visited);
+ findRefEdges(Index, &I, RefEdges, Visited, HasIFunc);
const auto *CB = dyn_cast<CallBase>(&I);
if (!CB) {
if (I.mayThrow())
@@ -571,7 +572,7 @@ static void computeFunctionSummary(
SmallPtrSet<const User *, 8> &Cache) {
for (const auto *I : Instrs) {
Cache.erase(I);
- HasIFunc = findRefEdges(Index, I, Edges, Cache);
+ findRefEdges(Index, I, Edges, Cache, HasIFunc);
}
};
@@ -803,7 +804,8 @@ static void computeVariableSummary(ModuleSummaryIndex &Index,
SmallVectorImpl<MDNode *> &Types) {
SetVector<ValueInfo, std::vector<ValueInfo>> RefEdges;
SmallPtrSet<const User *, 8> Visited;
- bool HasBlockAddress = findRefEdges(Index, &V, RefEdges, Visited);
+ bool Unused = false;
+ bool HasBlockAddress = findRefEdges(Index, &V, RefEdges, Visited, Unused);
bool NonRenamableLocal = isNonRenamableLocal(V);
GlobalValueSummary::GVFlags Flags(
V.getLinkage(), V.getVisibility(), NonRenamableLocal,
More information about the llvm-commits
mailing list