[PATCH] llvm-cov: Added -b option for branch probabilities.
Justin Bogner
mail at justinbogner.com
Thu Dec 12 14:12:18 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.
>>> +/// 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.
More information about the llvm-commits
mailing list