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

[Xinliang David Li via llvm-commits]

On Wed, Dec 16, 2015 at 11:05 AM, Diego Novillo <dnovillo at google.com> wrote:
> 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 :)

You can not compare moderate use of macros here with GCC code base
which is full of macros -- always all IR level APIs are macro based.
There are trade-offs of course -- but I don't want to get into
philosophical debate here.

The change looks good to me if Diego is ok with it.

David