[Driver] Support for -save-stats in AddGoldPlugin.

[fhahn]

Summary

This patch updates AddGoldPlugin to pass stats-file to the Gold plugin,
if -save-stats is passed. It also moves the save-stats option handling
to a helper function tools::getStatsFileName.

[tejohnson]

LGTM with comment nit

lib/Driver/ToolChains/CommonArgs.h

109

doxygen comment (looks like there aren't many to start with in this file, but good to add)

[compnerd]

Some cleanup suggestions included, but I like the change overall.

lib/Driver/ToolChains/CommonArgs.cpp

1283

Early return would be nicer.

const Arg *A = Args.getLastArg(options::OPT_save_stats_EQ);
if (!A)
  return {};

1297

Early return after the check would allow you to avoid the boolean and reduce indentation below too.

if (SaveStats != "cwd" && SaveStats != "obj") {
  D.Diag(drag::err_drv_invalid_value) << A->getAsString(Args) << SaveStats;
  return {};
}

lib/Driver/ToolChains/CommonArgs.h

63–64

While you're here, perhaps you can improve this a slight bit more?

D is unnecessary, ToolChain.getDriver() allows you to get the value anyways. I think it would be nicer to move the Input and Output parameters to before the IsThinLTO.

[fhahn]

Thank you very much for the review and the excellent suggestions. I simplified getStatsFileName, added a doxygen comment and changed the arguments of AddGoldPlugin as suggested

lib/Driver/ToolChains/Ananas.cpp

void ananas::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   }
 
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
   Args.AddAllArgs(CmdArgs,
                   {options::OPT_T_Group, options::OPT_e, options::OPT_s,
                    options::OPT_t, options::OPT_Z_Flag, options::OPT_r});
 
-  if (D.isUsingLTO())
-    AddGoldPlugin(ToolChain, Args, CmdArgs, D.getLTOMode() == LTOK_Thin, D);
+  if (D.isUsingLTO()) {
+    assert(!Inputs.empty() && "Must have at least one input.");
+    AddGoldPlugin(ToolChain, Args, CmdArgs, Output, Inputs[0],
+                  D.getLTOMode() == LTOK_Thin);
+  }
 
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
   if (ToolChain.ShouldLinkCXXStdlib(Args))
     ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))
     CmdArgs.push_back("-lc");
 

lib/Driver/ToolChains/Clang.cpp

   // Turn -fplugin=name.so into -load name.so
   for (const Arg *A : Args.filtered(options::OPT_fplugin_EQ)) {
     CmdArgs.push_back("-load");
     CmdArgs.push_back(A->getValue());
     A->claim();
   }
 
   // Setup statistics file output.
