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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits