dongjunduo added inline comments.

================
Comment at: clang/lib/Driver/Driver.cpp:4702
+
+  // Add or replace -ftime-trace<path>` to the correct one to all clang jobs
+  for (auto &J : C.getJobs()) {
----------------
Whitney wrote:
> Do you mean Add or replace the modified `-ftime-trace=<path>` to all clang 
> jobs?
Right


================
Comment at: clang/lib/Driver/Driver.cpp:4739
+
+      // replace `-ftime-trace=<path>`
+      auto &JArgs = J.getArguments();
----------------
Whitney wrote:
> should we also replace `-ftime-trace`?
The work before here is to infer the correct path to store the time-trace file.

After that, the <path> in `-ftime-trace=<path>` should be replaced by the 
infered correct path.

We do not need to replace `-ftime-trace` then.


================
Comment at: clang/test/Driver/check-time-trace.cpp:4
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o 
%T/exe/check-time-trace %s
+// RUN: cat %T/exe/check-time-trace*.json \
+// RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
----------------
Whitney wrote:
> what may be between `check-time-trace` and `.json`?
If we use `-ftime-trace` but no `-ftime-trace=<path>` to compile the source 
like "check-time-trace.cpp" to a executable file `check-time-trace`, the 
`check-time-trace.cpp` should be compiled to 
`check-time-trace-[random-string].o`, then linked to the `check-time-trace` by 
the linker. This random string is introduced by clang's own default logic.

The `-ftime-trace` records the time cost details of compilng source file to the 
object file (.cpp -> .o). If the time-trace file name isn't be specified, its 
default name is [object file's name].json, which is corresponding the object 
file.

Demo:

Cmd: `clang++ -ftime-trace -ftime-trace-granularity=0 -o check-time-trace 
check-time-trace.cpp`
Output: `check-time-trace`, `check-time-trace-a40601.json`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to