[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

Thu May 11 11:11:42 PDT 2023

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