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

[Vedant Kumar via Phabricator via cfe-commits]

vsk added subscribers: mzolotukhin, vsk.
vsk added a comment.

Thanks for working on this. Collecting better timing information in the frontend sgtm. It's cheap to do, and we can use the information to guide our efforts re: attacking the compile-time problem. Feel free to add me to future timing-related reviews. Regarding this specific patch:

Could you add a short test (perhaps in test/Misc?) that checks that -ftime-report prints out an entry for 'Preprocessing'?

It'd be nice to dump this timer from Preprocessor::PrintStats(), too.



================
Comment at: lib/Lex/Preprocessor.cpp:660
 
+  llvm::TimeRegion(PPOpts->ShowTimers ? &PreprocessingTimer : nullptr);
+
----------------
I wonder whether this is too fine-grained. I think setting up a timer in Preprocessor::Lex() might capture more information. Would you mind experimenting with that?


================
Comment at: lib/Lex/Preprocessor.cpp:660
 
+  llvm::TimeRegion(PPOpts->ShowTimers ? &PreprocessingTimer : nullptr);
+
----------------
vsk wrote:
> I wonder whether this is too fine-grained. I think setting up a timer in Preprocessor::Lex() might capture more information. Would you mind experimenting with that?
Nitpick: it may be useful to add PPOpts::getTimer(), in case we find more sites where we need to either get back the PP timer or nullptr.


https://reviews.llvm.org/D36492