Restrict call metadata based hotness detection to Sample PGO mode

[tejohnson]

Summary

Don't use the metadata on call instructions for determining hotness
unless we are in sample PGO mode, where it is needed because profile
counts are not accurate. In instrumentation mode this is not necessary
and does more harm than good when calls have VP metadata that hasn't
been properly scaled after transformations or dropped after constant
prop based devirtualization (both should be fixed, but we don't need
to do this in the first place for instrumentation PGO).

This required adjusting a number of tests to distinguish between sample
and instrumentation PGO handling.

[eraman]

I feel the underlying problem here is the use of VP metadata to get counts in sample PGO mode. Dehao, even when VP metadata is available, can we add branch_weights to the call? Then, only sample PGO will annotate branch weights to calls and that won't affect instrumented PGO.

[tejohnson]

In D32877#747647, @eraman wrote:

I feel the underlying problem here is the use of VP metadata to get counts in sample PGO mode. Dehao, even when VP metadata is available, can we add branch_weights to the call? Then, only sample PGO will annotate branch weights to calls and that won't affect instrumented PGO.

That sounds like potentially a good longer term change. But I'd still like for this to go in as an immediate fix for the issue, which is causing a noticeable impact on instrumentation PGO performance, and because there is no reason AFAIK for anything other than Sample PGO to need to look at call metadata for determining hotness.

[danielcdh]

In D32877#747667, @tejohnson wrote:

In D32877#747647, @eraman wrote:

I feel the underlying problem here is the use of VP metadata to get counts in sample PGO mode. Dehao, even when VP metadata is available, can we add branch_weights to the call? Then, only sample PGO will annotate branch weights to calls and that won't affect instrumented PGO.

That sounds like potentially a good longer term change. But I'd still like for this to go in as an immediate fix for the issue, which is causing a noticeable impact on instrumentation PGO performance, and because there is no reason AFAIK for anything other than Sample PGO to need to look at call metadata for determining hotness.

Just to confirm, with https://reviews.llvm.org/D32773, we should have recovered all the instrumented PGO performance, isn't it?

lib/Analysis/ProfileSummaryInfo.cpp

84

Looks like you can always get the Kind from Inst, why would you want to pass in the Summary?

[tejohnson]

In D32877#747704, @danielcdh wrote:

In D32877#747667, @tejohnson wrote:

In D32877#747647, @eraman wrote:

I feel the underlying problem here is the use of VP metadata to get counts in sample PGO mode. Dehao, even when VP metadata is available, can we add branch_weights to the call? Then, only sample PGO will annotate branch weights to calls and that won't affect instrumented PGO.

That sounds like potentially a good longer term change. But I'd still like for this to go in as an immediate fix for the issue, which is causing a noticeable impact on instrumentation PGO performance, and because there is no reason AFAIK for anything other than Sample PGO to need to look at call metadata for determining hotness.

Just to confirm, with https://reviews.llvm.org/D32773, we should have recovered all the instrumented PGO performance, isn't it?

You're right, I suspect that it may (will be checking after it is fully integrated here), but my concern is that there may still be other transformations that don't update the VP metadata properly (I mentioned earlier this week that e.g. it appears the metadata isn't being dropped when calls are devirtualized after constant prop, although I don't think this causes the performance issue). These things should be fixed for other reasons including Sample PGO accuracy, but it is not needed for instrumentation PGO in the first place - is there any reason for anything other than Sample PGO to look at this instead of the branch weights?

lib/Analysis/ProfileSummaryInfo.cpp

84

It involves doing some work that is already done when the summary is available, so this was added just for the case where it is invoked without a summary.

[eraman]

include/llvm/Analysis/ProfileSummaryInfo.h

62 ↗

