Skip to content

[memprof] Use std::optional (NFC) #88366

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
Apr 11, 2024

Conversation

kazutakahirata
Copy link
Contributor

No description provided.

@llvmbot llvmbot added the PGO Profile Guided Optimizations label Apr 11, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 11, 2024

@llvm/pr-subscribers-pgo

Author: Kazu Hirata (kazutakahirata)

Changes

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

2 Files Affected:

  • (modified) llvm/lib/ProfileData/InstrProfReader.cpp (+4-5)
  • (modified) llvm/unittests/ProfileData/InstrProfTest.cpp (+4-5)
diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp
index 884334ed070e8d..a35366a106a322 100644
--- a/llvm/lib/ProfileData/InstrProfReader.cpp
+++ b/llvm/lib/ProfileData/InstrProfReader.cpp
@@ -33,6 +33,7 @@
 #include <cstdint>
 #include <limits>
 #include <memory>
+#include <optional>
 #include <system_error>
 #include <utility>
 #include <vector>
@@ -1506,13 +1507,11 @@ IndexedInstrProfReader::getMemProfRecord(const uint64_t FuncNameHash) {
 
   // Setup a callback to convert from frame ids to frame using the on-disk
   // FrameData hash table.
-  memprof::FrameId LastUnmappedFrameId = 0;
-  bool HasFrameMappingError = false;
+  std::optional<memprof::FrameId> LastUnmappedFrameId;
   auto IdToFrameCallback = [&](const memprof::FrameId Id) {
     auto FrIter = MemProfFrameTable->find(Id);
     if (FrIter == MemProfFrameTable->end()) {
       LastUnmappedFrameId = Id;
-      HasFrameMappingError = true;
       return memprof::Frame(0, 0, 0, false);
     }
     return *FrIter;
@@ -1521,10 +1520,10 @@ IndexedInstrProfReader::getMemProfRecord(const uint64_t FuncNameHash) {
   memprof::MemProfRecord Record(*Iter, IdToFrameCallback);
 
   // Check that all frame ids were successfully converted to frames.
-  if (HasFrameMappingError) {
+  if (LastUnmappedFrameId) {
     return make_error<InstrProfError>(instrprof_error::hash_mismatch,
                                       "memprof frame not found for frame id " +
-                                          Twine(LastUnmappedFrameId));
+                                          Twine(*LastUnmappedFrameId));
   }
   return Record;
 }
diff --git a/llvm/unittests/ProfileData/InstrProfTest.cpp b/llvm/unittests/ProfileData/InstrProfTest.cpp
index 732f8fd792f8de..82701c47daf71c 100644
--- a/llvm/unittests/ProfileData/InstrProfTest.cpp
+++ b/llvm/unittests/ProfileData/InstrProfTest.cpp
@@ -19,6 +19,7 @@
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 #include <cstdarg>
+#include <optional>
 
 using namespace llvm;
 using ::testing::EndsWith;
@@ -433,21 +434,19 @@ TEST_F(InstrProfTest, test_memprof) {
   ASSERT_THAT_ERROR(RecordOr.takeError(), Succeeded());
   const memprof::MemProfRecord &Record = RecordOr.get();
 
-  memprof::FrameId LastUnmappedFrameId = 0;
-  bool HasFrameMappingError = false;
+  std::optional<memprof::FrameId> LastUnmappedFrameId;
   auto IdToFrameCallback = [&](const memprof::FrameId Id) {
     auto Iter = IdToFrameMap.find(Id);
     if (Iter == IdToFrameMap.end()) {
       LastUnmappedFrameId = Id;
-      HasFrameMappingError = true;
       return memprof::Frame(0, 0, 0, false);
     }
     return Iter->second;
   };
 
   const memprof::MemProfRecord WantRecord(IndexedMR, IdToFrameCallback);
-  ASSERT_FALSE(HasFrameMappingError)
-      << "could not map frame id: " << LastUnmappedFrameId;
+  ASSERT_FALSE(LastUnmappedFrameId.has_value())
+      << "could not map frame id: " << *LastUnmappedFrameId;
   EXPECT_THAT(WantRecord, EqualsRecord(Record));
 }
 

@kazutakahirata kazutakahirata merged commit db9a17a into llvm:main Apr 11, 2024
@kazutakahirata kazutakahirata deleted the pr_memprof_optional branch April 11, 2024 16:56
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