Resolve {GlobalValue,GloalIndirectSymol}::getBaseObject confusion

While both GlobalAlias and GlobalIFunc are GlobalIndirectSymbol, their
`getIndirectSymbol()` usage is quite different (GlobalIFunc's resolver
is an entity different from GlobalIFunc itself).

As discussed on https://lists.llvm.org/pipermail/llvm-dev/2020-September/144904.html
("[IR] Modelling of GlobalIFunc"), the name `getBaseObject` is confusing when
used with GlobalIFunc.

To resolve the confusion:

* Move GloalIndirectSymol::getBaseObject to GlobalAlias:: (GlobalIFunc should use `getResolver` instead)
* Change GlobalValue::getBaseObject not to inspect GlobalIFunc. Note: the function has 7 references.
* Add GlobalIFunc::getResolverFunction to peel off potential ConstantExpr indirection
  (`strlen` in `test/LTO/Resolution/X86/ifunc.ll`)

Note: GlobalIFunc::getResolver (like GlobalAlias::getAliasee which does not peel
off ConstantExpr indirection) is kept to be used by ValueEnumerator.

Reviewed By: ibookstein

Differential Revision: https://reviews.llvm.org/D109792

diff --git a/llvm/include/llvm/IR/GlobalAlias.h b/llvm/include/llvm/IR/GlobalAlias.h
index f2d9b96..f2c56f6 100644 (file)
--- a/llvm/include/llvm/IR/GlobalAlias.h
+++ b/llvm/include/llvm/IR/GlobalAlias.h
@@ -77,6 +77,12 @@ public:
     return getIndirectSymbol();
   }
 
