[llvm] [nfc][InstrProf]Remove 'offsetOf' when parsing indexed profiles (PR #93346)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-pgo

Author: Mingming Liu (minglotus-6)

<details>
<summary>Changes</summary>

- In `Header::readFromBuffer`, read the buffer in the forward direction by using `readNext`.
- When compute the header size, spell out the constant.

With the changes above, we can remove `offsetOf` in InstrProf.cpp

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


1 Files Affected:

- (modified) llvm/lib/ProfileData/InstrProf.cpp (+38-59) 


``````````diff
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index dcf6aac8b5996..8fc5d8e2a0bba 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -1627,66 +1627,46 @@ 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);
-}
-
-// Read a uint64_t from the specified buffer offset, and swap the bytes in
-// native endianness if necessary.
-static inline uint64_t read(const unsigned char *Buffer, size_t Offset) {
-  using namespace ::support;
-  return endian::read<uint64_t, llvm::endianness::little, unaligned>(Buffer +
-                                                                     Offset);
-}
-
 Expected<Header> Header::readFromBuffer(const unsigned char *Buffer) {
   using namespace support;
   static_assert(std::is_standard_layout_v<Header>,
-                "The header should be standard layout type since we use offset "
-                "of fields to read.");
+                "Use standard layout for Header for simplicity");
   Header H;
 
-  H.Magic = read(Buffer, offsetOf(&Header::Magic));
+  H.Magic =
+      endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Buffer);
   // Check the magic number.
   if (H.Magic != IndexedInstrProf::Magic)
     return make_error<InstrProfError>(instrprof_error::bad_magic);
 
   // Read the version.
-  H.Version = read(Buffer, offsetOf(&Header::Version));
+  H.Version =
+      endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Buffer);
   if (H.getIndexedProfileVersion() >
       IndexedInstrProf::ProfVersion::CurrentVersion)
     return make_error<InstrProfError>(instrprof_error::unsupported_version);
 
-  switch (H.getIndexedProfileVersion()) {
-    // When a new field is added in the header add a case statement here to
-    // populate it.
-    static_assert(
-        IndexedInstrProf::ProfVersion::CurrentVersion == Version12,
-        "Please update the reading code below if a new field has been added, "
-        "if not add a case statement to fall through to the latest version.");
-  case 12ull:
-    H.VTableNamesOffset = read(Buffer, offsetOf(&Header::VTableNamesOffset));
-    [[fallthrough]];
-  case 11ull:
-    [[fallthrough]];
-  case 10ull:
+  static_assert(IndexedInstrProf::ProfVersion::CurrentVersion == Version12,
+                "Please update the reading as needed when a new field is added "
+                "or when indexed profile version gets bumped.");
+
+  Buffer += sizeof(uint64_t); // Skip Header.Unused field.
+  H.HashType =
+      endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Buffer);
+  H.HashOffset =
+      endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Buffer);
+  if (H.getIndexedProfileVersion() >= 8)
+    H.MemProfOffset =
+        endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Buffer);
+  if (H.getIndexedProfileVersion() >= 9)
+    H.BinaryIdOffset =
+        endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Buffer);
+  if (H.getIndexedProfileVersion() >= 10)
     H.TemporalProfTracesOffset =
-        read(Buffer, offsetOf(&Header::TemporalProfTracesOffset));
-    [[fallthrough]];
-  case 9ull:
-    H.BinaryIdOffset = read(Buffer, offsetOf(&Header::BinaryIdOffset));
-    [[fallthrough]];
-  case 8ull:
-    H.MemProfOffset = read(Buffer, offsetOf(&Header::MemProfOffset));
-    [[fallthrough]];
-  default: // Version7 (when the backwards compatible header was introduced).
-    H.HashType = read(Buffer, offsetOf(&Header::HashType));
-    H.HashOffset = read(Buffer, offsetOf(&Header::HashOffset));
-  }
-
+        endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Buffer);
+  if (H.getIndexedProfileVersion() >= 12)
+    H.VTableNamesOffset =
+        endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Buffer);
   return H;
 }
 
@@ -1696,27 +1676,26 @@ uint64_t Header::getIndexedProfileVersion() const {
 
 size_t Header::size() const {
   switch (getIndexedProfileVersion()) {
-    // 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.
-    static_assert(IndexedInstrProf::ProfVersion::CurrentVersion == Version12,
-                  "Please update the size computation below if a new field has "
-                  "been added to the header, if not add a case statement to "
-                  "fall through to the latest version.");
+    // To retain backward compatibility, new fields must be appended to the end
+    // of the header, and byte offset of existing fields shouldn't change when
+    // indexed profile version gets incremented.
+    static_assert(
+        IndexedInstrProf::ProfVersion::CurrentVersion == Version12,
+        "Please update the size computation below if a new field has "
+        "been added to the header; for a version bump without new "
+        "fields, add a case statement to fall through to the latest version.");
   case 12ull:
-    return offsetOf(&Header::VTableNamesOffset) +
-           sizeof(Header::VTableNamesOffset);
+    return 72;
   case 11ull:
     [[fallthrough]];
   case 10ull:
-    return offsetOf(&Header::TemporalProfTracesOffset) +
-           sizeof(Header::TemporalProfTracesOffset);
+    return 64;
   case 9ull:
-    return offsetOf(&Header::BinaryIdOffset) + sizeof(Header::BinaryIdOffset);
+    return 56;
   case 8ull:
-    return offsetOf(&Header::MemProfOffset) + sizeof(Header::MemProfOffset);
+    return 48;
   default: // Version7 (when the backwards compatible header was introduced).
-    return offsetOf(&Header::HashOffset) + sizeof(Header::HashOffset);
+    return 40;
   }
 }
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/93346