[llvm] fa9b1be - [ThinLTO]Mark referencers of local ifunc not eligible for import (#92431)

via llvm-commits llvm-commits at lists.llvm.org
Thu May 16 16:16:23 PDT 2024


Author: Mingming Liu
Date: 2024-05-16T16:16:19-07:00
New Revision: fa9b1be45088dce1e4b602d451f118128b94237b

URL: https://github.com/llvm/llvm-project/commit/fa9b1be45088dce1e4b602d451f118128b94237b
DIFF: https://github.com/llvm/llvm-project/commit/fa9b1be45088dce1e4b602d451f118128b94237b.diff

LOG: [ThinLTO]Mark referencers of local ifunc not eligible for import (#92431)

If an ifunc has local linkage, do not add it into ref edges and mark its
referencer (a function or global variable) not eligible for import. An
ifunc doesn't have summary and ThinLTO cannot promote it. Importing the
referencer may cause linkage errors.

To reference a similar fix, https://reviews.llvm.org/D158961 marks
callers of local ifunc not eligible for import to fix
https://github.com/llvm/llvm-project/issues/58740

Added: 
    llvm/test/ThinLTO/X86/ref-ifunc.ll

Modified: 
    llvm/lib/Analysis/ModuleSummaryAnalysis.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
index c3d15afe6d9cb..c863936a51882 100644
--- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -95,9 +95,12 @@ extern cl::opt<unsigned> MaxNumVTableAnnotations;
 // 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.
+// Set `RefLocalLinkageIFunc` to true if the analyzed value references a
+// local-linkage ifunc.
 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;
   SmallVector<const User *, 32> Worklist;
   if (Visited.insert(CurUser).second)
@@ -119,8 +122,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
+          // sets `RefLocalLinkageIFunc` to true. The referencer is 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)
@@ -313,7 +326,8 @@ static void computeFunctionSummary(
 
   // Add personality function, prefix data and prologue data to function's ref
   // list.
-  findRefEdges(Index, &F, RefEdges, Visited);
+  bool HasLocalIFuncCallOrRef = false;
+  findRefEdges(Index, &F, RefEdges, Visited, HasLocalIFuncCallOrRef);
   std::vector<const Instruction *> NonVolatileLoads;
   std::vector<const Instruction *> NonVolatileStores;
 
@@ -326,7 +340,6 @@ 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 +385,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);
+            findRefEdges(Index, U, RefEdges, Visited, HasLocalIFuncCallOrRef);
           continue;
         }
       }
-      findRefEdges(Index, &I, RefEdges, Visited);
+      findRefEdges(Index, &I, RefEdges, Visited, HasLocalIFuncCallOrRef);
       const auto *CB = dyn_cast<CallBase>(&I);
       if (!CB) {
         if (I.mayThrow())
@@ -450,7 +463,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;
+            HasLocalIFuncCallOrRef = true;
         // Skip inline assembly calls.
         if (CI && CI->isInlineAsm())
           continue;
@@ -555,7 +568,7 @@ static void computeFunctionSummary(
                            SmallPtrSet<const User *, 8> &Cache) {
       for (const auto *I : Instrs) {
         Cache.erase(I);
-        findRefEdges(Index, I, Edges, Cache);
+        findRefEdges(Index, I, Edges, Cache, HasLocalIFuncCallOrRef);
       }
     };
 
@@ -631,9 +644,9 @@ static void computeFunctionSummary(
 #endif
 
   bool NonRenamableLocal = isNonRenamableLocal(F);
-  bool NotEligibleForImport = NonRenamableLocal ||
-                              HasInlineAsmMaybeReferencingInternal ||
-                              HasIndirBranchToBlockAddress || HasIFuncCall;
+  bool NotEligibleForImport =
+      NonRenamableLocal || HasInlineAsmMaybeReferencingInternal ||
+      HasIndirBranchToBlockAddress || HasLocalIFuncCallOrRef;
   GlobalValueSummary::GVFlags Flags(
       F.getLinkage(), F.getVisibility(), NotEligibleForImport,
       /* Live = */ false, F.isDSOLocal(), F.canBeOmittedFromSymbolTable(),
@@ -787,7 +800,10 @@ 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 RefLocalIFunc = false;
+  bool HasBlockAddress =
+      findRefEdges(Index, &V, RefEdges, Visited, RefLocalIFunc);
+  const bool NotEligibleForImport = (HasBlockAddress || RefLocalIFunc);
   bool NonRenamableLocal = isNonRenamableLocal(V);
   GlobalValueSummary::GVFlags Flags(
       V.getLinkage(), V.getVisibility(), NonRenamableLocal,
@@ -821,7 +837,7 @@ static void computeVariableSummary(ModuleSummaryIndex &Index,
                                                          RefEdges.takeVector());
   if (NonRenamableLocal)
     CantBePromoted.insert(V.getGUID());
-  if (HasBlockAddress)
+  if (NotEligibleForImport)
     GVarSummary->setNotEligibleToImport();
   if (!VTableFuncs.empty())
     GVarSummary->setVTableFuncs(VTableFuncs);

diff  --git a/llvm/test/ThinLTO/X86/ref-ifunc.ll b/llvm/test/ThinLTO/X86/ref-ifunc.ll
new file mode 100644
index 0000000000000..a43c080fbf3ad
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/ref-ifunc.ll
@@ -0,0 +1,52 @@
+; RUN: opt -module-summary %s -o %t.bc
+
+; RUN: llvm-dis %t.bc -o - | FileCheck %s
+
+; Tests that var and caller are not eligible to import and they don't have refs to ifunc 'callee'
+
+; CHECK: gv: (name: "var", summaries: (variable: ({{.*}}, flags: ({{.*}}notEligibleToImport: 1
+; CHECK-NOT: refs
+; CHECK-SAME: guid = 7919382516565939378
+
+; 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
+
+ at var = constant { [1 x ptr] } { [1 x ptr] [ptr @callee]}
+
+define void @dispatch(ptr %func) {
+    tail call void %func()
+    ret void
+}
+
+define void @caller() {
+  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 void @__cpu_indicator_init()


        


More information about the llvm-commits mailing list