Re: [PATCH]Partially fix PR61529, bound basic block frequency

[Renlin Li <renlin dot li at arm dot com>]

On 29/10/14 12:42, Teresa Johnson wrote:
> Hi Renlin,
>
> Are the incoming edge counts or probabilities insane in this case? I
> guess the patch is ok if we need to do this to handle those incoming
> insanitiles. But I can't approve patches myself.

Not really, it's just a little bigger than the limit.

For this particular test case, ABC is a threaded path.
B is the fallthrough basic block of A, D is a basic block split from B 
(used to be a self loop). A, B and D have roughly the same frequency ( 
8281, 9100, 8281).
When calculating the path_in_freq, frequencies from AB and DB edges are 
accumulated, and the final result is large than BB_FREQ_MAX.


          A
100% |
          |      9%
------>B---------->C
|         |
|100%| 91%
|         |
--------D



There are 2 suspicious points:
1, The BD edge is not correctly guessed at the profile stage. However, 
anyway it's heuristic, so I don't think, it's here the problem starts.
2, The BD edge is not eliminated before jump threading. But the jump 
threading pass will analysis the condition jump statement in B block (In 
this particular case, the BD probability should be zero), and makes the 
decision to thread it.

Later in the dom pass, the BD edge is indeed removed.

> However, this is a fix to code (r215739) committed after the ICE in
> the original bug report and in comment 2 were reported, so I wonder if
> it is just hiding the original problem. Originally this was reported
> to be due to r210538 - ccing Dehao who was the author of that patch.
> Dehao, did you get a chance to look at this bug and see why your
> change triggered it? It is possible that Dehao's patch simply
> amplified an even further upstream profile insanity, but it would be
> good to confirm.
>
> Thanks!
> Teresa
>
> On Wed, Oct 29, 2014 at 2:26 AM, Renlin Li<renlin.li@arm.com>  wrote:
> > Hi all,
> >
> > This is a simple patch to fix ICE in comment 2 of PR61529:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61529
> >
> > Bound checking code is added to make sure the frequency is within legal
> > range.
> >
> > As far as I have observed, r215830 patch fixes the glibc building ICE. And
> > this patch should fix the ICE while building the sample code in comment 2
> > using aarch64-none-elf toolchain. Until now, all the ICEs reported in this
> > bug ticket should be fixed.
> >
> > x86_64-unknown-linux-gnu bootstrap and regression test have been done, no
> > new issue.
> > aarch64-none-elf toolchain has been test on the model. No new regression.
> >
> > Is this Okay for trunk?
> >
> > gcc/ChangeLog:
> >
> > 2014-10-29  Renlin Li<Renlin.Li@arm.com>
> >       PR middle-end/61529
> >      * tree-ssa-threadupdate.c (compute_path_counts): Bound path_in_freq.