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
