[PATCH] D73142: Heat Coloring for CFGPrinter and CallPrinter

[David Li via Phabricator via llvm-commits]

davidxl added a comment.

In D73142#1855835 <https://reviews.llvm.org/D73142#1855835>, @knaumov wrote:

> Addressed @davidxl and @sfertile comments.
>
> Answering @davidxl 's comment on "The function entry count should have the information", line 73, HeatUtils.cpp:
>  This function counts the number of calls of one specific function by another specific function while Function.entryCount() returns number entries for this function.
>
> Answering @davidxl 's comment on "for the program's max count, max function count, get it from ProfileSummaryInfo.", line 99, HeatUtils.cpp:
>  ProfileSummaryInfo doesn't contain info on the maximum frequency in the module, it needs to be calculated.
>
> Updates:
>
> - Ran clang-format through all the files
> - Deleted some function which had no use (see hasProfiling function)
> - Deleted const unsigned variables


The max count info from a module can be misleading. The module itself in most of the cases are cold, so profile summary should be used for cases when profile data is available.

Another major issue is that when real profile is not available (when heurstic is used), the blockfrequency value is relative to the a function, so you can not really compare block frequency values across function boundaries.   For that, a round of synthetic profile count propagation is needed.



================
Comment at: llvm/lib/Analysis/CallPrinter.cpp:37
+
+static cl::opt<bool>
+    FullCallGraph("callgraph-full", cl::init(false), cl::Hidden,
----------------
By looking at the code, it seems that the difference of 'full' vs 'nonfull' is a whether it is a multi-graph or not.


================
Comment at: llvm/lib/Analysis/CallPrinter.cpp:71
+          auto CS = CallSite(U);
+          if (!CS.getCaller()->isDeclaration()) {
+            uint64_t Counter = getNumOfCalls(CS, LookupBFI);
----------------
How can this be? The caller of the callsite can not be a declaration.


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

https://reviews.llvm.org/D73142