Skip to content

[ProfileData] Refactor VTableNamePtr and CompressedVTableNamesLen (NFC) #94859

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
Jun 9, 2024

Conversation

kazutakahirata
Copy link
Contributor

VTableNamePtr and CompressedVTableNamesLen are always used together to
create a StringRef in getSymtab.

We can create the StringRef ahead of time in readHeader. This way,
IndexedInstrProfReader becomes a tiny bit simpler with fewer member
variables. Also, StringRef default-constructs itself with its Data
and Length set to nullptr and 0, respectively, which is exactly what
we need.

VTableNamePtr and CompressedVTableNamesLen are always used together to
create a StringRef in getSymtab.

We can create the StringRef ahead of time in readHeader.  This way,
IndexedInstrProfReader becomes a tiny bit simpler with fewer member
variables.  Also, StringRef default-constructs itself with its Data
and Length set to nullptr and 0, respectively, which is exactly what
we need.
@llvmbot llvmbot added the PGO Profile Guided Optimizations label Jun 8, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 8, 2024

@llvm/pr-subscribers-pgo

Author: Kazu Hirata (kazutakahirata)

Changes

VTableNamePtr and CompressedVTableNamesLen are always used together to
create a StringRef in getSymtab.

We can create the StringRef ahead of time in readHeader. This way,
IndexedInstrProfReader becomes a tiny bit simpler with fewer member
variables. Also, StringRef default-constructs itself with its Data
and Length set to nullptr and 0, respectively, which is exactly what
we need.


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

2 Files Affected:

  • (modified) llvm/include/llvm/ProfileData/InstrProfReader.h (+2-7)
  • (modified) llvm/lib/ProfileData/InstrProfReader.cpp (+5-4)
diff --git a/llvm/include/llvm/ProfileData/InstrProfReader.h b/llvm/include/llvm/ProfileData/InstrProfReader.h
index 34dba870d8da3..37f8e1612a878 100644
--- a/llvm/include/llvm/ProfileData/InstrProfReader.h
+++ b/llvm/include/llvm/ProfileData/InstrProfReader.h
@@ -693,15 +693,10 @@ class IndexedInstrProfReader : public InstrProfReader {
   /// Context sensitive profile summary data.
   std::unique_ptr<ProfileSummary> CS_Summary;
   IndexedMemProfReader MemProfReader;
-  /// VTableNamePtr points to the beginning of compressed vtable names.
-  /// When a symtab is constructed from profiles by llvm-profdata, the list of
-  /// names could be decompressed based on `VTableNamePtr` and
-  /// `CompressedVTableNamesLen`.
+  /// The compressed vtable names, to be used for symtab construction.
   /// A compiler that reads indexed profiles could construct symtab from module
   /// IR so it doesn't need the decompressed names.
-  const char *VTableNamePtr = nullptr;
-  /// The length of compressed vtable names.
-  uint64_t CompressedVTableNamesLen = 0;
+  StringRef VTableName;
   /// Total size of binary ids.
   uint64_t BinaryIdsSize{0};
   /// Start address of binary id length and data pairs.
diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp
index 7758363d9c952..ec78a50af2cb8 100644
--- a/llvm/lib/ProfileData/InstrProfReader.cpp
+++ b/llvm/lib/ProfileData/InstrProfReader.cpp
@@ -1403,14 +1403,16 @@ Error IndexedInstrProfReader::readHeader() {
   if (Header->getIndexedProfileVersion() >= 12) {
     const unsigned char *Ptr = Start + Header->VTableNamesOffset;
 
-    CompressedVTableNamesLen =
+    uint64_t CompressedVTableNamesLen =
         support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
 
     // Writer first writes the length of compressed string, and then the actual
     // content.
-    VTableNamePtr = (const char *)Ptr;
+    const char *VTableNamePtr = (const char *)Ptr;
     if (VTableNamePtr > (const char *)DataBuffer->getBufferEnd())
       return make_error<InstrProfError>(instrprof_error::truncated);
+
+    VTableName = StringRef(VTableNamePtr, CompressedVTableNamesLen);
   }
 
   if (Header->getIndexedProfileVersion() >= 10 &&
@@ -1466,8 +1468,7 @@ InstrProfSymtab &IndexedInstrProfReader::getSymtab() {
 
   auto NewSymtab = std::make_unique<InstrProfSymtab>();
 
-  if (Error E = NewSymtab->initVTableNamesFromCompressedStrings(
-          StringRef(VTableNamePtr, CompressedVTableNamesLen))) {
+  if (Error E = NewSymtab->initVTableNamesFromCompressedStrings(VTableName)) {
     auto [ErrCode, Msg] = InstrProfError::take(std::move(E));
     consumeError(error(ErrCode, Msg));
   }

Copy link
Contributor

@mingmingl-llvm mingmingl-llvm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@kazutakahirata kazutakahirata merged commit 521238d into llvm:main Jun 9, 2024
9 checks passed
@kazutakahirata kazutakahirata deleted the vtable_StringRef branch June 9, 2024 22:33
@HerrCai0907 HerrCai0907 mentioned this pull request Jun 13, 2024
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