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

[David Li via Phabricator via llvm-commits]

davidxl 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;
----------------
xur wrote:
> 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.
profile counter reader should do something when reading those values into instrProfRecord so that the special values are not leaked.


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:514
+    return;
   ProfRecord->scale(Numerator, Denominator, [&](instrprof_error E) {
     warn(toString(make_error<InstrProfError>(E)));
----------------
are the counts all -1 or -2 in this case? what is the point of scaling?


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

https://reviews.llvm.org/D132601