Skip to content

[llvm-profdata] Fix detailed summary format on Windows #124169

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

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

ellishg
Copy link
Contributor

@ellishg ellishg commented Jan 23, 2025

The detailed summary format was changed in #105915 which broke llvm/test/tools/llvm-profdata/general.proftext (XFAILed in #124165). Apparently the behavior of %lu is different between Linux and Windows, so I reverted back to using << style formats.

@llvmbot llvmbot added PGO Profile Guided Optimizations llvm:ir labels Jan 23, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 23, 2025

@llvm/pr-subscribers-pgo

@llvm/pr-subscribers-llvm-ir

Author: Ellis Hoag (ellishg)

Changes

The detailed summary format was changed in #105915 which broke llvm/test/tools/llvm-profdata/general.proftext (XFAILed in #124165). Apparently the behavior of %lu is different between Linux and Windows, so I reverted back to using &lt;&lt; style formats.


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

2 Files Affected:

  • (modified) llvm/lib/IR/ProfileSummary.cpp (+6-5)
  • (modified) llvm/test/tools/llvm-profdata/general.proftext (-2)
diff --git a/llvm/lib/IR/ProfileSummary.cpp b/llvm/lib/IR/ProfileSummary.cpp
index 59f29e5982dfb0..12ae81ca8d5f3f 100644
--- a/llvm/lib/IR/ProfileSummary.cpp
+++ b/llvm/lib/IR/ProfileSummary.cpp
@@ -259,10 +259,11 @@ void ProfileSummary::printSummary(raw_ostream &OS) const {
 void ProfileSummary::printDetailedSummary(raw_ostream &OS) const {
   OS << "Detailed summary:\n";
   for (const auto &Entry : DetailedSummary) {
-    OS << format("%lu blocks (%.2f%%) with count >= %lu account for %0.6g%% of "
-                 "the total counts.\n",
-                 Entry.NumCounts,
-                 NumCounts ? (100.f * Entry.NumCounts / NumCounts) : 0,
-                 Entry.MinCount, 100.f * Entry.Cutoff / Scale);
+    OS << Entry.NumCounts << " blocks "
+       << format("(%.2f%%)",
+                 NumCounts ? (100.f * Entry.NumCounts / NumCounts) : 0)
+       << " with count >= " << Entry.MinCount << " account for "
+       << format("%0.6g", 100.f * Entry.Cutoff / Scale)
+       << "% of the total counts.\n";
   }
 }
diff --git a/llvm/test/tools/llvm-profdata/general.proftext b/llvm/test/tools/llvm-profdata/general.proftext
index ca532f9a37116d..89762f2540f6a6 100644
--- a/llvm/test/tools/llvm-profdata/general.proftext
+++ b/llvm/test/tools/llvm-profdata/general.proftext
@@ -1,5 +1,3 @@
-# FIXME: Somehow this is failing on windows after https://github.com/llvm/llvm-project/pull/105915
-# XFAIL: system-windows
 # RUN: llvm-profdata merge -sparse=true %s -o %t.profdata
 
 # RUN: llvm-profdata merge -sparse=false %s -o %t.profdata.dense

@ellishg ellishg merged commit d87441a into llvm:main Jan 23, 2025
11 checks passed
@ellishg ellishg deleted the fix-profdata-summary-format branch January 23, 2025 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:ir PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants