[PATCH] D14893: [llvm-profdata] Change instr prof counter overflow to saturate rather than discard

[Sean Silva via llvm-commits]

silvas added inline comments.

================
Comment at: include/llvm/ProfileData/InstrProf.h:421
@@ +420,3 @@
+    Counts[I] = SaturatingAdd(Counts[I], Other.Counts[I]);
+    if (Other.Counts[I] > 0 && Counts[I] == CounterMax) {
+      Result = instrprof_error::counter_overflow;
----------------
davidxl wrote:
> silvas wrote:
> > Is this the right check? Seems like it would incorrectly fire when adding Counts[I] == "CounterMax - 1" and Other.Counts[I] == "1" since we are overwriting Counts[I]
> > The cleanest thing might be to add an optional argument `bool *ResultOverflowed = nullptr` to `SaturatingAdd`.
> Or perhaps just 
> 
> NewCount = SaturatingAdd(....);
> if (NewCount - Counts[I] != Other.Counts[I)
>     Result = ...
> 
>  Counts[I] = NewCount;
> 
> > Quoted Text
> 
> 
I like that!


http://reviews.llvm.org/D14893