[PATCH] D41915: [lldCOFF] Print detailed timing information with /VERBOSE

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/COFF/Driver.cpp:834-836
+  if (Args.hasArg(OPT_show_timing)) {
+    Config->ShowTiming = true;
+  }
----------------
For consistency:

  // Handle /showtiming
  if (Args.hasArg(OPT_show_timing))
    Config->ShowTiming = true;


================
Comment at: lld/include/lld/Common/LinkTimer.h:18
+
+struct LinkTimer {
+  ~LinkTimer() { end(); }
----------------
Since everything in `lld` namespace is related to linking, I'd name this Timer.


================
Comment at: lld/include/lld/Common/LinkTimer.h:21
+
+  void start(std::chrono::milliseconds &DurationToUpdate) {
+    assert(!this->DurationToUpdate);
----------------
Why don't you use nanosecond?


================
Comment at: lld/include/lld/Common/LinkTimer.h:30
+      return;
+    auto EndTime = std::chrono::system_clock::now();
+    *DurationToUpdate += std::chrono::duration_cast<std::chrono::milliseconds>(
----------------
Please write the actual type instead of `auto`.


================
Comment at: lld/include/lld/Common/LinkTimer.h:31
+    auto EndTime = std::chrono::system_clock::now();
+    *DurationToUpdate += std::chrono::duration_cast<std::chrono::milliseconds>(
+        EndTime - StartTime);
----------------
It is better to use chrono::high_resolution_clock, or at least chrono::steady_clock. system_clock is not guaranteed to be monotonic (so it can rewind.)


https://reviews.llvm.org/D41915