[PATCH] D52845: Update entry count for cold calls

[Wei Mi via Phabricator via llvm-commits]

wmi added inline comments.


================
Comment at: lib/IR/Function.cpp:1403-1404
 
+void Function::updateProfileData(int64_t entryDelta,
+                                 const ValueMap<const Value *, WeakTrackingVH> * VMap) {
+  auto CalleeCount = getEntryCount();
----------------
It seems to me a little bit weird to have some structure specific for inliner in the interface of llvm IR. It looks more like a transformation utility and better to stay under lib/Transforms/Utils/. We may create a new file for it if there is nowhere else to put. 

And maybe make the name more explicit like updateCalleeProfile. 


================
Comment at: lib/Transforms/IPO/SampleProfile.cpp:326
+    uint64_t entryCount;
+    uint64_t samples;
+  };
----------------
I don't find where samples is used?


================
Comment at: lib/Transforms/IPO/SampleProfile.cpp:848
             if ((isa<CallInst>(DI) || isa<InvokeInst>(DI)) &&
                 inlineCallInstruction(DI))
               LocalChanged = true;
----------------
Indirect call is skipped in the current patch, do you think it is better to handle it as well? I don't have strong opionion on this since it is cold. 


================
Comment at: lib/Transforms/IPO/SampleProfile.cpp:1488
   DenseSet<GlobalValue::GUID> InlinedGUIDs;
-  Changed |= inlineHotFunctions(F, InlinedGUIDs);
+  std::pair<bool,uint64_t> inlineResult = inlineHotFunctions(F, InlinedGUIDs);
+  Changed |= inlineResult.first;
----------------
inlineResult.second is never used. What is the intention to change the interface of inlineHotFunctions?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D52845/new/

https://reviews.llvm.org/D52845