[llvm] r269222 - [ProfileData] Use SoftInstrProfErrors to count soft errors, NFC
via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 29 16:57:26 PDT 2017
> On Jun 29, 2017, at 4:52 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Thu, Jun 29, 2017 at 4:48 PM <vsk at apple.com <mailto:vsk at apple.com>> wrote:
>> On Jun 29, 2017, at 4:38 PM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>>
>>
>>
>> On Thu, Jun 29, 2017 at 4:27 PM <vsk at apple.com <mailto:vsk at apple.com>> wrote:
>>> On Jun 29, 2017, at 4:11 PM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>>>
>>>
>>>
>>> On Thu, Jun 29, 2017 at 4:09 PM <vsk at apple.com <mailto:vsk at apple.com>> wrote:
>>>> On Jun 29, 2017, at 2:41 PM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>>>>
>>>>
>>>>
>>>> On Thu, Jun 29, 2017 at 2:24 PM Xinliang David Li <davidxl at google.com <mailto:davidxl at google.com>> wrote:
>>>> On Thu, Jun 29, 2017 at 2:16 PM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>>>>
>>>>
>>>> On Thu, Jun 29, 2017 at 10:03 AM <vsk at apple.com <mailto:vsk at apple.com>> wrote:
>>>> I'm in favor of removing the counters from this struct, but keeping it as a member of InstrProfRecord [*]
>>>>
>>>> I expected to use the counters in this struct to aid debugging, and to eventually implement better error reporting. This hasn't panned out: the counters haven't been much help (at least, not to me, I'm curious as to whether @davidxl has made use of them). It's totally fair to remove them until we have a better plan.
>>>>
>>>> Having a soft error tracker within InstrProfRecord makes error tracking very 'nice'. We can make sure an IPR isn't destroyed without its error state being considered. We can also handle copying/moving IPRs easily. @dblaikie The cost of a single instrprof_error field doesn't seem too high (at least, it'd be a lot better than what we have now). Would that work for you?
>>>>
>>>> I'd really be inclined to push for removing it entirely - keeping the error (even as a byte rather than a word) in the counters sub-struct would be a 25% increase in size for this sub-struct (it'd have 3 words for the Counts vector, one word for the unique_ptr to the value profiling stats, then adding even another byte would add a 5th word (due to padding/rounding up)).
>>>>
>>>> 25% increase is over-stated :) Remember that the counters vector also has dynamic allocated array with size that are multiples of uint64.
>>>>
>>>> Ah, the data I have at the moment points to the actual InstrProfRecords themselves, and the map entries they are within, account for 93% of memory usage (85% SmallDenseMap::allocateBuckets -> malloc/new/new[], 8% StringMapEntry::Create -> MallocAllocator::Allocate).
>>>>
>>>> The std::vector allocation for the Counts (the value profiling vectors are below the 1% threshold) are 1.32%.
>>>>
>>>> I think we can keep it for now unless we have data to show that the error field creates significant overhead (e.g > 5% overall memory increase).
>>>>
>>>> Sure - I can check, I think it's more than that.
>>>>
>>>> Yep, in my prototype, adding one word to the sub-struct increases peak memory usage from 5.1 -> 5.9 GB.
>>>
>>> This is enough to convince me we should get rid of the error member. It's more typing to thread the error state through callers, but it looks like it's worth it.
>>>
>>> Cool cool - I'm not sure how nice it'd be, but since we've moved to llvm::Error since the initial work - would it be possible/nice to just return Error from these functions, then it wouldn't be so many extra function parameters, etc?
>>
>> It's possible, but a bit cumbersome. E.g here's what error tracking looks like in a loop now:
>>
>> void merge(SoftInstrProfErrors &SIPE) {
>> for each counter {
>> add them
>> if (overflow)
>> SIPE.addError(counter_overflow);
>> }
>>
>> Were we to return Error everywhere, you'd need something like this:
>>
>> Error merge() {
>> Error E = Error::success();
>> for each counter {
>> add them
>> if (overflow)
>> if (!E)
>> E = make_error<...>(counter_overflow);
>> }
>> return E;
>> }
>>
>> Any chance of failing fast?
>>
>> Error merge() {
>> for each counter {
>> add them
>> if (overflow)
>> return make_error<...>(counter_overflow);
>> }
>> return Error::success();
>> }
>
> I don't think so, we need a way to resume these operations when we encounter a soft error.
>
> That's sort of what I was wondering about/questioning. Are these more like warnings than errors? Or why is it important to resume/continue after these failures?
They're more like warnings: it provides a heads-up to users when their profiles are getting stale (hash mismatches), or when imprecision is creeping in to the profiles (counter overflow). It's important that we be able to recover from these issues, because e.g clang-based profiling instrumentation is designed to work with profiles which are a bit stale. The original motivation was to make it possible for users to store profiles on some server, and then have project builds reuse that profile data, instead of obtaining fresh training data every time the project needs to be built.
vedant
>
> ("more what you'd call... guidelines" ;) )
>
>
>
>> If these Errors are recoverable (as might be indicated by the 'soft' in SoftInstrProfError) maybe it'd be better to pass a callback through (while you're adding this parameter everywhere anyway) - function_ref<void(Error)>
>>
>> That way one caller could pass a callback that does archive all the Errors like what SoftInstrProfError was doing (without baking it in) & existing callers can print the first Error and then use a boolean flag to ignore the rest?
>
> Having a callback sounds good to me, I don't know why I hadn't considered that.
>
> Dandy :)
>
>
> vedant
>
>> Feel free to tell me such redesigns are a bit out of scope, etc - only trying to offer some ideas, I'm mostly motivated to address the memory usage right now & don't mean to make it into a bigger project than it needs to be.
>>
>> - Dave
>>
>>
>> Also, every time you call a function which returns an Error, you have to decide how to update the current function's error state. To preserve the current behavior, you'd only update the current error state if it is Error::success(), otherwise you'd have to consumeError() the new error and drop it.
>>
>> So I think it'll be more convenient to keep the soft error tracking class, but have it passed around by callers. It costs one extra parameter per method but it might end up making things cleaner.
>>
>> Of course if you do try returning Error everywhere and find that it's simple/ergonomic I'm open to that.
>>
>> vedant
>>
>>>
>>> Anyway, whatever works best for you - just a thought.
>>>
>>>
>>> vedant
>>>
>>>>
>>>>
>>>> David
>>>>
>>>>
>>>> I agree that error handling is really important - but like the original implementation you posted, I think error handling at the point of failure is generally a better idea (rather than having to keep track of whether this InstrProfRecord has been queried for its failures or not, etc - you call the function that can fail, you handle the failure).
>>>>
>>>> - Dave
>>>>
>>>>
>>>> thanks,
>>>> vedant
>>>>
>>>> [*] I'm also volunteering to make the change, if there's agreement it's the right way to go :).
>>>>
>>>>> On Jun 29, 2017, at 9:44 AM, Xinliang David Li <davidxl at google.com <mailto:davidxl at google.com>> wrote:
>>>>>
>>>>> The size impact of this struct is indeed pretty large. We can consider compress the size of it significantly. For instance, making each member uint8_t. If the number of errors of each category > 255, it can be capped at 255 and the error message handling can be adjusted properly.
>>>>>
>>>>> David
>>>>>
>>>>> On Wed, Jun 28, 2017 at 11:57 PM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>>>>> This seems to have gone unused for a year so far - shall we remove it until there's an actual use case?
>>>>>
>>>>> I came across this because the sizeof(InstrProfRecord) contributes significantly to the memory footprint of llvm-profdata merge (I've made one big improvement so far (14GB -> 10GB for an example large profile)). I have a change in mind/prototyped that helps that (10GB -> 5GB) by moving the counters into a sub-struct of InstrProfRecord and using only that sub-struct in the InstrProfWriter (since it has the name and hash in the maps its using for lookup during merging - so they don't need to be duplicated in the values too). But this SoftInstrProfErrors is used from the counter-related functions and is quite large, so naively it would have to move into this sub-struct & take up lots of space.
>>>>>
>>>>> So at the very least I'd like to revisit the choice to make this a member, and instead go with the earlier version of this patch that wired it through function parameters instead - but given the lack of use, I think maybe it'd be better to remove this unused abstraction & go back to the simpler error handling that was present before.
>>>>>
>>>>> - Dave
>>>>>
>>>>> On Wed, May 11, 2016 at 12:48 PM Vedant Kumar via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>>>>> Author: vedantk
>>>>> Date: Wed May 11 14:42:19 2016
>>>>> New Revision: 269222
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=269222&view=rev <http://llvm.org/viewvc/llvm-project?rev=269222&view=rev>
>>>>> Log:
>>>>> [ProfileData] Use SoftInstrProfErrors to count soft errors, NFC
>>>>>
>>>>> Differential Revision: http://reviews.llvm.org/D20082 <http://reviews.llvm.org/D20082>
>>>>>
>>>>> Modified:
>>>>> llvm/trunk/include/llvm/ProfileData/InstrProf.h
>>>>> llvm/trunk/lib/ProfileData/InstrProf.cpp
>>>>> llvm/trunk/lib/ProfileData/InstrProfWriter.cpp
>>>>>
>>>>> Modified: llvm/trunk/include/llvm/ProfileData/InstrProf.h
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/InstrProf.h?rev=269222&r1=269221&r2=269222&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/InstrProf.h?rev=269222&r1=269221&r2=269222&view=diff>
>>>>> ==============================================================================
>>>>> --- llvm/trunk/include/llvm/ProfileData/InstrProf.h (original)
>>>>> +++ llvm/trunk/include/llvm/ProfileData/InstrProf.h Wed May 11 14:42:19 2016
>>>>> @@ -284,15 +284,51 @@ inline std::error_code make_error_code(i
>>>>> 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;
>>>>> -}
>>>>> +class SoftInstrProfErrors {
>>>>> + /// Count the number of soft instrprof_errors encountered and keep track of
>>>>> + /// the first such error for reporting purposes.
>>>>> +
>>>>> + /// The first soft error encountered.
>>>>> + instrprof_error FirstError;
>>>>> +
>>>>> + /// The number of hash mismatches.
>>>>> + unsigned NumHashMismatches;
>>>>> +
>>>>> + /// The number of count mismatches.
>>>>> + unsigned NumCountMismatches;
>>>>> +
>>>>> + /// The number of counter overflows.
>>>>> + unsigned NumCounterOverflows;
>>>>> +
>>>>> + /// The number of value site count mismatches.
>>>>> + unsigned NumValueSiteCountMismatches;
>>>>> +
>>>>> +public:
>>>>> + SoftInstrProfErrors()
>>>>> + : FirstError(instrprof_error::success), NumHashMismatches(0),
>>>>> + NumCountMismatches(0), NumCounterOverflows(0),
>>>>> + NumValueSiteCountMismatches(0) {}
>>>>> +
>>>>> + /// Track a soft error (\p IE) and increment its associated counter.
>>>>> + void addError(instrprof_error IE);
>>>>> +
>>>>> + /// Get the number of hash mismatches.
>>>>> + unsigned getNumHashMismatches() const { return NumHashMismatches; }
>>>>> +
>>>>> + /// Get the number of count mismatches.
>>>>> + unsigned getNumCountMismatches() const { return NumCountMismatches; }
>>>>> +
>>>>> + /// Get the number of counter overflows.
>>>>> + unsigned getNumCounterOverflows() const { return NumCounterOverflows; }
>>>>> +
>>>>> + /// Get the number of value site count mismatches.
>>>>> + unsigned getNumValueSiteCountMismatches() const {
>>>>> + return NumValueSiteCountMismatches;
>>>>> + }
>>>>> +
>>>>> + /// Return an error code for the first encountered error.
>>>>> + std::error_code getError() const { return make_error_code(FirstError); }
>>>>> +};
>>>>>
>>>>> namespace object {
>>>>> class SectionRef;
>>>>> @@ -465,19 +501,21 @@ struct InstrProfValueSiteRecord {
>>>>>
>>>>> /// Merge data from another InstrProfValueSiteRecord
>>>>> /// Optionally scale merged counts by \p Weight.
>>>>> - instrprof_error merge(InstrProfValueSiteRecord &Input, uint64_t Weight = 1);
>>>>> + void merge(SoftInstrProfErrors &SIPE, InstrProfValueSiteRecord &Input,
>>>>> + uint64_t Weight = 1);
>>>>> /// Scale up value profile data counts.
>>>>> - instrprof_error scale(uint64_t Weight);
>>>>> + void scale(SoftInstrProfErrors &SIPE, uint64_t Weight);
>>>>> };
>>>>>
>>>>> /// Profiling information for a single function.
>>>>> struct InstrProfRecord {
>>>>> - InstrProfRecord() {}
>>>>> + InstrProfRecord() : SIPE() {}
>>>>> InstrProfRecord(StringRef Name, uint64_t Hash, std::vector<uint64_t> Counts)
>>>>> - : Name(Name), Hash(Hash), Counts(std::move(Counts)) {}
>>>>> + : Name(Name), Hash(Hash), Counts(std::move(Counts)), SIPE() {}
>>>>> StringRef Name;
>>>>> uint64_t Hash;
>>>>> std::vector<uint64_t> Counts;
>>>>> + SoftInstrProfErrors SIPE;
>>>>>
>>>>> typedef std::vector<std::pair<uint64_t, uint64_t>> ValueMapType;
>>>>>
>>>>> @@ -512,11 +550,11 @@ struct InstrProfRecord {
>>>>>
>>>>> /// Merge the counts in \p Other into this one.
>>>>> /// Optionally scale merged counts by \p Weight.
>>>>> - instrprof_error merge(InstrProfRecord &Other, uint64_t Weight = 1);
>>>>> + void merge(InstrProfRecord &Other, uint64_t Weight = 1);
>>>>>
>>>>> /// Scale up profile counts (including value profile data) by
>>>>> /// \p Weight.
>>>>> - instrprof_error scale(uint64_t Weight);
>>>>> + void scale(uint64_t Weight);
>>>>>
>>>>> /// Sort value profile data (per site) by count.
>>>>> void sortValueData() {
>>>>> @@ -533,6 +571,9 @@ struct InstrProfRecord {
>>>>> getValueSitesForKind(Kind).clear();
>>>>> }
>>>>>
>>>>> + /// Get the error contained within the record's soft error counter.
>>>>> + std::error_code getError() const { return SIPE.getError(); }
>>>>> +
>>>>> private:
>>>>> std::vector<InstrProfValueSiteRecord> IndirectCallSites;
>>>>> const std::vector<InstrProfValueSiteRecord> &
>>>>> @@ -559,10 +600,10 @@ private:
>>>>>
>>>>> // Merge Value Profile data from Src record to this record for ValueKind.
>>>>> // Scale merged value counts by \p Weight.
>>>>> - instrprof_error mergeValueProfData(uint32_t ValueKind, InstrProfRecord &Src,
>>>>> - uint64_t Weight);
>>>>> + void mergeValueProfData(uint32_t ValueKind, InstrProfRecord &Src,
>>>>> + uint64_t Weight);
>>>>> // Scale up value profile data count.
>>>>> - instrprof_error scaleValueProfData(uint32_t ValueKind, uint64_t Weight);
>>>>> + void scaleValueProfData(uint32_t ValueKind, uint64_t Weight);
>>>>> };
>>>>>
>>>>> uint32_t InstrProfRecord::getNumValueKinds() const {
>>>>>
>>>>> Modified: llvm/trunk/lib/ProfileData/InstrProf.cpp
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/InstrProf.cpp?rev=269222&r1=269221&r2=269222&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/InstrProf.cpp?rev=269222&r1=269221&r2=269222&view=diff>
>>>>> ==============================================================================
>>>>> --- llvm/trunk/lib/ProfileData/InstrProf.cpp (original)
>>>>> +++ llvm/trunk/lib/ProfileData/InstrProf.cpp Wed May 11 14:42:19 2016
>>>>> @@ -80,6 +80,31 @@ const std::error_category &llvm::instrpr
>>>>>
>>>>> namespace llvm {
>>>>>
>>>>> +void SoftInstrProfErrors::addError(instrprof_error IE) {
>>>>> + if (IE == instrprof_error::success)
>>>>> + return;
>>>>> +
>>>>> + if (FirstError == instrprof_error::success)
>>>>> + FirstError = IE;
>>>>> +
>>>>> + switch (IE) {
>>>>> + case instrprof_error::hash_mismatch:
>>>>> + ++NumHashMismatches;
>>>>> + break;
>>>>> + case instrprof_error::count_mismatch:
>>>>> + ++NumCountMismatches;
>>>>> + break;
>>>>> + case instrprof_error::counter_overflow:
>>>>> + ++NumCounterOverflows;
>>>>> + break;
>>>>> + case instrprof_error::value_site_count_mismatch:
>>>>> + ++NumValueSiteCountMismatches;
>>>>> + break;
>>>>> + default:
>>>>> + llvm_unreachable("Not a soft error");
>>>>> + }
>>>>> +}
>>>>> +
>>>>> std::string getPGOFuncName(StringRef RawFuncName,
>>>>> GlobalValue::LinkageTypes Linkage,
>>>>> StringRef FileName,
>>>>> @@ -291,13 +316,13 @@ std::error_code readPGOFuncNameStrings(S
>>>>> return make_error_code(instrprof_error::success);
>>>>> }
>>>>>
>>>>> -instrprof_error InstrProfValueSiteRecord::merge(InstrProfValueSiteRecord &Input,
>>>>> - uint64_t Weight) {
>>>>> +void InstrProfValueSiteRecord::merge(SoftInstrProfErrors &SIPE,
>>>>> + InstrProfValueSiteRecord &Input,
>>>>> + uint64_t Weight) {
>>>>> 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)
>>>>> @@ -306,92 +331,80 @@ instrprof_error InstrProfValueSiteRecord
>>>>> bool Overflowed;
>>>>> I->Count = SaturatingMultiplyAdd(J->Count, Weight, I->Count, &Overflowed);
>>>>> if (Overflowed)
>>>>> - Result = instrprof_error::counter_overflow;
>>>>> + SIPE.addError(instrprof_error::counter_overflow);
>>>>> ++I;
>>>>> continue;
>>>>> }
>>>>> ValueData.insert(I, *J);
>>>>> }
>>>>> - return Result;
>>>>> }
>>>>>
>>>>> -instrprof_error InstrProfValueSiteRecord::scale(uint64_t Weight) {
>>>>> - instrprof_error Result = instrprof_error::success;
>>>>> +void InstrProfValueSiteRecord::scale(SoftInstrProfErrors &SIPE,
>>>>> + uint64_t Weight) {
>>>>> for (auto I = ValueData.begin(), IE = ValueData.end(); I != IE; ++I) {
>>>>> bool Overflowed;
>>>>> I->Count = SaturatingMultiply(I->Count, Weight, &Overflowed);
>>>>> if (Overflowed)
>>>>> - Result = instrprof_error::counter_overflow;
>>>>> + SIPE.addError(instrprof_error::counter_overflow);
>>>>> }
>>>>> - return Result;
>>>>> }
>>>>>
>>>>> // Merge Value Profile data from Src record to this record for ValueKind.
>>>>> // Scale merged value counts by \p Weight.
>>>>> -instrprof_error InstrProfRecord::mergeValueProfData(uint32_t ValueKind,
>>>>> - InstrProfRecord &Src,
>>>>> - uint64_t Weight) {
>>>>> +void InstrProfRecord::mergeValueProfData(uint32_t ValueKind,
>>>>> + InstrProfRecord &Src,
>>>>> + uint64_t Weight) {
>>>>> uint32_t ThisNumValueSites = getNumValueSites(ValueKind);
>>>>> uint32_t OtherNumValueSites = Src.getNumValueSites(ValueKind);
>>>>> - if (ThisNumValueSites != OtherNumValueSites)
>>>>> - return instrprof_error::value_site_count_mismatch;
>>>>> + if (ThisNumValueSites != OtherNumValueSites) {
>>>>> + SIPE.addError(instrprof_error::value_site_count_mismatch);
>>>>> + return;
>>>>> + }
>>>>> 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++)
>>>>> - MergeResult(Result, ThisSiteRecords[I].merge(OtherSiteRecords[I], Weight));
>>>>> - return Result;
>>>>> + ThisSiteRecords[I].merge(SIPE, OtherSiteRecords[I], Weight);
>>>>> }
>>>>>
>>>>> -instrprof_error InstrProfRecord::merge(InstrProfRecord &Other,
>>>>> - uint64_t Weight) {
>>>>> +void InstrProfRecord::merge(InstrProfRecord &Other, uint64_t Weight) {
>>>>> // If the number of counters doesn't match we either have bad data
>>>>> // or a hash collision.
>>>>> - if (Counts.size() != Other.Counts.size())
>>>>> - return instrprof_error::count_mismatch;
>>>>> -
>>>>> - instrprof_error Result = instrprof_error::success;
>>>>> + if (Counts.size() != Other.Counts.size()) {
>>>>> + SIPE.addError(instrprof_error::count_mismatch);
>>>>> + return;
>>>>> + }
>>>>>
>>>>> for (size_t I = 0, E = Other.Counts.size(); I < E; ++I) {
>>>>> bool Overflowed;
>>>>> Counts[I] =
>>>>> SaturatingMultiplyAdd(Other.Counts[I], Weight, Counts[I], &Overflowed);
>>>>> if (Overflowed)
>>>>> - Result = instrprof_error::counter_overflow;
>>>>> + SIPE.addError(instrprof_error::counter_overflow);
>>>>> }
>>>>>
>>>>> for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind)
>>>>> - MergeResult(Result, mergeValueProfData(Kind, Other, Weight));
>>>>> -
>>>>> - return Result;
>>>>> + mergeValueProfData(Kind, Other, Weight);
>>>>> }
>>>>>
>>>>> -instrprof_error InstrProfRecord::scaleValueProfData(uint32_t ValueKind,
>>>>> - uint64_t Weight) {
>>>>> +void InstrProfRecord::scaleValueProfData(uint32_t ValueKind, uint64_t Weight) {
>>>>> uint32_t ThisNumValueSites = getNumValueSites(ValueKind);
>>>>> std::vector<InstrProfValueSiteRecord> &ThisSiteRecords =
>>>>> getValueSitesForKind(ValueKind);
>>>>> - instrprof_error Result = instrprof_error::success;
>>>>> for (uint32_t I = 0; I < ThisNumValueSites; I++)
>>>>> - MergeResult(Result, ThisSiteRecords[I].scale(Weight));
>>>>> - return Result;
>>>>> + ThisSiteRecords[I].scale(SIPE, Weight);
>>>>> }
>>>>>
>>>>> -instrprof_error InstrProfRecord::scale(uint64_t Weight) {
>>>>> - instrprof_error Result = instrprof_error::success;
>>>>> +void InstrProfRecord::scale(uint64_t Weight) {
>>>>> for (auto &Count : this->Counts) {
>>>>> bool Overflowed;
>>>>> Count = SaturatingMultiply(Count, Weight, &Overflowed);
>>>>> - if (Overflowed && Result == instrprof_error::success) {
>>>>> - Result = instrprof_error::counter_overflow;
>>>>> - }
>>>>> + if (Overflowed)
>>>>> + SIPE.addError(instrprof_error::counter_overflow);
>>>>> }
>>>>> for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind)
>>>>> - MergeResult(Result, scaleValueProfData(Kind, Weight));
>>>>> -
>>>>> - return Result;
>>>>> + scaleValueProfData(Kind, Weight);
>>>>> }
>>>>>
>>>>> // 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=269222&r1=269221&r2=269222&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/InstrProfWriter.cpp?rev=269222&r1=269221&r2=269222&view=diff>
>>>>> ==============================================================================
>>>>> --- llvm/trunk/lib/ProfileData/InstrProfWriter.cpp (original)
>>>>> +++ llvm/trunk/lib/ProfileData/InstrProfWriter.cpp Wed May 11 14:42:19 2016
>>>>> @@ -166,22 +166,21 @@ std::error_code InstrProfWriter::addReco
>>>>> ProfileDataMap.insert(std::make_pair(I.Hash, InstrProfRecord()));
>>>>> InstrProfRecord &Dest = Where->second;
>>>>>
>>>>> - instrprof_error Result = instrprof_error::success;
>>>>> if (NewFunc) {
>>>>> // We've never seen a function with this name and hash, add it.
>>>>> Dest = std::move(I);
>>>>> // Fix up the name to avoid dangling reference.
>>>>> Dest.Name = FunctionData.find(Dest.Name)->getKey();
>>>>> if (Weight > 1)
>>>>> - Result = Dest.scale(Weight);
>>>>> + Dest.scale(Weight);
>>>>> } else {
>>>>> // We're updating a function we've seen before.
>>>>> - Result = Dest.merge(I, Weight);
>>>>> + Dest.merge(I, Weight);
>>>>> }
>>>>>
>>>>> Dest.sortValueData();
>>>>>
>>>>> - return Result;
>>>>> + return Dest.getError();
>>>>> }
>>>>>
>>>>> bool InstrProfWriter::shouldEncodeData(const ProfilingData &PD) {
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170629/102c1e4f/attachment.html>
More information about the llvm-commits
mailing list