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

Summary: This changes overflow handling during instrumentation profile merge. Rathar than throwing away records that would result in counter overflow, merged counts are instead clamped to the maximum representable value. A warning about counter overflow is still surfaced to the user as before.

Reviewers: dnovillo, davidxl, silvas

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D14893

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@254525 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/ProfileData/InstrProf.h b/include/llvm/ProfileData/InstrProf.h
index 13f6c70b3e835d7eb8445127c2a298986e016f3e..95648511910298c5e73e27cc04d05f6babd6703c 100644 (file)
--- a/include/llvm/ProfileData/InstrProf.h
+++ b/include/llvm/ProfileData/InstrProf.h
@@ -428,19 +428,22 @@ instrprof_error InstrProfRecord::merge(InstrProfRecord &Other) {
   if (Counts.size() != Other.Counts.size())
     return instrprof_error::count_mismatch;
 
   if (Counts.size() != Other.Counts.size())
     return instrprof_error::count_mismatch;
 

+  instrprof_error Result = instrprof_error::success;
+

   for (size_t I = 0, E = Other.Counts.size(); I < E; ++I) {
   for (size_t I = 0, E = Other.Counts.size(); I < E; ++I) {

-    if (Counts[I] + Other.Counts[I] < Counts[I])
-      return instrprof_error::counter_overflow;
-    Counts[I] += Other.Counts[I];
+    bool ResultOverflowed;
+    Counts[I] = SaturatingAdd(Counts[I], Other.Counts[I], ResultOverflowed);
+    if (ResultOverflowed)
+      Result = instrprof_error::counter_overflow;

   }
 
   for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind) {
   }
 
   for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind) {

-    instrprof_error result = mergeValueProfData(Kind, Other);
-    if (result != instrprof_error::success)
-      return result;
+    instrprof_error MergeValueResult = mergeValueProfData(Kind, Other);
+    if (MergeValueResult != instrprof_error::success)
+      Result = MergeValueResult;

   }
 
   }
 

