[PGO] Fix profile mismatch in Comdat function with pre-inliner

[xur]

Summary

Pre-instrumentation inline (pre-inliner) greatly improves the IR instrumentation code performance, among other benefits. One issue of the pre-inliner is it can introduce CFG-mismatch for COMDAT functions. This is due to the fact that the same COMDAT function may have different early inline decisions across different modules -- that means different copies of COMDAT functions will have different CFG checksum.

A simple example:

COMDAT function foo() --> bar()

foo() is defined in f1.cc and f2.cc. If bar() is available in f1.cc and is small enough to inline in pre-inliner, and it's not available in f2.cc. After pre-inline pass, f1.cc and f2.cc each has its own foo() with different IR (different CFG). If the version in f1.cc is chosen, we only have this version of counters in the profile (as we merge the COMDAT profile variables). We will have checksum mismatch when do a profile-use compilation in f2.cc.

In this patch, we propose a partially renaming the COMDAT group and its member function/variable so we have different profile counter for each version. We will post-fix the COMDAT function and the group name with its FunctionHash.

For the above case, if the foo() in f1.cc has a hash of 123456 and the foo() in f2.cc has a hash of 234567. foo() in f1.cc will be rename to foo.123456() (as well as the COMDAT name, and the profile variables in that function). foo() in f2.cc will be rename to foo.234567() (as well as the COMDAT name, and the profile variables in that function).

There are cases where two functions with the same FunctionHash might not have the same IR. For example,
foo() { bar(); goo(); }
bar() { a++; }
goo() { b++; }
in f1.cc bar() is inlined while in f2.cc goo() is inlined. From edge profile point of view, nothing will go wrong as both version has the same profile variables and all the counter updates will be captured.

The only potential problem is the indirect-call profiling. For example, if bar() contains an indirect-call. The version of foo() in f1.cc will also contain an indirect-call counter, while the version in f2.cc does not have. This will create a mismatch when reading value profiles. To address this, we add the number of indirect-calls to the function hash.

This is not bullet-proof solution. As same number of indirect-calls does not guarantee the indirect-calls are the same. For example, if goo() contains a different indirect-call. Both version in f1.cc and f2.cc has one indirect call, even though they have different call-sites. Our proposed method will treat the two versions of foo() as identical as they have the same function hash. But we believe this happens rarely. Also note that the worse case here is that we will promote a wrong indirect-target. There are not correctness issues.

Some implementation details:
(1) The mismatch also applies to AvailableExternallyLinkage functions.
We will convert AvailableExternallyLinkage functions to COMDAT functions, but not ExternalWeakLinkage functions.

(2) COMDAT group with multiple functions.
We only handle COMDAT group that having one COMDAT function to reduce the complexity. If a COMDAT group has multiple functions, we need to have a unique post-fix for all the functions. To do this, it requires to collect all the member functions and their hash, which is costly.

An alternate way is to do a post instrumentation fix-up on the instrumentation intrinsics. This is costly too.

We find multiple functions COMDAT groups are relatively rare (mostly in global static initializer). So we decided to only handle single member COMDAT groups.

Future work:
(1) Reduce the number of renamings
One optimization that reduces the number of renamings is to only apply the renaming to the COMDAT functions that preinline occurs. Unfortunately, currently we does not have an attribute for this.

(2) Darwin
Darwin does not use COMDAT, instead, it uses LinkOnce linkage. We will have a separated patch to deal the mismatch introduced by pre-inliner.

[silvas]

adding Jake

[jakev]

Just a couple nits from me.

lib/Transforms/Instrumentation/PGOInstrumentation.cpp

121

Does this need ZeroOrMore?

122

functin -> function

286

Can this just be ComdatMembers.size()?

325

Can this be called 'canRenameComdat'?

888

comdat.in -> comdat in

[silvas]

I think this is a good solution. LGTM although I'd like to wait for other reviewers to chime in.

FWIW, one alternative solution to this issue (and the static var name issue that Jake fixed recently) is to make it so that multiple functions with same name and different hash can coexist naturally. I haven't looked closely, but this would require changing quite a bit (e.g. indexed format, various API's, etc.). So at the moment I don't think it is worth it since this patch + Jake's patch are relatively small.

