On Wed, Oct 31, 2018 at 10:36 PM Jeff Law <l...@redhat.com> wrote: > > On 10/31/18 12:30 AM, bin.cheng wrote: > > Hi, > > In new profile probability/count infra, we have different precision quality > > categories, > > and probabilities/counts of different categories are not supposed to be > > compared or > > calculated. Though in general is an improvement, it introduces unexpected > > behavior. > > Specifically, class profile_probablity and profile_count themselves are > > implemented > > by comparing probabilities/counts against profile_count::zero(). while > > zero() is of > > profile_precision category, it's always compared different to zero of other > > precision > > categories including afdo. > > > > I can see two ways fixing this: 1) Treat zero as a common probability/count > > regardless > > of its category; 2) Provide an "is_zero" method rather than relying on "==" > > comparison > > against probability_count::zero(). 2) requires lots of code changes so I > > went with 1) > > in this patch set. This patch doesn't handle "always" but it might be. > > > > This patch also corrects a minor issue where we try to invert an > > uninitialized value. > > > > Bootstrap and test on x86_64 in patch set. Is it OK? > > > > Thanks, > > bin > > > > 2018-10-31 Bin Cheng <bin.ch...@linux.alibaba.com> > > > > * expmed.c (emit_store_flag_force): Use profile_probability::always. > > * profile-count.h (profile_probability::always): Add parameter. > > (profile_probability::operator==, profile_count::operator==): Treat > > ZERO as common probability/count regardless of its quality. > > (profile_probability::invert): Don't invert uninitialized probability. > > > > I'm really not sure the emit_store_flag_force change is right -- > essentially without external information I can't see how we can pass in > any probability here other than "we don't know" which is > profile_probability::uninitialized IIUC. > > You could potentially make an argument that a 50-50 probability is > reasonable here. That's profile_probability::even. But I just don't > see how profile_probability::always is right here. Thanks, I realized I mis-spotted that piece of code. Will discard the part.
Thanks, bin > > jeff
