ThinLTO renaming: use module hash instead of position in the summary

[mehdi_amini]

Summary

This is more robust to the link ordering

[tejohnson]

Adding pcc since this is different than his proposal to use a hash of external names in order to facilitate renaming in the compile step (and thus enable compiling some code to text early).

I think for now this is ok, as it is better than the current strategy with regards to link ordering effects, even if we want to eventually migrate to using the hash of external names.

One thing to note is that the module hash will itself need to change if pcc implements compiling some portions to text early, to ensure that the hash reflects differences in the non-bitcode portion.

[mehdi_amini]

I'm perfectly fine with the renaming not being stable (i.e. we can change it anytime).

[pcc]

Seems reasonable enough to me if we can change this later.

[tejohnson]

Then LGTM too. Thanks

[mehdi_amini]

Thanks, r266018

include/llvm/IR/ModuleSummaryIndex.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_IR_MODULESUMMARYINDEX_H
 #define LLVM_IR_MODULESUMMARYINDEX_H
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include <array>
 
public:
   /// assigning it the given module ID. Each module merged in should have
   /// a unique ID, necessary for consistent renaming of promoted
   /// static (local) variables.
   void mergeFrom(std::unique_ptr<ModuleSummaryIndex> Other,
                  uint64_t NextModuleId);
 
   /// Convenience method for creating a promoted global name
   /// for the given value name of a local, and its original module's ID.
