[clang] 49b87b0 - [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

Fangrui Song via cfe-commits cfe-commits at lists.llvm.org
Fri May 12 10:46:11 PDT 2023


Author: Fangrui Song
Date: 2023-05-12T10:46:06-07:00
New Revision: 49b87b05726b4395503230c3d400f93c626e6dce

URL: https://github.com/llvm/llvm-project/commit/49b87b05726b4395503230c3d400f93c626e6dce
DIFF: https://github.com/llvm/llvm-project/commit/49b87b05726b4395503230c3d400f93c626e6dce.diff

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

Inspired by D133662.
Close https://github.com/llvm/llvm-project/issues/57285

When -ftime-trace is specified and the driver performs both compilation and
linking phases, the trace files are currently placed in the temporary directory
(/tmp by default on *NIX). A more sensible behavior would be to derive the trace
file names from the -o option, similar to how GCC derives auxiliary and dump
file names. Use -dumpdir (D149193) to implement the -gsplit-dwarf like behavior.

The following script demonstrates the time trace filenames.

```
#!/bin/sh -e
PATH=/tmp/Rel/bin:$PATH                # adapt according to your build directory
mkdir -p d e f
echo 'int main() {}' > d/a.c
echo > d/b.c

a() { rm $1 || exit 1; }

clang -ftime-trace d/a.c d/b.c         # previously /tmp/[ab]-*.json
a a-a.json; a a-b.json
clang -ftime-trace d/a.c d/b.c -o e/x  # previously /tmp/[ab]-*.json
a e/x-a.json; a e/x-b.json
clang -ftime-trace d/a.c d/b.c -o e/x -dumpdir f/
a f/a.json; a f/b.json
clang -ftime-trace=f d/a.c d/b.c -o e/x
a f/a-*.json; a f/b-*.json

clang -c -ftime-trace d/a.c d/b.c
a a.json b.json
clang -c -ftime-trace=f d/a.c d/b.c
a f/a.json f/b.json

clang -c -ftime-trace d/a.c -o e/xa.o
a e/xa.json
clang -c -ftime-trace d/a.c -o e/xa.o -dumpdir f/g
a f/ga.json
```

The driver checks `-ftime-trace` and `-ftime-trace=`, infers the trace file
name, and passes `-ftime-trace=` to cc1. The `-ftime-trace` cc1 option is
removed.

With offloading, previously `-ftime-trace` is passed to all offloading
actions, causing the same trace file to be overwritten by host and
offloading actions. This patch doesn't attempt to support offloading (D133662),
but makes a sensible change (`OffloadingPrefix.empty()`) to ensure we don't
overwrite the trace file.

Minor behavior differences: the trace file is now a result file, which
will be removed upon an error. -ftime-trace-granularity=0, like
-ftime-trace, can now cause a -Wunused-command-line-argument warning.

Reviewed By: Maetveis

Differential Revision: https://reviews.llvm.org/D150282

Added: 
    

Modified: 
    clang/include/clang/Driver/Compilation.h
    clang/include/clang/Driver/Options.td
    clang/include/clang/Frontend/FrontendOptions.h
    clang/lib/Driver/Driver.cpp
    clang/lib/Driver/ToolChains/Clang.cpp
    clang/test/Driver/ftime-trace.cpp
    clang/tools/driver/cc1_main.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Driver/Compilation.h b/clang/include/clang/Driver/Compilation.h
index f58b5a8cc9fdb..36ae85c424514 100644
--- a/clang/include/clang/Driver/Compilation.h
+++ b/clang/include/clang/Driver/Compilation.h
@@ -112,6 +112,9 @@ class Compilation {
   /// only be removed if we crash.
   ArgStringMap FailureResultFiles;
 
+  /// -ftime-trace result files.
+  ArgStringMap TimeTraceFiles;
+
   /// Optional redirection for stdin, stdout, stderr.
   std::vector<std::optional<StringRef>> Redirects;
 
@@ -269,6 +272,14 @@ class Compilation {
     return Name;
   }
 
+  const char *getTimeTraceFile(const JobAction *JA) const {
+    return TimeTraceFiles.lookup(JA);
+  }
+  void addTimeTraceFile(const char *Name, const JobAction *JA) {
+    assert(!TimeTraceFiles.contains(JA));
+    TimeTraceFiles[JA] = Name;
+  }
+
   /// CleanupFile - Delete a given file.
   ///
   /// \param IssueErrors - Report failures as errors.

diff  --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 670003a3da847..ac2c3c2fa718b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3004,8 +3004,7 @@ def ftime_trace : Flag<["-"], "ftime-trace">, Group<f_Group>,
 Turn on time profiler. Generates JSON file based on output filename. Results
 can be analyzed with chrome://tracing or `Speedscope App
 <https://www.speedscope.app>`_ for flamegraph visualization.}]>,
-  Flags<[CC1Option, CoreOption]>,
-  MarshallingInfoFlag<FrontendOpts<"TimeTrace">>;
+  Flags<[CoreOption]>;
 def ftime_trace_granularity_EQ : Joined<["-"], "ftime-trace-granularity=">, Group<f_Group>,
   HelpText<"Minimum time granularity (in microseconds) traced by time profiler">,
   Flags<[CC1Option, CoreOption]>,

diff  --git a/clang/include/clang/Frontend/FrontendOptions.h b/clang/include/clang/Frontend/FrontendOptions.h
index 85183a3812b42..40aa144a0e369 100644
--- a/clang/include/clang/Frontend/FrontendOptions.h
+++ b/clang/include/clang/Frontend/FrontendOptions.h
@@ -283,9 +283,6 @@ class FrontendOptions {
   /// print the supported cpus for the current target
   unsigned PrintSupportedCPUs : 1;
 
-  /// Output time trace profile.
-  unsigned TimeTrace : 1;
-
   /// Show the -version text.
   unsigned ShowVersion : 1;
 
@@ -513,16 +510,16 @@ class FrontendOptions {
 public:
   FrontendOptions()
       : DisableFree(false), RelocatablePCH(false), ShowHelp(false),
-        ShowStats(false), AppendStats(false), TimeTrace(false),
-        ShowVersion(false), FixWhatYouCan(false), FixOnlyWarnings(false),
-        FixAndRecompile(false), FixToTemporaries(false),
-        ARCMTMigrateEmitARCErrors(false), SkipFunctionBodies(false),
-        UseGlobalModuleIndex(true), GenerateGlobalModuleIndex(true),
-        ASTDumpDecls(false), ASTDumpLookups(false),
-        BuildingImplicitModule(false), BuildingImplicitModuleUsesLock(true),
-        ModulesEmbedAllFiles(false), IncludeTimestamps(true),
-        UseTemporary(true), AllowPCMWithCompilerErrors(false),
-        ModulesShareFileManager(true), TimeTraceGranularity(500) {}
+        ShowStats(false), AppendStats(false), ShowVersion(false),
+        FixWhatYouCan(false), FixOnlyWarnings(false), FixAndRecompile(false),
+        FixToTemporaries(false), ARCMTMigrateEmitARCErrors(false),
+        SkipFunctionBodies(false), UseGlobalModuleIndex(true),
+        GenerateGlobalModuleIndex(true), ASTDumpDecls(false),
+        ASTDumpLookups(false), BuildingImplicitModule(false),
+        BuildingImplicitModuleUsesLock(true), ModulesEmbedAllFiles(false),
+        IncludeTimestamps(true), UseTemporary(true),
+        AllowPCMWithCompilerErrors(false), ModulesShareFileManager(true),
+        TimeTraceGranularity(500) {}
 
   /// getInputKindForExtension - Return the appropriate input kind for a file
   /// extension. For example, "c" would return Language::C.

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 879f3bf00d1cf..535d9c9de13d2 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -5234,6 +5234,37 @@ InputInfoList Driver::BuildJobsForAction(
   return Result;
 }
 
+static void handleTimeTrace(Compilation &C, const ArgList &Args,
+                            const JobAction *JA, const char *BaseInput,
+                            const InputInfo &Result) {
+  Arg *A =
+      Args.getLastArg(options::OPT_ftime_trace, options::OPT_ftime_trace_EQ);
+  if (!A)
+    return;
+  SmallString<128> Path;
+  if (A->getOption().matches(options::OPT_ftime_trace_EQ)) {
+    Path = A->getValue();
+    if (llvm::sys::fs::is_directory(Path)) {
+      SmallString<128> Tmp(Result.getFilename());
+      llvm::sys::path::replace_extension(Tmp, "json");
+      llvm::sys::path::append(Path, llvm::sys::path::filename(Tmp));
+    }
+  } else {
+    if (Arg *DumpDir = Args.getLastArgNoClaim(options::OPT_dumpdir)) {
+      // The trace file is ${dumpdir}${basename}.json. Note that dumpdir may not
+      // end with a path separator.
+      Path = DumpDir->getValue();
+      Path += llvm::sys::path::filename(BaseInput);
+    } else {
+      Path = Result.getFilename();
+    }
+    llvm::sys::path::replace_extension(Path, "json");
+  }
+  const char *ResultFile = C.getArgs().MakeArgString(Path);
+  C.addTimeTraceFile(ResultFile, JA);
+  C.addResultFile(ResultFile, JA);
+}
+
 InputInfoList Driver::BuildJobsForActionNoCache(
     Compilation &C, const Action *A, const ToolChain *TC, StringRef BoundArch,
     bool AtTopLevel, bool MultipleArchs, const char *LinkingOutput,
@@ -5483,6 +5514,8 @@ InputInfoList Driver::BuildJobsForActionNoCache(
                                              AtTopLevel, MultipleArchs,
                                              OffloadingPrefix),
                        BaseInput);
+    if (T->canEmitIR() && OffloadingPrefix.empty())
+      handleTimeTrace(C, Args, JA, BaseInput, Result);
   }
 
   if (CCCPrintBindings && !CCGenDiagnostics) {

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index a518cc487cfd5..189cbd19e1499 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6344,13 +6344,15 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_parseable_fixits);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_report);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_report_EQ);
-  Args.AddLastArg(CmdArgs, options::OPT_ftime_trace);
-  Args.AddLastArg(CmdArgs, options::OPT_ftime_trace_granularity_EQ);
-  Args.AddLastArg(CmdArgs, options::OPT_ftime_trace_EQ);
   Args.AddLastArg(CmdArgs, options::OPT_ftrapv);
   Args.AddLastArg(CmdArgs, options::OPT_malign_double);
   Args.AddLastArg(CmdArgs, options::OPT_fno_temp_file);
 
+  if (const char *Name = C.getTimeTraceFile(&JA)) {
+    CmdArgs.push_back(Args.MakeArgString("-ftime-trace=" + Twine(Name)));
+    Args.AddLastArg(CmdArgs, options::OPT_ftime_trace_granularity_EQ);
+  }
+
   if (Arg *A = Args.getLastArg(options::OPT_ftrapv_handler_EQ)) {
     CmdArgs.push_back("-ftrapv-handler");
     CmdArgs.push_back(A->getValue());

diff  --git a/clang/test/Driver/ftime-trace.cpp b/clang/test/Driver/ftime-trace.cpp
index 16c7ad53c69e5..24848d9fa1edc 100644
--- a/clang/test/Driver/ftime-trace.cpp
+++ b/clang/test/Driver/ftime-trace.cpp
@@ -31,6 +31,35 @@
 // CHECK:      "name": "process_name"
 // CHECK:      "name": "thread_name"
 
+// RUN: mkdir d e f && cp %s d/a.cpp && touch d/b.c
+
+// RUN: %clang -### -c -ftime-trace -ftime-trace-granularity=0 d/a.cpp -o e/a.o 2>&1 | FileCheck %s --check-prefix=COMPILE1
+// COMPILE1: -cc1{{.*}} "-ftime-trace=e/a.json" "-ftime-trace-granularity=0"
+
+// RUN: %clang -### -c -ftime-trace -ftime-trace-granularity=0 d/a.cpp d/b.c -dumpdir f/ 2>&1 | FileCheck %s --check-prefix=COMPILE2
+// COMPILE2: -cc1{{.*}} "-ftime-trace=f/a.json" "-ftime-trace-granularity=0"
+// COMPILE2: -cc1{{.*}} "-ftime-trace=f/b.json" "-ftime-trace-granularity=0"
+
+/// -o specifies the link output. Create ${output}-${basename}.json.
+// RUN: %clang -### -ftime-trace -ftime-trace-granularity=0 d/a.cpp d/b.c -o e/x 2>&1 | FileCheck %s --check-prefix=LINK1
+// LINK1: -cc1{{.*}} "-ftime-trace=e/x-a.json" "-ftime-trace-granularity=0"
+// LINK1: -cc1{{.*}} "-ftime-trace=e/x-b.json" "-ftime-trace-granularity=0"
+
+/// -dumpdir is f/g, not ending with a path separator. We create f/g${basename}.json.
+// RUN: %clang -### -ftime-trace -ftime-trace-granularity=0 d/a.cpp d/b.c -o e/x -dumpdir f/g 2>&1 | FileCheck %s --check-prefix=LINK2
+// LINK2: -cc1{{.*}} "-ftime-trace=f/ga.json" "-ftime-trace-granularity=0"
+// LINK2: -cc1{{.*}} "-ftime-trace=f/gb.json" "-ftime-trace-granularity=0"
+
+// RUN: %clang -### -ftime-trace=e -ftime-trace-granularity=0 d/a.cpp d/b.c -o f/x -dumpdir f/ 2>&1 | FileCheck %s --check-prefix=LINK3
+// LINK3: -cc1{{.*}} "-ftime-trace=e/a-{{[^.]*}}.json" "-ftime-trace-granularity=0"
+// LINK3: -cc1{{.*}} "-ftime-trace=e/b-{{[^.]*}}.json" "-ftime-trace-granularity=0"
+
+// RUN: %clang -### -ftime-trace -ftime-trace=e -ftime-trace-granularity=1 -xassembler d/a.cpp 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=UNUSED --implicit-check-not=warning:
+// UNUSED: warning: argument unused during compilation: '-ftime-trace'
+// UNUSED: warning: argument unused during compilation: '-ftime-trace=e'
+// UNUSED: warning: argument unused during compilation: '-ftime-trace-granularity=1'
+
 template <typename T>
 struct Struct {
   T Num;

diff  --git a/clang/tools/driver/cc1_main.cpp b/clang/tools/driver/cc1_main.cpp
index cb33692cd2623..9e7f8679b4cbd 100644
--- a/clang/tools/driver/cc1_main.cpp
+++ b/clang/tools/driver/cc1_main.cpp
@@ -213,9 +213,7 @@ int cc1_main(ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr) {
   bool Success = CompilerInvocation::CreateFromArgs(Clang->getInvocation(),
                                                     Argv, Diags, Argv0);
 
-  if (Clang->getFrontendOpts().TimeTrace ||
-      !Clang->getFrontendOpts().TimeTracePath.empty()) {
-    Clang->getFrontendOpts().TimeTrace = 1;
+  if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
     llvm::timeTraceProfilerInitialize(
         Clang->getFrontendOpts().TimeTraceGranularity, Argv0);
   }
@@ -257,16 +255,6 @@ int cc1_main(ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr) {
   llvm::TimerGroup::clearAll();
 
   if (llvm::timeTraceProfilerEnabled()) {
-    SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
-    llvm::sys::path::replace_extension(Path, "json");
-    if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
-      // replace the suffix to '.json' directly
-      SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
-      if (llvm::sys::fs::is_directory(TracePath))
-        llvm::sys::path::append(TracePath, llvm::sys::path::filename(Path));
-      Path.assign(TracePath);
-    }
-
     // It is possible that the compiler instance doesn't own a file manager here
     // if we're compiling a module unit. Since the file manager are owned by AST
     // when we're compiling a module unit. So the file manager may be invalid
@@ -280,7 +268,8 @@ int cc1_main(ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr) {
           Clang->getInvocation(), Clang->getDiagnostics()));
 
     if (auto profilerOutput = Clang->createOutputFile(
-            Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
+            Clang->getFrontendOpts().TimeTracePath, /*Binary=*/false,
+            /*RemoveFileOnSignal=*/false,
             /*useTemporary=*/false)) {
       llvm::timeTraceProfilerWrite(*profilerOutput);
       profilerOutput.reset();


        


More information about the cfe-commits mailing list