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. > ================ > 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? ================ > 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
