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

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 16 11:02:25 PST 2015


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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151216/4bc4854e/attachment.html>


More information about the llvm-commits mailing list