(On Diff #97862)

A major rationale for adding ProfileSummaryInfo as a separate analysis is to prevent Profilesummary from being directly manipulated, so I believe we shouldn't add an interface that takes ProfileSummary as a parameter. As Dehao mentions below, you anyway get the module from the instruction and get the kind from there, so this is not necessary. My concern there is this becomes unnecessarily expensive as we get an invariant value (kind) many times.

[tejohnson]

include/llvm/Analysis/ProfileSummaryInfo.h

62 ↗

(On Diff #97862)

A major rationale for adding ProfileSummaryInfo as a separate analysis is to prevent Profilesummary from being directly manipulated, so I believe we shouldn't add an interface that takes ProfileSummary as a parameter.

Note that this is only passed in when getProfileCount() is called from a ProfileSummaryInfo method (it passes in the Summary object it owns), so I am not sure what the concern is about directly manipulating it?

As Dehao mentions below, you anyway get the module from the instruction and get the kind from there, so this is not necessary. My concern there is this becomes unnecessarily expensive as we get an invariant value (kind) many times.

Exactly, that's why I don't think we should keep re-finding the profile metadata every time when it is called from the ProfileSummaryInfo.

[eraman]

include/llvm/Analysis/ProfileSummaryInfo.h

62 ↗

(On Diff #97862)

The concern is you now have a public method that takes ProfileSummaryInfo * (even though no one outside the class passes it). What you can do is have a private helper method that takes a ProfileSummaryInfo *. Within the class, call this directly. Note that
the class already owns the Summary. External users will still call the public method, where you can get the summary from the instruction and call the private method (with a comment about the overhead).

[tejohnson]

include/llvm/Analysis/ProfileSummaryInfo.h

62 ↗

(On Diff #97862)

Done

[tejohnson]

Address review comments

[tejohnson]

Update patch now that getProfileCount is not static. This greatly simplified
the code changes, but required adjusting a few more tests which needed
profile summary metadata.

[eraman]

lib/Analysis/ProfileSummaryInfo.cpp

80

I wonder if we should check if Summary is non-null and then the summary kind is PSK_Sample. There is one test case down below (inliner count update) where you had to attach the summary to the test case. Is there any reason the summary has to be present to get the count based on entry count and block frequency?

[tejohnson]

lib/Analysis/ProfileSummaryInfo.cpp

80

Do you mean only do the metadata-based hotness when computeSummary() returns true and the kind is PSK_Sample? I.e. if !computeSummary(), then assume instrumentation based? I could do that, it would mean a few less test changes.

[eraman]

lib/Analysis/ProfileSummaryInfo.cpp

80

Yes, that's what I should've written. As long as we have function entry counts, we should return the profile count.

[tejohnson]

Treat missing summary as instrumentation PGO, and remove unneeded test changes

[eraman]

LGTM

lib/Analysis/ProfileSummaryInfo.cpp

bool ProfileSummaryInfo::computeSummary() {
   return true;
 }
 
 Optional<uint64_t>
 ProfileSummaryInfo::getProfileCount(const Instruction *Inst,
                                     BlockFrequencyInfo *BFI) {
   if (!Inst)
     return None;
+  if (!computeSummary())
+    return None;
   assert((isa<CallInst>(Inst) || isa<InvokeInst>(Inst)) &&
          "We can only get profile count for call/invoke instruction.");
-  // Check if there is a profile metadata on the instruction. If it is present,
-  // determine hotness solely based on that.
+  if (Summary->getKind() == ProfileSummary::PSK_Sample) {
+    // In sample PGO mode, check if there is a profile metadata on the
+    // instruction. If it is present, determine hotness solely based on that,
+    // since the sampled entry count may not be accurate.
     uint64_t TotalCount;
     if (Inst->extractProfTotalWeight(TotalCount))
       return TotalCount;
+  }
   if (BFI)
     return BFI->getBlockProfileCount(Inst->getParent());
   return None;
 }
 
 /// Returns true if the function's entry is hot. If it returns false, it
 /// either means it is not hot or it is unknown whether it is hot or not (for
 /// example, no profile data is available).

test/Bitcode/thinlto-function-summary-callgraph-pgo.ll

 
 ; CHECK: <SOURCE_FILENAME
 ; CHECK-NEXT: <FUNCTION
 ; "func"
 ; CHECK-NEXT: <FUNCTION op0=4 op1=4
 ; CHECK:       <GLOBALVAL_SUMMARY_BLOCK
 ; CHECK-NEXT:    <VERSION
 ; See if the call to func is registered, using the expected hotness type.
-; CHECK-NEXT:    <PERMODULE_PROFILE {{.*}} op4=1 op5=2/>
+; CHECK-NEXT:    <PERMODULE_PROFILE {{.*}} op4=1 op5=3/>
 ; CHECK-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 ; CHECK: <STRTAB_BLOCK
 ; CHECK-NEXT: blob data = 'mainfunc'
 
 ; COMBINED:       <GLOBALVAL_SUMMARY_BLOCK
 ; COMBINED-NEXT:    <VERSION
 ; COMBINED-NEXT:    <VALUE_GUID op0=[[FUNCID:[0-9]+]] op1=7289175272376759421/>
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <COMBINED
 ; See if the call to func is registered, using the expected hotness type.
-; op6=2 which is hotnessType::None.
-; COMBINED-NEXT:    <COMBINED_PROFILE {{.*}} op5=[[FUNCID]] op6=2/>
+; op6=2 which is hotnessType::Hot.
+; COMBINED-NEXT:    <COMBINED_PROFILE {{.*}} op5=[[FUNCID]] op6=3/>
 ; COMBINED-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 
 ; ModuleID = 'thinlto-function-summary-callgraph.ll'
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 ; Function Attrs: nounwind uwtable
-define i32 @main() #0 !prof !2 {
+define i32 @main() #0 !prof !28 {
 entry:
     call void (...) @func()
     ret i32 0
 }
 
 declare void @func(...) #1
 
-!2 = !{!"function_entry_count", i64 1}
+!llvm.module.flags = !{!1}
+
+!1 = !{i32 1, !"ProfileSummary", !2}
+!2 = !{!3, !4, !5, !6, !7, !8, !9, !10}
+!3 = !{!"ProfileFormat", !"InstrProf"}
+!4 = !{!"TotalCount", i64 2}
+!5 = !{!"MaxCount", i64 1}
+!6 = !{!"MaxInternalCount", i64 0}
+!7 = !{!"MaxFunctionCount", i64 1}
+!8 = !{!"NumCounts", i64 2}
+!9 = !{!"NumFunctions", i64 2}
+!10 = !{!"DetailedSummary", !11}
+!11 = !{!12, !13, !14, !15, !16, !17, !17, !18, !18, !19, !20, !21, !22, !23, !24, !25, !26, !27}
+!12 = !{i32 10000, i64 0, i32 0}
+!13 = !{i32 100000, i64 0, i32 0}
+!14 = !{i32 200000, i64 0, i32 0}
+!15 = !{i32 300000, i64 0, i32 0}
+!16 = !{i32 400000, i64 0, i32 0}
+!17 = !{i32 500000, i64 1, i32 2}
+!18 = !{i32 600000, i64 1, i32 2}
+!19 = !{i32 700000, i64 1, i32 2}
+!20 = !{i32 800000, i64 1, i32 2}
+!21 = !{i32 900000, i64 1, i32 2}
+!22 = !{i32 950000, i64 1, i32 2}
+!23 = !{i32 990000, i64 1, i32 2}
+!24 = !{i32 999000, i64 1, i32 2}
+!25 = !{i32 999900, i64 1, i32 2}
+!26 = !{i32 999990, i64 1, i32 2}
+!27 = !{i32 999999, i64 1, i32 2}
+!28 = !{!"function_entry_count", i64 1}
+
 
 ; OLD: Index {{.*}} contains 1 nodes (1 functions, 0 alias, 0 globals) and 1 edges (0 refs and 1 calls)
 ; OLD-COMBINED: Index {{.*}} contains 2 nodes (2 functions, 0 alias, 0 globals) and 1 edges (0 refs and 1 calls)

test/Bitcode/thinlto-function-summary-callgraph-profile-summary.ll

 ; "none2"
 ; CHECK-NEXT: <FUNCTION op0=37 op1=5
 ; "none3"
 ; CHECK-NEXT: <FUNCTION op0=42 op1=5
 ; CHECK-LABEL:       <GLOBALVAL_SUMMARY_BLOCK
 ; CHECK-NEXT:    <VERSION
 ; CHECK-NEXT:    <VALUE_GUID op0=25 op1=123/>
 ; op4=hot1 op6=cold op8=hot2 op10=hot4 op12=none1 op14=hot3 op16=none2 op18=none3 op20=123
-; CHECK-NEXT:    <PERMODULE_PROFILE {{.*}} op4=1 op5=3 op6=5 op7=1 op8=2 op9=3 op10=4 op11=3 op12=6 op13=2 op14=3 op15=3 op16=7 op17=2 op18=8 op19=2 op20=25 op21=3/>
+; CHECK-NEXT:    <PERMODULE_PROFILE {{.*}} op4=1 op5=3 op6=5 op7=1 op8=2 op9=3 op10=4 op11=1 op12=6 op13=2 op14=3 op15=3 op16=7 op17=2 op18=8 op19=2 op20=25 op21=3/>
 ; CHECK-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 
 ; CHECK: <STRTAB_BLOCK
 ; CHECK-NEXT: blob data = 'hot_functionhot1hot2hot3hot4coldnone1none2none3'
 
 ; COMBINED:       <GLOBALVAL_SUMMARY_BLOCK
 ; COMBINED-NEXT:    <VERSION
 ; COMBINED-NEXT:    <VALUE_GUID

test/Bitcode/thinlto-function-summary-callgraph-sample-profile-summary.ll

 
 
 
 !llvm.module.flags = !{!1}
 !20 = !{!"function_entry_count", i64 110, i64 123}
 
 !1 = !{i32 1, !"ProfileSummary", !2}
 !2 = !{!3, !4, !5, !6, !7, !8, !9, !10}
-!3 = !{!"ProfileFormat", !"InstrProf"}
+!3 = !{!"ProfileFormat", !"SampleProfile"}
 !4 = !{!"TotalCount", i64 10000}
 !5 = !{!"MaxCount", i64 10}
 !6 = !{!"MaxInternalCount", i64 1}
 !7 = !{!"MaxFunctionCount", i64 1000}
 !8 = !{!"NumCounts", i64 3}
 !9 = !{!"NumFunctions", i64 3}
 !10 = !{!"DetailedSummary", !11}
 !11 = !{!12, !13, !14}
 !12 = !{i32 10000, i64 100, i32 1}
 !13 = !{i32 999000, i64 100, i32 1}
 !14 = !{i32 999999, i64 1, i32 2}
 !15 = !{!"branch_weights", i32 100}

test/Transforms/CodeGenPrepare/section-samplepgo.ll

define void @cold_func() !prof !16 {
   ret void
 }
 
 ; CHECK: ![[HOT_ID]] = !{!"function_section_prefix", !".hot"}
 ; CHECK: ![[COLD_ID]] = !{!"function_section_prefix", !".unlikely"}
 !llvm.module.flags = !{!1}
 !1 = !{i32 1, !"ProfileSummary", !2}
 !2 = !{!3, !4, !5, !6, !7, !8, !9, !10}
-!3 = !{!"ProfileFormat", !"InstrProf"}
+!3 = !{!"ProfileFormat", !"SampleProfile"}
 !4 = !{!"TotalCount", i64 10000}
 !5 = !{!"MaxCount", i64 1000}
 !6 = !{!"MaxInternalCount", i64 1}
 !7 = !{!"MaxFunctionCount", i64 1000}
 !8 = !{!"NumCounts", i64 3}
 !9 = !{!"NumFunctions", i64 3}
 !10 = !{!"DetailedSummary", !11}
 !11 = !{!12, !13, !14}
 !12 = !{i32 10000, i64 100, i32 1}
 !13 = !{i32 999000, i64 100, i32 1}
 !14 = !{i32 999999, i64 1, i32 2}
 !15 = !{!"function_entry_count", i64 1000}
 !16 = !{!"function_entry_count", i64 1}
 !17 = !{!"branch_weights", i32 80}
 !18 = !{!"branch_weights", i32 1}

test/Transforms/CodeGenPrepare/section.ll

 ; RUN: opt < %s -codegenprepare -S | FileCheck %s
 
 target triple = "x86_64-pc-linux-gnu"
 
 ; This tests that hot/cold functions get correct section prefix assigned
 
 ; CHECK: hot_func{{.*}}!section_prefix ![[HOT_ID:[0-9]+]]
 ; The entry is hot
 define void @hot_func() !prof !15 {
   ret void
 }
 
-; CHECK: hot_call_func{{.*}}!section_prefix ![[HOT_ID]]
-; The sum of 2 callsites are hot
-define void @hot_call_func() !prof !16 {
+; For instrumentation based PGO, we should only look at entry counts,
+; not call site VP metadata (which can exist on value profiled memcpy,
+; or possibly left behind after static analysis based devirtualization).
+; CHECK: cold_func1{{.*}}!section_prefix ![[COLD_ID:[0-9]+]]
+define void @cold_func1() !prof !16 {
   call void @hot_func(), !prof !17
   call void @hot_func(), !prof !17
   ret void
 }
 
-; CHECK-NOT: normal_func{{.*}}!section_prefix
-; The sum of all callsites are neither hot or cold
-define void @normal_func() !prof !16 {
+; CHECK: cold_func2{{.*}}!section_prefix
+define void @cold_func2() !prof !16 {
   call void @hot_func(), !prof !17
   call void @hot_func(), !prof !18
   call void @hot_func(), !prof !18
   ret void
 }
 
-; CHECK: cold_func{{.*}}!section_prefix ![[COLD_ID:[0-9]+]]
-; The entry and the callsite are both cold
-define void @cold_func() !prof !16 {
+; CHECK: cold_func3{{.*}}!section_prefix ![[COLD_ID]]
+define void @cold_func3() !prof !16 {
   call void @hot_func(), !prof !18
   ret void
 }
 
 ; CHECK: ![[HOT_ID]] = !{!"function_section_prefix", !".hot"}
 ; CHECK: ![[COLD_ID]] = !{!"function_section_prefix", !".unlikely"}
 !llvm.module.flags = !{!1}
 !1 = !{i32 1, !"ProfileSummary", !2}

test/Transforms/Inline/function-count-update-2.ll

 ; RUN: opt < %s -passes='require<profile-summary>,cgscc(inline)' -S | FileCheck %s
 
 ; This tests that the function count of a callee gets correctly updated after it
 ; has been inlined into a two callsites.
 
 ; CHECK: @callee() !prof [[COUNT:![0-9]+]]
-define i32 @callee() !prof !1 {
+define i32 @callee() !prof !15 {
   ret i32 0
 }
 
-define i32 @caller1() !prof !2 {
+define i32 @caller1() !prof !16 {
 ; CHECK-LABEL: @caller1
 ; CHECK-NOT: callee
 ; CHECK: ret
   %i = call i32 @callee()
   ret i32 %i
 }
 
-define i32 @caller2() !prof !3 {
+define i32 @caller2() !prof !17 {
 ; CHECK-LABEL: @caller2
 ; CHECK-NOT: callee
 ; CHECK: ret
   %i = call i32 @callee()
   ret i32 %i
 }
 
-!llvm.module.flags = !{!0}
+!llvm.module.flags = !{!1}
 ; CHECK: [[COUNT]] = !{!"function_entry_count", i64 0}
 !0 = !{i32 1, !"MaxFunctionCount", i32 1000}
-!1 = !{!"function_entry_count", i64 1000}
-!2 = !{!"function_entry_count", i64 600}
-!3 = !{!"function_entry_count", i64 400}
 
+!1 = !{i32 1, !"ProfileSummary", !2}
+!2 = !{!3, !4, !5, !6, !7, !8, !9, !10}
+!3 = !{!"ProfileFormat", !"InstrProf"}
+!4 = !{!"TotalCount", i64 2000}
+!5 = !{!"MaxCount", i64 1000}
+!6 = !{!"MaxInternalCount", i64 0}
+!7 = !{!"MaxFunctionCount", i64 1000}
+!8 = !{!"NumCounts", i64 3}
+!9 = !{!"NumFunctions", i64 3}
+!10 = !{!"DetailedSummary", !11}
+!11 = !{!12, !13, !14}
+!12 = !{i32 10000, i64 1000, i32 1}
+!13 = !{i32 999900, i64 600, i32 2}
+!14 = !{i32 999999, i64 600, i32 2}
+!15 = !{!"function_entry_count", i64 1000}
+!16 = !{!"function_entry_count", i64 600}
+!17 = !{!"function_entry_count", i64 400}

test/Transforms/Inline/function-count-update-3.ll

 ; known and hence the cost gets reduced.
 ; Estimated count of a->e callsite = C2 * (C1 / C4)
 ; Estimated count of a->e callsite = 250 * (200 / 500) = 100
 ; Remaining count of e = C3 - 100 = 500 - 100 = 400
 ; Remaining count of c = C4 - C1 - C5 = 500 - 200 - 300 = 0
 
 @data = external global i32
 
-define i32 @a(i32 %a1) !prof !1 {
+define i32 @a(i32 %a1) !prof !15 {
   %a2 = call i32 @c(i32 %a1, i32 1)
   ret i32 %a2
 }
 
-define i32 @b(i32 %b1) !prof !2 {
+define i32 @b(i32 %b1) !prof !16 {
   %b2 = call i32 @c(i32 %b1, i32 %b1)
   ret i32 %b2
 }
 
 declare void @ext();
 
 ; CHECK: @c(i32 %c1, i32 %c100) !prof [[COUNT1:![0-9]+]]
-define i32 @c(i32 %c1, i32 %c100) !prof !3 {
+define i32 @c(i32 %c1, i32 %c100) !prof !17 {
   call void @ext()
   %cond = icmp sle i32 %c1, 1
   br i1 %cond, label %cond_true, label %cond_false
 
 cond_false:
   ret i32 0
 
 cond_true:
   %c11 = call i32 @e(i32 %c100)
   ret i32 %c11
 }
 
 
 ; CHECK: @e(i32 %c1) !prof [[COUNT2:![0-9]+]]
-define i32 @e(i32 %c1) !prof !4 {
+define i32 @e(i32 %c1) !prof !18 {
   %cond = icmp sle i32 %c1, 1
   br i1 %cond, label %cond_true, label %cond_false
 
 cond_false:
   call void @ext()
   %c2 = load i32, i32* @data, align 4
   %c3 = add i32 %c1, %c2
   %c4 = mul i32 %c3, %c2
   %c5 = add i32 %c4, %c2
   %c6 = mul i32 %c5, %c2
   %c7 = add i32 %c6, %c2
   %c8 = mul i32 %c7, %c2
   %c9 = add i32 %c8, %c2
   %c10 = mul i32 %c9, %c2
   ret i32 %c10
 
 cond_true:
   ret i32 0
 }
 
-!llvm.module.flags = !{!0}
+!llvm.module.flags = !{!1}
 ; CHECK: [[COUNT1]] = !{!"function_entry_count", i64 0}
 ; CHECK: [[COUNT2]] = !{!"function_entry_count", i64 400}
-!0 = !{i32 1, !"MaxFunctionCount", i32 5000}
-!1 = !{!"function_entry_count", i64 200}
-!2 = !{!"function_entry_count", i64 300}
-!3 = !{!"function_entry_count", i64 500}
-!4 = !{!"function_entry_count", i64 500}
-
+!1 = !{i32 1, !"ProfileSummary", !2}
+!2 = !{!3, !4, !5, !6, !7, !8, !9, !10}
+!3 = !{!"ProfileFormat", !"InstrProf"}
+!4 = !{!"TotalCount", i64 6500}
+!5 = !{!"MaxCount", i64 5000}
+!6 = !{!"MaxInternalCount", i64 0}
+!7 = !{!"MaxFunctionCount", i64 5000}
+!8 = !{!"NumCounts", i64 4}
+!9 = !{!"NumFunctions", i64 4}
+!10 = !{!"DetailedSummary", !11}
+!11 = !{!12, !13, !14}
+!12 = !{i32 10000, i64 5000, i32 1}
+!13 = !{i32 999900, i64 5000, i32 1}
+!14 = !{i32 999999, i64 5000, i32 1}
+!15 = !{!"function_entry_count", i64 200}
+!16 = !{!"function_entry_count", i64 300}
+!17 = !{!"function_entry_count", i64 500}
+!18 = !{!"function_entry_count", i64 500}

test/Transforms/Inline/function-count-update.ll

 ; RUN: opt < %s -passes='require<profile-summary>,cgscc(inline)' -S | FileCheck %s
 
 ; This tests that the function count of two callees get correctly updated after
 ; they have been inlined into two back-to-back callsites in a single basic block
 ; in the caller. The callees have the alwaysinline attribute and so they get
 ; inlined both with the regular inliner pass and the always inline pass. In
 ; both cases, the new count of each callee is the original count minus callsite
 ; count which is 200 (since the caller's entry count is 400 and the block
 ; containing the calls have a relative block frequency of 0.5).
 
 ; CHECK: @callee1(i32 %n) #0 !prof [[COUNT1:![0-9]+]]
-define i32 @callee1(i32 %n) #0 !prof !1 {
+define i32 @callee1(i32 %n) #0 !prof !15 {
   %cond = icmp sle i32 %n, 10
   br i1 %cond, label %cond_true, label %cond_false
 
 cond_true:
   %r1 = add i32 %n, 1
   ret i32 %r1
 cond_false:
   %r2 = add i32 %n, 2
   ret i32 %r2
 }
 
 ; CHECK: @callee2(i32 %n) #0 !prof [[COUNT2:![0-9]+]]
-define i32 @callee2(i32 %n) #0 !prof !2 {
+define i32 @callee2(i32 %n) #0 !prof !16 {
   %r1 = add i32 %n, 1
   ret i32 %r1
 }
 
-define i32 @caller(i32 %n) !prof !3 {
+define i32 @caller(i32 %n) !prof !17 {
   %cond = icmp sle i32 %n, 100
   br i1 %cond, label %cond_true, label %cond_false
 
 cond_true:
   %i = call i32 @callee1(i32 %n)
   %j = call i32 @callee2(i32 %i)
   ret i32 %j
 cond_false:
   ret i32 0
 }
 
-!llvm.module.flags = !{!0}
+!llvm.module.flags = !{!1}
 ; CHECK: [[COUNT1]] = !{!"function_entry_count", i64 800}
 ; CHECK: [[COUNT2]] = !{!"function_entry_count", i64 1800}
-!0 = !{i32 1, !"MaxFunctionCount", i32 1000}
-!1 = !{!"function_entry_count", i64 1000}
-!2 = !{!"function_entry_count", i64 2000}
-!3 = !{!"function_entry_count", i64 400}
+!1 = !{i32 1, !"ProfileSummary", !2}
+!2 = !{!3, !4, !5, !6, !7, !8, !9, !10}
+!3 = !{!"ProfileFormat", !"InstrProf"}
+!4 = !{!"TotalCount", i64 3400}
+!5 = !{!"MaxCount", i64 2000}
+!6 = !{!"MaxInternalCount", i64 400}
+!7 = !{!"MaxFunctionCount", i64 2000}
+!8 = !{!"NumCounts", i64 3}
+!9 = !{!"NumFunctions", i64 3}
+!10 = !{!"DetailedSummary", !11}
+!11 = !{!12, !13, !14}
+!12 = !{i32 10000, i64 2000, i32 1}
+!13 = !{i32 999900, i64 1000, i32 2}
+!14 = !{i32 999999, i64 1000, i32 2}
+!15 = !{!"function_entry_count", i64 1000}
+!16 = !{!"function_entry_count", i64 2000}
+!17 = !{!"function_entry_count", i64 400}
 attributes #0 = { alwaysinline }
 

test/Transforms/Inline/prof-update.ll

 ; RUN: opt < %s -inline -S | FileCheck %s
 ; Checks if inliner updates branch_weights annotation for call instructions.
 
 declare void @ext();
 declare void @ext1();
 @func = global void ()* null
 
 ; CHECK: define void @callee(i32 %n) !prof ![[ENTRY_COUNT:[0-9]*]]
-define void  @callee(i32 %n) !prof !1 {
+define void  @callee(i32 %n) !prof !15 {
   %cond = icmp sle i32 %n, 10
   br i1 %cond, label %cond_true, label %cond_false
 cond_true:
 ; ext1 is optimized away, thus not updated.
 ; CHECK: call void @ext1(), !prof ![[COUNT_CALLEE1:[0-9]*]]
-  call void @ext1(), !prof !2
+  call void @ext1(), !prof !16
   ret void
 cond_false:
 ; ext is cloned and updated.
 ; CHECK: call void @ext(), !prof ![[COUNT_CALLEE:[0-9]*]]
-  call void @ext(), !prof !2
+  call void @ext(), !prof !16
   %f = load void ()*, void ()** @func
 ; CHECK: call void %f(), !prof ![[COUNT_IND_CALLEE:[0-9]*]] 
-  call void %f(), !prof !4
+  call void %f(), !prof !18
   ret void
 }
 
 ; CHECK: define void @caller()
 define void @caller() {
 ; CHECK: call void @ext(), !prof ![[COUNT_CALLER:[0-9]*]]
 ; CHECK: call void %f.i(), !prof ![[COUNT_IND_CALLER:[0-9]*]]
-  call void @callee(i32 15), !prof !3
+  call void @callee(i32 15), !prof !17
   ret void
 }
 
-!llvm.module.flags = !{!0}
-!0 = !{i32 1, !"MaxFunctionCount", i32 2000}
-!1 = !{!"function_entry_count", i64 1000}
-!2 = !{!"branch_weights", i64 2000}
-!3 = !{!"branch_weights", i64 400}
-!4 = !{!"VP", i32 0, i64 140, i64 111, i64 80, i64 222, i64 40, i64 333, i64 20}
+!llvm.module.flags = !{!1}
+!1 = !{i32 1, !"ProfileSummary", !2}
+!2 = !{!3, !4, !5, !6, !7, !8, !9, !10}
+!3 = !{!"ProfileFormat", !"SampleProfile"}
+!4 = !{!"TotalCount", i64 10000}
+!5 = !{!"MaxCount", i64 10}
+!6 = !{!"MaxInternalCount", i64 1}
+!7 = !{!"MaxFunctionCount", i64 2000}
+!8 = !{!"NumCounts", i64 2}
+!9 = !{!"NumFunctions", i64 2}
+!10 = !{!"DetailedSummary", !11}
+!11 = !{!12, !13, !14}
+!12 = !{i32 10000, i64 100, i32 1}
+!13 = !{i32 999000, i64 100, i32 1}
+!14 = !{i32 999999, i64 1, i32 2}
+!15 = !{!"function_entry_count", i64 1000}
+!16 = !{!"branch_weights", i64 2000}
+!17 = !{!"branch_weights", i64 400}
+!18 = !{!"VP", i32 0, i64 140, i64 111, i64 80, i64 222, i64 40, i64 333, i64 20}
 attributes #0 = { alwaysinline }
 ; CHECK: ![[ENTRY_COUNT]] = !{!"function_entry_count", i64 600}
 ; CHECK: ![[COUNT_CALLEE1]] = !{!"branch_weights", i64 2000}
 ; CHECK: ![[COUNT_CALLEE]] = !{!"branch_weights", i64 1200}
 ; CHECK: ![[COUNT_IND_CALLEE]] = !{!"VP", i32 0, i64 84, i64 111, i64 48, i64 222, i64 24, i64 333, i64 12}
 ; CHECK: ![[COUNT_CALLER]] = !{!"branch_weights", i64 800}
 ; CHECK: ![[COUNT_IND_CALLER]] = !{!"VP", i32 0, i64 56, i64 111, i64 32, i64 222, i64 16, i64 333, i64 8}

unittests/Analysis/ProfileSummaryInfoTest.cpp

TEST_F(ProfileSummaryInfoTest, InstrProf) {
   EXPECT_TRUE(PSI.isHotBB(BB3, &BFI));
 
   CallSite CS1(BB1->getFirstNonPHI());
   auto *CI2 = BB2->getFirstNonPHI();
   CallSite CS2(CI2);
 
   EXPECT_TRUE(PSI.isHotCallSite(CS1, &BFI));
   EXPECT_FALSE(PSI.isHotCallSite(CS2, &BFI));
+
+  // Test that adding an MD_prof metadata with a hot count on CS2 does not
+  // change its hotness as it has no effect in instrumented profiling.
+  MDBuilder MDB(M->getContext());
+  CI2->setMetadata(llvm::LLVMContext::MD_prof, MDB.createBranchWeights({400}));
+  EXPECT_FALSE(PSI.isHotCallSite(CS2, &BFI));
 }
 
 TEST_F(ProfileSummaryInfoTest, SampleProf) {
   auto M = makeLLVMModule("SampleProfile");
   Function *F = M->getFunction("f");
   ProfileSummaryInfo PSI = buildPSI(M.get());
 
   BasicBlock &BB0 = F->getEntryBlock();