[PATCH] D61914: [Support][Test] Time profiler: add regression test

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev marked 4 inline comments as done.
anton-afanasyev added a comment.

In D61914#1512021 <https://reviews.llvm.org/D61914#1512021>, @aganea wrote:

> Could you please move the test to a more approriate location? (ie. clang/trunk/test/Driver/)


Thanks, I've moved it there.



================
Comment at: clang/tools/driver/cc1_main.cpp:245
+
+    llvm::errs() << "Time trace json-file dumped to " << Path.str() << "\n";
+    llvm::errs()
----------------
aganea wrote:
> This seems a bit too chatty. Suround these two lines with `if (Config->Verbose)` ?
I don't think it should be done this way for several reasons:
1. This info is actually needed by user when one uses `--ftime-trace` since json-file is usually dumped to random filename located in `/tmp/`. Therefore `--ftime-trace` means `--ftime-trace --verbose` itself. Without this info user cannot understand where dumped file is. How can he know about `--verbose`?
2. Option `--ftime-report` is much more chatty, it dumps several tables to terminal. This option just dumps to file, but tells where file is.
3. I have not found elegant way to check `--verbose` option here, one should add it to `clang::FrontendOptions` or so.


================
Comment at: llvm/test/Support/check-time-trace.cxx:4
+
+// CHECK: "args":{"name":"clang"}
+
----------------
aganea wrote:
> I don't see any other `-ftime-trace` tests, I would add a few more exhaustive file format checks here.
Ok, I've added more checks, parsing json file by python.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61914/new/

https://reviews.llvm.org/D61914