Avoid duplicating uniqueness in ThinLTO when unique internal linkage names are used

[tmsriram]

Summary

Option -funique-internal-linkage-names already creates unique names for internal functions.

With ThinLTO, importing internal linkage functions uniqueifies their names to avoid two such functions from different modules colliding. However, -funique-internal-linkage-names already does that independently. This simple patch checks if unique names are already present before adding a unique suffix.

The benefits are that this reduces .strtab size.

[tejohnson]

llvm/lib/Transforms/Utils/FunctionImportUtils.cpp

266

Better to do this in ModuleSummaryIndex::getGlobalNameForLocal (called by getPromotedName) so that it automatically handles all cases. E.g. without that fix cross-module WPD to a local function (requiring promotion) will be broken (we will promote on the devirt side but no longer on the def side).

E.g. see llvm/test/ThinLTO/X86/devirt_promote.ll. Although unfortunately this test case will not catch the breakage since we have hardcoded IR names in the test, i.e. it isn't being generated by -funique-internal-linkage-names.

llvm/lib/Transforms/Utils/FunctionImportUtils.cpp

if (GlobalVariable *V = dyn_cast<GlobalVariable>(&GV)) {
           V->setInitializer(Constant::getNullValue(V->getValueType()));
       }
     }
   }
 
   if (GV.hasLocalLinkage() && shouldPromoteLocalToGlobal(&GV, VI)) {
     // Save the original name string before we rename GV below.
     auto Name = GV.getName().str();
+    // With -funique-internal-linkage-names, local functions already get unique
+    // names.  To avoid duplication, check if this suffix does not exist before
+    // uniqueifying the name.  This will directly reduce .strtab size.
+    if (Name.find(".__uniq.") == std::string::npos)
       GV.setName(getPromotedName(&GV));
     GV.setLinkage(getLinkage(&GV, /* DoPromote */ true));
     assert(!GV.hasLocalLinkage());
     GV.setVisibility(GlobalValue::HiddenVisibility);
 
     // If we are renaming a COMDAT leader, ensure that we record the COMDAT
     // for later renaming as well. This is required for COFF.
     if (const auto *C = GV.getComdat())
       if (C->getName() == Name)

llvm/test/ThinLTO/X86/export.ll

 ; Do setup work for all below tests: generate bitcode and combined index
 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/export.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto-action=thinlink -o %t3.bc %t1.bc %t2.bc
 
 ; Ensure statics are promoted/renamed correctly from this file.
 ; RUN: llvm-lto -thinlto-action=promote %t1.bc -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s
 ; CHECK-DAG: @staticvar.llvm.0 = hidden global
 ; CHECK-DAG: define hidden void @staticfunc.llvm.0
+; CHECK-DAG: define hidden void @uniqstaticfunc.__uniq.12345
+; CHECK-NOT: define hidden void @uniqstaticfunc.__uniq.12345.llvm.
 
 target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.11.0"
 
 @staticvar = internal global i32 1, align 4
 
 define void @callstaticfunc() #0 {
 entry:
   call void @staticfunc()
+  call void @uniqstaticfunc.__uniq.12345()
   ret void
 }
 
 define internal void @staticfunc() #0 {
 entry:
   %0 = load i32, i32* @staticvar, align 4
   ret void
 }
+
+define internal void @uniqstaticfunc.__uniq.12345() #0 {
+entry:
+  %0 = load i32, i32* @staticvar, align 4
+  ret void
+}