[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo added a comment. > Hey @dongjunduo, > Are you still interested in finishing this? If not I was thinking that I > would pick it up, so I can work on the follow ups. @Maetveis Sorry for delaying the issue due to some personal trifles... This part of the implementation has been

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo added a comment. In D131469#3798409 , @Maetveis wrote: > As discussed with @jamieschmeiser on D133662 > , I have left suggestions regarding the > approach I took for handling `-o` and passing the option to

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo added inline comments. Comment at: clang/lib/Driver/Driver.cpp:4516 + bool HasTimeTrace = C.getArgs().hasArg(options::OPT_ftime_trace); + bool HasTimeTraceFile = C.getArgs().hasArg(options::OPT_ftime_trace_EQ); + // Whether `-ftime-trace` or `-ftime-trace=` are

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo added a comment. @MaskRay ping 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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo added a comment. In D131469#3775741 , @MaskRay wrote: > I appreciate the detailed summary. It has sufficient information but I think > it can be rephased to be conciser. > I request changes for the verbosity and the possibly functionality

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo updated this revision to Diff 459423. dongjunduo added a comment. fix related nits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/ https://reviews.llvm.org/D131469 Files: clang/lib/Driver/Driver.cpp

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo updated this revision to Diff 458440. dongjunduo added a comment. fix windows path-check error Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/ https://reviews.llvm.org/D131469 Files: clang/lib/Driver/Driver.cpp

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo added a comment. @dyung @steven_wu A newer diff has been submitted just now. Agree with @dyung. The test has been changed into checking the output of "-###". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo updated this revision to Diff 458177. dongjunduo added a comment. Rewrite test by checking output of -### Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/ https://reviews.llvm.org/D131469 Files: clang/lib/Driver/Driver.cpp

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo added a comment. These related commits have been reverted temporarily. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/ https://reviews.llvm.org/D131469 ___ cfe-commits mailing list

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo added a comment. In D131469#3768288 , @dyung wrote: > In D131469#3768283 , @dongjunduo > wrote: > >> In D131469#3768282 , @dyung wrote: >> >>> In

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo added a comment. In D131469#3768282 , @dyung wrote: > In D131469#3768274 , @dongjunduo > wrote: > >> In D131469#3768260 , @dyung wrote: >> >>> The test you

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo added a comment. In D131469#3768260 , @dyung wrote: > The test you added is failing on the PS4 linux bot because the PS4 platform > requires an external linker that isn't present. Is linking necessary for your > test? Or can -S or even -c

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

This revision was automatically updated to reflect the committed changes. Closed by commit rG38941da066a7: [Clang] change default storing path of `-ftime-trace` (authored by dongjunduo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo added a comment. @jamieschmeiser @Whitney For now, the time-trace file's name is corresponding to the output file's name ([demo].o => [demo].json). The only fly in the ointment is that when the user hasn't given the object file's name by "-o", the object file may be stored in

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo updated this revision to Diff 456535. dongjunduo added a comment. Rewrite comments and commit log Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/ https://reviews.llvm.org/D131469 Files: clang/lib/Driver/Driver.cpp

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo updated this revision to Diff 456533. dongjunduo added a comment. Replace -ftime-trace with -ftime-trace= also Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/ https://reviews.llvm.org/D131469 Files:

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo marked an inline comment as done. dongjunduo added inline comments. Comment at: clang/lib/Driver/Driver.cpp:4702 + + // Add or replace -ftime-trace` to the correct one to all clang jobs + for (auto : C.getJobs()) { Whitney wrote: > dongjunduo

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo marked an inline comment as done. dongjunduo added inline comments. Comment at: clang/lib/Driver/Driver.cpp:4702 + + // Add or replace -ftime-trace` to the correct one to all clang jobs + for (auto : C.getJobs()) { Whitney wrote: > dongjunduo

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo added inline comments. Comment at: clang/lib/Driver/Driver.cpp:4702 + + // Add or replace -ftime-trace` to the correct one to all clang jobs + for (auto : C.getJobs()) { Whitney wrote: > Do you mean Add or replace the modified `-ftime-trace=` to

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo updated this revision to Diff 456282. dongjunduo marked 2 inline comments as done. dongjunduo added a comment. Restyle variables' name and comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo marked 2 inline comments as done. dongjunduo added inline comments. Comment at: clang/lib/Driver/Driver.cpp:4667 + // Whether `-ftime-trace` or `-ftime-trace=` are specified + if (!TimeTrace && !TimeTraceFile) return; + jamieschmeiser wrote: > The

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo updated this revision to Diff 454501. dongjunduo added a comment. Restyle code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/ https://reviews.llvm.org/D131469 Files: clang/lib/Driver/Driver.cpp

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo marked 7 inline comments as done. dongjunduo added inline comments. Comment at: clang/lib/Driver/Driver.cpp:4674 +for (auto : C.getJobs()) { + if (J.getSource().getKind() == Action::LinkJobClass && + !J.getOutputFilenames().empty()) {

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo updated this revision to Diff 454439. dongjunduo added a comment. Restyle code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/ https://reviews.llvm.org/D131469 Files: clang/lib/Driver/Driver.cpp

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo updated this revision to Diff 452058. dongjunduo added a comment. Add assert messages Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/ https://reviews.llvm.org/D131469 Files: clang/lib/Driver/Driver.cpp

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo updated this revision to Diff 452054. dongjunduo added a comment. Add necessary asserts Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/ https://reviews.llvm.org/D131469 Files: clang/lib/Driver/Driver.cpp

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo updated this revision to Diff 451874. dongjunduo added a comment. format cc1_main.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/ https://reviews.llvm.org/D131469 Files: clang/lib/Driver/Driver.cpp

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo marked 3 inline comments as done. dongjunduo added inline comments. Comment at: clang/lib/Driver/Driver.cpp:4786 +const std::string::size_type size = arg.size(); +char *buffer = new char[size + 1]; +memcpy(buffer, arg.c_str(), size + 1);

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo updated this revision to Diff 451865. dongjunduo added a comment. Restyle codes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/ https://reviews.llvm.org/D131469 Files: clang/lib/Driver/Driver.cpp

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo updated this revision to Diff 451857. dongjunduo added a comment. format code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/ https://reviews.llvm.org/D131469 Files: clang/lib/Driver/Driver.cpp

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo added a comment. In D131469#3715929 , @jamieschmeiser wrote: > You should not have debugging information in code that is up for review. If > this is debugging information that you plan to leave in for future purposes > (which I doubt is

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo updated this revision to Diff 451838. dongjunduo added a comment. fix stringRef bug Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/ https://reviews.llvm.org/D131469 Files: clang/lib/Driver/Driver.cpp

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo updated this revision to Diff 451804. dongjunduo added a comment. Add more debug info Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/ https://reviews.llvm.org/D131469 Files: clang/lib/Driver/Driver.cpp

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo updated this revision to Diff 451796. dongjunduo added a comment. Add more debug info Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/ https://reviews.llvm.org/D131469 Files: clang/lib/Driver/Driver.cpp

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

dongjunduo updated this revision to Diff 451785. dongjunduo added a comment. [Clang] add -### for debug Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131469/new/ https://reviews.llvm.org/D131469 Files: clang/lib/Driver/Driver.cpp

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

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. Change the default storing path of `-ftime-trace`. We can use `-ftime-trace` or `-ftime-trace=` to switch on

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGf5d9de8cc330: [Clang] Add a new clang option -ftime-trace=value (authored by dongjunduo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128048/new/

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

dongjunduo updated this revision to Diff 444948. dongjunduo added a comment. [Clang] fix mkdir error in ftime-trace= test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128048/new/ https://reviews.llvm.org/D128048 Files:

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

dongjunduo updated this revision to Diff 444898. dongjunduo added a comment. [Clang] restyle code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128048/new/ https://reviews.llvm.org/D128048 Files: clang/include/clang/Driver/Options.td

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

dongjunduo updated this revision to Diff 441574. dongjunduo added a comment. [Clang] format the source file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128048/new/ https://reviews.llvm.org/D128048 Files: clang/include/clang/Driver/Options.td

[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

[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:

[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"

[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:

[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

[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:

[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

[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:

[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

[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:

[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/

[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

[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

[PATCH] D128046: 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 a project: clang. Herald added a subscriber: cfe-commits. Add a new clang option "-ftime-trace-path". The time profiler traces the stages during the clang compile process.

[PATCH] D128044: add new 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. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D128044 Files: