[PATCH] Fix PR 23525 - Separate header mass propagation in irregular loops.

[Xinliang David Li]

This illustrates the problem mentioned by Duncan : the frequency of
SeventyFive should be close to half of the entry (which is 0.5), but the
computed frequency is too low (on the other hand, without the fix, it is
too high). Again this only applies to short trip counted irreducible loops.

David

On Wed, Jun 10, 2015 at 9:33 AM, Diego Novillo <dnovillo at google.com> wrote:

>
>
> On Wed, Jun 10, 2015 at 12:28 PM, Xinliang David Li <xinliangli at gmail.com>
> wrote:
>
>>
>>> > Secondly, I wonder whether this will regress cases we care about if you
>>> > don't incorporate the entry frequency point.  E.g., your patch would
>>> > make the frequencies *way* off for the following contrived example:
>>> >
>>> >     assert(n > 0);
>>> >     int i = n - 2;
>>> >     if (n % 2 == 0)
>>> >       goto SeventyFive;
>>> >
>>> >     for (; i < n; ++i) {
>>> >       if (i % n == 75) {
>>> >     SeventyFive:
>>> >         foo(i);
>>> >       } else {
>>> >         bar(i);
>>> >       }
>>> >     }
>>> >
>>> > whereas (I think) the current in-tree code would get it roughly correct
>>> > (by fluke?).
>>>
>>> Actually, my patch addresses this problem in particular.  This code is
>>> almost exactly the test case in PR 23525. The in-tree code gets it wrong
>>> the same way it gets PR 23525 wrong. The call to foo(i) is almost never
>>> taken vs the call to bar(i).
>>>
>>> We get the following frequencies for this code snippet (I wrapped it
>>> into a function that I called 100 times).  The left column is current
>>> trunk.  The right column is trunk plus my patch.  I redacted the floating
>>> point figures to make them more readable:
>>>
>>>
>>
>> Do you have a complete runnable example above?
>>
>
> Sure.
>
> $ cat foo.c
> #include <assert.h>
> #include <stdio.h>
>
> long X = 0;
>
> void foo(int n) {
>   assert(n > 0);
>   int i = n - 2;
>   if (n % 2 == 0)
>     goto SeventyFive;
>
>   for (; i < n; ++i) {
>     if (i % n == 75) {
> SeventyFive:
>       X += i;
>     } else {
>       X *= i;
>     }
>   }
> }
>
> int main() {
>   int i;
>   for (i = 1; i < 100; i++) foo(i);
>   printf("X = %ld\n", X);
>   return 0;
> }
>
> $ bin/clang -O0 -fprofile-instr-generate -Wall foo.c -o foo
> $ ./foo
> X = 1502145969101944128
> $ bin/llvm-profdata merge -o foo.prof default.profraw
> $ bin/clang -O0 -fprofile-instr-use=foo.prof -emit-llvm -S foo.c
> $ bin/opt -analyze -block-freq foo.ll
> Printing analysis 'Block Frequency Analysis' for function 'foo':
> block-frequency-info: foo
>  - entry: float = 1.0, int = 8388608
>  - cond.true: float = 0.9999990463, int = 8388600
>  - cond.false: float = 0.0000009536743164, int = 8
>  - : float = 0.0, int = 0
>  - cond.end: float = 0.9999990463, int = 8388600
>  - if.then: float = 0.4950490328, int = 4152772
>  - if.end: float = 0.5049500135, int = 4235827
>  - for.cond: float = 2.884612633, int = 24197884
>  - for.body: float = 1.499998569, int = 12582899
>  - if.then.5: float = 0.02980129608, int = 249991
>  - SeventyFive: float = 0.03438611242, int = 288451
>  - if.else: float = 1.470197273, int = 12332908
>  - if.end.7: float = 4.580994102, int = 38428163
>  - for.inc: float = 2.499997616, int = 20971499
>  - for.end: float = 0.9999990463, int = 8388600
>
> Printing analysis 'Block Frequency Analysis' for function 'main':
> block-frequency-info: main
>  - entry: float = 1.0, int = 8
>  - for.cond: float = 51.0, int = 407
>  - for.body: float = 50.0, int = 399
>  - for.inc: float = 50.0, int = 399
>  - for.end: float = 1.0, int = 8
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150610/bcc42278/attachment.html>