[LTO] Add function alias as function instead of data

[scui-ibm]

On AIX, for undefined functions, only the dotnamed symbols (the address of the function) are generated after linking (i.e., no named function symbol is generated).
 
Currently, all alias symbols are added as defined data symbols when parsing symbols in LTOModule (the Link Time Optimization library used by linker to optimization code at link time). On AIX, if the function alias is used in the native object, and only its dotnamed symbol is generated, the linker will have problem to match the dotnamed symbol from the native object and the defined symbol marked as data from the bitcode at LTO linktime.
 
This patch is to add function alias as function instead of data.

[llvmbot]

@llvm/pr-subscribers-backend-powerpc

@llvm/pr-subscribers-lto

Author: Shimin Cui (scui-ibm)

Changes

On AIX, for undefined functions, only the dotnamed symbols (the address of the function) are generated after linking (i.e., no named function symbol is generated).
 
Currently, all alias symbols are added as defined data symbols when parsing symbols in LTOModule (the Link Time Optimization library used by linker to optimization code at link time). On AIX, if the function alias is used in the native object, and only its dotnamed symbol is generated, the linker will have problem to match the dotnamed symbol from the native object and the defined symbol marked as data from the bitcode at LTO linktime.
 
This patch is to add function alias as function instead of data.
 
I haven’t been able to draft a test case for this using existing llvm tools. Please comment if you think we need add a test case for this or not, or if you have any insight on how to test it. Thanks.

---

Full diff: https://github.com/llvm/llvm-project/pull/112599.diff

2 Files Affected:

• (modified) llvm/include/llvm/LTO/legacy/LTOModule.h (+1-1)
• (modified) llvm/lib/LTO/LTOModule.cpp (+13-4)

diff --git a/llvm/include/llvm/LTO/legacy/LTOModule.h b/llvm/include/llvm/LTO/legacy/LTOModule.h
index 1b2de3b333855c..e861a56bcbac1d 100644
--- a/llvm/include/llvm/LTO/legacy/LTOModule.h
+++ b/llvm/include/llvm/LTO/legacy/LTOModule.h
@@ -195,7 +195,7 @@ struct LTOModule {
 
   /// Add a function symbol as defined to the list.
   void addDefinedFunctionSymbol(ModuleSymbolTable::Symbol Sym);
-  void addDefinedFunctionSymbol(StringRef Name, const Function *F);
+  void addDefinedFunctionSymbol(StringRef Name, const GlobalValue *F);
 
   /// Add a global symbol from module-level ASM to the defined list.
   void addAsmGlobalSymbol(StringRef, lto_symbol_attributes scope);
diff --git a/llvm/lib/LTO/LTOModule.cpp b/llvm/lib/LTO/LTOModule.cpp
index eac78069f4d2bc..00eb8adb4e1035 100644
--- a/llvm/lib/LTO/LTOModule.cpp
+++ b/llvm/lib/LTO/LTOModule.cpp
@@ -406,11 +406,16 @@ void LTOModule::addDefinedFunctionSymbol(ModuleSymbolTable::Symbol Sym) {
     Buffer.c_str();
   }
 
-  const Function *F = cast<Function>(cast<GlobalValue *>(Sym));
-  addDefinedFunctionSymbol(Buffer, F);
+  auto *GV = cast<GlobalValue *>(Sym);
+  assert((isa<Function>(GV) ||
+          (isa<GlobalAlias>(GV) &&
+           isa<Function>(cast<GlobalAlias>(GV)->getAliasee()))) &&
+         "Not function or function alias");
+
+  addDefinedFunctionSymbol(Buffer, GV);
 }
 
-void LTOModule::addDefinedFunctionSymbol(StringRef Name, const Function *F) {
+void LTOModule::addDefinedFunctionSymbol(StringRef Name, const GlobalValue *F) {
   // add to list of defined symbols
   addDefinedSymbol(Name, F, true);
 }
@@ -611,7 +616,11 @@ void LTOModule::parseSymbols() {
     }
 
     assert(isa<GlobalAlias>(GV));
-    addDefinedDataSymbol(Sym);
+
+    if (isa<Function>(cast<GlobalAlias>(GV)->getAliasee()))
+      addDefinedFunctionSymbol(Sym);
+    else
+      addDefinedDataSymbol(Sym);
   }
 
   // make symbols for all undefines

[teresajohnson]

This is the legacy LTO handling, which I am less familiar with. Added @cachemeifyoucan who is a user of that and can probably give better guidance on the patch and a test case.

That being said it seems reasonable to me to treat a function alias as a function not data, but again, I'm unfamiliar with the handling here. It would be really good to have a test case. The llvm-lto tool is typically used to invoke the legacy LTO handling. Can you have a simple IR test where there is an alias and check the resulting assembly after llvm-lto?

[cachemeifyoucan]

I would prefer a test case for this. I think the goal is to get attribute set correctly when querying from LTO legacy interface? I will just extend llvm-lto -list-symbols-only option to print attributes too to test your change.

[scui-ibm]

Thanks @teresajohnson and @cachemeifyoucan for your quick comments and suggestions. I'll extend llvm-lto -list-symbols-only option to print attributes and add a test to test this.

[cachemeifyoucan]

Thanks. LGTM!