lib/Transforms/Instrumentation/PGOInstrumentation.cpp

988

Is this variable Builder needed?

[xur]

lib/Transforms/Instrumentation/PGOInstrumentation.cpp

121

Should be Optional. Removed ZeroOrMore.

286

good suggestion. changed.

325

changed.

988

This is from a leftover of earlier merge. It should not be here. Thanks for catching this.

[xur]

Integrated the review suggestions from Jake and Sean.

Thanks,

-Rong

[davidxl]

lib/ProfileData/InstrProf.cpp

784 ↗

(On Diff #64931)

Please separate the restructuring into a NFC patch

lib/Transforms/Instrumentation/PGOInstrumentation.cpp

120

Change the name to "DoComdatRenaming"

[xur]

Address David's review comments:
(1) change option name.
(2) split the refactoring work into NFC patch:
https://reviews.llvm.org/D22643
D22643 [PGO] Make needsComdatForCounter() available (NFC)

[davidxl]

lib/Transforms/Instrumentation/PGOInstrumentation.cpp

338

Perhaps add an assert about the linkage?

341

Add // FIXME and some explanation of the limitation.

342

have have --> have

344

Also explain why having other variables are ok?

365

--> change the linkage to LinkOnceODR and put them into comdat. This is because after renaming, there is no backup external copy available for the function.

367

Add assertion about availableExternally linkage.

test/Transforms/PGOProfile/comdat_internal.ll

7

Can you use wild card and defined a variable for the hash val? There is no need to test the actual alue.

test/Transforms/PGOProfile/indirect_call_profile.ll

16

Same here

[xur]

lib/Transforms/Instrumentation/PGOInstrumentation.cpp

344

It turns out having variable in the comdat group is not safe to rename: if we rename the variable, we will create different copy of variable which is wrong. if we don't rename, we might run into duplicated symbol in linking.
So in the new patch, I disable the rename of the comdat groups with variables.

It's ok to have aliases though.

[xur]

This new patch addressed David's comments:
It refines the handling of comdat group with variables and aliases, and adds a test case to check handled and unhandled cases.

[davidxl]

lib/Transforms/Instrumentation/PGOInstrumentation.cpp

348

(2) variables can not be renamed, so we can not rename comdat function in a group including global vars.

393

Add assert that the alias target is still F

test/Transforms/PGOProfile/comdat_rename.ll

2

you may want to try this test case on other platform such as coff to make sure it works:

-mtriple=x86_64-pc-win32-coff

[xur]

lib/Transforms/Instrumentation/PGOInstrumentation.cpp

393

I'll assert GA's comdat is the same as OrigComdat.

test/Transforms/PGOProfile/comdat_rename.ll

2

The test failed for the AvailableExternallyLinkage case. The reason is in needsComdatForCounter().

For AvailableExternallyLinkage, since this is not ELF format target, we return false.

I'm wondering if we should move the checking for TargetTriple() after the linkage check. The target can have Comdat support for nonELF targets after all.

[davidxl]

test/Transforms/PGOProfile/comdat_rename.ll

2

you should explicitly add -mtripple (covering elf and coff). For coff case, do not check availableExternally (which is not supported yet with this change).

[xur]

Integrated David's comments.

Thanks,

-Rong

[davidxl]

As a follow up, perfhaps a new internal option can be introduced to force 'privatize a comdat function': when a comdat function has module context specific profile, it is can be useful to let it have its own copy of profile counters.

lgtm

[silvas]

Thanks David for the close review! (and of course thanks to Rong for the patch!)

lib/Transforms/Instrumentation/PGOInstrumentation.cpp

 #include "llvm/Transforms/PGOInstrumentation.h"
 #include "CFGMST.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Analysis/BlockFrequencyInfo.h"

 #include "llvm/Analysis/IndirectCallSiteVisitor.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/DiagnosticInfo.h"
+#include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InstIterator.h"
 #include "llvm/IR/Instructions.h"

 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include <algorithm>
 #include <string>
+#include <unordered_map>
 #include <utility>
 #include <vector>
 

     cl::desc("Max number of annotations for a single indirect "
              "call callsite"));
 
