<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Apr 22, 2014 at 2:55 PM, 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><br><div class="gmail_quote"><div class="">On Mon, Apr 21, 2014 at 5:41 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</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"><br>
  Mostly copy-editing, but some questions too.<br>
<br>
<br>
================<br>
Comment at: docs/UsersManual.rst:1073<br>
@@ +1072,3 @@<br>
+hardware counters, while your application executes. They are typically<br>
+very efficient and do not incur in a large runtime overhead. The<br>
+sample data collected by the profiler can be used during compilation<br>
----------------<br>
Should this be "do not incur a large runtime overhead" ?<br></blockquote><div><br></div></div><div>Done.</div><div class=""><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">


<br>
================<br>
Comment at: docs/UsersManual.rst:1075<br>
@@ +1074,3 @@<br>
+sample data collected by the profiler can be used during compilation<br>
+to determine what are the most executed areas of the code.<br>
+<br>
----------------<br>
"to determine what the most executed ares of the code are."<br>
<br></blockquote><div><br></div></div><div>Done.</div><div class=""><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">


================<br>
Comment at: docs/UsersManual.rst:1078<br>
@@ +1077,3 @@<br>
+In particular, sample profilers can provide execution counts for all<br>
+instructions in the code, information on branches taken and function<br>
+invocation. The compiler can use this information in its optimization<br>
----------------<br>
"and" instead of ",".<br>
<br></blockquote><div><br></div></div><div>Done.</div><div class=""><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">


================<br>
Comment at: docs/UsersManual.rst:1083<br>
@@ +1082,3 @@<br>
+basic blocks. Knowing that a function ``foo`` is called more<br>
+frequently than another ``bar`` helps the inliner.<br>
+<br>
----------------<br>
"another function ``bar``"<br>
<div><br></div></blockquote><div> </div></div><div>Done.</div><div class=""><div><br></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">


================<br>
Comment at: docs/UsersManual.rst:1092<br>
@@ +1091,3 @@<br>
+   usual build flags that you always build your application with. The only<br>
+   requirement is that you add ``-gline-tables-ony`` or ``-g`` to the<br>
+   command line. This is important for the profiler to be able to map<br>
----------------<br>
"``-gline-tables-only``"<br>
<br></blockquote><div><br></div></div><div>Done.</div><div class=""><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">


================<br>
Comment at: docs/UsersManual.rst:1131-1133<br>
@@ +1130,5 @@<br>
+<br>
+4. Build the code again using the collected profile. This step feeds<br>
+   the profile back to the optimizers. This should result in a binary<br>
+   that executes faster than the original one.<br>
+<br>
----------------<br>
Are you required to use the exact same arguments that you used before, other than the `-fprofile-sample-use=` argument? Either way, we should say so here.<br>
<br></blockquote><div><br></div></div><div>Done.</div><div class=""><div><br></div><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">


================<br>
Comment at: docs/UsersManual.rst:1165<br>
@@ +1164,3 @@<br>
+at the prologue of the function (second number). This head sample<br>
+count provides an indicator of how frequent is the function invoked.<br>
+<br>
----------------<br>
"how frequently the function is invoked."<br>
<br></blockquote><div><br></div></div><div>Done.</div><div class=""><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">


================<br>
Comment at: docs/UsersManual.rst:1164<br>
@@ +1163,3 @@<br>
+function (first number), and the total number of samples accumulated<br>
+at the prologue of the function (second number). This head sample<br>
+count provides an indicator of how frequent is the function invoked.<br>
----------------<br>
Should "at the prologue" be "in the prologue"?<br>
<br></blockquote><div><br></div></div><div>Done.</div><div class=""><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">


================<br>
Comment at: docs/UsersManual.rst:1176-1178<br>
@@ +1175,5 @@<br>
+<br>
+b. [OPTIONAL] Discriminator. This is used if the sampled program<br>
+   was compiled with DWARF discriminator support<br>
+   (<a href="http://wiki.dwarfstd.org/index.php?title=Path_Discriminators" target="_blank">http://wiki.dwarfstd.org/index.php?title=Path_Discriminators</a>)<br>
+<br>
----------------<br>
... and what is it, in that case? At a guess: "If present, this is a decimal integer representing the value of the DWARF discriminator register."<br>
<br></blockquote><div><br></div></div><div>Done.</div></div></div></div></blockquote><div><br></div><div>Thanks, this helps -- it explains why we'd want this. But it still doesn't answer my question of what value is specified here, and how it's represented.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div class=""><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">


================<br>
Comment at: docs/UsersManual.rst:1173-1174<br>
@@ +1172,4 @@<br>
+   always relative to the line where symbol of the function is<br>
+   defined. So, if the function has its header at line 280, the offset<br>
+   13 is at line 293 in the file.<br>
+<br>
----------------<br>
What happens if the line is in a different file (eg through macro expansion or inlining)?<br>
<br></blockquote><div><br></div></div><div>It will break down then. Though I don't think I have much sympathy for something like:</div><div><br></div><div>file1.h:</div><div>   int foo() {<br></div><div>     i1;</div>
<div>
     i2;</div><div>------------ </div><div>file2.h:</div><div>    i3;</div><div>    i4;</div><div>}</div><div><br></div><div>Or are you thinking something else?</div></div></div></div></blockquote><div><br></div><div>More mundane cases than that =) For instance:</div>
<div><br></div><div>file.def:</div><div>  FOO(a)</div><div>  FOO(b)</div><div>  FOO(c)</div><div>  #undef FOO</div><div><br></div><div>file.cc:</div><div>  void f() {</div><div>    #define FOO(x) x = 0;</div><div>    #include "file.def"</div>
<div>  }</div><div><br></div><div>or simply:</div><div><br></div><div>file.cc:</div><div>  void f() {</div><div>    assert(some_expensive_call());</div><div>  }</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div class=""><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">


================<br>
Comment at: docs/UsersManual.rst:1180-1181<br>
@@ +1179,4 @@<br>
+<br>
+c. Number of samples. This is the number of samples collected by<br>
+   the profiler at this source location.<br>
+<br>
----------------<br>
Decimal integer? Floating point? Something else? Is scientific notation OK? Is hexadecimal OK?<br>
<br></blockquote><div><br></div></div><div>Done.</div><div class=""><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">


================<br>
Comment at: docs/UsersManual.rst:1149-1150<br>
@@ +1148,4 @@<br>
+which correspond to each of the functions executed at runtime. Each<br>
+section has the following format (taken from<br>
+<a href="https://github.com/google/autofdo/blob/master/profile_writer.h" target="_blank">https://github.com/google/autofdo/blob/master/profile_writer.h</a>):<br>
+<br>
----------------<br>
Is the whitespace required to be a single space, or can it be any sequence of horizontal whitespace? Is any other whitespace allowed than that listed here?<br>
<br></blockquote><div><br></div></div><div>Done.</div><div class=""><div><br></div><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">


================<br>
Comment at: docs/UsersManual.rst:1193-1194<br>
@@ +1192,4 @@<br>
+   The above means that at relative line offset 130 there is a call<br>
+   instruction that calls one of ``foo()``, ``bar()`` and ``baz()``.<br>
+   With ``baz()`` being the relatively more frequent call target.<br>
+<br>
----------------<br>
". With" -> ", with"<br>
"frequent call" -> "frequently called"<br>
<br></blockquote><div><br></div></div><div>Done.</div><div><br></div><div><br></div><div>Thanks for the feedback!</div></div></div></div>
</blockquote></div><br></div></div>