Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Clang][Driver][LTO] Change the filename format for LTO'd stats file #70242

Closed
wants to merge 1 commit into from

Conversation

mshockwave
Copy link
Member

Use ".ld.stats" instead of ".stats" for stats file generated by LTO backend. The new extension makes it easier to search for LTO'd stats file and be consistent with LTO'd optimization remarks files' naming convention (i.e. *.opt.ld.yaml, as opposed to *.opt.yaml for normal opt remarks files).

Use "<output filename>.ld.stats" instead of "<base input filename>.stats"
for stats file generated by LTO backend. The new extension makes it
easier to search for LTO'd stats file and be consistent with LTO'd
optimization remarks files' naming convention (i.e. *.opt.ld.yaml,
as opposed to *.opt.yaml for normal opt remarks files).
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Oct 25, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 25, 2023

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Min-Yih Hsu (mshockwave)

Changes

Use "<output filename>.ld.stats" instead of "<base input filename>.stats" for stats file generated by LTO backend. The new extension makes it easier to search for LTO'd stats file and be consistent with LTO'd optimization remarks files' naming convention (i.e. *.opt.ld.yaml, as opposed to *.opt.yaml for normal opt remarks files).


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

3 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+15-4)
  • (modified) clang/lib/Driver/ToolChains/CommonArgs.h (+2-1)
  • (modified) clang/test/Driver/save-stats.c (+3-2)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index ad012d3d0d4b46f..98e20abc015c5fd 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -852,7 +852,8 @@ void tools::addLTOOptions(const ToolChain &ToolChain, const ArgList &Args,
         Args.MakeArgString(Twine(PluginOptPrefix) + "-stack-size-section"));
 
   // Setup statistics file output.
-  SmallString<128> StatsFile = getStatsFileName(Args, Output, Input, D);
+  SmallString<128> StatsFile =
+      getStatsFileName(Args, Output, Input, D, /*IsLTO=*/true);
   if (!StatsFile.empty())
     CmdArgs.push_back(
         Args.MakeArgString(Twine(PluginOptPrefix) + "stats-file=" + StatsFile));
@@ -1941,7 +1942,7 @@ void tools::AddRunTimeLibs(const ToolChain &TC, const Driver &D,
 SmallString<128> tools::getStatsFileName(const llvm::opt::ArgList &Args,
                                          const InputInfo &Output,
                                          const InputInfo &Input,
-                                         const Driver &D) {
+                                         const Driver &D, bool IsLTO) {
   const Arg *A = Args.getLastArg(options::OPT_save_stats_EQ);
   if (!A && !D.CCPrintInternalStats)
     return {};
@@ -1957,9 +1958,19 @@ SmallString<128> tools::getStatsFileName(const llvm::opt::ArgList &Args,
       return {};
     }
 
-    StringRef BaseName = llvm::sys::path::filename(Input.getBaseInput());
+    StringRef BaseName;
+    // For stats files generated by the LTO backend, we're using "<output
+    // filename>.ld.stats" for its name. Note that we don't query
+    // `options::OPT_flto_EQ` and `options::OPT_fno_lto` here to decide `IsLTO`
+    // because it also changes the stats filenames for the pre-link object (LLVM
+    // bitcode) files, which we want to keep the same.
+    if (IsLTO && Output.isFilename())
+      BaseName = llvm::sys::path::filename(Output.getFilename());
+    else
+      BaseName = llvm::sys::path::filename(Input.getBaseInput());
+
     llvm::sys::path::append(StatsFile, BaseName);
-    llvm::sys::path::replace_extension(StatsFile, "stats");
+    llvm::sys::path::replace_extension(StatsFile, IsLTO ? "ld.stats" : "stats");
   } else {
     assert(D.CCPrintInternalStats);
     StatsFile.assign(D.CCPrintInternalStatReportFilename.empty()
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.h b/clang/lib/Driver/ToolChains/CommonArgs.h
index f364c9793c9be62..bab80af5fb2f67a 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.h
+++ b/clang/lib/Driver/ToolChains/CommonArgs.h
@@ -184,7 +184,8 @@ SmallVector<StringRef> unifyTargetFeatures(ArrayRef<StringRef> Features);
 /// to.
 SmallString<128> getStatsFileName(const llvm::opt::ArgList &Args,
                                   const InputInfo &Output,
-                                  const InputInfo &Input, const Driver &D);
+                                  const InputInfo &Input, const Driver &D,
+                                  bool IsLTO = false);
 
 /// \p Flag must be a flag accepted by the driver.
 void addMultilibFlag(bool Enabled, const StringRef Flag,
diff --git a/clang/test/Driver/save-stats.c b/clang/test/Driver/save-stats.c
index ca8f2a457d4488c..44a6ddabde2a2e9 100644
--- a/clang/test/Driver/save-stats.c
+++ b/clang/test/Driver/save-stats.c
@@ -22,10 +22,11 @@
 // 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"
+// CHECK-LTO: "-plugin-opt=stats-file=save-stats.ld.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"
+// CHECK-LTO-OBJ: "-stats-file={{.*}}save-stats.stats"
+// CHECK-LTO-OBJ: "-plugin-opt=stats-file=obj/dir{{/|\\\\}}save-stats.ld.stats"
 
 // RUN: env CC_PRINT_INTERNAL_STAT=1 \
 // RUN: %clang -target x86_64-apple-darwin %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-ENV

@MaskRay
Copy link
Member

MaskRay commented Oct 26, 2023

I can understand the rationale, but adding this special case feels stranger to me..

@mshockwave
Copy link
Member Author

I can understand the rationale, but adding this special case feels stranger to me..

I'm fine with not having a special file extension for LTO'd stats file, hence closing this PR.
That said, it would be really helpful if you could help me to review a related PR #71197 .

@mshockwave mshockwave closed this Nov 3, 2023
@mshockwave mshockwave deleted the patch/lto-stats-filename branch November 3, 2023 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants