Skip to content

Conversation

@kazutakahirata
Copy link
Contributor

For Frames, we prefer the inline notation for the brevity.

For PortableMemInfoBlock, we go through all member fields and print
out those that are populated.

For Frames, we prefer the inline notation for the brevity.

For PortableMemInfoBlock, we go through all member fields and print
out those that are populated.
@llvmbot llvmbot added the PGO Profile Guided Optimizations label Dec 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 5, 2024

@llvm/pr-subscribers-pgo

Author: Kazu Hirata (kazutakahirata)

Changes

For Frames, we prefer the inline notation for the brevity.

For PortableMemInfoBlock, we go through all member fields and print
out those that are populated.


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

2 Files Affected:

  • (modified) llvm/include/llvm/ProfileData/MemProf.h (+12-2)
  • (modified) llvm/unittests/ProfileData/MemProfTest.cpp (+34)
diff --git a/llvm/include/llvm/ProfileData/MemProf.h b/llvm/include/llvm/ProfileData/MemProf.h
index fe017913f6de24..88666087007c91 100644
--- a/llvm/include/llvm/ProfileData/MemProf.h
+++ b/llvm/include/llvm/ProfileData/MemProf.h
@@ -1177,6 +1177,11 @@ template <> struct MappingTraits<memprof::Frame> {
     (void)Column;
     (void)IsInlineFrame;
   }
+
+  // Request the inline notation for brevity:
+  //
+  //   { Function: 123, LineOffset: 11, Column: 10; IsInlineFrame: true }
+  static const bool flow = true;
 };
 
 template <> struct CustomMappingTraits<memprof::PortableMemInfoBlock> {
@@ -1201,8 +1206,13 @@ template <> struct CustomMappingTraits<memprof::PortableMemInfoBlock> {
     Io.setError("Key is not a valid validation event");
   }
 
-  static void output(IO &Io, memprof::PortableMemInfoBlock &VI) {
-    llvm_unreachable("To be implemented");
+  static void output(IO &Io, memprof::PortableMemInfoBlock &MIB) {
+    auto Schema = MIB.getSchema();
+#define MIBEntryDef(NameTag, Name, Type)                                       \
+  if (Schema.test(llvm::to_underlying(memprof::Meta::Name)))                   \
+    Io.mapRequired(#Name, MIB.Name);
+#include "llvm/ProfileData/MIBEntryDef.inc"
+#undef MIBEntryDef
   }
 };
 
diff --git a/llvm/unittests/ProfileData/MemProfTest.cpp b/llvm/unittests/ProfileData/MemProfTest.cpp
index 2f8589bbfbb962..0f3af93b096852 100644
--- a/llvm/unittests/ProfileData/MemProfTest.cpp
+++ b/llvm/unittests/ProfileData/MemProfTest.cpp
@@ -807,4 +807,38 @@ TEST(MemProf, YAMLParser) {
   EXPECT_THAT(Record.CallSiteIds,
               ElementsAre(hashCallStack(CS3), hashCallStack(CS4)));
 }
+
+TEST(MemProf, YAMLWriterFrame) {
+  Frame F(12, 34, 56, true);
+
+  std::string Out;
+  llvm::raw_string_ostream OS(Out);
+  llvm::yaml::Output Yout(OS);
+  Yout << F;
+  EXPECT_EQ(Out, R"YAML(---
+{ Function: 12, LineOffset: 34, Column: 56, Inline: true }
+...
+)YAML");
+}
+
+TEST(MemProf, YAMLWriterMIB) {
+  MemInfoBlock MIB;
+  MIB.AllocCount = 111;
+  MIB.TotalSize = 222;
+  MIB.TotalLifetime = 333;
+  MIB.TotalLifetimeAccessDensity = 444;
+  PortableMemInfoBlock PMIB(MIB, llvm::memprof::getHotColdSchema());
+
+  std::string Out;
+  llvm::raw_string_ostream OS(Out);
+  llvm::yaml::Output Yout(OS);
+  Yout << PMIB;
+  EXPECT_EQ(Out, R"YAML(---
+AllocCount:      111
+TotalSize:       222
+TotalLifetime:   333
+TotalLifetimeAccessDensity: 444
+...
+)YAML");
+}
 } // namespace

@kazutakahirata kazutakahirata merged commit 7b8cf14 into llvm:main Dec 5, 2024
8 checks passed
@kazutakahirata kazutakahirata deleted the memprof_YAML_writer branch December 5, 2024 03:23
@dyung
Copy link
Collaborator

dyung commented Dec 5, 2024

@fhahn
Copy link
Contributor

fhahn commented Dec 5, 2024

Reverted for now as this has been breaking macOS builds for 8+ hours

@kazutakahirata
Copy link
Contributor Author

@dyung @fhahn Thank you for taking care of the revert! I'll take a look.

kazutakahirata added a commit that referenced this pull request Dec 5, 2024
For Frames, we prefer the inline notation for the brevity.

For PortableMemInfoBlock, we go through all member fields and print
out those that are populated.

This iteration works around the unavailability of
ScalarTraits<uintptr_t> on macOS.
@fhahn
Copy link
Contributor

fhahn commented Dec 6, 2024

Great thanks! Would also be good if you could keep an eye out for build failures from the build bots. I think https://lab.llvm.org/buildbot/#/builders/190 and https://lab.llvm.org/buildbot/#/builders/23/builds/5491 should send emails, Green Dragon unfortunately doesn't at the moment :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PGO Profile Guided Optimizations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants