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. jeff
