[PATCH] D71060: [LLD][ELF] Add time-trace to ELF LLD (2/2)

[Russell Gallop via Phabricator via llvm-commits]

russell.gallop marked 3 inline comments as done.
russell.gallop added inline comments.


================
Comment at: lld/ELF/Driver.cpp:533
+    SmallString<128> path(config->outputFile);
+    llvm::sys::path::replace_extension(path, "json");
+    if (!args.getLastArgValue(OPT_time_trace_eq).empty())
----------------
ruiu wrote:
> I think it is better to make -time-trace to just print the result to stdout, as it is convenient.
Hmm, I went for this as it was more consistent with the comparable compiler option.

I do find this behaviour useful if you are tracing a build such as llvm which has a lot of links. While this probably isn't typical, it does make it easy to send all of the link traces to separate files just by adding a single link option to all links.

What about an option like "-ftime-trace -" to send to stdout?


================
Comment at: lld/ELF/Driver.cpp:979
+  config->timeTraceGranularity =
+      args::getInteger(args, OPT_time_trace_granularity, 500u);
   config->trace = args.hasArg(OPT_trace);
----------------
ruiu wrote:
> Is u prefix supported?
> 
> edit: Oh, this is not for nanosecond but just an unsigned. I'd just remove `u`.
Thanks. Will fix.


================
Comment at: lld/ELF/Driver.cpp:1624
 template <class ELFT> void LinkerDriver::compileBitcodeFiles() {
+  llvm::TimeTraceScope timeScope("LTO", StringRef(""));
   // Compile bitcode files and replace bitcode symbols.
----------------
ruiu wrote:
> I'm curious what is supposed to pass as a second argument.
This is used for "Detail". E.g. optimisation passes specify which pass they are running on "OptModule" specifies which module it is running on. This helps to distinguish multiples of the same kind of scope in traces. I don't think there is anything useful to detail at this level.

I could add an overloaded constructor to TimeTraceScope with a default to not add "detail". That would cut down on the StringRef("") argument which come up quite a bit. It could also cut down on file size by not writing that block out.


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

https://reviews.llvm.org/D71060