+// Command line option to control appending FunctionHash to the name of a COMDAT
+// function. This is to avoid the hash mismatch caused by the preinliner.
+static cl::opt<bool> DoComdatRenaming(
+    "do-comdat-renaming", cl::init(true), cl::Hidden,
+    cl::desc("Append function hash to the name of COMDAT function to avoid "
+             "function hash mismatch due to the preinliner"));
+
 // Command line option to enable/disable the warning about missing profile
 // information.
 static cl::opt<bool> NoPGOWarnMissing("no-pgo-warn-missing", cl::init(false),

 private:
   Function &F;
   void computeCFGHash();
+  void renameComdatFunction();
+  // A map that stores the Comdat group in function F.
+  std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers;
 
 public:
   std::string FuncName;

                               Twine(FunctionHash) + "\t" + Str);
   }
 
-  FuncPGOInstrumentation(Function &Func, bool CreateGlobalVar = false,
-                         BranchProbabilityInfo *BPI = nullptr,
-                         BlockFrequencyInfo *BFI = nullptr)
-      : F(Func), FunctionHash(0), MST(F, BPI, BFI) {
+  FuncPGOInstrumentation(
+      Function &Func,
+      std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers,
+      bool CreateGlobalVar = false, BranchProbabilityInfo *BPI = nullptr,
+      BlockFrequencyInfo *BFI = nullptr)
+      : F(Func), ComdatMembers(ComdatMembers), FunctionHash(0),
+        MST(F, BPI, BFI) {
     FuncName = getPGOFuncName(F);
     computeCFGHash();
+    if (ComdatMembers.size())
+      renameComdatFunction();
     DEBUG(dumpInfo("after CFGMST"));
 
     NumOfPGOBB += MST.BBInfos.size();

     }
   }
   JC.update(Indexes);
-  FunctionHash = (uint64_t)MST.AllEdges.size() << 32 | JC.getCRC();
+  FunctionHash = (uint64_t)findIndirectCallSites(F).size() << 48 |
+                 (uint64_t)MST.AllEdges.size() << 32 | JC.getCRC();
+}
+
+// Check if we can safely rename this Comdat function.
+static bool canRenameComdat(
+    Function &F,
+    std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers) {
+  if (F.getName().empty())
+    return false;
+  if (!needsComdatForCounter(F, *(F.getParent())))
+    return false;
+  // Only safe to do if this function may be discarded if it is not used
+  // in the compilation unit.
+  if (!GlobalValue::isDiscardableIfUnused(F.getLinkage()))
+    return false;
+
+  // For AvailableExternallyLinkage functions.
+  if (!F.hasComdat()) {
+    assert(F.getLinkage() == GlobalValue::AvailableExternallyLinkage);
+    return true;
+  }
+
+  // FIXME: Current only handle those Comdat groups that only containing one
+  // function and function aliases. 
+  // (1) For a Comdat group containing multiple functions, we need to have a
+  // unique postfix based on the hashes for each function. There is a
+  // non-trivial code refactoring to do this efficiently.
+  // (2) For a Comdat group containing a variable member, we should not create
+  // multiple copies for the variable.
+  Comdat *C = F.getComdat();
+  for (auto &&CM : make_range(ComdatMembers.equal_range(C))) {
+    if (dyn_cast<GlobalAlias>(CM.second))
+      continue;
+    Function *FM = dyn_cast<Function>(CM.second);
+    if (FM != &F)
+      return false;
+  }
+  return true;
+}
+
+// Append the CFGHash to the Comdat function name.
+template <class Edge, class BBInfo>
+void FuncPGOInstrumentation<Edge, BBInfo>::renameComdatFunction() {
+  if (!canRenameComdat(F, ComdatMembers))
+    return;
+  std::string NewFuncName =
+      Twine(F.getName() + "." + Twine(FunctionHash)).str();
+  F.setName(Twine(NewFuncName));
+  FuncName = Twine(FuncName + "." + Twine(FunctionHash)).str();
+  Comdat *NewComdat;
+  Module *M = F.getParent();
+  // For AvailableExternallyLinkage functions, change the linkage to
+  // LinkOnceODR and put them into comdat. This is because after renaming, there
+  // is no backup external copy available for the function.
+  if (!F.hasComdat()) {
+    assert(F.getLinkage() == GlobalValue::AvailableExternallyLinkage);
+    NewComdat = M->getOrInsertComdat(StringRef(NewFuncName));
+    F.setLinkage(GlobalValue::LinkOnceODRLinkage);
+    F.setComdat(NewComdat);
+    return;
+  }
+
+  // This function belongs to a single function Comdat group.
+  Comdat *OrigComdat = F.getComdat();
+  std::string NewComdatName =
+      Twine(OrigComdat->getName() + "." + Twine(FunctionHash)).str();
+  NewComdat = M->getOrInsertComdat(StringRef(NewComdatName));
+  NewComdat->setSelectionKind(OrigComdat->getSelectionKind());
+
+  for (auto &&CM : make_range(ComdatMembers.equal_range(OrigComdat))) {
+    if (GlobalAlias *GA = dyn_cast<GlobalAlias>(CM.second)) {
+      // For aliases, change the name directly.
+      GA->setName(Twine(GA->getName() + "." + Twine(FunctionHash)));
+      continue;
+    }
+    // Must be a function.
+    Function *CF = dyn_cast<Function>(CM.second);
+    assert(CF);
+    CF->setComdat(NewComdat);
+  }
 }
 
 // Given a CFG E to be instrumented, find which BB to place the instrumented

 
 // Visit all edge and instrument the edges not in MST, and do value profiling.
 // Critical edges will be split.
