Update entry count for cold calls

Summary:
Profile sample files include the number of times each entry or inlined
call site is sampled. This is translated into the entry count metadta
on functions.

When sample data is being read, if a call site that was inlined
in the sample program is considered cold and not inlined, then
the entry count of the out-of-line functions does not reflect
the current compilation.

In this patch, we note call sites where the function was not inlined
and as a last action of the sample profile loading, we update the
called function's entry count to reflect the calls from these
call sites which are not included in the profile file.

Reviewers: danielcdh, wmi, Kader, modocache

Reviewed By: wmi

Subscribers: davidxl, eraman, llvm-commits

Differential Revision: https://reviews.llvm.org/D52845

llvm-svn: 352001

llvm/include/llvm/Transforms/Utils/Cloning.h

@@ -268,6 +268,13 @@ BasicBlock *DuplicateInstructionsInSplitBetween(BasicBlock *BB,
                                                 ValueToValueMapTy &ValueMapping,
                                                 DomTreeUpdater &DTU);
 
+/// Updates profile information by adjusting the entry count by adding
+/// entryDelta then scaling callsite information by the new count divided by the
+/// old count. VMap is used during inlinng to also update the new clone
+void updateProfileCallee(
+    Function *Callee, int64_t entryDelta,
+    const ValueMap<const Value *, WeakTrackingVH> *VMap = nullptr);
+
 } // end namespace llvm
 
 #endif // LLVM_TRANSFORMS_UTILS_CLONING_H

llvm/lib/Transforms/IPO/SampleProfile.cpp

