On 6/7/19 3:58 PM, Jan Hubicka wrote:
>>
>> Because I removed hist->hvalue.counters[2] where we stored total number of
>> executions.
>> It's back again so that _atomic suffix version makes sense again.
>
> OK and we do not care about race conditions on the other two counters?
We do care, but one can't probably implement that without a locking of the
critical section:
117 static inline void
118 __gcov_one_value_profiler_body (gcov_type *counters, gcov_type value,
119 int use_atomic)
120 {
121 if (value == counters[1])
122 counters[2]++;
123 else if (counters[2] == 0)
124 {
125 counters[2] = 1;
126 counters[1] = value;
127 }
128 else
129 counters[2]--;
Here you operate with 2 values which can change during a parallel execution.
>>
>> After we discussed that I decided to live with all counters in memory.
>> Reason is that
>> in write_one_data we would have to allocate temporary buffers and fill up
>> them here:
>>
>> 304 tag = gcov_read_unsigned ();
>> 305 length = gcov_read_unsigned ();
>> 306 if (tag != GCOV_TAG_FOR_COUNTER (t_ix)
>> 307 || length != GCOV_TAG_COUNTER_LENGTH (ci_ptr->num))
>> 308 goto read_mismatch;
>> 309 (*merge) (ci_ptr->values, ci_ptr->num);
>> 310 ci_ptr++;
>>
>> It would be quite nasty and I would like to mitigate libgcov profile crashes.
>
> Hmm, I see, if code was organized to directly write the output, it would
> be easier. I guess it is not overly important.
>
> Patch is OK
Thanks for the approval.
Martin
> Honza
>
