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

[David Li via llvm-commits]

davidxl 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;
----------------
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




http://reviews.llvm.org/D14893