[llvm] 9fd2cb2 - Revert "[InstrProf] Make the IndexedInstrProf header backwards compatible."

Snehasish Kumar via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 14 11:43:42 PST 2022


Author: Snehasish Kumar
Date: 2022-02-14T11:42:58-08:00
New Revision: 9fd2cb21fb3f763fc784eab198bf1297a24596fa

URL: https://github.com/llvm/llvm-project/commit/9fd2cb21fb3f763fc784eab198bf1297a24596fa
DIFF: https://github.com/llvm/llvm-project/commit/9fd2cb21fb3f763fc784eab198bf1297a24596fa.diff

LOG: Revert "[InstrProf] Make the IndexedInstrProf header backwards compatible."

This reverts commit 14cc41a0206a85d350767f8aff6e02bd4e7dd5d6. [2/4]

Added: 
    

Modified: 
    llvm/include/llvm/ProfileData/InstrProf.h
    llvm/lib/ProfileData/InstrProf.cpp
    llvm/lib/ProfileData/InstrProfReader.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index 83f599b90ae9b..a416eb28906e7 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -1028,16 +1028,6 @@ struct Header {
   uint64_t Unused; // Becomes unused since version 4
   uint64_t HashType;
   uint64_t HashOffset;
-  // New fields should only be added at the end to ensure that the size
-  // computation is correct. The methods below need to be updated to ensure that
-  // the new field is read correctly.
-
-  // Reads a header struct from the buffer.
-  static Expected<Header> readFromBuffer(const unsigned char *Buffer);
-
-  // Returns the size of the header in bytes for all valid fields based on the
-  // version. I.e a older version header will return a smaller size.
-  size_t size() const;
 };
 
 // Profile summary data recorded in the profile data file in indexed

diff  --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index 6bdca486205ef..07d467305ae5a 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -51,7 +51,6 @@
 #include <memory>
 #include <string>
 #include <system_error>
-#include <type_traits>
 #include <utility>
 #include <vector>
 
@@ -1312,59 +1311,4 @@ void OverlapStats::dump(raw_fd_ostream &OS) const {
   }
 }
 
-namespace IndexedInstrProf {
-// A C++14 compatible version of the offsetof macro.
-template <typename T1, typename T2>
-inline size_t constexpr offsetOf(T1 T2::*Member) {
-  constexpr T2 Object{};
-  return size_t(&(Object.*Member)) - size_t(&Object);
-}
-
-static inline uint64_t read(const unsigned char *Buffer, size_t Offset) {
-  return *reinterpret_cast<const uint64_t *>(Buffer + Offset);
-}
-
-Expected<Header> Header::readFromBuffer(const unsigned char *Buffer) {
-  using namespace support;
-  static_assert(std::is_standard_layout<Header>::value,
-                "The header should be standard layout type since we use offset "
-                "of fields to read.");
-  Header H;
-
-  H.Magic = read(Buffer, offsetOf(&Header::Magic));
-  // Check the magic number.
-  uint64_t Magic = endian::byte_swap<uint64_t, little>(H.Magic);
-  if (Magic != IndexedInstrProf::Magic)
-    return make_error<InstrProfError>(instrprof_error::bad_magic);
-
-  // Read the version.
-  H.Version = read(Buffer, offsetOf(&Header::Version));
-  uint64_t FormatVersion = endian::byte_swap<uint64_t, little>(H.Version);
-  if (GET_VERSION(FormatVersion) >
-      IndexedInstrProf::ProfVersion::CurrentVersion)
-    return make_error<InstrProfError>(instrprof_error::unsupported_version);
-
-  switch (GET_VERSION(FormatVersion)) {
-  // When a new field is added in the header add a case statement here to
-  // populate it.
-  default:
-    H.HashType = read(Buffer, offsetOf(&Header::HashType));
-    H.HashOffset = read(Buffer, offsetOf(&Header::HashOffset));
-  }
-
-  return H;
-}
-
-size_t Header::size() const {
-  switch (GET_VERSION(Version)) {
-  // When a new field is added to the header add a case statement here to
-  // compute the size as offset of the new field + size of the new field. This
-  // relies on the field being added to the end of the list.
-  default:
-    return offsetOf(&Header::HashOffset) + sizeof(Header::HashOffset);
-  }
-}
-
-} // namespace IndexedInstrProf
-
 } // end namespace llvm

diff  --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp
index c45b8cde2cda0..bc990755e0e47 100644
--- a/llvm/lib/ProfileData/InstrProfReader.cpp
+++ b/llvm/lib/ProfileData/InstrProfReader.cpp
@@ -934,17 +934,24 @@ Error IndexedInstrProfReader::readHeader() {
   if ((const unsigned char *)DataBuffer->getBufferEnd() - Cur < 24)
     return error(instrprof_error::truncated);
 
-  auto HeaderOr = IndexedInstrProf::Header::readFromBuffer(Start);
-  if (!HeaderOr)
-    return HeaderOr.takeError();
+  auto *Header = reinterpret_cast<const IndexedInstrProf::Header *>(Cur);
+  Cur += sizeof(IndexedInstrProf::Header);
 
-  const IndexedInstrProf::Header *Header = &HeaderOr.get();
-  Cur += Header->size();
+  // Check the magic number.
+  uint64_t Magic = endian::byte_swap<uint64_t, little>(Header->Magic);
+  if (Magic != IndexedInstrProf::Magic)
+    return error(instrprof_error::bad_magic);
+
+  // Read the version.
+  uint64_t FormatVersion = endian::byte_swap<uint64_t, little>(Header->Version);
+  if (GET_VERSION(FormatVersion) >
+      IndexedInstrProf::ProfVersion::CurrentVersion)
+    return error(instrprof_error::unsupported_version);
 
-  Cur = readSummary((IndexedInstrProf::ProfVersion)Header->Version, Cur,
+  Cur = readSummary((IndexedInstrProf::ProfVersion)FormatVersion, Cur,
                     /* UseCS */ false);
-  if (Header->Version & VARIANT_MASK_CSIR_PROF)
-    Cur = readSummary((IndexedInstrProf::ProfVersion)Header->Version, Cur,
+  if (FormatVersion & VARIANT_MASK_CSIR_PROF)
+    Cur = readSummary((IndexedInstrProf::ProfVersion)FormatVersion, Cur,
                       /* UseCS */ true);
 
   // Read the hash type and start offset.
@@ -956,8 +963,9 @@ Error IndexedInstrProfReader::readHeader() {
   uint64_t HashOffset = endian::byte_swap<uint64_t, little>(Header->HashOffset);
 
   // The rest of the file is an on disk hash table.
-  auto IndexPtr = std::make_unique<InstrProfReaderIndex<OnDiskHashTableImplV3>>(
-      Start + HashOffset, Cur, Start, HashType, Header->Version);
+  auto IndexPtr =
+      std::make_unique<InstrProfReaderIndex<OnDiskHashTableImplV3>>(
+          Start + HashOffset, Cur, Start, HashType, FormatVersion);
 
   // Load the remapping table now if requested.
   if (RemappingBuffer) {


        


More information about the llvm-commits mailing list