[llvm] a3beb34 - Reland "[InstrProf] Make the IndexedInstrProf header backwards compatible."
Snehasish Kumar via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 17 11:43:49 PST 2022
Author: Snehasish Kumar
Date: 2022-02-17T11:40:32-08:00
New Revision: a3beb34015fcc5b61e804736247781a80554443a
URL: https://github.com/llvm/llvm-project/commit/a3beb34015fcc5b61e804736247781a80554443a
DIFF: https://github.com/llvm/llvm-project/commit/a3beb34015fcc5b61e804736247781a80554443a.diff
LOG: Reland "[InstrProf] Make the IndexedInstrProf header backwards compatible."
This reverts commit 9fd2cb21fb3f763fc784eab198bf1297a24596fa.
Fixes an issue on big endian systems where the format version
was not converted to little endian prior to passing to GET_VERSION.
Differential Revision: https://reviews.llvm.org/D118390
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 a416eb28906e..c015e8e4b43d 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -1028,6 +1028,20 @@ 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;
+
+ // Returns the format version in little endian. The header retains the version
+ // in native endian of the compiler runtime.
+ uint64_t formatVersion() 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 07d467305ae5..6e53b0a27699 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -51,6 +51,7 @@
#include <memory>
#include <string>
#include <system_error>
+#include <type_traits>
#include <utility>
#include <vector>
@@ -1311,4 +1312,63 @@ 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);
+}
+
+uint64_t Header::formatVersion() const {
+ using namespace support;
+ return endian::byte_swap<uint64_t, little>(Version);
+}
+
+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));
+ if (GET_VERSION(H.formatVersion()) >
+ IndexedInstrProf::ProfVersion::CurrentVersion)
+ return make_error<InstrProfError>(instrprof_error::unsupported_version);
+
+ switch (GET_VERSION(H.formatVersion())) {
+ // When a new field is added in the header add a case statement here to
+ // populate it.
+ default: // Version7 (when the backwards compatible header was introduced).
+ H.HashType = read(Buffer, offsetOf(&Header::HashType));
+ H.HashOffset = read(Buffer, offsetOf(&Header::HashOffset));
+ }
+
+ return H;
+}
+
+size_t Header::size() const {
+ switch (GET_VERSION(formatVersion())) {
+ // 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: // Version7 (when the backwards compatible header was introduced).
+ 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 bc990755e0e4..d1e3438a6f41 100644
--- a/llvm/lib/ProfileData/InstrProfReader.cpp
+++ b/llvm/lib/ProfileData/InstrProfReader.cpp
@@ -934,24 +934,17 @@ Error IndexedInstrProfReader::readHeader() {
if ((const unsigned char *)DataBuffer->getBufferEnd() - Cur < 24)
return error(instrprof_error::truncated);
- auto *Header = reinterpret_cast<const IndexedInstrProf::Header *>(Cur);
- Cur += sizeof(IndexedInstrProf::Header);
+ auto HeaderOr = IndexedInstrProf::Header::readFromBuffer(Start);
+ if (!HeaderOr)
+ return HeaderOr.takeError();
- // 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);
+ const IndexedInstrProf::Header *Header = &HeaderOr.get();
+ Cur += Header->size();
- Cur = readSummary((IndexedInstrProf::ProfVersion)FormatVersion, Cur,
+ Cur = readSummary((IndexedInstrProf::ProfVersion)Header->formatVersion(), Cur,
/* UseCS */ false);
- if (FormatVersion & VARIANT_MASK_CSIR_PROF)
- Cur = readSummary((IndexedInstrProf::ProfVersion)FormatVersion, Cur,
+ if (Header->formatVersion() & VARIANT_MASK_CSIR_PROF)
+ Cur = readSummary((IndexedInstrProf::ProfVersion)Header->formatVersion(), Cur,
/* UseCS */ true);
// Read the hash type and start offset.
@@ -963,9 +956,8 @@ 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, FormatVersion);
+ auto IndexPtr = std::make_unique<InstrProfReaderIndex<OnDiskHashTableImplV3>>(
+ Start + HashOffset, Cur, Start, HashType, Header->formatVersion());
// Load the remapping table now if requested.
if (RemappingBuffer) {
More information about the llvm-commits
mailing list