[PATCH] llvm-cov: Added -b option for branch probabilities.

[Yuchen Wu]

> Yuchen Wu <yuchenericwu at hotmail.com> writes:
>>>> +// It is unlikely--but possible--for multiple functions to be on
>>>> the same line.
>>>> +// But optimize for the common case.
>>>
>>> What is this comment talking about?
>>
>> The reason FunctionVector exists is because LineData needs to be able
>> to store all of the functions that are on a single line. The comment
>> is to explain that it's unlikely two functions will be on the same
>> line, so optimize for the common case by setting the SmallVector
>> typedef to contain a single element.
>
> I may have been subtly implying you should improve the comment :)

Okay done.

>>>> +typedef SmallVector<const GCOVFunction *, 1> FunctionVector;
>>>> +typedef DenseMap<uint32_t, FunctionVector> FunctionLines;
>>>> typedef SmallVector<const GCOVBlock *, 4> BlockVector;
>>>> -typedef DenseMap<uint32_t, BlockVector> LineData;
>>>> +typedef DenseMap<uint32_t, BlockVector> BlockLines;
>>
>>>> +// 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.


>> +/// 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. 		 	   		  
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-llvm-cov-Added-b-option-for-branch-probabilities.patch
Type: application/octet-stream
Size: 21867 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131212/8a9b9679/attachment.obj>