-  return instrprof_error::success;
+  return Result;

 }
 
 inline support::endianness getHostEndianness() {
 }
 
 inline support::endianness getHostEndianness() {

diff --git a/lib/ProfileData/InstrProfWriter.cpp b/lib/ProfileData/InstrProfWriter.cpp
index f9cc2afe3da0d893f040dcfa469c19ad49516605..78bec012eeb24d08511f1f0da99161a69cfa145f 100644 (file)
--- a/lib/ProfileData/InstrProfWriter.cpp
+++ b/lib/ProfileData/InstrProfWriter.cpp
@@ -107,22 +107,23 @@ std::error_code InstrProfWriter::addRecord(InstrProfRecord &&I) {
   std::tie(Where, NewFunc) =
       ProfileDataMap.insert(std::make_pair(I.Hash, InstrProfRecord()));
   InstrProfRecord &Dest = Where->second;
   std::tie(Where, NewFunc) =
       ProfileDataMap.insert(std::make_pair(I.Hash, InstrProfRecord()));
   InstrProfRecord &Dest = Where->second;

+
+  instrprof_error Result;

   if (NewFunc) {
     // We've never seen a function with this name and hash, add it.
     Dest = std::move(I);
   if (NewFunc) {
     // We've never seen a function with this name and hash, add it.
     Dest = std::move(I);

+    Result = instrprof_error::success;

   } else {
     // We're updating a function we've seen before.
   } else {
     // We're updating a function we've seen before.

-    instrprof_error MergeResult = Dest.merge(I);
-    if (MergeResult != instrprof_error::success) {
-      return MergeResult;
-    }
+    Result = Dest.merge(I);

   }
 
   // We keep track of the max function count as we go for simplicity.
   }
 
   // We keep track of the max function count as we go for simplicity.

+  // Update this statistic no matter the result of the merge.

   if (Dest.Counts[0] > MaxFunctionCount)
     MaxFunctionCount = Dest.Counts[0];
 
   if (Dest.Counts[0] > MaxFunctionCount)
     MaxFunctionCount = Dest.Counts[0];
 

-  return instrprof_error::success;
+  return Result;

 }
 
 std::pair<uint64_t, uint64_t> InstrProfWriter::writeImpl(raw_ostream &OS) {
 }
 
 std::pair<uint64_t, uint64_t> InstrProfWriter::writeImpl(raw_ostream &OS) {

diff --git a/test/tools/llvm-profdata/overflow.proftext b/test/tools/llvm-profdata/overflow.proftext
index cbf3bf1618230a7877dcf01048395af4cd01d0b5..b8401ffd84dfe329c4677b02c821c33704252e5e 100644 (file)
--- a/test/tools/llvm-profdata/overflow.proftext
+++ b/test/tools/llvm-profdata/overflow.proftext
@@ -1,12 +1,20 @@
-# RUN: llvm-profdata merge %s -o %t.out 2>&1 | FileCheck %s
-# CHECK: overflow.proftext: overflow: Counter overflow
+# RUN: llvm-profdata merge %s -o %t.out 2>&1 | FileCheck %s --check-prefix=MERGE
+# RUN: llvm-profdata show %t.out | FileCheck %s --check-prefix=SHOW
+# MERGE: overflow.proftext: overflow: Counter overflow
+# SHOW: Total functions: 1
+# SHOW: Maximum function count: 18446744073709551615
+# SHOW: Maximum internal block count: 18446744073709551615

 
 overflow
 1
 
 overflow
 1

-1
+3
+18446744073709551615

 9223372036854775808
 9223372036854775808

+18446744073709551615

 
 overflow
 1
 
 overflow
 1

-1
+3
+9223372036854775808

 9223372036854775808
 9223372036854775808

+0

diff --git a/unittests/ProfileData/InstrProfTest.cpp b/unittests/ProfileData/InstrProfTest.cpp
index 2f3adb65a0ee735199046187b693dc84e555ff18..635a5431a5133f62c51ee2eb657451ab7be4d524 100644 (file)
--- a/unittests/ProfileData/InstrProfTest.cpp
+++ b/unittests/ProfileData/InstrProfTest.cpp
@@ -354,7 +354,7 @@ TEST_F(InstrProfTest, get_icall_data_merge1_saturation) {
   const uint64_t Max = std::numeric_limits<uint64_t>::max();
 
   InstrProfRecord Record1("caller", 0x1234, {1});
   const uint64_t Max = std::numeric_limits<uint64_t>::max();
 
   InstrProfRecord Record1("caller", 0x1234, {1});

-  InstrProfRecord Record2("caller", 0x1234, {1});
+  InstrProfRecord Record2("caller", 0x1234, {Max});

   InstrProfRecord Record3("callee1", 0x1235, {3, 4});
 
   Record1.reserveSites(IPVK_IndirectCallTarget, 1);
   InstrProfRecord Record3("callee1", 0x1235, {3, 4});
 
   Record1.reserveSites(IPVK_IndirectCallTarget, 1);


@@ -375,6 +375,9 @@ TEST_F(InstrProfTest, get_icall_data_merge1_saturation) {
   // Verify saturation of counts.
   ErrorOr<InstrProfRecord> R = Reader->getInstrProfRecord("caller", 0x1234);
   ASSERT_TRUE(NoError(R.getError()));
   // Verify saturation of counts.
   ErrorOr<InstrProfRecord> R = Reader->getInstrProfRecord("caller", 0x1234);
   ASSERT_TRUE(NoError(R.getError()));

+
+  ASSERT_EQ(Max, R.get().Counts[0]);
+

   ASSERT_EQ(1U, R.get().getNumValueSites(IPVK_IndirectCallTarget));
   std::unique_ptr<InstrProfValueData[]> VD =
           R.get().getValueForSite(IPVK_IndirectCallTarget, 0);
   ASSERT_EQ(1U, R.get().getNumValueSites(IPVK_IndirectCallTarget));
   std::unique_ptr<InstrProfValueData[]> VD =
           R.get().getValueForSite(IPVK_IndirectCallTarget, 0);