MaskRay marked an inline comment as done. MaskRay added a comment. In D150282#4334927 <https://reviews.llvm.org/D150282#4334927>, @Maetveis wrote:
> LGTM, but I don't feel like I have the experience to "formally" approve. I > left a nice-to have suggestion too, but feel free to ignore, if you feel its > out of scope. Thanks. You can certainly give your review opinion as you've studied and discussed the feature a lot! (https://llvm.org/docs/Phabricator.html#:~:text=participated%20might) If we land a change like this patch, D133662 <https://reviews.llvm.org/D133662> can be turned to address the offloading side issue. It doesn't work without or with this patch; there is only one single JSON output file. I think supporting offloading will make some extensive changes to `GetNamedOutputPath`, so I avoid doing that in this patch. I am not an offloading user and don't understand its behavior well enough. ================ Comment at: clang/lib/Driver/Driver.cpp:5514 BaseInput); + handleTimeTrace(C, Args, JA, BaseInput, Result); } ---------------- Maetveis wrote: > One thing D133662 had that this change doesn't is that `--ftime-trace` was > reported as unused when there was no job that invoked clang (e.g. if using > the driver to link object files only). > > I am not sure its worth the effort, but it would be nice. IIRC in D133662 I > did this by having a method (`supportsTimeTrace`) on Tools to query support. > > I can also do this if you feel its out of scope here, and if there's no > objection to it. Thanks! This is a worthy change. I'll add a test separately. I think we can reuse `canEmitIR` instead of adding `supportsTimeTrace`. Another frontend flang-new defines `canEmitIR` to true as well. It's a work-in-progress and can support `-ftime-trace` later. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150282/new/ https://reviews.llvm.org/D150282 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
