[llvm] r307516 - llvm-profdata: Reduce memory usage by using Error callback rather than member

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 10 14:45:18 PDT 2017


Thank you!

vedant

> On Jul 9, 2017, at 8:04 PM, David Blaikie via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: dblaikie
> Date: Sun Jul  9 20:04:59 2017
> New Revision: 307516
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=307516&view=rev
> Log:
> llvm-profdata: Reduce memory usage by using Error callback rather than member
> 
> Reduces llvm-profdata memory usage on a large profile from 7.8GB to 5.1GB.
> 
> The ProfData API now supports reporting all the errors/warnings rather
> than only the first, though llvm-profdata ignores everything after the
> first for now to preserve existing behavior. (if there's a desire for
> other behavior, happy to implement that - but might be as well left for
> a separate patch)
> 
> Reviewers: davidxl
> 
> Differential Revision: https://reviews.llvm.org/D35149
> 
> Modified:
>    llvm/trunk/include/llvm/ProfileData/InstrProf.h
>    llvm/trunk/include/llvm/ProfileData/InstrProfWriter.h
>    llvm/trunk/lib/ProfileData/InstrProf.cpp
>    llvm/trunk/lib/ProfileData/InstrProfWriter.cpp
>    llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp
>    llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp
>    llvm/trunk/unittests/ProfileData/InstrProfTest.cpp
> 
> Modified: llvm/trunk/include/llvm/ProfileData/InstrProf.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/InstrProf.h?rev=307516&r1=307515&r2=307516&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ProfileData/InstrProf.h (original)
> +++ llvm/trunk/include/llvm/ProfileData/InstrProf.h Sun Jul  9 20:04:59 2017
> @@ -581,16 +581,15 @@ struct InstrProfValueSiteRecord {
> 
>   /// Merge data from another InstrProfValueSiteRecord
>   /// Optionally scale merged counts by \p Weight.
> -  void merge(SoftInstrProfErrors &SIPE, InstrProfValueSiteRecord &Input,
> -             uint64_t Weight = 1);
> +  void merge(InstrProfValueSiteRecord &Input, uint64_t Weight,
> +             function_ref<void(instrprof_error)> Warn);
>   /// Scale up value profile data counts.
> -  void scale(SoftInstrProfErrors &SIPE, uint64_t Weight);
> +  void scale(uint64_t Weight, function_ref<void(instrprof_error)> Warn);
> };
> 
> /// Profiling information for a single function.
> struct InstrProfRecord {
>   std::vector<uint64_t> Counts;
> -  SoftInstrProfErrors SIPE;
> 
>   InstrProfRecord() = default;
>   InstrProfRecord(std::vector<uint64_t> Counts) : Counts(std::move(Counts)) {}
> @@ -653,11 +652,12 @@ struct InstrProfRecord {
> 
>   /// Merge the counts in \p Other into this one.
>   /// Optionally scale merged counts by \p Weight.
> -  void merge(InstrProfRecord &Other, uint64_t Weight = 1);
> +  void merge(InstrProfRecord &Other, uint64_t Weight,
> +             function_ref<void(instrprof_error)> Warn);
> 
>   /// Scale up profile counts (including value profile data) by
>   /// \p Weight.
> -  void scale(uint64_t Weight);
> +  void scale(uint64_t Weight, function_ref<void(instrprof_error)> Warn);
> 
>   /// Sort value profile data (per site) by count.
>   void sortValueData() {
> @@ -675,9 +675,6 @@ struct InstrProfRecord {
>   /// Clear value data entries
>   void clearValueData() { ValueData = nullptr; }
> 
> -  /// Get the error contained within the record's soft error counter.
> -  Error takeError() { return SIPE.takeError(); }
> -
> private:
>   struct ValueProfData {
>     std::vector<InstrProfValueSiteRecord> IndirectCallSites;
> @@ -729,11 +726,13 @@ private:
> 
>   // Merge Value Profile data from Src record to this record for ValueKind.
>   // Scale merged value counts by \p Weight.
> -  void mergeValueProfData(uint32_t ValueKind, InstrProfRecord &Src,
> -                          uint64_t Weight);
> +  void mergeValueProfData(uint32_t ValkeKind, InstrProfRecord &Src,
> +                          uint64_t Weight,
> +                          function_ref<void(instrprof_error)> Warn);
> 
>   // Scale up value profile data count.
> -  void scaleValueProfData(uint32_t ValueKind, uint64_t Weight);
> +  void scaleValueProfData(uint32_t ValueKind, uint64_t Weight,
> +                          function_ref<void(instrprof_error)> Warn);
> };
> 
> struct NamedInstrProfRecord : InstrProfRecord {
> 
> Modified: llvm/trunk/include/llvm/ProfileData/InstrProfWriter.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/InstrProfWriter.h?rev=307516&r1=307515&r2=307516&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ProfileData/InstrProfWriter.h (original)
> +++ llvm/trunk/include/llvm/ProfileData/InstrProfWriter.h Sun Jul  9 20:04:59 2017
> @@ -51,10 +51,15 @@ public:
>   /// Add function counts for the given function. If there are already counts
>   /// for this function and the hash and number of counts match, each counter is
>   /// summed. Optionally scale counts by \p Weight.
> -  Error addRecord(NamedInstrProfRecord &&I, uint64_t Weight = 1);
> +  void addRecord(NamedInstrProfRecord &&I, uint64_t Weight,
> +                 function_ref<void(Error)> Warn);
> +  void addRecord(NamedInstrProfRecord &&I, function_ref<void(Error)> Warn) {
> +    addRecord(std::move(I), 1, Warn);
> +  }
> 
>   /// Merge existing function counts from the given writer.
> -  Error mergeRecordsFromWriter(InstrProfWriter &&IPW);
> +  void mergeRecordsFromWriter(InstrProfWriter &&IPW,
> +                              function_ref<void(Error)> Warn);
> 
>   /// Write the profile to \c OS
>   void write(raw_fd_ostream &OS);
> @@ -87,8 +92,8 @@ public:
>   void setOutputSparse(bool Sparse);
> 
> private:
> -  Error addRecord(StringRef Name, uint64_t Hash, InstrProfRecord &&I,
> -                  uint64_t Weight = 1);
> +  void addRecord(StringRef Name, uint64_t Hash, InstrProfRecord &&I,
> +                 uint64_t Weight, function_ref<void(Error)> Warn);
>   bool shouldEncodeData(const ProfilingData &PD);
>   void writeImpl(ProfOStream &OS);
> };
> 
> Modified: llvm/trunk/lib/ProfileData/InstrProf.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/InstrProf.cpp?rev=307516&r1=307515&r2=307516&view=diff
> ==============================================================================
> --- llvm/trunk/lib/ProfileData/InstrProf.cpp (original)
> +++ llvm/trunk/lib/ProfileData/InstrProf.cpp Sun Jul  9 20:04:59 2017
> @@ -460,9 +460,9 @@ Error readPGOFuncNameStrings(StringRef N
>   return Error::success();
> }
> 
> -void InstrProfValueSiteRecord::merge(SoftInstrProfErrors &SIPE,
> -                                     InstrProfValueSiteRecord &Input,
> -                                     uint64_t Weight) {
> +void InstrProfValueSiteRecord::merge(InstrProfValueSiteRecord &Input,
> +                                     uint64_t Weight,
> +                                     function_ref<void(instrprof_error)> Warn) {
>   this->sortByTargetValues();
>   Input.sortByTargetValues();
>   auto I = ValueData.begin();
> @@ -475,7 +475,7 @@ void InstrProfValueSiteRecord::merge(Sof
>       bool Overflowed;
>       I->Count = SaturatingMultiplyAdd(J->Count, Weight, I->Count, &Overflowed);
>       if (Overflowed)
> -        SIPE.addError(instrprof_error::counter_overflow);
> +        Warn(instrprof_error::counter_overflow);
>       ++I;
>       continue;
>     }
> @@ -483,25 +483,25 @@ void InstrProfValueSiteRecord::merge(Sof
>   }
> }
> 
> -void InstrProfValueSiteRecord::scale(SoftInstrProfErrors &SIPE,
> -                                     uint64_t Weight) {
> +void InstrProfValueSiteRecord::scale(uint64_t Weight,
> +                                     function_ref<void(instrprof_error)> Warn) {
>   for (auto I = ValueData.begin(), IE = ValueData.end(); I != IE; ++I) {
>     bool Overflowed;
>     I->Count = SaturatingMultiply(I->Count, Weight, &Overflowed);
>     if (Overflowed)
> -      SIPE.addError(instrprof_error::counter_overflow);
> +      Warn(instrprof_error::counter_overflow);
>   }
> }
> 
> // Merge Value Profile data from Src record to this record for ValueKind.
> // Scale merged value counts by \p Weight.
> -void InstrProfRecord::mergeValueProfData(uint32_t ValueKind,
> -                                         InstrProfRecord &Src,
> -                                         uint64_t Weight) {
> +void InstrProfRecord::mergeValueProfData(
> +    uint32_t ValueKind, InstrProfRecord &Src, uint64_t Weight,
> +    function_ref<void(instrprof_error)> Warn) {
>   uint32_t ThisNumValueSites = getNumValueSites(ValueKind);
>   uint32_t OtherNumValueSites = Src.getNumValueSites(ValueKind);
>   if (ThisNumValueSites != OtherNumValueSites) {
> -    SIPE.addError(instrprof_error::value_site_count_mismatch);
> +    Warn(instrprof_error::value_site_count_mismatch);
>     return;
>   }
>   if (!ThisNumValueSites)
> @@ -511,14 +511,15 @@ void InstrProfRecord::mergeValueProfData
>   MutableArrayRef<InstrProfValueSiteRecord> OtherSiteRecords =
>       Src.getValueSitesForKind(ValueKind);
>   for (uint32_t I = 0; I < ThisNumValueSites; I++)
> -    ThisSiteRecords[I].merge(SIPE, OtherSiteRecords[I], Weight);
> +    ThisSiteRecords[I].merge(OtherSiteRecords[I], Weight, Warn);
> }
> 
> -void InstrProfRecord::merge(InstrProfRecord &Other, uint64_t Weight) {
> +void InstrProfRecord::merge(InstrProfRecord &Other, uint64_t Weight,
> +                            function_ref<void(instrprof_error)> Warn) {
>   // If the number of counters doesn't match we either have bad data
>   // or a hash collision.
>   if (Counts.size() != Other.Counts.size()) {
> -    SIPE.addError(instrprof_error::count_mismatch);
> +    Warn(instrprof_error::count_mismatch);
>     return;
>   }
> 
> @@ -527,27 +528,30 @@ void InstrProfRecord::merge(InstrProfRec
>     Counts[I] =
>         SaturatingMultiplyAdd(Other.Counts[I], Weight, Counts[I], &Overflowed);
>     if (Overflowed)
> -      SIPE.addError(instrprof_error::counter_overflow);
> +      Warn(instrprof_error::counter_overflow);
>   }
> 
>   for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind)
> -    mergeValueProfData(Kind, Other, Weight);
> +    mergeValueProfData(Kind, Other, Weight, Warn);
> }
> 
> -void InstrProfRecord::scaleValueProfData(uint32_t ValueKind, uint64_t Weight) {
> +void InstrProfRecord::scaleValueProfData(
> +    uint32_t ValueKind, uint64_t Weight,
> +    function_ref<void(instrprof_error)> Warn) {
>   for (auto &R : getValueSitesForKind(ValueKind))
> -    R.scale(SIPE, Weight);
> +    R.scale(Weight, Warn);
> }
> 
> -void InstrProfRecord::scale(uint64_t Weight) {
> +void InstrProfRecord::scale(uint64_t Weight,
> +                            function_ref<void(instrprof_error)> Warn) {
>   for (auto &Count : this->Counts) {
>     bool Overflowed;
>     Count = SaturatingMultiply(Count, Weight, &Overflowed);
>     if (Overflowed)
> -      SIPE.addError(instrprof_error::counter_overflow);
> +      Warn(instrprof_error::counter_overflow);
>   }
>   for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind)
> -    scaleValueProfData(Kind, Weight);
> +    scaleValueProfData(Kind, Weight, Warn);
> }
> 
> // Map indirect call target name hash to name string.
> 
> Modified: llvm/trunk/lib/ProfileData/InstrProfWriter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/InstrProfWriter.cpp?rev=307516&r1=307515&r2=307516&view=diff
> ==============================================================================
> --- llvm/trunk/lib/ProfileData/InstrProfWriter.cpp (original)
> +++ llvm/trunk/lib/ProfileData/InstrProfWriter.cpp Sun Jul  9 20:04:59 2017
> @@ -176,14 +176,16 @@ void InstrProfWriter::setOutputSparse(bo
>   this->Sparse = Sparse;
> }
> 
> -Error InstrProfWriter::addRecord(NamedInstrProfRecord &&I, uint64_t Weight) {
> +void InstrProfWriter::addRecord(NamedInstrProfRecord &&I, uint64_t Weight,
> +                                function_ref<void(Error)> Warn) {
>   auto Name = I.Name;
>   auto Hash = I.Hash;
> -  return addRecord(Name, Hash, std::move(I), Weight);
> +  addRecord(Name, Hash, std::move(I), Weight, Warn);
> }
> 
> -Error InstrProfWriter::addRecord(StringRef Name, uint64_t Hash,
> -                                 InstrProfRecord &&I, uint64_t Weight) {
> +void InstrProfWriter::addRecord(StringRef Name, uint64_t Hash,
> +                                InstrProfRecord &&I, uint64_t Weight,
> +                                function_ref<void(Error)> Warn) {
>   auto &ProfileDataMap = FunctionData[Name];
> 
>   bool NewFunc;
> @@ -192,27 +194,28 @@ Error InstrProfWriter::addRecord(StringR
>       ProfileDataMap.insert(std::make_pair(Hash, InstrProfRecord()));
>   InstrProfRecord &Dest = Where->second;
> 
> +  auto MapWarn = [&](instrprof_error E) {
> +    Warn(make_error<InstrProfError>(E));
> +  };
> +
>   if (NewFunc) {
>     // We've never seen a function with this name and hash, add it.
>     Dest = std::move(I);
>     if (Weight > 1)
> -      Dest.scale(Weight);
> +      Dest.scale(Weight, MapWarn);
>   } else {
>     // We're updating a function we've seen before.
> -    Dest.merge(I, Weight);
> +    Dest.merge(I, Weight, MapWarn);
>   }
> 
>   Dest.sortValueData();
> -
> -  return Dest.takeError();
> }
> 
> -Error InstrProfWriter::mergeRecordsFromWriter(InstrProfWriter &&IPW) {
> +void InstrProfWriter::mergeRecordsFromWriter(InstrProfWriter &&IPW,
> +                                             function_ref<void(Error)> Warn) {
>   for (auto &I : IPW.FunctionData)
>     for (auto &Func : I.getValue())
> -      if (Error E = addRecord(I.getKey(), Func.first, std::move(Func.second)))
> -        return E;
> -  return Error::success();
> +      addRecord(I.getKey(), Func.first, std::move(Func.second), 1, Warn);
> }
> 
> bool InstrProfWriter::shouldEncodeData(const ProfilingData &PD) {
> 
> Modified: llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp?rev=307516&r1=307515&r2=307516&view=diff
> ==============================================================================
> --- llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp (original)
> +++ llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp Sun Jul  9 20:04:59 2017
> @@ -159,14 +159,20 @@ static void loadInput(const WeightedFile
> 
>   for (auto &I : *Reader) {
>     const StringRef FuncName = I.Name;
> -    if (Error E = WC->Writer.addRecord(std::move(I), Input.Weight)) {
> +    bool Reported = false;
> +    WC->Writer.addRecord(std::move(I), Input.Weight, [&](Error E) {
> +      if (Reported) {
> +        consumeError(std::move(E));
> +        return;
> +      }
> +      Reported = true;
>       // Only show hint the first time an error occurs.
>       instrprof_error IPE = InstrProfError::take(std::move(E));
>       std::unique_lock<std::mutex> ErrGuard{WC->ErrLock};
>       bool firstTime = WC->WriterErrorCodes.insert(IPE).second;
>       handleMergeWriterError(make_error<InstrProfError>(IPE), Input.Filename,
>                              FuncName, firstTime);
> -    }
> +    });
>   }
>   if (Reader->hasError())
>     WC->Err = Reader->getError();
> @@ -174,8 +180,15 @@ static void loadInput(const WeightedFile
> 
> /// Merge the \p Src writer context into \p Dst.
> static void mergeWriterContexts(WriterContext *Dst, WriterContext *Src) {
> -  if (Error E = Dst->Writer.mergeRecordsFromWriter(std::move(Src->Writer)))
> +  bool Reported = false;
> +  Dst->Writer.mergeRecordsFromWriter(std::move(Src->Writer), [&](Error E) {
> +    if (Reported) {
> +      consumeError(std::move(E));
> +      return;
> +    }
> +    Reported = true;
>     Dst->Err = std::move(E);
> +  });
> }
> 
> static void mergeInstrProfile(const WeightedFileVector &Inputs,
> 
> Modified: llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp?rev=307516&r1=307515&r2=307516&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp (original)
> +++ llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp Sun Jul  9 20:04:59 2017
> @@ -303,9 +303,10 @@ TEST_P(CoverageMappingTest, correct_dese
>   }
> }
> 
> +static const auto Err = [](Error E) { FAIL(); };
> +
> TEST_P(CoverageMappingTest, load_coverage_for_more_than_two_files) {
> -  EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func", 0x1234, {0}}),
> -                    Succeeded());
> +  ProfileWriter.addRecord({"func", 0x1234, {0}}, Err);
> 
>   const char *FileNames[] = {"bar", "baz", "foo"};
>   static const unsigned N = array_lengthof(FileNames);
> @@ -326,17 +327,15 @@ TEST_P(CoverageMappingTest, load_coverag
> }
> 
> TEST_P(CoverageMappingTest, load_coverage_with_bogus_function_name) {
> -  EXPECT_THAT_ERROR(ProfileWriter.addRecord({"", 0x1234, {10}}), Succeeded());
> +  ProfileWriter.addRecord({"", 0x1234, {10}}, Err);
>   startFunction("", 0x1234);
>   addCMR(Counter::getCounter(0), "foo", 1, 1, 5, 5);
>   EXPECT_TRUE(ErrorEquals(coveragemap_error::malformed, loadCoverageMapping()));
> }
> 
> TEST_P(CoverageMappingTest, load_coverage_for_several_functions) {
> -  EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func1", 0x1234, {10}}),
> -                    Succeeded());
> -  EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func2", 0x2345, {20}}),
> -                    Succeeded());
> +  ProfileWriter.addRecord({"func1", 0x1234, {10}}, Err);
> +  ProfileWriter.addRecord({"func2", 0x2345, {20}}, Err);
> 
>   startFunction("func1", 0x1234);
>   addCMR(Counter::getCounter(0), "foo", 1, 1, 5, 5);
> @@ -380,8 +379,7 @@ TEST_P(CoverageMappingTest, expansion_ge
> }
> 
> TEST_P(CoverageMappingTest, basic_coverage_iteration) {
> -  EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func", 0x1234, {30, 20, 10, 0}}),
> -                    Succeeded());
> +  ProfileWriter.addRecord({"func", 0x1234, {30, 20, 10, 0}}, Err);
> 
>   startFunction("func", 0x1234);
>   addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
> @@ -429,8 +427,7 @@ TEST_P(CoverageMappingTest, uncovered_fu
> }
> 
> TEST_P(CoverageMappingTest, combine_regions) {
> -  EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func", 0x1234, {10, 20, 30}}),
> -                    Succeeded());
> +  ProfileWriter.addRecord({"func", 0x1234, {10, 20, 30}}, Err);
> 
>   startFunction("func", 0x1234);
>   addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
> @@ -448,8 +445,7 @@ TEST_P(CoverageMappingTest, combine_regi
> }
> 
> TEST_P(CoverageMappingTest, restore_combined_counter_after_nested_region) {
> -  EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func", 0x1234, {10, 20, 40}}),
> -                    Succeeded());
> +  ProfileWriter.addRecord({"func", 0x1234, {10, 20, 40}}, Err);
> 
>   startFunction("func", 0x1234);
>   addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
> @@ -469,10 +465,8 @@ TEST_P(CoverageMappingTest, restore_comb
> // If CodeRegions and ExpansionRegions cover the same area,
> // only counts of CodeRegions should be used.
> TEST_P(CoverageMappingTest, dont_combine_expansions) {
> -  EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func", 0x1234, {10, 20}}),
> -                    Succeeded());
> -  EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func", 0x1234, {0, 0}}),
> -                    Succeeded());
> +  ProfileWriter.addRecord({"func", 0x1234, {10, 20}}, Err);
> +  ProfileWriter.addRecord({"func", 0x1234, {0, 0}}, Err);
> 
>   startFunction("func", 0x1234);
>   addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
> @@ -492,8 +486,7 @@ TEST_P(CoverageMappingTest, dont_combine
> 
> // If an area is covered only by ExpansionRegions, they should be combinated.
> TEST_P(CoverageMappingTest, combine_expansions) {
> -  EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func", 0x1234, {2, 3, 7}}),
> -                    Succeeded());
> +  ProfileWriter.addRecord({"func", 0x1234, {2, 3, 7}}, Err);
> 
>   startFunction("func", 0x1234);
>   addCMR(Counter::getCounter(1), "include1", 1, 1, 1, 10);
> @@ -514,8 +507,7 @@ TEST_P(CoverageMappingTest, combine_expa
> }
> 
> TEST_P(CoverageMappingTest, strip_filename_prefix) {
> -  EXPECT_THAT_ERROR(ProfileWriter.addRecord({"file1:func", 0x1234, {0}}),
> -                    Succeeded());
> +  ProfileWriter.addRecord({"file1:func", 0x1234, {0}}, Err);
> 
>   startFunction("file1:func", 0x1234);
>   addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
> @@ -529,8 +521,7 @@ TEST_P(CoverageMappingTest, strip_filena
> }
> 
> TEST_P(CoverageMappingTest, strip_unknown_filename_prefix) {
> -  EXPECT_THAT_ERROR(ProfileWriter.addRecord({"<unknown>:func", 0x1234, {0}}),
> -                    Succeeded());
> +  ProfileWriter.addRecord({"<unknown>:func", 0x1234, {0}}, Err);
> 
>   startFunction("<unknown>:func", 0x1234);
>   addCMR(Counter::getCounter(0), "", 1, 1, 9, 9);
> @@ -544,10 +535,8 @@ TEST_P(CoverageMappingTest, strip_unknow
> }
> 
> TEST_P(CoverageMappingTest, dont_detect_false_instantiations) {
> -  EXPECT_THAT_ERROR(ProfileWriter.addRecord({"foo", 0x1234, {10}}),
> -                    Succeeded());
> -  EXPECT_THAT_ERROR(ProfileWriter.addRecord({"bar", 0x2345, {20}}),
> -                    Succeeded());
> +  ProfileWriter.addRecord({"foo", 0x1234, {10}}, Err);
> +  ProfileWriter.addRecord({"bar", 0x2345, {20}}, Err);
> 
>   startFunction("foo", 0x1234);
>   addCMR(Counter::getCounter(0), "expanded", 1, 1, 1, 10);
> @@ -565,8 +554,7 @@ TEST_P(CoverageMappingTest, dont_detect_
> }
> 
> TEST_P(CoverageMappingTest, load_coverage_for_expanded_file) {
> -  EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func", 0x1234, {10}}),
> -                    Succeeded());
> +  ProfileWriter.addRecord({"func", 0x1234, {10}}, Err);
> 
>   startFunction("func", 0x1234);
>   addCMR(Counter::getCounter(0), "expanded", 1, 1, 1, 10);
> @@ -582,8 +570,7 @@ TEST_P(CoverageMappingTest, load_coverag
> }
> 
> TEST_P(CoverageMappingTest, skip_duplicate_function_record) {
> -  EXPECT_THAT_ERROR(ProfileWriter.addRecord({"func", 0x1234, {1}}),
> -                    Succeeded());
> +  ProfileWriter.addRecord({"func", 0x1234, {1}}, Err);
> 
>   startFunction("func", 0x1234);
>   addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
> 
> Modified: llvm/trunk/unittests/ProfileData/InstrProfTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ProfileData/InstrProfTest.cpp?rev=307516&r1=307515&r2=307516&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/ProfileData/InstrProfTest.cpp (original)
> +++ llvm/trunk/unittests/ProfileData/InstrProfTest.cpp Sun Jul  9 20:04:59 2017
> @@ -64,9 +64,13 @@ TEST_P(MaybeSparseInstrProfTest, write_a
>   ASSERT_TRUE(Reader->begin() == Reader->end());
> }
> 
> +static const auto Err = [](Error E) {
> +  consumeError(std::move(E));
> +  FAIL();
> +};
> +
> TEST_P(MaybeSparseInstrProfTest, write_and_read_one_function) {
> -  EXPECT_THAT_ERROR(Writer.addRecord({"foo", 0x1234, {1, 2, 3, 4}}),
> -                    Succeeded());
> +  Writer.addRecord({"foo", 0x1234, {1, 2, 3, 4}}, Err);
>   auto Profile = Writer.writeBuffer();
>   readProfile(std::move(Profile));
> 
> @@ -83,8 +87,8 @@ TEST_P(MaybeSparseInstrProfTest, write_a
> }
> 
> TEST_P(MaybeSparseInstrProfTest, get_instr_prof_record) {
> -  EXPECT_THAT_ERROR(Writer.addRecord({"foo", 0x1234, {1, 2}}), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({"foo", 0x1235, {3, 4}}), Succeeded());
> +  Writer.addRecord({"foo", 0x1234, {1, 2}}, Err);
> +  Writer.addRecord({"foo", 0x1235, {3, 4}}, Err);
>   auto Profile = Writer.writeBuffer();
>   readProfile(std::move(Profile));
> 
> @@ -108,8 +112,8 @@ TEST_P(MaybeSparseInstrProfTest, get_ins
> }
> 
> TEST_P(MaybeSparseInstrProfTest, get_function_counts) {
> -  EXPECT_THAT_ERROR(Writer.addRecord({"foo", 0x1234, {1, 2}}), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({"foo", 0x1235, {3, 4}}), Succeeded());
> +  Writer.addRecord({"foo", 0x1234, {1, 2}}, Err);
> +  Writer.addRecord({"foo", 0x1235, {3, 4}}, Err);
>   auto Profile = Writer.writeBuffer();
>   readProfile(std::move(Profile));
> 
> @@ -135,15 +139,15 @@ TEST_P(MaybeSparseInstrProfTest, get_fun
> 
> // Profile data is copied from general.proftext
> TEST_F(InstrProfTest, get_profile_summary) {
> -  EXPECT_THAT_ERROR(Writer.addRecord({"func1", 0x1234, {97531}}), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({"func2", 0x1234, {0, 0}}), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({"func3",
> -                                      0x1234,
> -                                      {2305843009213693952, 1152921504606846976,
> -                                       576460752303423488, 288230376151711744,
> -                                       144115188075855872, 72057594037927936}}),
> -                    Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({"func4", 0x1234, {0}}), Succeeded());
> +  Writer.addRecord({"func1", 0x1234, {97531}}, Err);
> +  Writer.addRecord({"func2", 0x1234, {0, 0}}, Err);
> +  Writer.addRecord(
> +      {"func3",
> +       0x1234,
> +       {2305843009213693952, 1152921504606846976, 576460752303423488,
> +        288230376151711744, 144115188075855872, 72057594037927936}},
> +      Err);
> +  Writer.addRecord({"func4", 0x1234, {0}}, Err);
>   auto Profile = Writer.writeBuffer();
>   readProfile(std::move(Profile));
> 
> @@ -194,13 +198,12 @@ TEST_F(InstrProfTest, get_profile_summar
> }
> 
> TEST_F(InstrProfTest, test_writer_merge) {
> -  EXPECT_THAT_ERROR(Writer.addRecord({"func1", 0x1234, {42}}), Succeeded());
> +  Writer.addRecord({"func1", 0x1234, {42}}, Err);
> 
>   InstrProfWriter Writer2;
> -  EXPECT_THAT_ERROR(Writer2.addRecord({"func2", 0x1234, {0, 0}}), Succeeded());
> +  Writer2.addRecord({"func2", 0x1234, {0, 0}}, Err);
> 
> -  EXPECT_THAT_ERROR(Writer.mergeRecordsFromWriter(std::move(Writer2)),
> -                    Succeeded());
> +  Writer.mergeRecordsFromWriter(std::move(Writer2), Err);
> 
>   auto Profile = Writer.writeBuffer();
>   readProfile(std::move(Profile));
> @@ -239,10 +242,10 @@ TEST_P(MaybeSparseInstrProfTest, get_ica
>   InstrProfValueData VD3[] = {{(uint64_t)callee1, 1}};
>   Record1.addValueData(IPVK_IndirectCallTarget, 3, VD3, 1, nullptr);
> 
> -  EXPECT_THAT_ERROR(Writer.addRecord(std::move(Record1)), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({"callee1", 0x1235, {3, 4}}), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({"callee2", 0x1235, {3, 4}}), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({"callee3", 0x1235, {3, 4}}), Succeeded());
> +  Writer.addRecord(std::move(Record1), Err);
> +  Writer.addRecord({"callee1", 0x1235, {3, 4}}, Err);
> +  Writer.addRecord({"callee2", 0x1235, {3, 4}}, Err);
> +  Writer.addRecord({"callee3", 0x1235, {3, 4}}, Err);
>   auto Profile = Writer.writeBuffer();
>   readProfile(std::move(Profile));
> 
> @@ -274,7 +277,7 @@ TEST_P(MaybeSparseInstrProfTest, annotat
>   InstrProfValueData VD0[] = {{1000, 1}, {2000, 2}, {3000, 3}, {5000, 5},
>                               {4000, 4}, {6000, 6}};
>   Record.addValueData(IPVK_IndirectCallTarget, 0, VD0, 6, nullptr);
> -  EXPECT_THAT_ERROR(Writer.addRecord(std::move(Record)), Succeeded());
> +  Writer.addRecord(std::move(Record), Err);
>   auto Profile = Writer.writeBuffer();
>   readProfile(std::move(Profile));
>   Expected<InstrProfRecord> R = Reader->getInstrProfRecord("caller", 0x1234);
> @@ -379,10 +382,10 @@ TEST_P(MaybeSparseInstrProfTest, get_ica
>   InstrProfValueData VD3[] = {{(uint64_t)callee1, 1}};
>   Record1.addValueData(IPVK_IndirectCallTarget, 3, VD3, 1, nullptr);
> 
> -  EXPECT_THAT_ERROR(Writer.addRecord(std::move(Record1), 10), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({"callee1", 0x1235, {3, 4}}), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({"callee2", 0x1235, {3, 4}}), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({"callee3", 0x1235, {3, 4}}), Succeeded());
> +  Writer.addRecord(std::move(Record1), 10, Err);
> +  Writer.addRecord({"callee1", 0x1235, {3, 4}}, Err);
> +  Writer.addRecord({"callee2", 0x1235, {3, 4}}, Err);
> +  Writer.addRecord({"callee3", 0x1235, {3, 4}}, Err);
>   auto Profile = Writer.writeBuffer();
>   readProfile(std::move(Profile));
> 
> @@ -422,10 +425,10 @@ TEST_P(MaybeSparseInstrProfTest, get_ica
>   InstrProfValueData VD3[] = {{(uint64_t)callee1, 1}};
>   Record1.addValueData(IPVK_IndirectCallTarget, 3, VD3, 1, nullptr);
> 
> -  EXPECT_THAT_ERROR(Writer.addRecord(std::move(Record1)), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({"callee1", 0x1235, {3, 4}}), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({"callee2", 0x1235, {3, 4}}), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({"callee3", 0x1235, {3, 4}}), Succeeded());
> +  Writer.addRecord(std::move(Record1), Err);
> +  Writer.addRecord({"callee1", 0x1235, {3, 4}}, Err);
> +  Writer.addRecord({"callee2", 0x1235, {3, 4}}, Err);
> +  Writer.addRecord({"callee3", 0x1235, {3, 4}}, Err);
> 
>   // Set big endian output.
>   Writer.setValueProfDataEndianness(support::big);
> @@ -501,15 +504,15 @@ TEST_P(MaybeSparseInstrProfTest, get_ica
>                                {uint64_t(callee3), 3}};
>   Record12.addValueData(IPVK_IndirectCallTarget, 4, VD42, 3, nullptr);
> 
> -  EXPECT_THAT_ERROR(Writer.addRecord(std::move(Record11)), Succeeded());
> +  Writer.addRecord(std::move(Record11), Err);
>   // Merge profile data.
> -  EXPECT_THAT_ERROR(Writer.addRecord(std::move(Record12)), Succeeded());
> +  Writer.addRecord(std::move(Record12), Err);
> 
> -  EXPECT_THAT_ERROR(Writer.addRecord({callee1, 0x1235, {3, 4}}), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({callee2, 0x1235, {3, 4}}), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({callee3, 0x1235, {3, 4}}), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({callee3, 0x1235, {3, 4}}), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({callee4, 0x1235, {3, 5}}), Succeeded());
> +  Writer.addRecord({callee1, 0x1235, {3, 4}}, Err);
> +  Writer.addRecord({callee2, 0x1235, {3, 4}}, Err);
> +  Writer.addRecord({callee3, 0x1235, {3, 4}}, Err);
> +  Writer.addRecord({callee3, 0x1235, {3, 4}}, Err);
> +  Writer.addRecord({callee4, 0x1235, {3, 5}}, Err);
>   auto Profile = Writer.writeBuffer();
>   readProfile(std::move(Profile));
> 
> @@ -564,35 +567,37 @@ TEST_P(MaybeSparseInstrProfTest, get_ica
> 
>   const uint64_t Max = std::numeric_limits<uint64_t>::max();
> 
> -  auto Result1 = Writer.addRecord({"foo", 0x1234, {1}});
> -  ASSERT_EQ(InstrProfError::take(std::move(Result1)),
> -            instrprof_error::success);
> +  instrprof_error Result;
> +  auto Err = [&](Error E) { Result = InstrProfError::take(std::move(E)); };
> +  Result = instrprof_error::success;
> +  Writer.addRecord({"foo", 0x1234, {1}}, Err);
> +  ASSERT_EQ(Result, instrprof_error::success);
> 
>   // Verify counter overflow.
> -  auto Result2 = Writer.addRecord({"foo", 0x1234, {Max}});
> -  ASSERT_EQ(InstrProfError::take(std::move(Result2)),
> -            instrprof_error::counter_overflow);
> -
> -  auto Result3 = Writer.addRecord({bar, 0x9012, {8}});
> -  ASSERT_EQ(InstrProfError::take(std::move(Result3)),
> -            instrprof_error::success);
> +  Result = instrprof_error::success;
> +  Writer.addRecord({"foo", 0x1234, {Max}}, Err);
> +  ASSERT_EQ(Result, instrprof_error::counter_overflow);
> +
> +  Result = instrprof_error::success;
> +  Writer.addRecord({bar, 0x9012, {8}}, Err);
> +  ASSERT_EQ(Result, instrprof_error::success);
> 
>   NamedInstrProfRecord 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(InstrProfError::take(std::move(Result4)),
> -            instrprof_error::success);
> +  Result = instrprof_error::success;
> +  Writer.addRecord(std::move(Record4), Err);
> +  ASSERT_EQ(Result, instrprof_error::success);
> 
>   // Verify value data counter overflow.
>   NamedInstrProfRecord 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(InstrProfError::take(std::move(Result5)),
> -            instrprof_error::counter_overflow);
> +  Result = instrprof_error::success;
> +  Writer.addRecord(std::move(Record5), Err);
> +  ASSERT_EQ(Result, instrprof_error::counter_overflow);
> 
>   auto Profile = Writer.writeBuffer();
>   readProfile(std::move(Profile));
> @@ -643,9 +648,9 @@ TEST_P(MaybeSparseInstrProfTest, get_ica
>   Record12.addValueData(IPVK_IndirectCallTarget, 0, VD1, 255, nullptr);
>   Record12.addValueData(IPVK_IndirectCallTarget, 1, nullptr, 0, nullptr);
> 
> -  EXPECT_THAT_ERROR(Writer.addRecord(std::move(Record11)), Succeeded());
> +  Writer.addRecord(std::move(Record11), Err);
>   // Merge profile data.
> -  EXPECT_THAT_ERROR(Writer.addRecord(std::move(Record12)), Succeeded());
> +  Writer.addRecord(std::move(Record12), Err);
> 
>   auto Profile = Writer.writeBuffer();
>   readProfile(std::move(Profile));
> @@ -790,11 +795,9 @@ TEST_P(MaybeSparseInstrProfTest, value_p
> }
> 
> TEST_P(MaybeSparseInstrProfTest, get_max_function_count) {
> -  EXPECT_THAT_ERROR(Writer.addRecord({"foo", 0x1234, {1ULL << 31, 2}}),
> -                    Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({"bar", 0, {1ULL << 63}}), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({"baz", 0x5678, {0, 0, 0, 0}}),
> -                    Succeeded());
> +  Writer.addRecord({"foo", 0x1234, {1ULL << 31, 2}}, Err);
> +  Writer.addRecord({"bar", 0, {1ULL << 63}}, Err);
> +  Writer.addRecord({"baz", 0x5678, {0, 0, 0, 0}}, Err);
>   auto Profile = Writer.writeBuffer();
>   readProfile(std::move(Profile));
> 
> @@ -802,8 +805,8 @@ TEST_P(MaybeSparseInstrProfTest, get_max
> }
> 
> TEST_P(MaybeSparseInstrProfTest, get_weighted_function_counts) {
> -  EXPECT_THAT_ERROR(Writer.addRecord({"foo", 0x1234, {1, 2}}, 3), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({"foo", 0x1235, {3, 4}}, 5), Succeeded());
> +  Writer.addRecord({"foo", 0x1234, {1, 2}}, 3, Err);
> +  Writer.addRecord({"foo", 0x1235, {3, 4}}, 5, Err);
>   auto Profile = Writer.writeBuffer();
>   readProfile(std::move(Profile));
> 
> @@ -991,11 +994,12 @@ TEST_P(MaybeSparseInstrProfTest, instr_p
> }
> 
> TEST_F(SparseInstrProfTest, preserve_no_records) {
> -  EXPECT_THAT_ERROR(Writer.addRecord({"foo", 0x1234, {0}}), Succeeded());
> -  EXPECT_THAT_ERROR(Writer.addRecord({"bar", 0x4321, {0, 0}}), Succeeded());
> +  Writer.addRecord({"foo", 0x1234, {0}}, Err);
> +  Writer.addRecord({"bar", 0x4321, {0, 0}}, Err);
>   // FIXME: I'm guessing this data should be different, but the original author
>   // should check/update this test so it doesn't produce errors.
> -  consumeError(Writer.addRecord({"bar", 0x4321, {0, 0, 0}}));
> +  Writer.addRecord({"bar", 0x4321, {0, 0, 0}},
> +                   [](Error E) { consumeError(std::move(E)); });
> 
>   auto Profile = Writer.writeBuffer();
>   readProfile(std::move(Profile));
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list