On Tue, Apr 22, 2014 at 2:55 PM, Diego Novillo <[email protected]> wrote:
> > > > On Mon, Apr 21, 2014 at 5:41 PM, Richard Smith <[email protected]>wrote: > >> >> Mostly copy-editing, but some questions too. >> >> >> ================ >> Comment at: docs/UsersManual.rst:1073 >> @@ +1072,3 @@ >> +hardware counters, while your application executes. They are typically >> +very efficient and do not incur in a large runtime overhead. The >> +sample data collected by the profiler can be used during compilation >> ---------------- >> Should this be "do not incur a large runtime overhead" ? >> > > Done. > > >> >> ================ >> Comment at: docs/UsersManual.rst:1075 >> @@ +1074,3 @@ >> +sample data collected by the profiler can be used during compilation >> +to determine what are the most executed areas of the code. >> + >> ---------------- >> "to determine what the most executed ares of the code are." >> >> > Done. > > >> ================ >> Comment at: docs/UsersManual.rst:1078 >> @@ +1077,3 @@ >> +In particular, sample profilers can provide execution counts for all >> +instructions in the code, information on branches taken and function >> +invocation. The compiler can use this information in its optimization >> ---------------- >> "and" instead of ",". >> >> > Done. > > >> ================ >> Comment at: docs/UsersManual.rst:1083 >> @@ +1082,3 @@ >> +basic blocks. Knowing that a function ``foo`` is called more >> +frequently than another ``bar`` helps the inliner. >> + >> ---------------- >> "another function ``bar``" >> >> > Done. > > ================ >> Comment at: docs/UsersManual.rst:1092 >> @@ +1091,3 @@ >> + usual build flags that you always build your application with. The >> only >> + requirement is that you add ``-gline-tables-ony`` or ``-g`` to the >> + command line. This is important for the profiler to be able to map >> ---------------- >> "``-gline-tables-only``" >> >> > Done. > > >> ================ >> Comment at: docs/UsersManual.rst:1131-1133 >> @@ +1130,5 @@ >> + >> +4. Build the code again using the collected profile. This step feeds >> + the profile back to the optimizers. This should result in a binary >> + that executes faster than the original one. >> + >> ---------------- >> 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. >> >> > Done. > > > >> ================ >> Comment at: docs/UsersManual.rst:1165 >> @@ +1164,3 @@ >> +at the prologue of the function (second number). This head sample >> +count provides an indicator of how frequent is the function invoked. >> + >> ---------------- >> "how frequently the function is invoked." >> >> > Done. > > >> ================ >> Comment at: docs/UsersManual.rst:1164 >> @@ +1163,3 @@ >> +function (first number), and the total number of samples accumulated >> +at the prologue of the function (second number). This head sample >> +count provides an indicator of how frequent is the function invoked. >> ---------------- >> Should "at the prologue" be "in the prologue"? >> >> > Done. > > >> ================ >> Comment at: docs/UsersManual.rst:1176-1178 >> @@ +1175,5 @@ >> + >> +b. [OPTIONAL] Discriminator. This is used if the sampled program >> + was compiled with DWARF discriminator support >> + (http://wiki.dwarfstd.org/index.php?title=Path_Discriminators) >> + >> ---------------- >> ... 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." >> >> > Done. > 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. ================ >> Comment at: docs/UsersManual.rst:1173-1174 >> @@ +1172,4 @@ >> + always relative to the line where symbol of the function is >> + defined. So, if the function has its header at line 280, the offset >> + 13 is at line 293 in the file. >> + >> ---------------- >> What happens if the line is in a different file (eg through macro >> expansion or inlining)? >> >> > It will break down then. Though I don't think I have much sympathy for > something like: > > file1.h: > int foo() { > i1; > i2; > ------------ > file2.h: > i3; > i4; > } > > Or are you thinking something else? > More mundane cases than that =) For instance: file.def: FOO(a) FOO(b) FOO(c) #undef FOO file.cc: void f() { #define FOO(x) x = 0; #include "file.def" } or simply: file.cc: void f() { assert(some_expensive_call()); } ================ >> Comment at: docs/UsersManual.rst:1180-1181 >> @@ +1179,4 @@ >> + >> +c. Number of samples. This is the number of samples collected by >> + the profiler at this source location. >> + >> ---------------- >> Decimal integer? Floating point? Something else? Is scientific notation >> OK? Is hexadecimal OK? >> >> > Done. > > >> ================ >> Comment at: docs/UsersManual.rst:1149-1150 >> @@ +1148,4 @@ >> +which correspond to each of the functions executed at runtime. Each >> +section has the following format (taken from >> +https://github.com/google/autofdo/blob/master/profile_writer.h): >> + >> ---------------- >> 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? >> >> > Done. > > > >> ================ >> Comment at: docs/UsersManual.rst:1193-1194 >> @@ +1192,4 @@ >> + The above means that at relative line offset 130 there is a call >> + instruction that calls one of ``foo()``, ``bar()`` and ``baz()``. >> + With ``baz()`` being the relatively more frequent call target. >> + >> ---------------- >> ". With" -> ", with" >> "frequent call" -> "frequently called" >> >> > Done. > > > Thanks for the feedback! >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