+  const GlobalObject *getBaseObject() const;
+  GlobalObject *getBaseObject() {
+    return const_cast<GlobalObject *>(
+        static_cast<const GlobalAlias *>(this)->getBaseObject());
+  }
+
   static bool isValidLinkage(LinkageTypes L) {
     return isExternalLinkage(L) || isLocalLinkage(L) ||
       isWeakLinkage(L) || isLinkOnceLinkage(L);

diff --git a/llvm/include/llvm/IR/GlobalIFunc.h b/llvm/include/llvm/IR/GlobalIFunc.h
index ddd29c8..3931693 100644 (file)
--- a/llvm/include/llvm/IR/GlobalIFunc.h
+++ b/llvm/include/llvm/IR/GlobalIFunc.h
@@ -57,11 +57,15 @@ public:
   void setResolver(Constant *Resolver) {
     setIndirectSymbol(Resolver);
   }
-  const Constant *getResolver() const {
-    return getIndirectSymbol();
-  }
-  Constant *getResolver() {
-    return getIndirectSymbol();
+  const Constant *getResolver() const { return getIndirectSymbol(); }
+  Constant *getResolver() { return getIndirectSymbol(); }
+
+  // Return the resolver function after peeling off potential ConstantExpr
+  // indirection.
+  const Function *getResolverFunction() const;
+  Function *getResolverFunction() {
+    return const_cast<Function *>(
+        static_cast<const GlobalIFunc *>(this)->getResolverFunction());
   }
 
   // Methods for support type inquiry through isa, cast, and dyn_cast:

diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp
index b1c6dcc..a8dc0e2 100644 (file)
--- a/llvm/lib/IR/Globals.cpp
+++ b/llvm/lib/IR/Globals.cpp
@@ -283,7 +283,7 @@ bool GlobalObject::canIncreaseAlignment() const {
 const GlobalObject *GlobalValue::getBaseObject() const {
   if (auto *GO = dyn_cast<GlobalObject>(this))
     return GO;
-  if (auto *GA = dyn_cast<GlobalIndirectSymbol>(this))
+  if (auto *GA = dyn_cast<GlobalAlias>(this))
     return GA->getBaseObject();
   return nullptr;
 }
@@ -464,11 +464,6 @@ findBaseObject(const Constant *C, DenseSet<const GlobalAlias *> &Aliases) {
   return nullptr;
 }
 
-const GlobalObject *GlobalIndirectSymbol::getBaseObject() const {
-  DenseSet<const GlobalAlias *> Aliases;
-  return findBaseObject(getOperand(0), Aliases);
-}
-
 //===----------------------------------------------------------------------===//
 // GlobalAlias Implementation
 //===----------------------------------------------------------------------===//
@@ -524,6 +519,11 @@ void GlobalAlias::setAliasee(Constant *Aliasee) {
   setIndirectSymbol(Aliasee);
 }
 
+const GlobalObject *GlobalAlias::getBaseObject() const {
+  DenseSet<const GlobalAlias *> Aliases;
+  return findBaseObject(getOperand(0), Aliases);
+}
+
 //===----------------------------------------------------------------------===//
 // GlobalIFunc Implementation
 //===----------------------------------------------------------------------===//
@@ -550,3 +550,8 @@ void GlobalIFunc::removeFromParent() {
 void GlobalIFunc::eraseFromParent() {
   getParent()->getIFuncList().erase(getIterator());
 }
+
+const Function *GlobalIFunc::getResolverFunction() const {
+  DenseSet<const GlobalAlias *> Aliases;
+  return cast<Function>(findBaseObject(getIndirectSymbol(), Aliases));
+}

diff --git a/llvm/lib/Object/IRSymtab.cpp b/llvm/lib/Object/IRSymtab.cpp
index 746b008..9208f90 100644 (file)
--- a/llvm/lib/Object/IRSymtab.cpp
+++ b/llvm/lib/Object/IRSymtab.cpp
@@ -284,9 +284,13 @@ Error Builder::addSymbol(const ModuleSymbolTable &Msymtab,
   }
 
   const GlobalObject *Base = GV->getBaseObject();
-  if (!Base)
-    return make_error<StringError>("Unable to determine comdat of alias!",
-                                   inconvertibleErrorCode());
+  if (!Base) {
+    if (isa<GlobalIFunc>(GV))
+      Base = cast<GlobalIFunc>(GV)->getResolverFunction();
+    if (!Base)
+      return make_error<StringError>("Unable to determine comdat of alias!",
+                                     inconvertibleErrorCode());
+  }
   if (const Comdat *C = Base->getComdat()) {
     Expected<int> ComdatIndexOrErr = getComdatIndex(C, GV->getParent());
     if (!ComdatIndexOrErr)

diff --git a/llvm/lib/Object/ModuleSymbolTable.cpp b/llvm/lib/Object/ModuleSymbolTable.cpp
index 9a79de7..5ae81c9 100644 (file)
--- a/llvm/lib/Object/ModuleSymbolTable.cpp
+++ b/llvm/lib/Object/ModuleSymbolTable.cpp
@@ -204,7 +204,7 @@ uint32_t ModuleSymbolTable::getSymbolFlags(Symbol S) const {
     if (GVar->isConstant())
       Res |= BasicSymbolRef::SF_Const;
   }
-  if (dyn_cast_or_null<Function>(GV->getBaseObject()))
+  if (isa_and_nonnull<Function>(GV->getBaseObject()) || isa<GlobalIFunc>(GV))
     Res |= BasicSymbolRef::SF_Executable;
   if (isa<GlobalAlias>(GV))
     Res |= BasicSymbolRef::SF_Indirect;

diff --git a/llvm/lib/Transforms/Utils/SplitModule.cpp b/llvm/lib/Transforms/Utils/SplitModule.cpp
index 32f2f4e..2fd3eed 100644 (file)
--- a/llvm/lib/Transforms/Utils/SplitModule.cpp
+++ b/llvm/lib/Transforms/Utils/SplitModule.cpp
@@ -125,9 +125,11 @@ static void findPartitions(Module &M, ClusterIDMapType &ClusterIDMap,
 
     // For aliases we should not separate them from their aliasees regardless
     // of linkage.
-    if (auto *GIS = dyn_cast<GlobalIndirectSymbol>(&GV)) {
+    if (auto *GIS = dyn_cast<GlobalAlias>(&GV)) {
       if (const GlobalObject *Base = GIS->getBaseObject())
         GVtoClusterMap.unionSets(&GV, Base);
+    } else if (auto *GIS = dyn_cast<GlobalIFunc>(&GV)) {
+      GVtoClusterMap.unionSets(&GV, GIS->getResolverFunction());
     }
 
     if (const Function *F = dyn_cast<Function>(&GV)) {
@@ -225,9 +227,12 @@ static void externalize(GlobalValue *GV) {
 
 // Returns whether GV should be in partition (0-based) I of N.
 static bool isInPartition(const GlobalValue *GV, unsigned I, unsigned N) {
-  if (auto *GIS = dyn_cast<GlobalIndirectSymbol>(GV))
+  if (auto *GIS = dyn_cast<GlobalAlias>(GV)) {
     if (const GlobalObject *Base = GIS->getBaseObject())
       GV = Base;
+  } else if (auto *GIS = dyn_cast<GlobalIFunc>(GV)) {
+    GV = GIS->getResolverFunction();
+  }
 
   StringRef Name;
   if (const Comdat *C = GV->getComdat())

diff --git a/llvm/test/LTO/Resolution/X86/ifunc.ll b/llvm/test/LTO/Resolution/X86/ifunc.ll
index 7192ea2..afe7c8c 100644 (file)
--- a/llvm/test/LTO/Resolution/X86/ifunc.ll
+++ b/llvm/test/LTO/Resolution/X86/ifunc.ll
@@ -1,15 +1,23 @@
 ; RUN: opt -module-summary -o %t.bc %s
-; RUN: llvm-lto2 run %t.bc -r %t.bc,foo,pl -o %t2
+; RUN: llvm-lto2 run %t.bc -r %t.bc,foo,pl -r %t.bc,strlen,pl -o %t2
 ; RUN: llvm-nm %t2.1 | FileCheck %s
 ; CHECK: i foo
-; CHECK: t foo_ifunc
+; CHECK: t foo_resolver
+; CHECK: i strlen
+; CHECK: t strlen_resolver
 
 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"
 
-@foo = ifunc i32 (i32), i64 ()* @foo_ifunc
+@foo = ifunc i32 (i32), i64 ()* @foo_resolver
+@strlen = ifunc i64 (i8*), bitcast (i64 (i8*)* ()* @strlen_resolver to i64 (i8*)*)
 
-define internal i64 @foo_ifunc() {
+define internal i64 @foo_resolver() {
 entry:
   ret i64 0
 }
+
+define internal i64 (i8*)* @strlen_resolver() {
+entry:
+  ret i64 (i8*)* null
+}