[PGO] Handle and report overflow during profile merge for all types of data

[slingn]

Summary

Surface counter overflow when merging profile data. Merging still occurs on overflow but counts saturate to the maximum representable value. Overflow is reported to the user.

[davidxl]

include/llvm/ProfileData/SampleProf.h

299 ↗

(On Diff #42937)

May be introducing a macro for the repeated patterns:

#define SET_RESULT(R, FinalR) \

if (R != ....) \
   FinalR = R;

[slingn]

Updated for davidxl, dnovillo feedback.

include/llvm/ProfileData/SampleProf.h

299 ↗

(On Diff #42937)

Went with inline function implementation as discussed on llvm-commits.

[davidxl]

include/llvm/ProfileData/InstrProf.h

187 ↗

(On Diff #43029)

Nit: how about UpdateResult -- it seems a better name.

include/llvm/ProfileData/SampleProf.h

145 ↗

(On Diff #43029)

There are also lots of if(..) return patterns that can be simplified. This case you will need a macro:

#define RETURN_IF(Cond, Err) \

if(Cond) \
    return Err;

[slingn]

include/llvm/ProfileData/InstrProf.h

187 ↗

(On Diff #43029)

How about 'MergeResult'?

include/llvm/ProfileData/SampleProf.h

145 ↗

(On Diff #43029)

I don't think we want to add macros to header files like this as macros don't respect namespaces. I'll see if I can simplify in another way.

[davidxl]

include/llvm/ProfileData/InstrProf.h

187 ↗

(On Diff #43029)

Sounds fine.

include/llvm/ProfileData/SampleProf.h

145 ↗

(On Diff #43029)

you can undef it somewhere. Note that 'assert' before the change is a macro too :)

[slingn]

include/llvm/ProfileData/SampleProf.h

145 ↗

(On Diff #43029)

Yes, macros are unavoidable. But I'd still prefer to avoid them. :)

In any case, I think this will be cleaned up quite a bit by D15385 - adding a fused multiply-add saturation function. I'll update that review to depend on this one.

[slingn]

Updated for davidxl feedback.

llvm/trunk/include/llvm/ProfileData/InstrProf.h

enum class instrprof_error {
   counter_overflow,
   value_site_count_mismatch
 };
 
 inline std::error_code make_error_code(instrprof_error E) {
   return std::error_code(static_cast<int>(E), instrprof_category());
 }
 
+inline instrprof_error MergeResult(instrprof_error &Accumulator,
+                                   instrprof_error Result) {
+  // Prefer first error encountered as later errors may be secondary effects of
+  // the initial problem.
+  if (Accumulator == instrprof_error::success &&
+      Result != instrprof_error::success)
+    Accumulator = Result;
+  return Accumulator;
+}
+
 enum InstrProfValueKind : uint32_t {
 #define VALUE_PROF_KIND(Enumerator, Value) Enumerator = Value,
 #include "llvm/ProfileData/InstrProfData.inc"
 };
 
 struct InstrProfStringTable {
   // Set of string values in profiling data.
   StringSet<> StringValueSet;
void sortByTargetValues() {
     ValueData.sort(
         [](const InstrProfValueData &left, const InstrProfValueData &right) {
           return left.Value < right.Value;
         });
   }
 
   /// Merge data from another InstrProfValueSiteRecord
   /// Optionally scale merged counts by \p Weight.
-  void mergeValueData(InstrProfValueSiteRecord &Input, uint64_t Weight = 1) {
+  instrprof_error mergeValueData(InstrProfValueSiteRecord &Input,
+                                 uint64_t Weight = 1) {
     this->sortByTargetValues();
     Input.sortByTargetValues();
     auto I = ValueData.begin();
     auto IE = ValueData.end();
+    instrprof_error Result = instrprof_error::success;
     for (auto J = Input.ValueData.begin(), JE = Input.ValueData.end(); J != JE;
          ++J) {
       while (I != IE && I->Value < J->Value)
         ++I;
       if (I != IE && I->Value == J->Value) {
-        // FIXME: Improve handling of counter overflow.
         uint64_t JCount = J->Count;
         bool Overflowed;
         if (Weight > 1) {
           JCount = SaturatingMultiply(JCount, Weight, &Overflowed);
-          assert(!Overflowed && "Value data counter overflowed!");
+          if (Overflowed)
+            Result = instrprof_error::counter_overflow;
         }
         I->Count = SaturatingAdd(I->Count, JCount, &Overflowed);
-        assert(!Overflowed && "Value data counter overflowed!");
+        if (Overflowed)
+          Result = instrprof_error::counter_overflow;
         ++I;
         continue;
       }
       ValueData.insert(I, *J);
     }
+    return Result;
   }
 };
 
 /// Profiling information for a single function.
 struct InstrProfRecord {
   InstrProfRecord() {}
   InstrProfRecord(StringRef Name, uint64_t Hash, std::vector<uint64_t> Counts)
       : Name(Name), Hash(Hash), Counts(std::move(Counts)) {}
instrprof_error mergeValueProfData(uint32_t ValueKind, InstrProfRecord &Src,
     uint32_t ThisNumValueSites = getNumValueSites(ValueKind);
     uint32_t OtherNumValueSites = Src.getNumValueSites(ValueKind);
     if (ThisNumValueSites != OtherNumValueSites)
       return instrprof_error::value_site_count_mismatch;
     std::vector<InstrProfValueSiteRecord> &ThisSiteRecords =
         getValueSitesForKind(ValueKind);
     std::vector<InstrProfValueSiteRecord> &OtherSiteRecords =
         Src.getValueSitesForKind(ValueKind);
+    instrprof_error Result = instrprof_error::success;
     for (uint32_t I = 0; I < ThisNumValueSites; I++)
-      ThisSiteRecords[I].mergeValueData(OtherSiteRecords[I], Weight);
-    return instrprof_error::success;
+      MergeResult(Result, ThisSiteRecords[I].mergeValueData(OtherSiteRecords[I],
+                                                            Weight));
+    return Result;
   }
 };
 
 uint32_t InstrProfRecord::getNumValueKinds() const {
   uint32_t NumValueKinds = 0;
   for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind)
     NumValueKinds += !(getValueSitesForKind(Kind).empty());
   return NumValueKinds;
if (Weight > 1) {
       if (Overflowed)
         Result = instrprof_error::counter_overflow;
     }
     Counts[I] = SaturatingAdd(Counts[I], OtherCount, &Overflowed);
     if (Overflowed)
       Result = instrprof_error::counter_overflow;
   }
 
-  for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind) {
-    instrprof_error MergeValueResult = mergeValueProfData(Kind, Other, Weight);
-    if (MergeValueResult != instrprof_error::success)
-      Result = MergeValueResult;
-  }
+  for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind)
+    MergeResult(Result, mergeValueProfData(Kind, Other, Weight));
 
   return Result;
 }
 
 inline support::endianness getHostEndianness() {
   return sys::IsLittleEndianHost ? support::little : support::big;
 }
 
 
 // Include definitions for value profile data
 #define INSTR_PROF_VALUE_PROF_DATA
 #include "llvm/ProfileData/InstrProfData.inc"
 
  /*
- * Initialize the record for runtime value profile data. 
+ * Initialize the record for runtime value profile data.
  * Return 0 if the initialization is successful, otherwise
  * return 1.
  */
 int initializeValueProfRuntimeRecord(ValueProfRuntimeRecord *RuntimeRecord,
                                      const uint16_t *NumValueSites,
                                      ValueProfNode **Nodes);
 
 /* Release memory allocated for the runtime record.  */

llvm/trunk/include/llvm/ProfileData/SampleProf.h

enum class sampleprof_error {
   bad_magic,
   unsupported_version,
   too_large,
   truncated,
   malformed,
   unrecognized_format,
   unsupported_writing_format,
   truncated_name_table,
-  not_implemented
+  not_implemented,
+  counter_overflow
 };
 
 inline std::error_code make_error_code(sampleprof_error E) {
   return std::error_code(static_cast<int>(E), sampleprof_category());
 }
 
+inline sampleprof_error MergeResult(sampleprof_error &Accumulator,
+                                    sampleprof_error Result) {
+  // Prefer first error encountered as later errors may be secondary effects of
+  // the initial problem.
+  if (Accumulator == sampleprof_error::success &&
+      Result != sampleprof_error::success)
+    Accumulator = Result;
+  return Accumulator;
+}
+
 } // end namespace llvm
 
 namespace std {
 template <>
 struct is_error_code_enum<llvm::sampleprof_error> : std::true_type {};
 }
 
 namespace llvm {
public:
 
   SampleRecord() : NumSamples(0), CallTargets() {}
 
   /// Increment the number of samples for this record by \p S.
   /// Optionally scale sample count \p S by \p Weight.
   ///
   /// Sample counts accumulate using saturating arithmetic, to avoid wrapping
   /// around unsigned integers.
-  void addSamples(uint64_t S, uint64_t Weight = 1) {
-    // FIXME: Improve handling of counter overflow.
+  sampleprof_error addSamples(uint64_t S, uint64_t Weight = 1) {
     bool Overflowed;
     if (Weight > 1) {
       S = SaturatingMultiply(S, Weight, &Overflowed);
-      assert(!Overflowed && "Sample counter overflowed!");
+      if (Overflowed)
+        return sampleprof_error::counter_overflow;
     }
     NumSamples = SaturatingAdd(NumSamples, S, &Overflowed);
-    assert(!Overflowed && "Sample counter overflowed!");
+    if (Overflowed)
+      return sampleprof_error::counter_overflow;
+
+    return sampleprof_error::success;
   }
 
   /// Add called function \p F with samples \p S.
   /// Optionally scale sample count \p S by \p Weight.
   ///
   /// Sample counts accumulate using saturating arithmetic, to avoid wrapping
   /// around unsigned integers.
-  void addCalledTarget(StringRef F, uint64_t S, uint64_t Weight = 1) {
-    // FIXME: Improve handling of counter overflow.
+  sampleprof_error addCalledTarget(StringRef F, uint64_t S,
+                                   uint64_t Weight = 1) {
     uint64_t &TargetSamples = CallTargets[F];
     bool Overflowed;
     if (Weight > 1) {
       S = SaturatingMultiply(S, Weight, &Overflowed);
-      assert(!Overflowed && "Called target counter overflowed!");
+      if (Overflowed)
+        return sampleprof_error::counter_overflow;
     }
     TargetSamples = SaturatingAdd(TargetSamples, S, &Overflowed);
-    assert(!Overflowed && "Called target counter overflowed!");
+    if (Overflowed)
+      return sampleprof_error::counter_overflow;
+
+    return sampleprof_error::success;
   }
 
   /// Return true if this sample record contains function calls.
   bool hasCalls() const { return CallTargets.size() > 0; }
 
   uint64_t getSamples() const { return NumSamples; }
   const CallTargetMap &getCallTargets() const { return CallTargets; }
 
   /// Merge the samples in \p Other into this record.
   /// Optionally scale sample counts by \p Weight.
-  void merge(const SampleRecord &Other, uint64_t Weight = 1) {
-    addSamples(Other.getSamples(), Weight);
-    for (const auto &I : Other.getCallTargets())
-      addCalledTarget(I.first(), I.second, Weight);
+  sampleprof_error merge(const SampleRecord &Other, uint64_t Weight = 1) {
+    sampleprof_error Result = addSamples(Other.getSamples(), Weight);
+    for (const auto &I : Other.getCallTargets()) {
+      MergeResult(Result, addCalledTarget(I.first(), I.second, Weight));
+    }
+    return Result;
   }
 
   void print(raw_ostream &OS, unsigned Indent) const;
   void dump() const;
 
 private:
   uint64_t NumSamples;
   CallTargetMap CallTargets;

 /// This data structure contains all the collected samples for the body
 /// of a function. Each sample corresponds to a LineLocation instance
 /// within the body of the function.
 class FunctionSamples {
 public:
   FunctionSamples() : TotalSamples(0), TotalHeadSamples(0) {}
   void print(raw_ostream &OS = dbgs(), unsigned Indent = 0) const;
   void dump() const;
-  void addTotalSamples(uint64_t Num, uint64_t Weight = 1) {
-    // FIXME: Improve handling of counter overflow.
+  sampleprof_error addTotalSamples(uint64_t Num, uint64_t Weight = 1) {
     bool Overflowed;
     if (Weight > 1) {
       Num = SaturatingMultiply(Num, Weight, &Overflowed);
-      assert(!Overflowed && "Total samples counter overflowed!");
+      if (Overflowed)
+        return sampleprof_error::counter_overflow;
     }
     TotalSamples = SaturatingAdd(TotalSamples, Num, &Overflowed);
-    assert(!Overflowed && "Total samples counter overflowed!");
+    if (Overflowed)
+      return sampleprof_error::counter_overflow;
+
+    return sampleprof_error::success;
   }
-  void addHeadSamples(uint64_t Num, uint64_t Weight = 1) {
-    // FIXME: Improve handling of counter overflow.
+  sampleprof_error addHeadSamples(uint64_t Num, uint64_t Weight = 1) {
     bool Overflowed;
     if (Weight > 1) {
       Num = SaturatingMultiply(Num, Weight, &Overflowed);
-      assert(!Overflowed && "Total head samples counter overflowed!");
+      if (Overflowed)
+        return sampleprof_error::counter_overflow;
     }
     TotalHeadSamples = SaturatingAdd(TotalHeadSamples, Num, &Overflowed);
-    assert(!Overflowed && "Total head samples counter overflowed!");
+    if (Overflowed)
+      return sampleprof_error::counter_overflow;
+
+    return sampleprof_error::success;
   }
-  void addBodySamples(uint32_t LineOffset, uint32_t Discriminator, uint64_t Num,
-                      uint64_t Weight = 1) {
-    BodySamples[LineLocation(LineOffset, Discriminator)].addSamples(Num,
-                                                                    Weight);
+  sampleprof_error addBodySamples(uint32_t LineOffset, uint32_t Discriminator,
+                                  uint64_t Num, uint64_t Weight = 1) {
+    return BodySamples[LineLocation(LineOffset, Discriminator)].addSamples(
+        Num, Weight);
   }
-  void addCalledTargetSamples(uint32_t LineOffset, uint32_t Discriminator,
+  sampleprof_error addCalledTargetSamples(uint32_t LineOffset,
+                                          uint32_t Discriminator,
                                           std::string FName, uint64_t Num,
                                           uint64_t Weight = 1) {
-    BodySamples[LineLocation(LineOffset, Discriminator)].addCalledTarget(
+    return BodySamples[LineLocation(LineOffset, Discriminator)].addCalledTarget(
         FName, Num, Weight);
   }
 
   /// Return the number of samples collected at the given location.
   /// Each location is specified by \p LineOffset and \p Discriminator.
   /// If the location is not found in profile, return error.
   ErrorOr<uint64_t> findSamplesAt(uint32_t LineOffset,
                                   uint32_t Discriminator) const {
public:
 
   /// Return all the callsite samples collected in the body of the function.
   const CallsiteSampleMap &getCallsiteSamples() const {
     return CallsiteSamples;
   }
 
   /// Merge the samples in \p Other into this one.
   /// Optionally scale samples by \p Weight.
-  void merge(const FunctionSamples &Other, uint64_t Weight = 1) {
-    addTotalSamples(Other.getTotalSamples(), Weight);
-    addHeadSamples(Other.getHeadSamples(), Weight);
+  sampleprof_error merge(const FunctionSamples &Other, uint64_t Weight = 1) {
+    sampleprof_error Result = sampleprof_error::success;
+    MergeResult(Result, addTotalSamples(Other.getTotalSamples(), Weight));
+    MergeResult(Result, addHeadSamples(Other.getHeadSamples(), Weight));
     for (const auto &I : Other.getBodySamples()) {
       const LineLocation &Loc = I.first;
       const SampleRecord &Rec = I.second;
-      BodySamples[Loc].merge(Rec, Weight);
+      MergeResult(Result, BodySamples[Loc].merge(Rec, Weight));
     }
     for (const auto &I : Other.getCallsiteSamples()) {
       const CallsiteLocation &Loc = I.first;
       const FunctionSamples &Rec = I.second;
-      functionSamplesAt(Loc).merge(Rec, Weight);
+      MergeResult(Result, functionSamplesAt(Loc).merge(Rec, Weight));
     }
+    return Result;
   }
 
 private:
   /// Total number of samples collected inside this function.
   ///
   /// Samples are cumulative, they include all the samples collected
   /// inside this function and all its inlined callees.
   uint64_t TotalSamples;

llvm/trunk/lib/ProfileData/SampleProf.cpp

std::string message(int IE) const override {
     case sampleprof_error::unrecognized_format:
       return "Unrecognized sample profile encoding format";
     case sampleprof_error::unsupported_writing_format:
       return "Profile encoding format unsupported for writing operations";
     case sampleprof_error::truncated_name_table:
       return "Truncated function name table";
     case sampleprof_error::not_implemented:
       return "Unimplemented feature";
+    case sampleprof_error::counter_overflow:
+      return "Counter overflow";
     }
     llvm_unreachable("A value of sampleprof_error has no message.");
   }
 };
 }
 
 static ManagedStatic<SampleProfErrorCategoryType> ErrorCategory;
 

llvm/trunk/lib/ProfileData/SampleProfReader.cpp

 /// \brief Load samples from a text file.
 ///
 /// See the documentation at the top of the file for an explanation of
 /// the expected format.
 ///
 /// \returns true if the file was loaded successfully, false otherwise.
 std::error_code SampleProfileReaderText::read() {
   line_iterator LineIt(*Buffer, /*SkipBlanks=*/true, '#');
+  sampleprof_error Result = sampleprof_error::success;
 
   InlineCallStack InlineStack;
 
   for (; !LineIt.is_at_eof(); ++LineIt) {
     if ((*LineIt)[(*LineIt).find_first_not_of(' ')] == '#')
       continue;
     // Read the header of each function.
     //
if ((*LineIt)[0] != ' ') {
       StringRef FName;
       if (!ParseHead(*LineIt, FName, NumSamples, NumHeadSamples)) {
         reportError(LineIt.line_number(),
                     "Expected 'mangled_name:NUM:NUM', found " + *LineIt);
         return sampleprof_error::malformed;
       }
       Profiles[FName] = FunctionSamples();
       FunctionSamples &FProfile = Profiles[FName];
-      FProfile.addTotalSamples(NumSamples);
-      FProfile.addHeadSamples(NumHeadSamples);
+      MergeResult(Result, FProfile.addTotalSamples(NumSamples));
+      MergeResult(Result, FProfile.addHeadSamples(NumHeadSamples));
       InlineStack.clear();
       InlineStack.push_back(&FProfile);
     } else {
       uint64_t NumSamples;
       StringRef FName;
       DenseMap<StringRef, uint64_t> TargetCountMap;
       bool IsCallsite;
       uint32_t Depth, LineOffset, Discriminator;
       if (!ParseLine(*LineIt, IsCallsite, Depth, NumSamples, LineOffset,
                      Discriminator, FName, TargetCountMap)) {
         reportError(LineIt.line_number(),
                     "Expected 'NUM[.NUM]: NUM[ mangled_name:NUM]*', found " +
                         *LineIt);
         return sampleprof_error::malformed;
       }
       if (IsCallsite) {
         while (InlineStack.size() > Depth) {
           InlineStack.pop_back();
         }
         FunctionSamples &FSamples = InlineStack.back()->functionSamplesAt(
             CallsiteLocation(LineOffset, Discriminator, FName));
-        FSamples.addTotalSamples(NumSamples);
+        MergeResult(Result, FSamples.addTotalSamples(NumSamples));
         InlineStack.push_back(&FSamples);
       } else {
         while (InlineStack.size() > Depth) {
           InlineStack.pop_back();
         }
         FunctionSamples &FProfile = *InlineStack.back();
         for (const auto &name_count : TargetCountMap) {
-          FProfile.addCalledTargetSamples(LineOffset, Discriminator,
-                                          name_count.first, name_count.second);
+          MergeResult(Result, FProfile.addCalledTargetSamples(
+                                  LineOffset, Discriminator, name_count.first,
+                                  name_count.second));
         }
-        FProfile.addBodySamples(LineOffset, Discriminator, NumSamples);
+        MergeResult(Result, FProfile.addBodySamples(LineOffset, Discriminator,
+                                                    NumSamples));
       }
     }
   }
 
-  return sampleprof_error::success;
+  return Result;
 }
 
 bool SampleProfileReaderText::hasFormat(const MemoryBuffer &Buffer) {
   bool result = false;
 
   // Check that the first non-comment line is a valid function header.
   line_iterator LineIt(Buffer, /*SkipBlanks=*/true, '#');
   if (!LineIt.is_at_eof()) {

llvm/trunk/test/tools/llvm-profdata/Inputs/overflow-instr.proftext

+overflow
+1
+3
+18446744073709551615
+9223372036854775808
+18446744073709551615

llvm/trunk/test/tools/llvm-profdata/Inputs/overflow-sample.proftext

+_Z3bari:18446744073709551615:1000
+ 1: 18446744073709551615
+_Z3fooi:18446744073709551615:1000
+ 1: 18446744073709551615
+main:1000:0
+ 1: 500 _Z3bari:18446744073709551615
+ 2: 500 _Z3fooi:18446744073709551615

llvm/trunk/test/tools/llvm-profdata/overflow-instr.test

+Tests for overflow when merging instrumented profiles.
+
+1- Merge profile having maximum counts with itself and verify overflow detected and saturation occurred
+RUN: llvm-profdata merge -instr %p/Inputs/overflow-instr.proftext %p/Inputs/overflow-instr.proftext -o %t.out 2>&1 | FileCheck %s -check-prefix=MERGE_OVERFLOW
+RUN: llvm-profdata show -instr %t.out | FileCheck %s --check-prefix=SHOW_OVERFLOW
+MERGE_OVERFLOW: {{.*}}: overflow: Counter overflow
+SHOW_OVERFLOW: Total functions: 1
+SHOW_OVERFLOW-NEXT: Maximum function count: 18446744073709551615
+SHOW_OVERFLOW-NEXT: Maximum internal block count: 18446744073709551615
+
+2- Merge profile having maximum counts by itself and verify no overflow
+RUN: llvm-profdata merge -instr %p/Inputs/overflow-instr.proftext -o %t.out 2>&1 | FileCheck %s -check-prefix=MERGE_NO_OVERFLOW -allow-empty
+RUN: llvm-profdata show -instr %t.out | FileCheck %s --check-prefix=SHOW_NO_OVERFLOW
+MERGE_NO_OVERFLOW-NOT: {{.*}}: overflow: Counter overflow
+SHOW_NO_OVERFLOW: Total functions: 1
+SHOW_NO_OVERFLOW-NEXT: Maximum function count: 18446744073709551615
+SHOW_NO_OVERFLOW-NEXT: Maximum internal block count: 18446744073709551615

llvm/trunk/test/tools/llvm-profdata/overflow-sample.test

+Tests for overflow when merging sampled profiles.
+
+1- Merge profile having maximum counts with itself and verify overflow detected
+RUN: llvm-profdata merge -sample %p/Inputs/overflow-sample.proftext %p/Inputs/overflow-sample.proftext -o %t.out 2>&1 | FileCheck %s -check-prefix=MERGE_OVERFLOW
+RUN: llvm-profdata show -sample %t.out | FileCheck %s --check-prefix=SHOW_OVERFLOW
+MERGE_OVERFLOW: {{.*}}: main: Counter overflow
+SHOW_OVERFLOW: Function: main: 2000, 0, 2 sampled lines
+SHOW_OVERFLOW-NEXT: Samples collected in the function's body {
+SHOW_OVERFLOW-NEXT:   1: 1000, calls: _Z3bari:18446744073709551615
+SHOW_OVERFLOW-NEXT:   2: 1000, calls: _Z3fooi:18446744073709551615
+SHOW_OVERFLOW-NEXT: }
+SHOW_OVERFLOW-NEXT: No inlined callsites in this function
+SHOW_OVERFLOW-NEXT: Function: _Z3fooi: 18446744073709551615, 2000, 1 sampled lines
+SHOW_OVERFLOW-NEXT: Samples collected in the function's body {
+SHOW_OVERFLOW-NEXT:   1: 18446744073709551615
+SHOW_OVERFLOW-NEXT: }
+SHOW_OVERFLOW-NEXT: No inlined callsites in this function
+SHOW_OVERFLOW-NEXT: Function: _Z3bari: 18446744073709551615, 2000, 1 sampled lines
+SHOW_OVERFLOW-NEXT: Samples collected in the function's body {
+SHOW_OVERFLOW-NEXT:   1: 18446744073709551615
+SHOW_OVERFLOW-NEXT: }
+SHOW_OVERFLOW-NEXT: No inlined callsites in this function
+
+2- Merge profile having maximum counts by itself and verify no overflow
+RUN: llvm-profdata merge -sample %p/Inputs/overflow-sample.proftext -o %t.out 2>&1 | FileCheck %s -allow-empty -check-prefix=MERGE_NO_OVERFLOW
+RUN: llvm-profdata show -sample %t.out | FileCheck %s --check-prefix=SHOW_NO_OVERFLOW
+MERGE_NO_OVERFLOW-NOT: {{.*}}: main: Counter overflow
+SHOW_NO_OVERFLOW: Function: main: 1000, 0, 2 sampled lines
+SHOW_NO_OVERFLOW-NEXT: Samples collected in the function's body {
+SHOW_NO_OVERFLOW-NEXT:   1: 500, calls: _Z3bari:18446744073709551615
+SHOW_NO_OVERFLOW-NEXT:   2: 500, calls: _Z3fooi:18446744073709551615
+SHOW_NO_OVERFLOW-NEXT: }
+SHOW_NO_OVERFLOW-NEXT: No inlined callsites in this function
+SHOW_NO_OVERFLOW-NEXT: Function: _Z3fooi: 18446744073709551615, 1000, 1 sampled lines
+SHOW_NO_OVERFLOW-NEXT: Samples collected in the function's body {
+SHOW_NO_OVERFLOW-NEXT:   1: 18446744073709551615
+SHOW_NO_OVERFLOW-NEXT: }
+SHOW_NO_OVERFLOW-NEXT: No inlined callsites in this function
+SHOW_NO_OVERFLOW-NEXT: Function: _Z3bari: 18446744073709551615, 1000, 1 sampled lines
+SHOW_NO_OVERFLOW-NEXT: Samples collected in the function's body {
+SHOW_NO_OVERFLOW-NEXT:   1: 18446744073709551615
+SHOW_NO_OVERFLOW-NEXT: }
+SHOW_NO_OVERFLOW-NEXT: No inlined callsites in this function

llvm/trunk/test/tools/llvm-profdata/overflow.proftext

-# 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
-3
-18446744073709551615
-9223372036854775808
-18446744073709551615
-
-overflow
-1
-3
-9223372036854775808
-9223372036854775808
-0

llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp

if (std::error_code EC = Reader->read())
       exitWithErrorCode(EC, Input.Filename);
 
     StringMap<FunctionSamples> &Profiles = Reader->getProfiles();
     for (StringMap<FunctionSamples>::iterator I = Profiles.begin(),
                                               E = Profiles.end();
          I != E; ++I) {
       StringRef FName = I->first();
       FunctionSamples &Samples = I->second;
-      ProfileMap[FName].merge(Samples, Input.Weight);
+      sampleprof_error Result = ProfileMap[FName].merge(Samples, Input.Weight);
+      if (Result != sampleprof_error::success) {
+        std::error_code EC = make_error_code(Result);
+        handleMergeWriterError(EC, Input.Filename, FName);
+      }
     }
   }
   Writer->write(ProfileMap);
 }
 
 static WeightedFile parseWeightedFile(const StringRef &WeightedFilename) {
   StringRef WeightStr, FileName;
   std::tie(WeightStr, FileName) = WeightedFilename.split(',');

llvm/trunk/unittests/ProfileData/InstrProfTest.cpp

TEST_F(InstrProfTest, get_icall_data_merge1) {
   ASSERT_EQ(4U, VD_4[1].Count);
   ASSERT_EQ(StringRef((const char *)VD_4[2].Value, 7), StringRef("callee1"));
   ASSERT_EQ(2U, VD_4[2].Count);
 }
 
 TEST_F(InstrProfTest, get_icall_data_merge1_saturation) {
   const uint64_t Max = std::numeric_limits<uint64_t>::max();
 
-  InstrProfRecord Record1("caller", 0x1234, {1});
-  InstrProfRecord Record2("caller", 0x1234, {Max});
-  InstrProfRecord Record3("callee1", 0x1235, {3, 4});
-
-  Record1.reserveSites(IPVK_IndirectCallTarget, 1);
-  InstrProfValueData VD1[] = {{(uint64_t) "callee1", 1}};
-  Record1.addValueData(IPVK_IndirectCallTarget, 0, VD1, 1, nullptr);
-
-  Record2.reserveSites(IPVK_IndirectCallTarget, 1);
-  // FIXME: Improve handling of counter overflow. ValueData asserts on overflow.
-  //  InstrProfValueData VD2[] = {{(uint64_t) "callee1", Max}};
-  InstrProfValueData VD2[] = {{(uint64_t) "callee1", 1}};
-  Record2.addValueData(IPVK_IndirectCallTarget, 0, VD2, 1, nullptr);
-
-  Writer.addRecord(std::move(Record1));
-  Writer.addRecord(std::move(Record2));
-  Writer.addRecord(std::move(Record3));
+  InstrProfRecord Record1("foo", 0x1234, {1});
+  auto Result1 = Writer.addRecord(std::move(Record1));
+  ASSERT_EQ(Result1, instrprof_error::success);
+
+  // Verify counter overflow.
+  InstrProfRecord Record2("foo", 0x1234, {Max});
+  auto Result2 = Writer.addRecord(std::move(Record2));
+  ASSERT_EQ(Result2, instrprof_error::counter_overflow);
+
+  InstrProfRecord Record3("bar", 0x9012, {8});
+  auto Result3 = Writer.addRecord(std::move(Record3));
+  ASSERT_EQ(Result3, instrprof_error::success);
+
+  InstrProfRecord Record4("baz", 0x5678, {3, 4});
+  Record4.reserveSites(IPVK_IndirectCallTarget, 1);
+  InstrProfValueData VD4[] = {{(uint64_t) "bar", 1}};
+  Record4.addValueData(IPVK_IndirectCallTarget, 0, VD4, 1, nullptr);
+  auto Result4 = Writer.addRecord(std::move(Record4));
+  ASSERT_EQ(Result4, instrprof_error::success);
+
+  // Verify value data counter overflow.
+  InstrProfRecord Record5("baz", 0x5678, {5, 6});
+  Record5.reserveSites(IPVK_IndirectCallTarget, 1);
+  InstrProfValueData VD5[] = {{(uint64_t) "bar", Max}};
+  Record5.addValueData(IPVK_IndirectCallTarget, 0, VD5, 1, nullptr);
+  auto Result5 = Writer.addRecord(std::move(Record5));
+  ASSERT_EQ(Result5, instrprof_error::counter_overflow);
 
   auto Profile = Writer.writeBuffer();
   readProfile(std::move(Profile));
 
   // 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));
+  ErrorOr<InstrProfRecord> ReadRecord1 =
+      Reader->getInstrProfRecord("foo", 0x1234);
+  ASSERT_TRUE(NoError(ReadRecord1.getError()));
+  ASSERT_EQ(Max, ReadRecord1.get().Counts[0]);
+
+  ErrorOr<InstrProfRecord> ReadRecord2 =
+      Reader->getInstrProfRecord("baz", 0x5678);
+  ASSERT_EQ(1U, ReadRecord2.get().getNumValueSites(IPVK_IndirectCallTarget));
   std::unique_ptr<InstrProfValueData[]> VD =
-          R.get().getValueForSite(IPVK_IndirectCallTarget, 0);
-  ASSERT_EQ(StringRef("callee1"), StringRef((const char *)VD[0].Value, 7));
-
-  // FIXME: Improve handling of counter overflow. ValueData asserts on overflow.
-  //  ASSERT_EQ(Max, VD[0].Count);
-  ASSERT_EQ(2U, VD[0].Count);
+      ReadRecord2.get().getValueForSite(IPVK_IndirectCallTarget, 0);
+  ASSERT_EQ(StringRef("bar"), StringRef((const char *)VD[0].Value, 3));
+  ASSERT_EQ(Max, VD[0].Count);
 }
 
 // Synthesize runtime value profile data.
 ValueProfNode Site1Values[5] = {{{uint64_t("callee1"), 400}, &Site1Values[1]},
                                 {{uint64_t("callee2"), 1000}, &Site1Values[2]},
                                 {{uint64_t("callee3"), 500}, &Site1Values[3]},
                                 {{uint64_t("callee4"), 300}, &Site1Values[4]},
                                 {{uint64_t("callee5"), 100}, 0}};

llvm/trunk/unittests/ProfileData/SampleProfTest.cpp

 TEST_F(SampleProfTest, roundtrip_text_profile) {
   testRoundTrip(SampleProfileFormat::SPF_Text);
 }
 
 TEST_F(SampleProfTest, roundtrip_binary_profile) {
   testRoundTrip(SampleProfileFormat::SPF_Binary);
 }
 
+TEST_F(SampleProfTest, sample_overflow_saturation) {
+  const uint64_t Max = std::numeric_limits<uint64_t>::max();
+  sampleprof_error Result;
+
+  StringRef FooName("_Z3fooi");
+  FunctionSamples FooSamples;
+  Result = FooSamples.addTotalSamples(1);
+  ASSERT_EQ(Result, sampleprof_error::success);
+
+  Result = FooSamples.addHeadSamples(1);
+  ASSERT_EQ(Result, sampleprof_error::success);
+
+  Result = FooSamples.addBodySamples(10, 0, 1);
+  ASSERT_EQ(Result, sampleprof_error::success);
+
+  Result = FooSamples.addTotalSamples(Max);
+  ASSERT_EQ(Result, sampleprof_error::counter_overflow);
+  ASSERT_EQ(FooSamples.getTotalSamples(), Max);
+
+  Result = FooSamples.addHeadSamples(Max);
+  ASSERT_EQ(Result, sampleprof_error::counter_overflow);
+  ASSERT_EQ(FooSamples.getHeadSamples(), Max);
+
+  Result = FooSamples.addBodySamples(10, 0, Max);
+  ASSERT_EQ(Result, sampleprof_error::counter_overflow);
+  ErrorOr<uint64_t> BodySamples = FooSamples.findSamplesAt(10, 0);
+  ASSERT_FALSE(BodySamples.getError());
+  ASSERT_EQ(BodySamples.get(), Max);
+}
+
 } // end anonymous namespace