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

[Diego Novillo via llvm-commits]

On Wed, Dec 16, 2015 at 2:02 PM, Xinliang David Li <xinliangli at gmail.com>
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.
>

Semantic overload.  These macros tend to grow other things and they are a
royal pain when stepping through with a debugger.  I spent a long time
trying to get rid of these macros in GCC.  I don't want another codebase
riddled with macros :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151216/73b9d32d/attachment.html>