-  if (const Arg *A = Args.getLastArg(options::OPT_save_stats_EQ)) {
-    StringRef SaveStats = A->getValue();
-
-    SmallString<128> StatsFile;
-    bool DoSaveStats = false;
-    if (SaveStats == "obj") {
-      if (Output.isFilename()) {
-        StatsFile.assign(Output.getFilename());
-        llvm::sys::path::remove_filename(StatsFile);
-      }
-      DoSaveStats = true;
-    } else if (SaveStats == "cwd") {
-      DoSaveStats = true;
-    } else {
-      D.Diag(diag::err_drv_invalid_value) << A->getAsString(Args) << SaveStats;
-    }
-
-    if (DoSaveStats) {
-      StringRef BaseName = llvm::sys::path::filename(Input.getBaseInput());
-      llvm::sys::path::append(StatsFile, BaseName);
-      llvm::sys::path::replace_extension(StatsFile, "stats");
-      CmdArgs.push_back(Args.MakeArgString(Twine("-stats-file=") +
-                                           StatsFile));
-    }
-  }
+  SmallString<128> StatsFile = getStatsFileName(Args, Output, Input, D);
+  if (!StatsFile.empty())
+    CmdArgs.push_back(Args.MakeArgString(Twine("-stats-file=") + StatsFile));
 
   // Forward -Xclang arguments to -cc1, and -mllvm arguments to the LLVM option
   // parser.
   // -finclude-default-header flag is for preprocessor,
   // do not pass it to other cc1 commands when save-temps is enabled
   if (C.getDriver().isSaveTempsEnabled() &&
       !isa<PreprocessJobAction>(JA)) {
     for (auto Arg : Args.filtered(options::OPT_Xclang)) {

lib/Driver/ToolChains/CloudABI.cpp

void cloudabi::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   }
 
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
   Args.AddAllArgs(CmdArgs,
                   {options::OPT_T_Group, options::OPT_e, options::OPT_s,
                    options::OPT_t, options::OPT_Z_Flag, options::OPT_r});
 
-  if (D.isUsingLTO())
-    AddGoldPlugin(ToolChain, Args, CmdArgs, D.getLTOMode() == LTOK_Thin, D);
+  if (D.isUsingLTO()) {
+    assert(!Inputs.empty() && "Must have at least one input.");
+    AddGoldPlugin(ToolChain, Args, CmdArgs, Output, Inputs[0],
+                  D.getLTOMode() == LTOK_Thin);
+  }
 
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
   if (ToolChain.ShouldLinkCXXStdlib(Args))
     ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
     CmdArgs.push_back("-lc");
     CmdArgs.push_back("-lcompiler_rt");

lib/Driver/ToolChains/CommonArgs.h

 const char *SplitDebugName(const llvm::opt::ArgList &Args,
                            const InputInfo &Input);
 
 void SplitDebugInfo(const ToolChain &TC, Compilation &C, const Tool &T,
                     const JobAction &JA, const llvm::opt::ArgList &Args,
                     const InputInfo &Output, const char *OutFile);
 
 void AddGoldPlugin(const ToolChain &ToolChain, const llvm::opt::ArgList &Args,
-                   llvm::opt::ArgStringList &CmdArgs, bool IsThinLTO,
-                   const Driver &D);
+                   llvm::opt::ArgStringList &CmdArgs, const InputInfo &Output,
+                   const InputInfo &Input, bool IsThinLTO);
 
 std::tuple<llvm::Reloc::Model, unsigned, bool>
 ParsePICArgs(const ToolChain &ToolChain, const llvm::opt::ArgList &Args);
 
 unsigned ParseFunctionAlignment(const ToolChain &TC,
                                 const llvm::opt::ArgList &Args);
 
 void AddAssemblerKPIC(const ToolChain &ToolChain,
void AddTargetFeature(const llvm::opt::ArgList &Args,
                       llvm::opt::OptSpecifier OffOpt, StringRef FeatureName);
 
 std::string getCPUName(const llvm::opt::ArgList &Args, const llvm::Triple &T,
                        bool FromAs = false);
 
 void handleTargetFeaturesGroup(const llvm::opt::ArgList &Args,
                                std::vector<StringRef> &Features,
                                llvm::opt::OptSpecifier Group);
 
+/// Handles the -save-stats option and returns the filename to save statistics
+/// to.
+SmallString<128> getStatsFileName(const llvm::opt::ArgList &Args,
+                                  const InputInfo &Output,
+                                  const InputInfo &Input, const Driver &D);
 } // end namespace tools
 } // end namespace driver
 } // end namespace clang
 
 #endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_COMMONARGS_H

lib/Driver/ToolChains/CommonArgs.cpp

 //===--- CommonArgs.cpp - Args handling for multiple toolchains -*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "CommonArgs.h"
-#include "InputInfo.h"
-#include "Hexagon.h"
 #include "Arch/AArch64.h"
 #include "Arch/ARM.h"
 #include "Arch/Mips.h"
 #include "Arch/PPC.h"
 #include "Arch/SystemZ.h"
 #include "Arch/X86.h"
+#include "Hexagon.h"
+#include "InputInfo.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/ObjCRuntime.h"
 #include "clang/Basic/Version.h"
 #include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Action.h"
 #include "clang/Driver/Compilation.h"

 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/Compression.h"
