[PATCH] D128048: Add a new clang option "-ftime-trace-path"

dongjunduo marked an inline comment as done. dongjunduo added inline comments. Comment at: clang/test/Driver/check-time-trace.cpp:9 +// RUN: | FileCheck %s +// RUN: %clangxx -S -ftime-trace=%T -ftime-trace-granularity=0 -o %T/check-time-trace %s +// RUN: cat %T/check-time-trac

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

dongjunduo updated this revision to Diff 441572. dongjunduo added a comment. [Clang] change test cases to different directory Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128048/new/ https://reviews.llvm.org/D128048 Files: clang/include/clang/D

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

Whitney added inline comments. Comment at: clang/test/Driver/check-time-trace.cpp:9 +// RUN: | FileCheck %s +// RUN: %clangxx -S -ftime-trace=%T -ftime-trace-granularity=0 -o %T/check-time-trace %s +// RUN: cat %T/check-time-trace.json \ By default, the JSON f

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

dongjunduo added a comment. Hi @jamieschmeiser @Whitney @MaskRay, I have changed "-ftime-trace-path" to "-ftime-trace". It is a well-spelling option name. The user can turn on the time-trace by: - specifying "**-ftime-trace**" only and output it to the default directory (the same as the "-o" o

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

dongjunduo updated this revision to Diff 441368. dongjunduo added a comment. [Clang] change "-ftime-trace-path" to "-ftime-trace" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128048/new/ https://reviews.llvm.org/D128048 Files: clang/include/cla

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:2819 "'per-pass-run': one report for each pass invocation">; def ftime_trace : Flag<["-"], "ftime-trace">, Group, HelpText<"Turn on time profiler. Generates JSON file based on output

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

dongjunduo added inline comments. Comment at: clang/include/clang/Driver/Options.td:2819 "'per-pass-run': one report for each pass invocation">; def ftime_trace : Flag<["-"], "ftime-trace">, Group, HelpText<"Turn on time profiler. Generates JSON file based on outp

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

jamieschmeiser added a comment. In D128048#3607295 , @MaskRay wrote: > You may add `-ftime-trace=` instead of introducing a new spelling. I like this idea. There is an example of an optional argument on an option with -print-changed. Repository: rG

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

MaskRay added a comment. You may add `-ftime-trace=` instead of introducing a new spelling. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128048/new/ https://reviews.llvm.org/D128048 ___ cfe-commits mail

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

dongjunduo updated this revision to Diff 439661. dongjunduo added a comment. [Clang] rewrite test case of "-ftime-trace-path" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128048/new/ https://reviews.llvm.org/D128048 Files: clang/include/clang/D

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

jamieschmeiser added inline comments. Comment at: clang/test/Driver/check-time-trace-path.cpp:5 +// RUN: | FileCheck %s + +// CHECK: "beginningOfTime": {{[0-9]{16},}} This test is the same as the one in check-time-trace.cpp except for the path feature. A

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

dongjunduo added a comment. Hi @jamieschmeiser @Whitney , I have changed the approach from directory-store to path-store, so that the user can specify the aim path to store the time trace json file. If "-ftime-trace-path" is not specified, it will follow the default behavior. In addition, I a

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

dongjunduo updated this revision to Diff 439389. dongjunduo added a comment. [Clang] change directory-store to path-store Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128048/new/ https://reviews.llvm.org/D128048 Files: clang/include/clang/Drive

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

jamieschmeiser added a comment. In D128048#3601588 , @Whitney wrote: > In D128048#3601579 , > @jamieschmeiser wrote: > >> Can you please use git rebase -i to collapse all the changes into a single >> change? If

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

dongjunduo marked 4 inline comments as done. dongjunduo added a comment. In D128048#3601579 , @jamieschmeiser wrote: > Can you please use git rebase -i to collapse all the changes into a single > change? If this isn't done, it is difficult to know what

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

Whitney added a comment. In D128048#3601579 , @jamieschmeiser wrote: > Can you please use git rebase -i to collapse all the changes into a single > change? If this isn't done, it is difficult to know what is being reviewed > as the changes only show t

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

jamieschmeiser added a comment. Can you please use git rebase -i to collapse all the changes into a single change? If this isn't done, it is difficult to know what is being reviewed as the changes only show the differences since your last revision, not all of the changes. Repository: rG LL

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

dongjunduo updated this revision to Diff 438577. dongjunduo added a comment. [Clang] update help text of "ftime-trace-path" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128048/new/ https://reviews.llvm.org/D128048 Files: clang/include/clang/Dri

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

Whitney added inline comments. Comment at: clang/include/clang/Driver/Options.td:2832 +def ftime_trace_path : Joined<["-"], "ftime-trace-path=">, Group, + HelpText<"Path which stores the output files of time profiler">, + Flags<[CC1Option, CoreOption]>, jamiesc

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

dongjunduo added a comment. In D128048#3591829 , @Whitney wrote: > Can you please add some test cases? I have found that new-option unit tests are not included in `clang/test` or `clang/unit-tests`. Could u tell me where I should write the test cases?

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

dongjunduo updated this revision to Diff 438303. dongjunduo marked an inline comment as done. dongjunduo added a comment. [Clang] change unclear help text pf "-ftime-trace-path" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128048/new/ https://revi

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

dongjunduo updated this revision to Diff 438289. dongjunduo added a comment. [Clang] Restore the old behaviors when "-ftime-trace-path" is not specified Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128048/new/ https://reviews.llvm.org/D128048 Fil

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

jamieschmeiser requested changes to this revision. jamieschmeiser added a comment. This revision now requires changes to proceed. This is a good start. You should mention in the summary that when -c is not specified, the compiler is invoked with -o pointing at /tmp so the .json file will curren

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

Whitney added a comment. Can you please add some test cases? Comment at: clang/tools/driver/cc1_main.cpp:257 if (llvm::timeTraceProfilerEnabled()) { -SmallString<128> Path(Clang->getFrontendOpts().OutputFile); -llvm::sys::path::replace_extension(Path, "json"); +S

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

dongjunduo created this revision. Herald added a project: All. dongjunduo requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. The time profiler traces the stages during the clang compile process. Each compiling stage of a single source