Skip to content

Conversation

@kazutakahirata
Copy link
Contributor

This patch adds a quick validity check to
InstrProfWriter::addMemProfData. Specifically, we check to see if we
have all (or none) of the MemProf profile components (frames, call
stacks, records).

The credit goes to Teresa Johnson for suggesting this assert.

This patch adds a quick validity check to
InstrProfWriter::addMemProfData.  Specifically, we check to see if we
have all (or none) of the MemProf profile components (frames, call
stacks, records).

The credit goes to Teresa Johnson for suggesting this assert.
@llvmbot llvmbot added the PGO Profile Guided Optimizations label Nov 23, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 23, 2024

@llvm/pr-subscribers-pgo

Author: Kazu Hirata (kazutakahirata)

Changes

This patch adds a quick validity check to
InstrProfWriter::addMemProfData. Specifically, we check to see if we
have all (or none) of the MemProf profile components (frames, call
stacks, records).

The credit goes to Teresa Johnson for suggesting this assert.


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

1 Files Affected:

  • (modified) llvm/lib/ProfileData/InstrProfWriter.cpp (+8-3)
diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index 725ff9256fd4a0..4470e2c58105c2 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -351,9 +351,14 @@ bool InstrProfWriter::addMemProfCallStack(
 
 bool InstrProfWriter::addMemProfData(memprof::IndexedMemProfData Incoming,
                                      function_ref<void(Error)> Warn) {
-  // TODO: Once we remove support for MemProf format Version V1, assert that
-  // the three components (frames, call stacks, and records) are either all
-  // empty or populated.
+  // Return immediately if everything is empty.
+  if (Incoming.Frames.empty() && Incoming.CallStacks.empty() &&
+      Incoming.Records.empty())
+    return true;
+
+  // Otherwise, every component must be non-empty.
+  assert(!Incoming.Frames.empty() && !Incoming.CallStacks.empty() &&
+         !Incoming.Records.empty());
 
   if (MemProfData.Frames.empty())
     MemProfData.Frames = std::move(Incoming.Frames);

@kazutakahirata kazutakahirata merged commit 9e3215a into llvm:main Nov 25, 2024
10 checks passed
@kazutakahirata kazutakahirata deleted the memprof_addMemProfData_assert branch November 25, 2024 05:08
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.

3 participants