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

[dongjunduo via Phabricator via cfe-commits]

dongjunduo marked an inline comment as done.
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:
> dongjunduo wrote:
> > Whitney wrote:
> > > dongjunduo wrote:
> > > > Whitney wrote:
> > > > > Do you mean Add or replace the modified `-ftime-trace=<path>` to all clang jobs?
> > > > Right
> > > ic, can you please have the comment updated?
> > Done at line:4703
> Let me clarify...can you please add `=` in between `-ftime-trace` and `<path>` for the comment on line 4703?
Sorry, I may have made a spelling mistake : ( hh


================
Comment at: clang/lib/Driver/Driver.cpp:4739
+
+      // replace `-ftime-trace=<path>`
+      auto &JArgs = J.getArguments();
----------------
Whitney wrote:
> dongjunduo wrote:
> > Whitney wrote:
> > > dongjunduo wrote:
> > > > 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.
> > > What happens when `-ftime-trace` is given by the user? Do you have both `-ftime-trace=<path>` and `-ftime-trace` as arguments?
> > It doesn't matter that either "-ftime-trace" or "-ftime-trace=<path>" or both of them are given by the user.
> > 
> > The TimeProfiler is switched on when either "-ftime-trace" and "-ftime-trace=<path>"  is specified. Then, 
> > 
> > * If "-ftime-trace=<path>" is specified, the driver use <path>.
> > * If only "-ftime-trace" is specified, the <path> can be infered, then a new option "-ftime-trace=<infered_path>" may be added to the clang.
> In the case of only "-ftime-trace" is specified, isn't it better to only pass  "-ftime-trace=<infered_path>" to the compile step instead of both  "-ftime-trace=<infered_path>" and "-ftime-trace"?
You are right... May be it's more suitable... I wll remove the "-ftime-trace" in this situration then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469