[clang][flang] Support -time in both clang and flang

[tarunprabhu]

The -time option prints timing information for the subcommands (compiler, linker) in a format similar to that used by gcc/gfortran (we use more digits of precision).

This partially addresses requests from #89888

[llvmbot]

@llvm/pr-subscribers-clang-driver
@llvm/pr-subscribers-flang-driver

@llvm/pr-subscribers-clang

Author: Tarun Prabhu (tarunprabhu)

Changes

The -time option prints timing information for the subcommands (compiler, linker) in a format similar to that used by gcc/gfortran (we use more digits of precision).

This partially addresses requests from #89888

---

Full diff: https://github.com/llvm/llvm-project/pull/109165.diff

5 Files Affected:

• (modified) clang/include/clang/Driver/Options.td (+1)
• (modified) clang/lib/Driver/Compilation.cpp (+21)
• (modified) clang/lib/Driver/Driver.cpp (+3)
• (added) clang/test/Driver/time.c (+24)
• (added) flang/test/Driver/time.f90 (+22)

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 7f123335ce8cfa..6c97025985cb7b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5841,6 +5841,7 @@ def print_enabled_extensions : Flag<["-", "--"], "print-enabled-extensions">,
 def : Flag<["-"], "mcpu=help">, Alias<print_supported_cpus>;
 def : Flag<["-"], "mtune=help">, Alias<print_supported_cpus>;
 def time : Flag<["-"], "time">,
+  Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>,
   HelpText<"Time individual commands">;
 def traditional_cpp : Flag<["-", "--"], "traditional-cpp">,
   Visibility<[ClangOption, CC1Option]>,
diff --git a/clang/lib/Driver/Compilation.cpp b/clang/lib/Driver/Compilation.cpp
index ad077d5bbfa69a..aadaa236246a27 100644
--- a/clang/lib/Driver/Compilation.cpp
+++ b/clang/lib/Driver/Compilation.cpp
@@ -21,6 +21,9 @@
 #include "llvm/Option/OptSpecifier.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Format.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Timer.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TargetParser/Triple.h"
 #include <cassert>
