[PATCH] D36492: [time-report] Add preprocessor timer

Brian Gesiak via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 14 07:42:36 PST 2018


modocache abandoned this revision.
modocache added inline comments.


================
Comment at: include/clang/Lex/PreprocessorOptions.h:165
 public:
-  PreprocessorOptions() : UsePredefines(true), DetailedRecord(false),
+  PreprocessorOptions() : PPTimer("preprocessor", "Preprocessing"),
+                          UsePredefines(true),
----------------
avt77 wrote:
> modocache wrote:
> > eduardo-elizondo wrote:
> > > Should this be named "Lexing Time" or "Lexing" instead of "Preprocessing"?
> > Good idea! Now that the timer's being started up in the `Preprocessor::Lex` method, it probably should be named "Lexing". Alternatively, I could move this into, say,` Lexer::Lex`. I guess there's not much of a distinction in Clang between "lexing" and "preprocessing."
> > 
> > I had originally picked this name because that's what appears in `gcc -ftime-report`, but I guess we don't need to keep the names the same.
> Preprocessing does not means lexing. It includes much more, for example, file inclusion, include pathes elaboration, macros expansion, etc. We could merge all these parts or we could output the info for evey single piece. In any case I think we need a group of timers related to frontend (or maybe several groups: for pp, parser, Sema, codegen...). And maybe we need special switches to disable/enable the groups (-ftime-report could be used as some super switch for all of them). As result we need some repository of the timers and most probably this repository should be kept in PreprocessorOpts.
> 
> If there are no objections I can take this job on me and come up with new more general version of the patch. If it's OK then I'd like to collect the proposes about the required details of the time profile: what exactly we'd like to see in output? We have gcc output as an initial example but maybe we need more (or less) details?
> If there are no objections I can take this job on me and come up with new more general version of the patch. If it's OK then I'd like to collect the proposes about the required details of the time profile: what exactly we'd like to see in output? We have gcc output as an initial example but maybe we need more (or less) details?

Yes, absolutely! This sounds fantastic, thank you for doing this work.

I'll abandon this diff so it doesn't clog up anyone else's review queues. Thanks again!


https://reviews.llvm.org/D36492





More information about the cfe-commits mailing list