[PATCH] D132601: [llvm-profdata] Improve profile supplementation

[Rong Xu via Phabricator via llvm-commits]

xur added inline comments.


================
Comment at: llvm/lib/ProfileData/ProfileSummaryBuilder.cpp:226
   // Skip invalid count.
-  if (Count == (uint64_t)-1)
+  if (Count == (uint64_t)-1 || Count == (uint64_t)-2)
     return;
----------------
davidxl wrote:
> probably worth defining symbolic constants for these values.
> 
> Also how can we prevent these values are accidentally used somewhere else?
Symbolic constants seem reasonable.

These values should not be encountered in real profiles. 2^64 is too large for any counter values to come close.
But we do need to special handling of these values. 
It's possible to misuse -- I think overlap function does not handle this well.
There are not too many places to use the counters directly. We just need to check all of them. 

There might be other ways to implement -- like extending the profile format. But this is the simplest way.


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:509
   const unsigned MultiplyFactor = 3;
-  uint64_t Numerator = HotInstrThreshold * MultiplyFactor;
+  uint64_t Threshold = (SetToHot ? HotInstrThreshold : ColdInstrThreshold);
+  uint64_t Numerator = Threshold * MultiplyFactor;
----------------
davidxl wrote:
> why using coldthreshold?
We want to set the entry count so that the function will be warm

The entry count  needs to be greater than ColdThreshold but less than HotThreshold.

I think I need to add code to make sure it's less than HotThreshold.


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

https://reviews.llvm.org/D132601