[clang] [flang] [clang][flang] Support -time in both clang and flang (PR #109165)
Tarun Prabhu via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 12 08:07:38 PST 2024
https://github.com/tarunprabhu updated https://github.com/llvm/llvm-project/pull/109165
>From 11ee94ed124941fdbd70a1ea46021fa6618d2582 Mon Sep 17 00:00:00 2001
From: Tarun Prabhu <tarun.prabhu at gmail.com>
Date: Wed, 18 Sep 2024 09:49:26 -0600
Subject: [PATCH 1/7] [clang][flang] Support -time in both clang and flang
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
---
clang/include/clang/Driver/Options.td | 1 +
clang/lib/Driver/Compilation.cpp | 21 +++++++++++++++++++++
clang/lib/Driver/Driver.cpp | 3 +++
clang/test/Driver/time.c | 24 ++++++++++++++++++++++++
flang/test/Driver/time.f90 | 22 ++++++++++++++++++++++
5 files changed, 71 insertions(+)
create mode 100644 clang/test/Driver/time.c
create mode 100644 flang/test/Driver/time.f90
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 70f9c75944b0db..69a269c8a52d47 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5896,6 +5896,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 93e85f7dffe35a..a0fd459967a6b2 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1315,6 +1315,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
>From 29dac37af3c42770659da6c1d7c269d6cfac4f9f Mon Sep 17 00:00:00 2001
From: Tarun Prabhu <tarun.prabhu at gmail.com>
Date: Wed, 18 Sep 2024 13:18:45 -0600
Subject: [PATCH 2/7] Disable the Fortran test on Windows because it does not
produce any output causing the test to fail.
---
flang/test/Driver/time.f90 | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/flang/test/Driver/time.f90 b/flang/test/Driver/time.f90
index dda86e08960135..e39d1f28af74d2 100644
--- a/flang/test/Driver/time.f90
+++ b/flang/test/Driver/time.f90
@@ -1,3 +1,9 @@
+! TODO: For some reason, on Windows, nothing is printed to stderr which causes
+! the checks to fail. It is not clear why this is, so disable this on Windows
+! until the root cause can be determined.
+!
+! UNSUPPORTED: system-windows
+
! 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
>From e7e79c4a5513d09444ae23c5c52725929968fc6b Mon Sep 17 00:00:00 2001
From: Tarun Prabhu <tarun.prabhu at gmail.com>
Date: Mon, 23 Sep 2024 09:49:35 -0600
Subject: [PATCH 3/7] Address reviewer comments. Remove unnecessary -O3 flag in
test run lines. Improve checks to handle corner case.
---
clang/test/Driver/time.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/clang/test/Driver/time.c b/clang/test/Driver/time.c
index 1e19f29ab76a32..814c4ecc742ac6 100644
--- a/clang/test/Driver/time.c
+++ b/clang/test/Driver/time.c
@@ -5,16 +5,17 @@
// 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: %clang -time -c -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: %clang -time -S -emit-llvm -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: %clang -time -S -o /dev/null %s 2>&1 \
// RUN: | FileCheck %s --check-prefix=COMPILE-ONLY
-// RUN: %clang -time -O3 -o /dev/null %s 2>&1 \
+// RUN: %clang -time -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-ONLY-NOT: {{.}}
// COMPILE-AND-LINK: # {{.+}} {{[0-9]+(.[0-9]+)?}} {{[0-9]+(.[0-9]+)?}}
// COMPILE-AND-LINK: # {{.+}} {{[0-9]+(.[0-9]+)?}} {{[0-9]+(.[0-9]+)?}}
>From 517550ded623653f2d3ea25c017348430eb80b75 Mon Sep 17 00:00:00 2001
From: Tarun Prabhu <tarun.prabhu at gmail.com>
Date: Mon, 23 Sep 2024 09:52:51 -0600
Subject: [PATCH 4/7] Address reviewer comments. Same fixes to Fortran.
---
flang/test/Driver/time.f90 | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/flang/test/Driver/time.f90 b/flang/test/Driver/time.f90
index e39d1f28af74d2..cb94a5333f6e33 100644
--- a/flang/test/Driver/time.f90
+++ b/flang/test/Driver/time.f90
@@ -11,16 +11,17 @@
! 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: %flang -time -c -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: %flang -time -S -o /dev/null %s 2>&1 \
! RUN: | FileCheck %s --check-prefix=COMPILE-ONLY
-! RUN: %flang -time -O3 -o /dev/null %s 2>&1 \
+! RUN: %flang -time -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-ONLY-NOT: {{.}}
! COMPILE-AND-LINK: # {{.+}} {{[0-9]+(.[0-9]+)?}} {{[0-9]+(.[0-9]+)?}}
! COMPILE-AND-LINK: # {{.+}} {{[0-9]+(.[0-9]+)?}} {{[0-9]+(.[0-9]+)?}}
>From c5717475078b56e41e0e3ec50b14f2f04d9da6a1 Mon Sep 17 00:00:00 2001
From: Tarun Prabhu <tarun at lanl.gov>
Date: Fri, 11 Oct 2024 08:50:54 -0600
Subject: [PATCH 5/7] Add explicit target triple and enable the tests only on
Linux on x86_64. Add a comment to the tests explaining the reason for this
restriction.
---
clang/test/Driver/time.c | 16 ++++++++++++----
flang/test/Driver/time.f90 | 15 +++++++++++----
2 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/clang/test/Driver/time.c b/clang/test/Driver/time.c
index 814c4ecc742ac6..a9a95b073afb73 100644
--- a/clang/test/Driver/time.c
+++ b/clang/test/Driver/time.c
@@ -5,13 +5,21 @@
// could be something different, just check that whatever is printed to stderr
// looks like timing information.
-// RUN: %clang -time -c -o /dev/null %s 2>&1 \
+// Ideally, this should be tested on various platforms, but that requires the
+// the full toolchain, including a linker to be present. The initial author of
+// the test only had access to Linux on x86 which is why this is only enabled
+// there. More platforms ought to be added if possible.
+
+// REQUIRES: x86-registered-target
+// REQUIRES: x86_64-linux
+
+// RUN: %clang --target=x86_64-pc-linux -time -c -o /dev/null %s 2>&1 \
// RUN: | FileCheck %s --check-prefix=COMPILE-ONLY
-// RUN: %clang -time -S -emit-llvm -o /dev/null %s 2>&1 \
+// RUN: %clang --target=x86_64-pc-linux -time -S -emit-llvm -o /dev/null %s 2>&1 \
// RUN: | FileCheck %s --check-prefix=COMPILE-ONLY
-// RUN: %clang -time -S -o /dev/null %s 2>&1 \
+// RUN: %clang --target=x86_64-pc-linux -time -S -o /dev/null %s 2>&1 \
// RUN: | FileCheck %s --check-prefix=COMPILE-ONLY
-// RUN: %clang -time -o /dev/null %s 2>&1 \
+// RUN: %clang --target=x86_64-pc-linux -time -o /dev/null %s 2>&1 \
// RUN: | FileCheck %s --check-prefix=COMPILE-AND-LINK
// COMPILE-ONLY: # {{.+}} {{[0-9]+(.[0-9]+)?}} {{[0-9]+(.[0-9]+)?}}
diff --git a/flang/test/Driver/time.f90 b/flang/test/Driver/time.f90
index cb94a5333f6e33..98d4fac4e805e9 100644
--- a/flang/test/Driver/time.f90
+++ b/flang/test/Driver/time.f90
@@ -11,13 +11,20 @@
! potentially be something different, just check that whatever is printed to
! stderr looks like timing information.
-! RUN: %flang -time -c -o /dev/null %s 2>&1 \
+! Ideally, this should be tested on various platforms, but that requires the
+! the full toolchain, including a linker to be present. The initial author of
+! the test only had access to Linux on x86 which is why this is only enabled
+! there. More platforms ought to be added if possible.
+
+! REQUIRES: x86_64-linux
+
+! RUN: %flang --target=x86_64-linux -time -c -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: %flang --target=x86_64-linux -time -S -emit-llvm -O3 -o /dev/null %s 2>&1 \
! RUN: | FileCheck %s --check-prefix=COMPILE-ONLY
-! RUN: %flang -time -S -o /dev/null %s 2>&1 \
+! RUN: %flang --target=x86_64-linux -time -S -o /dev/null %s 2>&1 \
! RUN: | FileCheck %s --check-prefix=COMPILE-ONLY
-! RUN: %flang -time -o /dev/null %s 2>&1 \
+! RUN: %flang --target=x86_64-linux -time -o /dev/null %s 2>&1 \
! RUN: | FileCheck %s --check-prefix=COMPILE-AND-LINK
! COMPILE-ONLY: # {{.+}} {{[0-9]+(.[0-9]+)?}} {{[0-9]+(.[0-9]+)?}}
>From 61e51a744d5f450dc9693d514131d8db7d1de5fb Mon Sep 17 00:00:00 2001
From: Tarun Prabhu <tarun at lanl.gov>
Date: Mon, 14 Oct 2024 14:26:41 -0600
Subject: [PATCH 6/7] Address reviewer comments.
---
clang/lib/Driver/Compilation.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Driver/Compilation.cpp b/clang/lib/Driver/Compilation.cpp
index aadaa236246a27..43124a6c997e31 100644
--- a/clang/lib/Driver/Compilation.cpp
+++ b/clang/lib/Driver/Compilation.cpp
@@ -202,7 +202,7 @@ int Compilation::ExecuteCommand(const Command &C,
// us knowing about it.
llvm::TimeRecord StartTime;
if (getArgs().hasArg(options::OPT_time)) {
- StartTime = llvm::TimeRecord::getCurrentTime(true);
+ StartTime = llvm::TimeRecord::getCurrentTime(/*Start=*/true);
}
std::string Error;
@@ -212,7 +212,7 @@ int Compilation::ExecuteCommand(const Command &C,
PostCallback(C, Res);
if (getArgs().hasArg(options::OPT_time)) {
- llvm::TimeRecord Time = llvm::TimeRecord::getCurrentTime(false);
+ llvm::TimeRecord Time = llvm::TimeRecord::getCurrentTime(/*Start=*/false);
Time -= StartTime;
llvm::StringRef Name = llvm::sys::path::filename(C.getExecutable());
llvm::errs() << "# " << Name << " "
>From 60b63dc02e71a38087499d005703bf70d50f32db Mon Sep 17 00:00:00 2001
From: Tarun Prabhu <tarun at lanl.gov>
Date: Tue, 15 Oct 2024 07:52:51 -0600
Subject: [PATCH 7/7] Address reviewer comments. Make output compatible with
GCC
---
clang/lib/Driver/Compilation.cpp | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Driver/Compilation.cpp b/clang/lib/Driver/Compilation.cpp
index 43124a6c997e31..6965e59367c95d 100644
--- a/clang/lib/Driver/Compilation.cpp
+++ b/clang/lib/Driver/Compilation.cpp
@@ -201,9 +201,8 @@ int Compilation::ExecuteCommand(const Command &C,
// 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)) {
+ if (getArgs().hasArg(options::OPT_time))
StartTime = llvm::TimeRecord::getCurrentTime(/*Start=*/true);
- }
std::string Error;
bool ExecutionFailed;
@@ -216,8 +215,8 @@ int Compilation::ExecuteCommand(const Command &C,
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";
+ << llvm::format("%0.2f", Time.getUserTime()) << " "
+ << llvm::format("%0.2f", Time.getSystemTime()) << "\n";
}
if (!Error.empty()) {
More information about the cfe-commits
mailing list