[PATCH] llvm-cov: Added -b option for branch probabilities.
Yuchen Wu
yuchenericwu at hotmail.com
Thu Dec 12 14:32:16 PST 2013
> Yuchen Wu <yuchenericwu at hotmail.com> writes:
>>>>>> +// Safe integer division, returns 0 if divisor is 0.
>>>>>> +static uint32_t safeDiv(uint64_t Numerator, uint64_t Divisor) {
>>>>>> + if (Divisor == 0)
>>>>>> + return 0;
>>>>>> + return Numerator/Divisor;
>>>>>> +}
>>>>>
>>>>> Why do you need this?
>>>>>
>>>>>> +
>>>>>> +// This custom division function mimics gcov's branch ouputs:
>>>>>> +// - Round to closest whole number
>>>>>> +// - Only output 0% or 100% if it's exactly that value
>>>>>> +static uint32_t branchDiv(uint64_t Numerator, uint64_t Divisor) {
>>>>>> + uint8_t Res = safeDiv(Numerator*100+Divisor/2, Divisor);
>>>>>> + if (Res == 0 && Numerator)
>>>>>> + return 1;
>>>>>
>>>>> This seems like pretty strange behaviour if Divisor == 0.
>>>>
>>>> The safeDiv and branchDiv functions need to return 0 if it's asked to
>>>> divide 0 by 0, which actually happens quite often.
>>>
>>> Firstl, branchDiv returns 1%, not zero - that seems arbitrary and
>>> insane. Second, you still haven't answered why this happens / is
>>> accepted. This feels like it's papering over a problem somewhere else to
>>> me.
>>
>> branchDiv will only return 1% when it's asked to do something like
>> 1/100,000. This is to differentiate between a branch that is never
>> executed and one that is rarely executed. In the case where it's asked
>> to divide 0 by 0, it will do the right thing and output 0%. This is to
>> match gcov's arithmetic.
>>
>
> Okay, that's clearer if you just say "if (!Numerator) return 0;" before
> you do any math, isn't it? 10000/0 returns 1, though, which is pretty
> strange.
Fair enough, I've changed safeDiv and branchDiv to still return NaN if that happens.
>>>> +/// printFunctionSummary - Print function and block summary.
>>>> +void FileInfo::printFunctionSummary(raw_fd_ostream &OS, const
>>>> LineData &Line,
>>>> + uint32_t LineIndex) const {
>>>> + FunctionLines::const_iterator FuncsIt = Line.Functions.find(LineIndex);
>>>> + if (FuncsIt != Line.Functions.end()) {
>>>
>>> Better to return early here and save a level of indentation.
>>
>> Okay, I moved the check to the caller of the function instead.
>
> I meant to reverse the condition and return immediately.
Yeah but I think it's simpler to check outside printFunctionSummary anyway. Reduces the number of arguments by 1.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-llvm-cov-Added-b-option-for-branch-probabilities.patch
Type: application/octet-stream
Size: 21914 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131212/19834a2b/attachment.obj>
More information about the llvm-commits
mailing list