FYI, Fixed in r198416. Thanks, Teresa
On Thu, Apr 25, 2013 at 10:19 PM, Teresa Johnson <tejohn...@google.com> wrote: > Reproduced. This looks like another instance of a case I found testing > my follow-on patch: the helper routines have some assertion checking > that is too strict for the broader usage where we may be scaling > counts up and not just down. I am verifying and will send a patch in > the morning that suppresses this assert, which is the approach I am > taking in the follow-on patch also coming tomorrow. > > Teresa > > On Thu, Apr 25, 2013 at 3:29 PM, H.J. Lu <hjl.to...@gmail.com> wrote: >> On Fri, Apr 5, 2013 at 7:18 AM, Teresa Johnson <tejohn...@google.com> wrote: >>> On Thu, Mar 28, 2013 at 2:27 AM, Richard Biener >>> <richard.guent...@gmail.com> wrote: >>>> On Wed, Mar 27, 2013 at 6:22 PM, Teresa Johnson <tejohn...@google.com> >>>> wrote: >>>>> I found that the node weight updates on cloned nodes during ipa-cp were >>>>> leading to incorrect/insane weights. Both the original and new node weight >>>>> computations used truncating divides, leading to a loss of total node >>>>> weight. >>>>> I have fixed this by making both rounding integer divides. >>>>> >>>>> Bootstrapped and tested on x86-64-unknown-linux-gnu. Ok for trunk? >>>> >>>> I'm sure we can outline a rounding integer divide inline function on >>>> gcov_type. To gcov-io.h, I suppose. >>>> >>>> Otherwise this looks ok to me. >>> >>> Thanks. I went ahead and worked on outlining this functionality. In >>> the process of doing so, I discovered that there was already a method >>> in basic-block.h to do part of this: apply_probability(), which does >>> the rounding divide by REG_BR_PROB_BASE. There is a related function >>> combine_probabilities() that takes 2 int probabilities instead of a >>> gcov_type and an int probability. I decided to use apply_probability() >>> in ipa-cp, and add a new macro GCOV_COMPUTE_SCALE to basic-block.h to >>> compute the scale factor/probability via a rounding divide. So the >>> ipa-cp changes I made use both GCOV_COMPUTE_SCALE and >>> apply_probability. >>> >>> I then went through all the code to look for instances where we were >>> computing scale factors/probabilities and performing scaling. I found >>> a mix of existing uses of apply/combine_probabilities, uses of RDIV, >>> inlined rounding divides, and truncating divides. I think it would be >>> good to unify all of this. As a first step, I replaced all inline code >>> sequences that were already doing rounding divides to compute scale >>> factors/probabilities or do the scaling, to instead use the >>> appropriate helper function/macro described above. For these >>> locations, there should be no change to behavior. >>> >>> There are a number of places where there are truncating divides right >>> now. Since changing those may impact the resulting behavior, for this >>> patch I simply added a comment as to which helper they should use. As >>> soon as this patch goes in I am planning to change those to use the >>> appropriate helper and test performance, and then will send that patch >>> for review. So for this patch, the only place where behavior is >>> changed is in ipa-cp which was my original change. >>> >>> New patch is attached. Bootstrapped (both bootstrap and >>> profiledbootstrap) and tested on x86-64-unknown-linux-gnu. Ok for >>> trunk? >>> >> >> This caused: >> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57077 >> >> >> H.J. > > > > -- > Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413 -- Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413