[PATCH] D15547: [PGO] Handle and report overflow during profile merge for all types of data
David Li via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 16 10:49:22 PST 2015
davidxl added inline comments.
================
Comment at: include/llvm/ProfileData/InstrProf.h:187
@@ -186,1 +186,3 @@
+inline instrprof_error AccumulateResult(instrprof_error &Accumulator,
+ instrprof_error Result) {
----------------
slingn wrote:
> davidxl wrote:
> > Nit: how about UpdateResult -- it seems a better name.
> How about 'MergeResult'?
Sounds fine.
================
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;
----------------
slingn wrote:
> 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.
you can undef it somewhere. Note that 'assert' before the change is a macro too :)
http://reviews.llvm.org/D15547
More information about the llvm-commits
mailing list