-static void instrumentOneFunc(Function &F, Module *M,
-                              BranchProbabilityInfo *BPI,
-                              BlockFrequencyInfo *BFI) {
+static void instrumentOneFunc(
+    Function &F, Module *M, BranchProbabilityInfo *BPI, BlockFrequencyInfo *BFI,
+    std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers) {
   unsigned NumCounters = 0;
-  FuncPGOInstrumentation<PGOEdge, BBInfo> FuncInfo(F, true, BPI, BFI);
+  FuncPGOInstrumentation<PGOEdge, BBInfo> FuncInfo(F, ComdatMembers, true, BPI,
+                                                   BFI);
   for (auto &E : FuncInfo.MST.AllEdges) {
     if (!E->InMST && !E->Removed)
       NumCounters++;
   }
-
   uint32_t I = 0;
   Type *I8PtrTy = Type::getInt8PtrTy(M->getContext());
   for (auto &E : FuncInfo.MST.AllEdges) {

 
 class PGOUseFunc {
 public:
-  PGOUseFunc(Function &Func, Module *Modu, BranchProbabilityInfo *BPI = nullptr,
+  PGOUseFunc(Function &Func, Module *Modu,
+             std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers,
+             BranchProbabilityInfo *BPI = nullptr,
              BlockFrequencyInfo *BFI = nullptr)
-      : F(Func), M(Modu), FuncInfo(Func, false, BPI, BFI),
+      : F(Func), M(Modu), FuncInfo(Func, ComdatMembers, false, BPI, BFI),
         FreqAttr(FFA_Normal) {}
 
   // Read counts for the instrumented BB from profile.

   // Return the function hotness from the profile.
   FuncFreqAttr getFuncFreqAttr() const { return FreqAttr; }
 
+  // Return the function hash.
+  uint64_t getFuncHash() const { return FuncInfo.FunctionHash; }
   // Return the profile record for this function;
   InstrProfRecord &getProfileRecord() { return ProfileRecord; }
 

         StringRef(INSTR_PROF_QUOTE(IR_LEVEL_PROF_VERSION_VAR))));
 }
 
+// Collect the set of members for each Comdat in module M and store
+// in ComdatMembers.
+static void collectComdatMembers(
+    Module &M,
+    std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers) {
+  if (!DoComdatRenaming)
+    return;
+  for (Function &F : M)
+    if (Comdat *C = F.getComdat())
+      ComdatMembers.insert(std::make_pair(C, &F));
+  for (GlobalVariable &GV : M.globals())
+    if (Comdat *C = GV.getComdat())
+      ComdatMembers.insert(std::make_pair(C, &GV));
+  for (GlobalAlias &GA : M.aliases())
+    if (Comdat *C = GA.getComdat())
+      ComdatMembers.insert(std::make_pair(C, &GA));
+}
+
 static bool InstrumentAllFunctions(
     Module &M, function_ref<BranchProbabilityInfo *(Function &)> LookupBPI,
     function_ref<BlockFrequencyInfo *(Function &)> LookupBFI) {
   createIRLevelProfileFlagVariable(M);
+  std::unordered_multimap<Comdat *, GlobalValue *> ComdatMembers;
+  collectComdatMembers(M, ComdatMembers);
+
   for (auto &F : M) {
     if (F.isDeclaration())
       continue;
     auto *BPI = LookupBPI(F);
     auto *BFI = LookupBFI(F);
-    instrumentOneFunc(F, &M, BPI, BFI);
+    instrumentOneFunc(F, &M, BPI, BFI, ComdatMembers);
   }
   return true;
 }

     return false;
   }
 
+  std::unordered_multimap<Comdat *, GlobalValue *> ComdatMembers;
+  collectComdatMembers(M, ComdatMembers);
   std::vector<Function *> HotFunctions;
   std::vector<Function *> ColdFunctions;
   for (auto &F : M) {

       continue;
     auto *BPI = LookupBPI(F);
     auto *BFI = LookupBFI(F);
-    PGOUseFunc Func(F, &M, BPI, BFI);
+    PGOUseFunc Func(F, &M, ComdatMembers, BPI, BFI);
     if (!Func.readCounters(PGOReader.get()))
       continue;
     Func.populateCounters();

     F->addFnAttr(llvm::Attribute::Cold);
     DEBUG(dbgs() << "Set cold attribute to function: " << F->getName() << "\n");
   }
-
   return true;
 }
 

