Re: [PATCH] D22593: [Profile] document %h and %m

2016-07-20 Thread Xinliang David Li via cfe-commits
ok David On Wed, Jul 20, 2016 at 4:32 PM, Sean Silva wrote: > silvas accepted this revision. > silvas added a comment. > This revision is now accepted and ready to land. > > LGTM (also, another small suggestion). > > > > Comment at: docs/UsersManual.rst:1502 > @@ +1501,3 @@ > +

Re: [PATCH] D22593: [Profile] document %h and %m

2016-07-20 Thread David Li via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL276207: [Profile] Document new profile file name modifiers (authored by davidxl). Changed prior to commit: https://reviews.llvm.org/D22593?vs=64793&id=64794#toc Repository: rL LLVM https://reviews.l

Re: [PATCH] D22593: [Profile] document %h and %m

2016-07-20 Thread Sean Silva via cfe-commits
silvas accepted this revision. silvas added a comment. This revision is now accepted and ready to land. LGTM (also, another small suggestion). Comment at: docs/UsersManual.rst:1502 @@ +1501,3 @@ + multiple raw profiles dumped from different processes (running on the same or +

Re: [PATCH] D22593: [Profile] document %h and %m

2016-07-20 Thread David Li via cfe-commits
davidxl updated this revision to Diff 64793. davidxl added a comment. Addressed review comments. I still think %4m etc is an advanced feature that needs more explanation. We can delay that to a later time. https://reviews.llvm.org/D22593 Files: docs/UsersManual.rst Index: docs/UsersManual.

Re: [PATCH] D22593: [Profile] document %h and %m

2016-07-20 Thread David Li via cfe-commits
davidxl added inline comments. Comment at: docs/UsersManual.rst:1500 @@ +1499,3 @@ + name. When this specifier is used, the profiler runtime will substitute ``%m`` + with a unique integer identifier associated with the instrumented binary. Multiple + profiles dumped from

Re: [PATCH] D22593: [Profile] document %h and %m

2016-07-20 Thread Vedant Kumar via cfe-commits
vsk added a comment. In https://reviews.llvm.org/D22593#490490, @silvas wrote: > LGTM with some small wording nits. > > We may want to extend this to mention number modifier to `%m` (e.g. `%4m`). > Perhaps it is better to leave that to more advanced documentation -- your > experiments showed th

Re: [PATCH] D22593: [Profile] document %h and %m

2016-07-20 Thread Sean Silva via cfe-commits
silvas added a comment. LGTM with some small wording nits. We may want to extend this to mention number modifier to `%m` (e.g. `%4m`). Perhaps it is better to leave that to more advanced documentation -- your experiments showed that even just 1 merge pool is quite scalable IIRC. =

Re: [PATCH] D22593: [Profile] document %h and %m

2016-07-20 Thread David Li via cfe-commits
davidxl updated this revision to Diff 64759. davidxl added a comment. Fixed a typo https://reviews.llvm.org/D22593 Files: docs/UsersManual.rst Index: docs/UsersManual.rst === --- docs/UsersManual.rst +++ docs/UsersManual.rst @@

[PATCH] D22593: [Profile] document %h and %m

2016-07-20 Thread David Li via cfe-commits
davidxl created this revision. davidxl added reviewers: vsk, silvas. davidxl added a subscriber: cfe-commits. Added documentation for %h and %m specifiers. %m specifier which specifies the number of copies is not documented yet (treated as internal for now) https://reviews.llvm.org/D22593 File