[PATCH] D15547: [PGO] Handle and report overflow during profile merge for all types of data

Nathan Slingerland via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 16 11:06:06 PST 2015


David I think most of these will be cleaned up by your earlier suggestion
of a 'fused' multiply-add method.

I like that way better since it's all right there in the code.

On Wed, Dec 16, 2015 at 11:02 AM, Xinliang David Li via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

>
>
> On Wed, Dec 16, 2015 at 10:51 AM, Diego Novillo via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>>
>>
>> On Wed, Dec 16, 2015 at 1:45 PM, Nathan Slingerland <slingn at gmail.com>
>> wrote:
>>
>>>
>>> ================
>>> Comment at: include/llvm/ProfileData/SampleProf.h:145
>>> @@ -134,2 +144,3 @@
>>>        S = SaturatingMultiply(S, Weight, &Overflowed);
>>> -      assert(!Overflowed && "Sample counter overflowed!");
>>> +      if (Overflowed)
>>> +        return sampleprof_error::counter_overflow;
>>> ----------------
>>> davidxl wrote:
>>> > There are also lots of if(..) return patterns that can be simplified.
>>> This case you will need a macro:
>>> >
>>> > #define RETURN_IF(Cond, Err) \
>>> >     if(Cond) \
>>> >         return Err;
>>> I don't think we want to add macros to header files like this as macros
>>> don't respect namespaces. I'll see if I can simplify in another way.
>>>
>>
>> Agreed.  I don't really like macros of any kind in headers (well,
>> anywhere really).  Particularly the kind that obscure control flow.  I'd
>> rather see the control flow in the code.
>>
>
> Why do you think this obscure control flow? The name indicates it is a
> conditional return.
>
> David
>
>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>
> _______________________________________________
> 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/20151216/cfddcbb2/attachment.html>


More information about the llvm-commits mailing list