[llvm] r269222 - [ProfileData] Use SoftInstrProfErrors to count soft errors, NFC
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 29 16:38:32 PDT 2017
On Thu, Jun 29, 2017 at 4:27 PM <vsk at apple.com> wrote:
> On Jun 29, 2017, at 4:11 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Thu, Jun 29, 2017 at 4:09 PM <vsk at apple.com> wrote:
>
>> On Jun 29, 2017, at 2:41 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>
>>
>> On Thu, Jun 29, 2017 at 2:24 PM Xinliang David Li <davidxl at google.com>
>> wrote:
>>
>>> On Thu, Jun 29, 2017 at 2:16 PM, David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Thu, Jun 29, 2017 at 10:03 AM <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();
}
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?
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>
>>>>> 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>
>>>>> 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> 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
>>>>>>> Log:
>>>>>>> [ProfileData] Use SoftInstrProfErrors to count soft errors, NFC
>>>>>>>
>>>>>>> Differential Revision: 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
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- 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
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- 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
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- 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
>>>>>>> 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/760541b7/attachment.html>
More information about the llvm-commits
mailing list