[PATCH] D43521: [ThinLTO] Compute synthetic function entry count

[Easwaran Raman via Phabricator via llvm-commits]

eraman marked 8 inline comments as done.
eraman added a comment.

I have to bump up the index version to 6 after rebasing and fix a few failing test cases.



================
Comment at: lib/LTO/ThinLTOCodeGenerator.cpp:203
+                      const FunctionImporter::ImportMapTy &ImportList,
+                      const GVSummaryMapTy &DefinedGlobals) {
   auto Loader = [&](StringRef Identifier) {
----------------
tejohnson wrote:
> I don't see this new parameter getting used anywhere. I think it might be leftover from your earlier approach (along with the other changes to this file)?
Yes, this is leftover from earlier iterations


================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:847
+
+void llvm::computeSyntheticCounts(ModuleSummaryIndex &Index) {
+  using Scaled64 = ScaledNumber<uint64_t>;
----------------
tejohnson wrote:
> I assume that eventually we would want to gate this based on whether we have FDO or not - how will that work?
I think eventually we want this to work even when profile data is available by filling in the synthesized count for only those functions which do not have an entry count (a hybrid mode). What I have done now is to gate this on -enable-import-entry-count. To avoid declaring this as extern, I have moved the computeSyntheticCount to FunctionImportUtils.cpp  (where the flag is defined) and bail out if the flag is false. 



================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:851
+  auto GetCallSiteRelFreq = [](FunctionSummary::EdgeTy &Edge) {
+    // FIXME: Handle fake/invalid edges?
+    return Scaled64(Edge.second.RelBlockFreq, -CalleeInfo::ScaleShift);
----------------
tejohnson wrote:
> What is a fake/invalid edge in the index?
These are the edges from the fake root node to all nodes without parent (see calculateCallGraphRoot). But there is no need to handle them separately (and the FIXME should be removed). The Edge.second.RelBlockFreq will be 0 and hence this lambda will return 0 for the fake edges which is the right thing to do. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D43521