[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:24:48 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/4] [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/4] 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/4] 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,

>From ab084e0b6a77295f6ba0128333a4a2c370d0ee16 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Thu, 16 May 2024 10:24:01 -0700
Subject: [PATCH 4/4] fix a bug

---
 llvm/lib/Analysis/ModuleSummaryAnalysis.cpp | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
index 36cff096f33aa..2e1c215be4b3f 100644
--- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -89,11 +89,11 @@ extern cl::opt<unsigned> MaxNumVTableAnnotations;
 // the set of GlobalValue references encountered. Invoked either on an
 // Instruction or a GlobalVariable (which walks its initializer).
 //
-// 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
+// Return true if any user references a block address, and sets
+// `RefLocalLinkageIFunc` to true if the analyzed value references local ifunc.
+// - If a local-linkage ifunc is referenced, the analyzed value is not eligible
+// for import.
+// - Additionally, global vars will be 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
@@ -804,8 +804,9 @@ static void computeVariableSummary(ModuleSummaryIndex &Index,
                                    SmallVectorImpl<MDNode *> &Types) {
   SetVector<ValueInfo, std::vector<ValueInfo>> RefEdges;
   SmallPtrSet<const User *, 8> Visited;
-  bool Unused = false;
-  bool HasBlockAddress = findRefEdges(Index, &V, RefEdges, Visited, Unused);
+  bool RefIFunc = false;
+  bool HasBlockAddress = findRefEdges(Index, &V, RefEdges, Visited, RefIFunc);
+  const bool NotEligibleForImport = (HasBlockAddress || RefIFunc);
   bool NonRenamableLocal = isNonRenamableLocal(V);
   GlobalValueSummary::GVFlags Flags(
       V.getLinkage(), V.getVisibility(), NonRenamableLocal,
@@ -839,7 +840,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);



More information about the llvm-commits mailing list