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

[Nathan Slingerland via llvm-commits]

slingn added inline comments.

================
Comment at: include/llvm/ProfileData/InstrProf.h:421
@@ +420,3 @@
+    uint64_t NewCount = SaturatingAdd(Counts[I], Other.Counts[I]);
+    if (NewCount - Counts[I] != Other.Counts[I])
+      Result = instrprof_error::counter_overflow;
----------------
The inline check for SaturatingAdd() overflow works well here, but thinking about the final weighted profile change - we need a way to check for SaturatingMultiply() overflow at multiple points in the code. 

I'll follow up this change with Sean's suggestion of adding a third optional argument to SaturatingAdd()/SaturatingMultiply() so the overflow check code can be encapsulated there.

================
Comment at: lib/ProfileData/InstrProfWriter.cpp:118
@@ -115,4 +117,3 @@
     instrprof_error MergeResult = Dest.merge(I);
-    if (MergeResult != instrprof_error::success) {
-      return MergeResult;
-    }
+    if (Result == instrprof_error::success)
+      Result = MergeResult;
----------------
davidxl wrote:
> This line does not look right. Is it needed?
Good point. Not needed.


http://reviews.llvm.org/D14893