test/Transforms/PGOProfile/Inputs/indirect_call.proftext

 :ir
 bar
 # Func Hash:
-12884901887
[phone]
 # Num Counters:
 1
 # Counter Values:

test/Transforms/PGOProfile/comdat_internal.ll

 target triple = "x86_64-unknown-linux-gnu"
 
 $foo = comdat any
+; CHECK: $foo.[[FOO_HASH:[0-9]+]] = comdat any
 
 ; CHECK: $__llvm_profile_raw_version = comdat any
-; CHECK: $__profv__stdin__foo = comdat any
+; CHECK: $__profv__stdin__foo.[[FOO_HASH]] = comdat any
 
 @bar = global i32 ()* @foo, align 8
 
 ; CHECK: @__llvm_profile_raw_version = constant i64 {{[0-9]+}}, comdat
-; CHECK: @__profn__stdin__foo = private constant [11 x i8] c"<stdin>:foo"
-; CHECK: @__profc__stdin__foo = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profv__stdin__foo), align 8
-; CHECK: @__profd__stdin__foo = private global { i64, i64, i64*, i8*, i8*, i32, [1 x i16] } { i64 -5640069336071256030, i64 12884901887, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__stdin__foo, i32 0, i32 0), i8*
+; CHECK: @__profn__stdin__foo.[[FOO_HASH]] = private constant [23 x i8] c"<stdin>:foo.[[FOO_HASH]]"
+; CHECK: @__profc__stdin__foo.[[FOO_HASH]] = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profv__stdin__foo.[[FOO_HASH]]), align 8
+; CHECK: @__profd__stdin__foo.[[FOO_HASH]] = private global { i64, i64, i64*, i8*, i8*, i32, [1 x i16] } { i64 6965568665848889497, i64 [[FOO_HASH]], i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__stdin__foo.[[FOO_HASH]], i32 0, i32 0), i8* null
 ; CHECK-NOT: bitcast (i32 ()* @foo to i8*)
-; CHECK-SAME: null
-; CHECK-SAME: , i8* null, i32 1, [1 x i16] zeroinitializer }, section "__llvm_prf_data", comdat($__profv__stdin__foo), align 8
+; CHECK-SAME: , i8* null, i32 1, [1 x i16] zeroinitializer }, section "__llvm_prf_data", comdat($__profv__stdin__foo.[[FOO_HASH]]), align 8
 ; CHECK: @__llvm_prf_nm
 ; CHECK: @llvm.used
 

test/Transforms/PGOProfile/comdat_rename.ll