@@ -194,11 +197,29 @@ int Compilation::ExecuteCommand(const Command &C,
   if (LogOnly)
     return 0;
 
+  // We don't use any timers or llvm::TimeGroup's because those are tied into
+  // the global static timer list which, in principle, could be cleared without
+  // us knowing about it.
+  llvm::TimeRecord StartTime;
+  if (getArgs().hasArg(options::OPT_time)) {
+    StartTime = llvm::TimeRecord::getCurrentTime(true);
+  }
+
   std::string Error;
   bool ExecutionFailed;
   int Res = C.Execute(Redirects, &Error, &ExecutionFailed);
   if (PostCallback)
     PostCallback(C, Res);
+
+  if (getArgs().hasArg(options::OPT_time)) {
+    llvm::TimeRecord Time = llvm::TimeRecord::getCurrentTime(false);
+    Time -= StartTime;
+    llvm::StringRef Name = llvm::sys::path::filename(C.getExecutable());
+    llvm::errs() << "# " << Name << " "
+                 << llvm::format("%0.5f", Time.getUserTime()) << " "
+                 << llvm::format("%0.5f", Time.getSystemTime()) << "\n";
+  }
+
   if (!Error.empty()) {
     assert(Res && "Error string set with 0 result code!");
     getDriver().Diag(diag::err_drv_command_failure) << Error;
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index efe398dd531da7..c8b45e9be5328b 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1289,6 +1289,9 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
   // Ignore -pipe.
   Args.ClaimAllArgs(options::OPT_pipe);
 
+  // Ignore -time.
+  Args.ClaimAllArgs(options::OPT_time);
+
   // Extract -ccc args.
   //
   // FIXME: We need to figure out where this behavior should live. Most of it
diff --git a/clang/test/Driver/time.c b/clang/test/Driver/time.c
new file mode 100644
index 00000000000000..1e19f29ab76a32
--- /dev/null
+++ b/clang/test/Driver/time.c
@@ -0,0 +1,24 @@
+// The -time option prints timing information for the various subcommands in a
+// format similar to that used by gcc. When compiling and linking, this will
+// include the time to call clang-${LLVM_VERSION_MAJOR} and the linker. Since
+// the name of the linker could vary across platforms, and name of the compiler
+// could be something different, just check that whatever is printed to stderr
+// looks like timing information.
+
+// RUN: %clang -time -c -O3 -o /dev/null %s 2>&1 \
+// RUN:     | FileCheck %s --check-prefix=COMPILE-ONLY
+// RUN: %clang -time -S -emit-llvm -O3 -o /dev/null %s 2>&1 \
+// RUN:     | FileCheck %s --check-prefix=COMPILE-ONLY
+// RUN: %clang -time -S -O3 -o /dev/null %s 2>&1 \
+// RUN:     | FileCheck %s --check-prefix=COMPILE-ONLY
+// RUN: %clang -time -O3 -o /dev/null %s 2>&1 \
+// RUN:     | FileCheck %s --check-prefix=COMPILE-AND-LINK
+
+// COMPILE-ONLY: # {{.+}} {{[0-9]+(.[0-9]+)?}} {{[0-9]+(.[0-9]+)?}}
+
+// COMPILE-AND-LINK: # {{.+}} {{[0-9]+(.[0-9]+)?}} {{[0-9]+(.[0-9]+)?}}
+// COMPILE-AND-LINK: # {{.+}} {{[0-9]+(.[0-9]+)?}} {{[0-9]+(.[0-9]+)?}}
+
+int main() {
+  return 0;
+}
diff --git a/flang/test/Driver/time.f90 b/flang/test/Driver/time.f90
new file mode 100644
index 00000000000000..dda86e08960135
--- /dev/null
+++ b/flang/test/Driver/time.f90
@@ -0,0 +1,22 @@
+! The -time option prints timing information for the various subcommands in a
+! format similar to that used by gfortran. When compiling and linking, this will
+! include the time to call flang-${LLVM_VERSION_MAJOR} and the linker. Since the
+! name of the linker could vary across platforms, and the flang name could also
+! potentially be something different, just check that whatever is printed to
+! stderr looks like timing information.
+
+! RUN: %flang -time -c -O3 -o /dev/null %s 2>&1 \
+! RUN:     | FileCheck %s --check-prefix=COMPILE-ONLY
+! RUN: %flang -time -S -emit-llvm -O3 -o /dev/null %s 2>&1 \
+! RUN:     | FileCheck %s --check-prefix=COMPILE-ONLY
+! RUN: %flang -time -S -O3 -o /dev/null %s 2>&1 \
+! RUN:     | FileCheck %s --check-prefix=COMPILE-ONLY
+! RUN: %flang -time -O3 -o /dev/null %s 2>&1 \
+! RUN:     | FileCheck %s --check-prefix=COMPILE-AND-LINK
+
+! COMPILE-ONLY: # {{.+}} {{[0-9]+(.[0-9]+)?}} {{[0-9]+(.[0-9]+)?}}
+
+! COMPILE-AND-LINK: # {{.+}} {{[0-9]+(.[0-9]+)?}} {{[0-9]+(.[0-9]+)?}}
+! COMPILE-AND-LINK: # {{.+}} {{[0-9]+(.[0-9]+)?}} {{[0-9]+(.[0-9]+)?}}
+
+end program

[tarunprabhu]

The buildkite is failing on Windows because flang does not print anything on Windows. It is not clear to me why this is the case. I do not have access to a windows machine on which to debug this. The test, time.f90 has been disabled on Windows with a comment explaining why it has been disabled.

[MaskRay]

This requires a hard-coded target triple like --target=x86_64 and REQUIRES: x86-registered-target.

Delete unneeded -O3.

[tarunprabhu]

Wouldn't this restrict this test to running only when that target has been enabled? Is there a way to set the target to some target that has been enabled in that build? I would like to avoid restricting where the test runs. Perhaps I am missing something.

[tarunprabhu]

@MaskRay, am I right in understanding that the changes that you have requested would restrict the test to run only on x86? If not, is there somewhere this is explained so I understand this better. Thanks.

[MaskRay]

If the default target is certain niche OSes AIX, z/OS , the output could be very different. While we are comfortable with Linux on many architectures, certain architectures could have strange behaviors (MIPS). So just stick with x86_64-linux

Running all spawned processes (without -###) has more requirements on the tools (e.g. ld must be available). It's better to use -### for driver tests, if feasible.

[tarunprabhu]

Thanks for the explanation. I have restricted the tests to only run on x86_64-linux with a comment explaining the reasoning for the restriction.

[tblah]

Thanks for this!

[kiranchandramohan]

If this is not expected to work on Windows, then should we have CLOption and DXCOption? Or are these just added for compatibility with the previous setting?

[tarunprabhu]

I don't see any reason why it should not work on Windows. But there was a failing Windows buildkite, so I temporarily disabled the test. I will try again to see if it was a transient issue. If the issue still persists, I will add a comment indicating that the option should work on Windows, but that the original author did not have a means to test it and, therefore, disabled it. Since some of the tests require a full toolchain to be present, @MaskRay suggested being conservative about where the tests were run.

I have generally kept CLOption and DXCOption when the original did not explicitly specify anything for compatibility.

[kiranchandramohan]

Thanks Tarun. LGTM.

[tarunprabhu]

@MaskRay, have all your comments been addressed?

[tarunprabhu]

Gentle ping, @MaskRay

[tarunprabhu]

Thank you all for the reviews.