<div dir="ltr">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.<div><br></div><div>David</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jun 10, 2015 at 9:33 AM, Diego Novillo <span dir="ltr"><<a href="mailto:dnovillo@google.com" target="_blank">dnovillo@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Wed, Jun 10, 2015 at 12:28 PM, Xinliang David Li <span dir="ltr"><<a href="mailto:xinliangli@gmail.com" target="_blank">xinliangli@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><span><br>> Secondly, I wonder whether this will regress cases we care about if you<br>> don't incorporate the entry frequency point.  E.g., your patch would<br>> make the frequencies *way* off for the following contrived example:<br>><br>>     assert(n > 0);<br>>     int i = n - 2;<br>>     if (n % 2 == 0)<br>>       goto SeventyFive;<br>><br>>     for (; i < n; ++i) {<br>>       if (i % n == 75) {<br>>     SeventyFive:<br>>         foo(i);<br>>       } else {<br>>         bar(i);<br>>       }<br>>     }<br>><br>> whereas (I think) the current in-tree code would get it roughly correct<br>> (by fluke?).<br><br></span>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).<br><br>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:<br><br></div></blockquote><div><br></div><div><br></div></div></div><div>Do you have a complete runnable example above?</div></div></div></div></blockquote><div><br></div></span><div>Sure.</div><div><br></div><div>$ cat foo.c</div><div><div>#include <assert.h></div><div>#include <stdio.h></div><div><br></div><div>long X = 0;</div><div><br></div><div>void foo(int n) {</div><span class=""><div>  assert(n > 0);</div><div>  int i = n - 2;</div><div>  if (n % 2 == 0)</div><div>    goto SeventyFive;</div><div><br></div><div>  for (; i < n; ++i) {</div><div>    if (i % n == 75) {</div><div>SeventyFive:</div></span><div>      X += i;</div><div>    } else {</div><div>      X *= i;</div><div>    }</div><div>  }</div><div>}</div><div><br></div><div>int main() {</div><div>  int i;</div><div>  for (i = 1; i < 100; i++) foo(i);</div><div>  printf("X = %ld\n", X);</div><div>  return 0;</div><div>}</div></div><div><br></div><div>$ bin/clang -O0 -fprofile-instr-generate -Wall foo.c -o foo</div><div><div>$ ./foo</div><div>X = 1502145969101944128</div><div>$ bin/llvm-profdata merge -o foo.prof default.profraw<br></div><div>$ bin/clang -O0 -fprofile-instr-use=foo.prof -emit-llvm -S foo.c</div></div><div><div>$ bin/opt -analyze -block-freq foo.ll</div><div>Printing analysis 'Block Frequency Analysis' for function 'foo':</div><span class=""><div>block-frequency-info: foo</div><div> - entry: float = 1.0, int = 8388608</div></span><div> - cond.true: float = 0.9999990463, int = 8388600</div><div> - cond.false: float = 0.0000009536743164, int = 8</div><span class=""><div> - : float = 0.0, int = 0</div></span><div> - cond.end: float = 0.9999990463, int = 8388600</div><div> - if.then: float = 0.4950490328, int = 4152772</div><div> - if.end: float = 0.<a href="tel:5049500135" value="+15049500135" target="_blank">5049500135</a>, int = 4235827</div><div> - for.cond: float = 2.884612633, int = 24197884</div><div> - for.body: float = 1.499998569, int = 12582899</div><div> - if.then.5: float = 0.02980129608, int = 249991</div><div> - SeventyFive: float = 0.03438611242, int = 288451</div><div> - if.else: float = 1.470197273, int = 12332908</div><div> - if.end.7: float = 4.580994102, int = 38428163</div><div> - for.inc: float = 2.499997616, int = 20971499</div><div> - for.end: float = 0.9999990463, int = 8388600</div><div><br></div><div>Printing analysis 'Block Frequency Analysis' for function 'main':</div><div>block-frequency-info: main</div><span class=""><div> - entry: float = 1.0, int = 8</div></span><div> - for.cond: float = 51.0, int = 407</div><div> - for.body: float = 50.0, int = 399</div><div> - for.inc: float = 50.0, int = 399</div><div> - for.end: float = 1.0, int = 8</div></div></div></div></div>
</blockquote></div><br></div>