@@ -318,6 +318,14 @@ class SampleProfileLoader {
 
   /// Optimization Remark Emitter used to emit diagnostic remarks.
   OptimizationRemarkEmitter *ORE = nullptr;
+
+  // Information recorded when we declined to inline a call site
+  // because we have determined it is too cold is accumulated for
+  // each callee function. Initially this is just the entry count.
+  struct NotInlinedProfileInfo {
+    uint64_t entryCount;
+  };
+  DenseMap<Function *, NotInlinedProfileInfo> notInlinedCallInfo;
 };
 
 class SampleProfileLoaderLegacyPass : public ModulePass {
@@ -778,6 +786,8 @@ bool SampleProfileLoader::inlineCallInstruction(Instruction *I) {
 bool SampleProfileLoader::inlineHotFunctions(
     Function &F, DenseSet<GlobalValue::GUID> &InlinedGUIDs) {
   DenseSet<Instruction *> PromotedInsns;
+
+  DenseMap<Instruction *, const FunctionSamples *> localNotInlinedCallSites;
   bool Changed = false;
   while (true) {
     bool LocalChanged = false;
@@ -790,6 +800,8 @@ bool SampleProfileLoader::inlineHotFunctions(
         if ((isa<CallInst>(I) || isa<InvokeInst>(I)) &&
             !isa<IntrinsicInst>(I) && (FS = findCalleeFunctionSamples(I))) {
           Candidates.push_back(&I);
+          if (FS->getEntrySamples() > 0)
+            localNotInlinedCallSites.try_emplace(&I, FS);
           if (callsiteIsHot(FS, PSI))
             Hot = true;
         }
@@ -835,8 +847,10 @@ bool SampleProfileLoader::inlineHotFunctions(
             PromotedInsns.insert(I);
             // If profile mismatches, we should not attempt to inline DI.
             if ((isa<CallInst>(DI) || isa<InvokeInst>(DI)) &&
-                inlineCallInstruction(DI))
+                inlineCallInstruction(DI)) {
+              localNotInlinedCallSites.erase(I);
               LocalChanged = true;
+            }
           } else {
             LLVM_DEBUG(dbgs()
                        << "\nFailed to promote indirect call to "
@@ -845,8 +859,10 @@ bool SampleProfileLoader::inlineHotFunctions(
         }
       } else if (CalledFunction && CalledFunction->getSubprogram() &&
                  !CalledFunction->isDeclaration()) {
-        if (inlineCallInstruction(I))
+        if (inlineCallInstruction(I)) {
+          localNotInlinedCallSites.erase(I);
           LocalChanged = true;
+        }
       } else if (IsThinLTOPreLink) {
         findCalleeFunctionSamples(*I)->findInlinedFunctions(
             InlinedGUIDs, F.getParent(), PSI->getOrCompHotCountThreshold());
@@ -858,6 +874,18 @@ bool SampleProfileLoader::inlineHotFunctions(
       break;
     }
   }
+
+  // Accumulate not inlined callsite information into notInlinedSamples
+  for (const auto &Pair : localNotInlinedCallSites) {
+    Instruction *I = Pair.getFirst();
+    Function *Callee = CallSite(I).getCalledFunction();
+    if (!Callee || Callee->isDeclaration())
+      continue;
+    const FunctionSamples *FS = Pair.getSecond();
+    auto pair =
+        notInlinedCallInfo.try_emplace(Callee, NotInlinedProfileInfo{0});
+    pair.first->second.entryCount += FS->getEntrySamples();
+  }
   return Changed;
 }
 
@@ -1600,6 +1628,12 @@ bool SampleProfileLoader::runOnModule(Module &M, ModuleAnalysisManager *AM,
       clearFunctionData();
       retval |= runOnFunction(F, AM);
     }
+
+  // Account for cold calls not inlined....
+  for (const std::pair<Function *, NotInlinedProfileInfo> &pair :
+       notInlinedCallInfo)
+    updateProfileCallee(pair.first, pair.second.entryCount);
+
   return retval;
 }
 

llvm/lib/Transforms/Utils/InlineFunction.cpp

@@ -1447,47 +1447,45 @@ static void updateCallProfile(Function *Callee, const ValueToValueMapTy &VMap,
       CalleeEntryCount.getCount() < 1)
     return;
   auto CallSiteCount = PSI ? PSI->getProfileCount(TheCall, CallerBFI) : None;
-  uint64_t CallCount =
+  int64_t CallCount =
       std::min(CallSiteCount.hasValue() ? CallSiteCount.getValue() : 0,
                CalleeEntryCount.getCount());
-
-  for (auto const &Entry : VMap)
-    if (isa<CallInst>(Entry.first))
-      if (auto *CI = dyn_cast_or_null<CallInst>(Entry.second))
-        CI->updateProfWeight(CallCount, CalleeEntryCount.getCount());
-  for (BasicBlock &BB : *Callee)
-    // No need to update the callsite if it is pruned during inlining.
-    if (VMap.count(&BB))
-      for (Instruction &I : BB)
-        if (CallInst *CI = dyn_cast<CallInst>(&I))
-          CI->updateProfWeight(CalleeEntryCount.getCount() - CallCount,
-                               CalleeEntryCount.getCount());
+  updateProfileCallee(Callee, -CallCount, &VMap);
 }
 
-/// Update the entry count of callee after inlining.
-///
-/// The callsite's block count is subtracted from the callee's function entry
-/// count.
-static void updateCalleeCount(BlockFrequencyInfo *CallerBFI, BasicBlock *CallBB,
-                              Instruction *CallInst, Function *Callee,
-                              ProfileSummaryInfo *PSI) {
-  // If the callee has a original count of N, and the estimated count of
-  // callsite is M, the new callee count is set to N - M. M is estimated from
-  // the caller's entry count, its entry block frequency and the block frequency
-  // of the callsite.
+void llvm::updateProfileCallee(
+    Function *Callee, int64_t entryDelta,
+    const ValueMap<const Value *, WeakTrackingVH> *VMap) {
   auto CalleeCount = Callee->getEntryCount();
-  if (!CalleeCount.hasValue() || !PSI)
-    return;
-  auto CallCount = PSI->getProfileCount(CallInst, CallerBFI);
-  if (!CallCount.hasValue())
+  if (!CalleeCount.hasValue())
     return;
+
+  uint64_t priorEntryCount = CalleeCount.getCount();
+  uint64_t newEntryCount = priorEntryCount;
+
   // Since CallSiteCount is an estimate, it could exceed the original callee
-  // count and has to be set to 0.
-  if (CallCount.getValue() > CalleeCount.getCount())
-    CalleeCount.setCount(0);
+  // count and has to be set to 0 so guard against underflow.
+  if (entryDelta < 0 && static_cast<uint64_t>(-entryDelta) > priorEntryCount)
+    newEntryCount = 0;
   else
-    CalleeCount.setCount(CalleeCount.getCount() - CallCount.getValue());
-  Callee->setEntryCount(CalleeCount);
+    newEntryCount = priorEntryCount + entryDelta;
+
+  Callee->setEntryCount(newEntryCount);
+
+  // During inlining ?
+  if (VMap) {
+    uint64_t cloneEntryCount = priorEntryCount - newEntryCount;
+    for (auto const &Entry : *VMap)
+      if (isa<CallInst>(Entry.first))
+        if (auto *CI = dyn_cast_or_null<CallInst>(Entry.second))
+          CI->updateProfWeight(cloneEntryCount, priorEntryCount);
+  }
+  for (BasicBlock &BB : *Callee)
+    // No need to update the callsite if it is pruned during inlining.
+    if (!VMap || VMap->count(&BB))
+      for (Instruction &I : BB)
+        if (CallInst *CI = dyn_cast<CallInst>(&I))
+          CI->updateProfWeight(newEntryCount, priorEntryCount);
 }
 
 /// This function inlines the called function into the basic block of the
@@ -1683,8 +1681,6 @@ llvm::InlineResult llvm::InlineFunction(CallSite CS, InlineFunctionInfo &IFI,
 
     updateCallProfile(CalledFunc, VMap, CalledFunc->getEntryCount(), TheCall,
                       IFI.PSI, IFI.CallerBFI);
-    // Update the profile count of callee.
-    updateCalleeCount(IFI.CallerBFI, OrigBB, TheCall, CalledFunc, IFI.PSI);
 
     // Inject byval arguments initialization.
     for (std::pair<Value*, Value*> &Init : ByValInit)

llvm/test/Transforms/SampleProfile/Inputs/entry_counts_cold.prof

@@ -0,0 +1,20 @@
+top:200:100
+ 1: 100 foo:100
+ 2: 100
+ 3: 2
+ 4: 100
+ 1: foo:100
+    2: 100
+    3: 100 bar:100
+    4: 100
+ 3: bar:2
+    1: 2
+    2: 2
+foo:200:150
+ 2: 150
+ 3: 150  bar:150
+ 4: 150
+bar:450:300
+ 1: 300 baz:300
+ 2: 300
+ 3: 300

llvm/test/Transforms/SampleProfile/entry_counts_cold.ll

@@ -0,0 +1,170 @@
+; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/entry_counts_cold.prof -S | FileCheck %s
+; ModuleID = 'temp.bc'
+source_filename = "temp.c"
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.14.0"
+
+; Function Attrs: nounwind ssp uwtable
+; CHECK: define i32 @top({{.*}} !prof [[TOP:![0-9]+]] 
+define i32 @top(i32* %p) #0 !dbg !8 {
+entry:
+  %p.addr = alloca i32*, align 8
+  store i32* %p, i32** %p.addr, align 8, !tbaa !15
+  call void @llvm.dbg.declare(metadata i32** %p.addr, metadata !14, metadata !DIExpression()), !dbg !19
+  %0 = load i32*, i32** %p.addr, align 8, !dbg !20, !tbaa !15
+  %call = call i32 @foo(i32* %0), !dbg !21
+; foo is inlined
+; CHECK-NOT: call i32 @foo
+; CHECK: call i32 @bar
+  %1 = load i32*, i32** %p.addr, align 8, !dbg !22, !tbaa !15
+  %2 = load i32, i32* %1, align 4, !dbg !24, !tbaa !25
+  %tobool = icmp ne i32 %2, 0, !dbg !24
+  br i1 %tobool, label %if.then, label %if.end, !dbg !27
+
+if.then:                                          ; preds = %entry
+  %3 = load i32*, i32** %p.addr, align 8, !dbg !28, !tbaa !15
+; bar is not inlined
+; CHECK: call i32 @bar
+  %call1 = call i32 @bar(i32* %3), !dbg !29
+  br label %if.end, !dbg !29
+
+if.end:                                           ; preds = %if.then, %entry
+  ret i32 0, !dbg !30
+}
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+; Function Attrs: nounwind ssp uwtable
+; CHECK: define i32 @foo({{.*}} !prof [[FOO:![0-9]+]] 
+define i32 @foo(i32* %p) #0 !dbg !31 {
+entry:
+  %p.addr = alloca i32*, align 8
+  %a = alloca i32, align 4
+  store i32* %p, i32** %p.addr, align 8, !tbaa !15
+  call void @llvm.dbg.declare(metadata i32** %p.addr, metadata !33, metadata !DIExpression()), !dbg !35
+  %0 = bitcast i32* %a to i8*, !dbg !36
+  call void @llvm.lifetime.start.p0i8(i64 4, i8* %0) #4, !dbg !36
+  call void @llvm.dbg.declare(metadata i32* %a, metadata !34, metadata !DIExpression()), !dbg !37
+  %1 = load i32*, i32** %p.addr, align 8, !dbg !38, !tbaa !15
+  %arrayidx = getelementptr inbounds i32, i32* %1, i64 3, !dbg !38
+  %2 = load i32, i32* %arrayidx, align 4, !dbg !38, !tbaa !25
+  %3 = load i32*, i32** %p.addr, align 8, !dbg !39, !tbaa !15
+  %arrayidx1 = getelementptr inbounds i32, i32* %3, i64 2, !dbg !39
+  %4 = load i32, i32* %arrayidx1, align 4, !dbg !40, !tbaa !25
+  %add = add nsw i32 %4, %2, !dbg !40
+  store i32 %add, i32* %arrayidx1, align 4, !dbg !40, !tbaa !25
+  %5 = load i32*, i32** %p.addr, align 8, !dbg !41, !tbaa !15
+  %call = call i32 @bar(i32* %5), !dbg !42
+  store i32 %call, i32* %a, align 4, !dbg !43, !tbaa !25
+  %6 = load i32, i32* %a, align 4, !dbg !44, !tbaa !25
+  %add2 = add nsw i32 %6, 1, !dbg !45
+  %7 = bitcast i32* %a to i8*, !dbg !46
+  call void @llvm.lifetime.end.p0i8(i64 4, i8* %7) #4, !dbg !46
+  ret i32 %add2, !dbg !47
+}
+
+; Function Attrs: nounwind ssp uwtable
+; CHECK: define i32 @bar({{.*}} !prof [[BAR:![0-9]+]] 
+define i32 @bar(i32* %p) #0 !dbg !48 {
+entry:
+  %p.addr = alloca i32*, align 8
+  store i32* %p, i32** %p.addr, align 8, !tbaa !15
+  call void @llvm.dbg.declare(metadata i32** %p.addr, metadata !50, metadata !DIExpression()), !dbg !51
+  ; CHECK: call void (...) @baz{{.*}} !prof [[BAZ:![0-9]+]]
+  call void (...) @baz(), !dbg !52
+  %0 = load i32*, i32** %p.addr, align 8, !dbg !53, !tbaa !15
+  %arrayidx = getelementptr inbounds i32, i32* %0, i64 2, !dbg !53
+  %1 = load i32, i32* %arrayidx, align 4, !dbg !53, !tbaa !25
+  %2 = load i32*, i32** %p.addr, align 8, !dbg !54, !tbaa !15
+  %arrayidx1 = getelementptr inbounds i32, i32* %2, i64 1, !dbg !54
+  %3 = load i32, i32* %arrayidx1, align 4, !dbg !55, !tbaa !25
+  %add = add nsw i32 %3, %1, !dbg !55
+  store i32 %add, i32* %arrayidx1, align 4, !dbg !55, !tbaa !25
+  %4 = load i32*, i32** %p.addr, align 8, !dbg !56, !tbaa !15
+  %arrayidx2 = getelementptr inbounds i32, i32* %4, i64 3, !dbg !56
+  %5 = load i32, i32* %arrayidx2, align 4, !dbg !56, !tbaa !25
+  ret i32 %5, !dbg !57
+}
+
+; Function Attrs: argmemonly nounwind
+declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture) #2
+
+; Function Attrs: argmemonly nounwind
+declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture) #2
+
+declare void @baz(...) #3
+
+attributes #0 = { nounwind ssp uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="penryn" "target-features"="+cx16,+fxsr,+mmx,+sahf,+sse,+sse2,+sse3,+sse4.1,+ssse3,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind readnone speculatable }
+attributes #2 = { argmemonly nounwind }
+attributes #3 = { "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="penryn" "target-features"="+cx16,+fxsr,+mmx,+sahf,+sse,+sse2,+sse3,+sse4.1,+ssse3,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #4 = { nounwind }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+; CHECK: [[TOP]] = !{!"function_entry_count", i64 101}
+; CHECK: [[FOO]] = !{!"function_entry_count", i64 151}
+; CHECK: [[BAR]] = !{!"function_entry_count", i64 303}
+; CHECK: [[BAZ]] = !{!"branch_weights", i64 303}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 8.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: GNU)
+!1 = !DIFile(filename: "temp.c", directory: "llvm/test/Transforms/SampleProfile")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{i32 7, !"PIC Level", i32 2}
+!7 = !{!"clang version 8.0.0"}
+!8 = distinct !DISubprogram(name: "top", scope: !1, file: !1, line: 5, type: !9, scopeLine: 5, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !13)
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11, !12}
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !11, size: 64)
+!13 = !{!14}
+!14 = !DILocalVariable(name: "p", arg: 1, scope: !8, file: !1, line: 5, type: !12)
+!15 = !{!16, !16, i64 0}
+!16 = !{!"any pointer", !17, i64 0}
+!17 = !{!"omnipotent char", !18, i64 0}
+!18 = !{!"Simple C/C++ TBAA"}
+!19 = !DILocation(line: 5, column: 14, scope: !8)
+!20 = !DILocation(line: 6, column: 7, scope: !8)
+!21 = !DILocation(line: 6, column: 3, scope: !8)
+!22 = !DILocation(line: 7, column: 8, scope: !23)
+!23 = distinct !DILexicalBlock(scope: !8, file: !1, line: 7, column: 7)
+!24 = !DILocation(line: 7, column: 7, scope: !23)
+!25 = !{!26, !26, i64 0}
+!26 = !{!"int", !17, i64 0}
+!27 = !DILocation(line: 7, column: 7, scope: !8)
+!28 = !DILocation(line: 8, column: 9, scope: !23)
+!29 = !DILocation(line: 8, column: 5, scope: !23)
+!30 = !DILocation(line: 9, column: 3, scope: !8)
+!31 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 12, type: !9, scopeLine: 12, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !32)
+!32 = !{!33, !34}
+!33 = !DILocalVariable(name: "p", arg: 1, scope: !31, file: !1, line: 12, type: !12)
+!34 = !DILocalVariable(name: "a", scope: !31, file: !1, line: 13, type: !11)
+!35 = !DILocation(line: 12, column: 14, scope: !31)
+!36 = !DILocation(line: 13, column: 3, scope: !31)
+!37 = !DILocation(line: 13, column: 7, scope: !31)
+!38 = !DILocation(line: 14, column: 11, scope: !31)
+!39 = !DILocation(line: 14, column: 3, scope: !31)
+!40 = !DILocation(line: 14, column: 8, scope: !31)
+!41 = !DILocation(line: 15, column: 11, scope: !31)
+!42 = !DILocation(line: 15, column: 7, scope: !31)
+!43 = !DILocation(line: 15, column: 5, scope: !31)
+!44 = !DILocation(line: 16, column: 10, scope: !31)
+!45 = !DILocation(line: 16, column: 11, scope: !31)
+!46 = !DILocation(line: 17, column: 1, scope: !31)
+!47 = !DILocation(line: 16, column: 3, scope: !31)
+!48 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 19, type: !9, scopeLine: 19, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !49)
+!49 = !{!50}
+!50 = !DILocalVariable(name: "p", arg: 1, scope: !48, file: !1, line: 19, type: !12)
+!51 = !DILocation(line: 19, column: 15, scope: !48)
+!52 = !DILocation(line: 20, column: 3, scope: !48)
+!53 = !DILocation(line: 21, column: 11, scope: !48)
+!54 = !DILocation(line: 21, column: 3, scope: !48)
+!55 = !DILocation(line: 21, column: 8, scope: !48)
+!56 = !DILocation(line: 22, column: 10, scope: !48)
+!57 = !DILocation(line: 22, column: 3, scope: !48)