[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:52:16 PDT 2017
On Thu, Jun 29, 2017 at 4:48 PM <vsk at apple.com> wrote:
> On Jun 29, 2017, at 4:38 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> 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();
> }
>
>
> 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?
("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>
>>>>>> 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/c6278c68/attachment-0001.html>
More information about the llvm-commits
mailing list