Refactor TimeProfiler write methods (NFC)

[amonshiz]

Summary

Added a write method for TimeTrace that takes two strings representing
file names. The first is any file name that may have been provided by the
user via time-trace-file flag, and the second is a fallback that should
be configured by the caller. This method makes it cleaner to write the
trace output because there is no longer a need to check file names at the
caller and simplifies future TimeTrace usages.

[rnk]

Once the error handling is fixed up, I'm not sure this is that much better than what came before, but I could be wrong.

llvm/lib/Support/TimeProfiler.cpp

296

Maybe passing the error back to the caller (as the Error type, perhaps?) would be better. LLD would hook it into its diagnostic machinery and clang to its own. Otherwise, the user can supply an unwritable path which will cause clang/lld to print a banner asking them to file a bug report, which is incorrect, it's a usage bug, not a compiler bug.

[amonshiz]

I'm not sure this is that much better than what came before, but I could be wrong.

The benefit of this change is that in the next diff in this stack I add additional usage of the TimeTraceScope to LLVM passes/pass managers. In doing so there is another need to essentially flush the trace info to a file and also validate the file name provided. It made sense to move handling whether or not a user provided a file name to a consistent place.

llvm/lib/Support/TimeProfiler.cpp

296

Will do! Sounds like a good improvement.

[amonshiz]

Add Error based handling

Thanks for the review @rnk

[modocache]

This looks good to me, it sounds like all of @rnk's comments were addressed, and I'd like to unblock D74516 that's stacked on top of this, so I'm going to accept. @amonshiz do you want me to commit this for you? (And @rnk, any comments before I do?)

[amonshiz]

@modocache that would be great if you could land this for me!

lld/ELF/Driver.cpp

case ELF64BEKind:
       link<ELF64BE>(args);
       break;
     default:
       llvm_unreachable("unknown Config->EKind");
     }
   }
 
   if (config->timeTraceEnabled) {
-    // Write the result of the time trace profiler.
-    std::string path = args.getLastArgValue(OPT_time_trace_file_eq).str();
-    if (path.empty())
-      path = (config->outputFile + ".time-trace").str();
-    std::error_code ec;
-    raw_fd_ostream os(path, ec, sys::fs::OF_Text);
-    if (ec) {
-      error("cannot open " + path + ": " + ec.message());
+    if (auto E = timeTraceProfilerWrite(args.getLastArgValue(OPT_time_trace_file_eq).str(),
+                                        config->outputFile)) {
+      handleAllErrors(std::move(E), [&](const StringError &SE) {
+        error(SE.getMessage());
+      });
       return;
     }
-    timeTraceProfilerWrite(os);
+
     timeTraceProfilerCleanup();
   }
 }
 
 static std::string getRpath(opt::InputArgList &args) {
   std::vector<StringRef> v = args::getStrings(args, OPT_rpath);
   return llvm::join(v.begin(), v.end(), ":");
 }

llvm/include/llvm/Support/TimeProfiler.h

 //===- llvm/Support/TimeProfiler.h - Hierarchical Time Profiler -*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_SUPPORT_TIME_PROFILER_H
 #define LLVM_SUPPORT_TIME_PROFILER_H
 
+#include "llvm/Support/Error.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace llvm {
 
 struct TimeTraceProfiler;
 TimeTraceProfiler *getTimeTraceProfilerInstance();
 
 /// Initialize the time trace profiler.
 /// This sets up the global \p TimeTraceProfilerInstance
 /// variable to be the profiler instance.
 void timeTraceProfilerInitialize(unsigned TimeTraceGranularity,
                                  StringRef ProcName);
 
 /// Cleanup the time trace profiler, if it was initialized.
 void timeTraceProfilerCleanup();
 
 /// Finish a time trace profiler running on a worker thread.
 void timeTraceProfilerFinishThread();
 
 /// Is the time trace profiler enabled, i.e. initialized?
 inline bool timeTraceProfilerEnabled() {
   return getTimeTraceProfilerInstance() != nullptr;
 }
 
-/// Write profiling data to output file.
+/// Write profiling data to output stream.
 /// Data produced is JSON, in Chrome "Trace Event" format, see
 /// https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview
 void timeTraceProfilerWrite(raw_pwrite_stream &OS);
 
+/// Write profiling data to a file.
+/// The function will write to \p PreferredFileName if provided, if not
+/// then will write to \p FallbackFileName appending .time-trace.
+/// Returns a StringError indicating a failure if the function is
+/// unable to open the file for writing.
+Error timeTraceProfilerWrite(StringRef PreferredFileName,
+                             StringRef FallbackFileName);
+
 /// Manually begin a time section, with the given \p Name and \p Detail.
 /// Profiler copies the string data, so the pointers can be given into
 /// temporaries. Time sections can be hierarchical; every Begin must have a
 /// matching End pair but they can nest.
 void timeTraceProfilerBegin(StringRef Name, StringRef Detail);
 void timeTraceProfilerBegin(StringRef Name,
                             llvm::function_ref<std::string()> Detail);
 

llvm/lib/Support/TimeProfiler.cpp

 }
 
 void timeTraceProfilerWrite(raw_pwrite_stream &OS) {
   assert(TimeTraceProfilerInstance != nullptr &&
          "Profiler object can't be null");
   TimeTraceProfilerInstance->Write(OS);
 }
 
+Error timeTraceProfilerWrite(StringRef PreferredFileName,
+                             StringRef FallbackFileName) {
+  assert(TimeTraceProfilerInstance != nullptr &&
+         "Profiler object can't be null");
+
+  std::string Path = PreferredFileName.str();
+  if (Path.empty()) {
+    Path = FallbackFileName == "-" ? "out" : FallbackFileName.str();
+    Path += ".time-trace";
+  }
+
+  std::error_code EC;
+  raw_fd_ostream OS(Path, EC, sys::fs::OF_Text);
+  if (EC)
+    return createStringError(EC, "Could not open " + Path);
+
+  timeTraceProfilerWrite(OS);
+  return Error::success();
+}
+
 void timeTraceProfilerBegin(StringRef Name, StringRef Detail) {
   if (TimeTraceProfilerInstance != nullptr)
     TimeTraceProfilerInstance->begin(std::string(Name),
                                      [&]() { return std::string(Detail); });
 }
 
 void timeTraceProfilerBegin(StringRef Name,
                             llvm::function_ref<std::string()> Detail) {