+; RUN: opt < %s -pgo-instr-gen -S | FileCheck %s
+; RUN: opt < %s -passes=pgo-instr-gen -S | FileCheck %s
+
+; Rename Comdat group and its function.
+$f = comdat any
+; CHECK: $f.[[SINGLEBB_HASH:[0-9]+]] = comdat any
+define linkonce_odr void @f() comdat($f) {
+  ret void
+}
+
+; Not rename Comdat with right linkage.
+$nf = comdat any
+; CHECK: $nf = comdat any
+define void @nf() comdat($nf) {
+  ret void
+}
+
+; Not rename Comdat with variable members.
+$f_with_var = comdat any
+; CHECK: $f_with_var = comdat any
+@var = global i32 0, comdat($f_with_var)
+define linkonce_odr void @f_with_var() comdat($f_with_var) {
+  %tmp = load i32, i32* @var, align 4
+  %inc = add nsw i32 %tmp, 1
+  store i32 %inc, i32* @var, align 4
+  ret void
+}
+
+; Not rename Comdat with multiple functions.
+$tf = comdat any
+; CHECK: $tf = comdat any
+define linkonce void @tf() comdat($tf) {
+  ret void
+}
+define linkonce void @tf2() comdat($tf) {
+  ret void
+}
+
+; Renaming Comdat with aliases.
+$f_with_alias = comdat any
+; CHECK: $f_with_alias.[[SINGLEBB_HASH]] = comdat any
+@af = alias void (...), bitcast (void ()* @f_with_alias to void (...)*)
+; CHECK-DAG: @af.[[SINGLEBB_HASH]] = alias void (...), bitcast (void ()* @f_with_alias.[[SINGLEBB_HASH]] to
+define linkonce_odr void @f_with_alias() comdat($f_with_alias) {
+  ret void
+}
+
+; Rename AvailableExternallyLinkage functions
+; CHECK-DAG: $aef.[[SINGLEBB_HASH]] = comdat any
+define available_externally void @aef() {
+; CHECK: define linkonce_odr void @aef.[[SINGLEBB_HASH]]() comdat {
+  ret void
+}
+
+

test/Transforms/PGOProfile/indirect_call_profile.ll

 define void @foo() {
 entry:
 ; GEN: entry:
-; GEN-NEXT: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_foo, i32 0, i32 0), i64 12884901887, i32 1, i32 0)
+; GEN-NEXT: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_foo, i32 0, i32 0), i64 [[FOO_HASH:[0-9]+]], i32 1, i32 0)
   %tmp = load void ()*, void ()** @bar, align 8
 ; GEN: [[ICALL_TARGET:%[0-9]+]] = ptrtoint void ()* %tmp to i64
-; GEN-NEXT: call void @llvm.instrprof.value.profile(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_foo, i32 0, i32 0), i64 12884901887, i64 [[ICALL_TARGET]], i32 0, i32 0)
+; GEN-NEXT: call void @llvm.instrprof.value.profile(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_foo, i32 0, i32 0), i64 [[FOO_HASH]], i64 [[ICALL_TARGET]], i32 0, i32 0)
   call void %tmp()
   ret void
 }

   invoke void %tmp2()
           to label %bb10 unwind label %bb2
 ; GEN: [[ICALL_TARGET2:%[0-9]+]] = ptrtoint void ()* %tmp2 to i64
-; GEN-NEXT: call void @llvm.instrprof.value.profile(i8* getelementptr inbounds ([4 x i8], [4 x i8]* @__profn_foo2, i32 0, i32 0), i64 38432627612, i64 [[ICALL_TARGET2]], i32 0, i32 0)
+; GEN-NEXT: call void @llvm.instrprof.value.profile(i8* getelementptr inbounds ([4 x i8], [4 x i8]* @__profn_foo2, i32 0, i32 0), i64 [[FOO2_HASH:[0-9]+]], i64 [[ICALL_TARGET2]], i32 0, i32 0)
 
 bb2:                                              ; preds = %bb
   %tmp3 = landingpad { i8*, i32 }

 }
 
 ; Test that comdat function's address is recorded.
-; LOWER: @__profd_foo3 = linkonce_odr{{.*}}@foo3
+; LOWER: @__profd_foo3.[[FOO3_HASH:[0-9]+]] = linkonce_odr{{.*}}@foo3.[[FOO3_HASH]]
 ; Function Attrs: nounwind uwtable
 define linkonce_odr i32 @foo3()  comdat  {
   ret i32 1