+#include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetParser.h"

 }
 
 // CloudABI uses -ffunction-sections and -fdata-sections by default.
 bool tools::isUseSeparateSections(const llvm::Triple &Triple) {
   return Triple.getOS() == llvm::Triple::CloudABI;
 }
 
 void tools::AddGoldPlugin(const ToolChain &ToolChain, const ArgList &Args,
-                          ArgStringList &CmdArgs, bool IsThinLTO,
-                          const Driver &D) {
+                          ArgStringList &CmdArgs, const InputInfo &Output,
+                          const InputInfo &Input, bool IsThinLTO) {
   // Tell the linker to load the plugin. This has to come before AddLinkerInputs
   // as gold requires -plugin to come before any -plugin-opt that -Wl might
   // forward.
   CmdArgs.push_back("-plugin");
 
 #if defined(LLVM_ON_WIN32)
   const char *Suffix = ".dll";
 #elif defined(__APPLE__)
else if (A->getOption().matches(options::OPT_O0))
       OOpt = "0";
     if (!OOpt.empty())
       CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=O") + OOpt));
   }
 
   if (IsThinLTO)
     CmdArgs.push_back("-plugin-opt=thinlto");
 
-  if (unsigned Parallelism = getLTOParallelism(Args, D))
+  if (unsigned Parallelism = getLTOParallelism(Args, ToolChain.getDriver()))
     CmdArgs.push_back(
         Args.MakeArgString("-plugin-opt=jobs=" + Twine(Parallelism)));
 
   // If an explicit debugger tuning argument appeared, pass it along.
   if (Arg *A = Args.getLastArg(options::OPT_gTune_Group,
                                options::OPT_ggdbN_Group)) {
     if (A->getOption().matches(options::OPT_glldb))
       CmdArgs.push_back("-plugin-opt=-debugger-tune=lldb");
#endif
   if (Args.hasFlag(options::OPT_fdata_sections, options::OPT_fno_data_sections,
                    UseSeparateSections)) {
     CmdArgs.push_back("-plugin-opt=-data-sections");
   }
 
   if (Arg *A = getLastProfileSampleUseArg(Args)) {
     StringRef FName = A->getValue();
     if (!llvm::sys::fs::exists(FName))
-      D.Diag(diag::err_drv_no_such_file) << FName;
+      ToolChain.getDriver().Diag(diag::err_drv_no_such_file) << FName;
     else
       CmdArgs.push_back(
           Args.MakeArgString(Twine("-plugin-opt=sample-profile=") + FName));
   }
 
   // Need this flag to turn on new pass manager via Gold plugin.
   if (Args.hasFlag(options::OPT_fexperimental_new_pass_manager,
                    options::OPT_fno_experimental_new_pass_manager,
                    /* Default */ ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER)) {
     CmdArgs.push_back("-plugin-opt=new-pass-manager");
   }
 
+  // Setup statistics file output.
+  SmallString<128> StatsFile =
+      getStatsFileName(Args, Output, Input, ToolChain.getDriver());
+  if (!StatsFile.empty())
+    CmdArgs.push_back(
+        Args.MakeArgString(Twine("-plugin-opt=stats-file=") + StatsFile));
 }
 
 void tools::addArchSpecificRPath(const ToolChain &TC, const ArgList &Args,
                                  ArgStringList &CmdArgs) {
   if (!Args.hasFlag(options::OPT_frtlib_add_rpath,
                     options::OPT_fno_rtlib_add_rpath, false))
     return;
 
void tools::AddOpenMPLinkerScript(const ToolChain &TC, Compilation &C,
 
   if (EC) {
     C.getDriver().Diag(clang::diag::err_unable_to_make_temp) << EC.message();
     return;
   }
 
   Lksf << LksBuffer;
 }
+
+SmallString<128> tools::getStatsFileName(const llvm::opt::ArgList &Args,
+                                         const InputInfo &Output,
+                                         const InputInfo &Input,
+                                         const Driver &D) {
+  const Arg *A = Args.getLastArg(options::OPT_save_stats_EQ);
+  if (!A)
+    return {};
+
+  StringRef SaveStats = A->getValue();
+  SmallString<128> StatsFile;
+  if (SaveStats == "obj" && Output.isFilename()) {
+    StatsFile.assign(Output.getFilename());
+    llvm::sys::path::remove_filename(StatsFile);
+  } else if (SaveStats != "cwd") {
+    D.Diag(diag::err_drv_invalid_value) << A->getAsString(Args) << SaveStats;
+    return {};
+  }
+
+  StringRef BaseName = llvm::sys::path::filename(Input.getBaseInput());
+  llvm::sys::path::append(StatsFile, BaseName);
+  llvm::sys::path::replace_extension(StatsFile, "stats");
+  return StatsFile;
+}

lib/Driver/ToolChains/FreeBSD.cpp

void freebsd::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
   Args.AddAllArgs(CmdArgs, options::OPT_T_Group);
   Args.AddAllArgs(CmdArgs, options::OPT_e);
   Args.AddAllArgs(CmdArgs, options::OPT_s);
   Args.AddAllArgs(CmdArgs, options::OPT_t);
   Args.AddAllArgs(CmdArgs, options::OPT_Z_Flag);
   Args.AddAllArgs(CmdArgs, options::OPT_r);
 
-  if (D.isUsingLTO())
-    AddGoldPlugin(ToolChain, Args, CmdArgs, D.getLTOMode() == LTOK_Thin, D);
+  if (D.isUsingLTO()) {
+    assert(!Inputs.empty() && "Must have at least one input.");
+    AddGoldPlugin(ToolChain, Args, CmdArgs, Output, Inputs[0],
+                  D.getLTOMode() == LTOK_Thin);
+  }
 
   bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
   bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
     addOpenMPRuntime(CmdArgs, ToolChain, Args);
     if (D.CCCIsCXX()) {

lib/Driver/ToolChains/Gnu.cpp

if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
     ToolChain.AddFastMathRuntimeIfAvailable(Args, CmdArgs);
   }
 
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   Args.AddAllArgs(CmdArgs, options::OPT_u);
 
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
 
-  if (D.isUsingLTO())
-    AddGoldPlugin(ToolChain, Args, CmdArgs, D.getLTOMode() == LTOK_Thin, D);
+  if (D.isUsingLTO()) {
+    assert(!Inputs.empty() && "Must have at least one input.");
+    AddGoldPlugin(ToolChain, Args, CmdArgs, Output, Inputs[0],
+                  D.getLTOMode() == LTOK_Thin);
+  }
 
   if (Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
     CmdArgs.push_back("--no-demangle");
 
   bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
   bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
   // The profile runtime also needs access to system libraries.

test/Driver/save-stats.c

 // CHECK-OBJ: "-o" "obj/dir{{/|\\\\}}save-stats.o"
 
 // RUN: %clang -target x86_64-apple-darwin -save-stats=obj -c %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-OBJ-NOO
 // CHECK-OBJ-NOO: "-stats-file=save-stats.stats"
 // CHECK-OBJ-NOO: "-o" "save-stats.o"
 
 // RUN: %clang -target x86_64-apple-darwin -save-stats=bla -c %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-INVALID
 // CHECK-INVALID: invalid value 'bla' in '-save-stats=bla'
+
+// RUN: %clang -target x86_64-linux-unknown -save-stats -flto -o obj/dir/save-stats.exe %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-LTO
+// CHECK-LTO: "-stats-file=save-stats.stats"
+// CHECK-LTO: "-o" "obj/dir{{/|\\\\}}save-stats.exe"
+// CHECK-LTO: "-plugin-opt=stats-file=save-stats.stats"
+
+// RUN: %clang -target x86_64-linux-unknown -save-stats=obj -flto -o obj/dir/save-stats.exe %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-LTO-OBJ
+// CHECK-LTO-OBJ: "-plugin-opt=stats-file=obj/dir/save-stats.stats"