-  static std::string getGlobalNameForLocal(StringRef Name, uint64_t ModId) {
+  static std::string getGlobalNameForLocal(StringRef Name, ModuleHash ModHash) {
     SmallString<256> NewName(Name);
     NewName += ".llvm.";
-    raw_svector_ostream(NewName) << ModId;
+    NewName += utohexstr(ModHash[0]); // Take the first 32 bits
     return NewName.str();
   }
 
   /// Add a new module path with the given \p Hash, mapped to the given \p
   /// ModID, and return an iterator to the entry in the index.
   ModulePathStringTableTy::iterator
   addModulePath(StringRef ModPath, uint64_t ModId,
                 ModuleHash Hash = ModuleHash{{0}}) {

lib/Transforms/Utils/FunctionImportUtils.cpp

std::string FunctionImportGlobalProcessing::getName(const GlobalValue *SGV) {
   // the promoted name uniquely identifies the copy in the original module,
   // using the ID assigned during combined index creation. When importing,
   // we rename all locals (not just those that are promoted) in order to
   // avoid naming conflicts between locals imported from different modules.
   if (SGV->hasLocalLinkage() &&
       (doPromoteLocalToGlobal(SGV) || isPerformingImport()))
     return ModuleSummaryIndex::getGlobalNameForLocal(
         SGV->getName(),
-        ImportIndex.getModuleId(SGV->getParent()->getModuleIdentifier()));
+        ImportIndex.getModuleHash(SGV->getParent()->getModuleIdentifier()));
   return SGV->getName();
 }
 
 GlobalValue::LinkageTypes
 FunctionImportGlobalProcessing::getLinkage(const GlobalValue *SGV) {
   // Any local variable that is referenced by an exported function needs
   // to be promoted to global scope. Since we don't currently know which
   // functions reference which local variables/functions, we must treat

test/Linker/funcimport.ll

 ; First ensure that the ThinLTO handling in llvm-link and llvm-lto handles
 ; bitcode without summary sections gracefully.
 ; RUN: llvm-as %s -o %t.bc
 ; RUN: llvm-as %p/Inputs/funcimport.ll -o %t2.bc
 ; RUN: llvm-link %t.bc -summary-index=%t.bc -S
 ; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc
 
 ; Do setup work for all below tests: generate bitcode and combined index
 ; RUN: llvm-as -module-summary %s -o %t.bc
 ; RUN: llvm-as -module-summary %p/Inputs/funcimport.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc
 
 ; Ensure statics are promoted/renamed correctly from this file (all but
 ; constant variable need promotion).
 ; RUN: llvm-link %t.bc -summary-index=%t3.thinlto.bc -S | FileCheck %s --check-prefix=EXPORTSTATIC
-; EXPORTSTATIC-DAG: @staticvar.llvm.1 = hidden global
+; EXPORTSTATIC-DAG: @staticvar.llvm.{{.*}} = hidden global
 ; EXPORTSTATIC-DAG: @staticconstvar = internal unnamed_addr constant
-; EXPORTSTATIC-DAG: @P.llvm.1 = hidden global void ()* null
-; EXPORTSTATIC-DAG: define hidden i32 @staticfunc.llvm.1
-; EXPORTSTATIC-DAG: define hidden void @staticfunc2.llvm.1
+; EXPORTSTATIC-DAG: @P.llvm.{{.*}} = hidden global void ()* null
+; EXPORTSTATIC-DAG: define hidden i32 @staticfunc.llvm.
+; EXPORTSTATIC-DAG: define hidden void @staticfunc2.llvm.
 
 ; Ensure that both weak alias to an imported function and strong alias to a
 ; non-imported function are correctly turned into declarations.
 ; Also ensures that alias to a linkonce function is turned into a declaration
 ; and that the associated linkonce function is not in the output, as it is
 ; lazily linked and never referenced/materialized.
 ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=globalfunc1:%t.bc -S | FileCheck %s --check-prefix=IMPORTGLOB1
 ; IMPORTGLOB1-DAG: define available_externally void @globalfunc1

 ; available_externally.
 ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=linkoncefunc:%t.bc -S | FileCheck %s --check-prefix=IMPORTGLOB5
 ; IMPORTGLOB5-DAG: linkoncealias = alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
 ; IMPORTGLOB5-DAG: define linkonce_odr void @linkoncefunc()
 
 ; Ensure that imported static variable and function references are correctly
 ; promoted and renamed (including static constant variable).
 ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=referencestatics:%t.bc -S | FileCheck %s --check-prefix=IMPORTSTATIC
-; IMPORTSTATIC-DAG: @staticvar.llvm.1 = external hidden global
-; IMPORTSTATIC-DAG: @staticconstvar.llvm.1 = internal unnamed_addr constant
+; IMPORTSTATIC-DAG: @staticvar.llvm.{{.*}} = external hidden global
+; IMPORTSTATIC-DAG: @staticconstvar.llvm.{{.*}} = internal unnamed_addr constant
 ; IMPORTSTATIC-DAG: define available_externally i32 @referencestatics
-; IMPORTSTATIC-DAG: %call = call i32 @staticfunc.llvm.1
-; IMPORTSTATIC-DAG: %0 = load i32, i32* @staticvar.llvm.1
-; IMPORTSTATIC-DAG: declare hidden i32 @staticfunc.llvm.1
+; IMPORTSTATIC-DAG: %call = call i32 @staticfunc.llvm.
+; IMPORTSTATIC-DAG: %0 = load i32, i32* @staticvar.llvm.
+; IMPORTSTATIC-DAG: declare hidden i32 @staticfunc.llvm.
 
 ; Ensure that imported global (external) function and variable references
 ; are handled correctly (including referenced variable imported as
 ; available_externally definition)
 ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=referenceglobals:%t.bc -S | FileCheck %s --check-prefix=IMPORTGLOBALS
 ; IMPORTGLOBALS-DAG: @globalvar = external global
 ; IMPORTGLOBALS-DAG: declare void @globalfunc1()
 ; IMPORTGLOBALS-DAG: define available_externally i32 @referenceglobals
 
 ; Ensure that common variable correctly imported as common defition.
 ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=referencecommon:%t.bc -S | FileCheck %s --check-prefix=IMPORTCOMMON
 ; IMPORTCOMMON-DAG: @commonvar = external global
 ; IMPORTCOMMON-DAG: define available_externally i32 @referencecommon
 
 ; Ensure that imported static function pointer correctly promoted and renamed.
 ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=callfuncptr:%t.bc -S | FileCheck %s --check-prefix=IMPORTFUNCPTR
-; IMPORTFUNCPTR-DAG: @P.llvm.1 = external hidden global void ()*
+; IMPORTFUNCPTR-DAG: @P.llvm.{{.*}} = external hidden global void ()*
 ; IMPORTFUNCPTR-DAG: define available_externally void @callfuncptr
-; IMPORTFUNCPTR-DAG: %0 = load void ()*, void ()** @P.llvm.1
+; IMPORTFUNCPTR-DAG: %0 = load void ()*, void ()** @P.llvm.
 
 ; Ensure that imported weak function reference/definition handled properly.
 ; Imported weak_any definition should be skipped with warning, and imported
 ; reference should turned into an external_weak declaration.
 ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=callweakfunc:%t.bc -import=weakfunc:%t.bc -S 2>&1 | FileCheck %s --check-prefix=IMPORTWEAKFUNC
 ; IMPORTWEAKFUNC-DAG: Ignoring import request for weak-any function weakfunc
 ; IMPORTWEAKFUNC-DAG: declare extern_weak void @weakfunc
 ; IMPORTWEAKFUNC-DAG: define available_externally void @callweakfunc

test/Transforms/FunctionImport/adjustable_threshold.ll

 ; Do setup work for all below tests: generate bitcode and combined index
 ; RUN: llvm-as -module-summary %s -o %t.bc
 ; RUN: llvm-as -module-summary %p/Inputs/adjustable_threshold.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc
 
 ; Test import with default progressive instruction factor
 ; RUN: opt -function-import -summary-file %t3.thinlto.bc %t.bc -import-instr-limit=10 -S | FileCheck %s --check-prefix=CHECK --check-prefix=INSTLIM-DEFAULT
-; INSTLIM-DEFAULT: call void @staticfunc2.llvm.2()
+; INSTLIM-DEFAULT: call void @staticfunc2.llvm.
 
 ; Test import with a reduced progressive instruction factor
 ; RUN: opt -function-import -summary-file %t3.thinlto.bc %t.bc -import-instr-limit=10 -import-instr-evolution-factor=0.5 -S | FileCheck %s --check-prefix=CHECK --check-prefix=INSTLIM-PROGRESSIVE
 ; INSTLIM-PROGRESSIVE-NOT: call void @staticfunc
 
 
 
 declare void @globalfunc1()

test/Transforms/FunctionImport/funcimport.ll

 ; Do setup work for all below tests: generate bitcode and combined index
 ; RUN: llvm-as -module-summary %s -o %t.bc
 ; RUN: llvm-as -module-summary %p/Inputs/funcimport.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto -print-summary-global-ids -o %t3 %t.bc %t2.bc 2>&1 | FileCheck %s --check-prefix=GUID
 
 ; Do the import now
 ; RUN: opt -function-import -stats -print-imports -summary-file %t3.thinlto.bc %t.bc -S 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=INSTLIMDEF
 ; "-stats" requires +Asserts.
 ; REQUIRES: asserts
 
 ; Test import with smaller instruction limit
 ; RUN: opt -function-import -summary-file %t3.thinlto.bc %t.bc -import-instr-limit=5 -S | FileCheck %s --check-prefix=CHECK --check-prefix=INSTLIM5
-; INSTLIM5-NOT: @staticfunc.llvm.2
+; INSTLIM5-NOT: @staticfunc.llvm.
 
 define i32 @main() #0 {
 entry:
   call void (...) @weakalias()
   call void (...) @analias()
   call void (...) @linkoncealias()
   %call = call i32 (...) @referencestatics()
   %call1 = call i32 (...) @referenceglobals()

 ; INSTLIMDEF-DAG: define available_externally i32 @referencestatics(i32 %i)
 ; INSTLIM5-DAG: declare i32 @referencestatics(...)
 declare i32 @referencestatics(...) #1
 
 ; The import of referencestatics will expose call to staticfunc that
 ; should in turn be imported as a promoted/renamed and hidden function.
 ; Ensure that the call is to the properly-renamed function.
 ; INSTLIMDEF-DAG: Import staticfunc
-; INSTLIMDEF-DAG: %call = call i32 @staticfunc.llvm.2()
-; INSTLIMDEF-DAG: define available_externally hidden i32 @staticfunc.llvm.2()
+; INSTLIMDEF-DAG: %call = call i32 @staticfunc.llvm.
+; INSTLIMDEF-DAG: define available_externally hidden i32 @staticfunc.llvm.
 
 ; INSTLIMDEF-DAG: Import referenceglobals
 ; CHECK-DAG: define available_externally i32 @referenceglobals(i32 %i)
 declare i32 @referenceglobals(...) #1
 
 ; The import of referenceglobals will expose call to globalfunc1 that
 ; should in turn be imported.
 ; INSTLIMDEF-DAG: Import globalfunc1
 ; CHECK-DAG: define available_externally void @globalfunc1()
 
 ; INSTLIMDEF-DAG: Import referencecommon
 ; CHECK-DAG: define available_externally i32 @referencecommon(i32 %i)
 declare i32 @referencecommon(...) #1
 
 ; INSTLIMDEF-DAG: Import setfuncptr
 ; CHECK-DAG: define available_externally void @setfuncptr()
 declare void @setfuncptr(...) #1
 
 ; INSTLIMDEF-DAG: Import callfuncptr
 ; CHECK-DAG: define available_externally void @callfuncptr()
 declare void @callfuncptr(...) #1
 
 ; Ensure that all uses of local variable @P which has used in setfuncptr
 ; and callfuncptr are to the same promoted/renamed global.
-; CHECK-DAG: @P.llvm.2 = external hidden global void ()*
-; CHECK-DAG: %0 = load void ()*, void ()** @P.llvm.2,
-; CHECK-DAG: store void ()* @staticfunc2.llvm.2, void ()** @P.llvm.2,
+; CHECK-DAG: @P.llvm.{{.*}} = external hidden global void ()*
+; CHECK-DAG: %0 = load void ()*, void ()** @P.llvm.
+; CHECK-DAG: store void ()* @staticfunc2.llvm.{{.*}}, void ()** @P.llvm.
 
 ; Won't import weak func
 ; CHECK-DAG: declare void @weakfunc(...)
 declare void @weakfunc(...) #1
 
 ; INSTLIMDEF-DAG: Import funcwithpersonality
-; INSTLIMDEF-DAG: define available_externally hidden void @funcwithpersonality.llvm.2() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
-; INSTLIM5-DAG: declare hidden void @funcwithpersonality.llvm.2()
+; INSTLIMDEF-DAG: define available_externally hidden void @funcwithpersonality.llvm.{{.*}}() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+; INSTLIM5-DAG: declare hidden void @funcwithpersonality.llvm.{{.*}}()
 
 ; INSTLIMDEF-DAG: Import globalfunc2
 ; INSTLIMDEF-DAG: 11 function-import - Number of functions imported
 
 ; The actual GUID values will depend on path to test.
 ; GUID-DAG: GUID {{.*}} is weakalias
 ; GUID-DAG: GUID {{.*}} is referenceglobals
 ; GUID-DAG: GUID {{.*}} is weakfunc