[PATCH] D14401: [PGO] Make indexed value profile data more compact and add structural definitions for the data format
Justin Bogner via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 6 15:43:59 PST 2015
David Li <davidxl at google.com> writes:
> davidxl updated this revision to Diff 39580.
> davidxl added a comment.
>
> Update the patch according to Vedant's comments.
>
>
> http://reviews.llvm.org/D14401
>
> Files:
> include/llvm/ProfileData/InstrProf.h
> lib/ProfileData/InstrProfReader.cpp
> lib/ProfileData/InstrProfWriter.cpp
>
> Index: lib/ProfileData/InstrProfWriter.cpp
> ===================================================================
> --- lib/ProfileData/InstrProfWriter.cpp
> +++ lib/ProfileData/InstrProfWriter.cpp
> @@ -51,20 +51,7 @@
> M += ProfRecord.Counts.size() * sizeof(uint64_t);
>
> // Value data
> - M += sizeof(uint64_t); // Number of value kinds with value sites.
> - for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind) {
> - uint32_t NumValueSites = ProfRecord.getNumValueSites(Kind);
> - if (NumValueSites == 0)
> - continue;
> - M += sizeof(uint64_t); // Value kind
> - M += sizeof(uint64_t); // The number of value sites for given value kind
> - for (uint32_t I = 0; I < NumValueSites; I++) {
> - M += sizeof(uint64_t); // Number of value data pairs at a value site
> - uint64_t NumValueDataForSite =
> - ProfRecord.getNumValueDataForSite(Kind, I);
> - M += 2 * sizeof(uint64_t) * NumValueDataForSite; // Value data pairs
> - }
> - }
> + M += IndexedInstrProf::ValueProfData::getSize(ProfileData.second);
> }
> LE.write<offset_type>(M);
>
> @@ -87,38 +74,15 @@
> for (uint64_t I : ProfRecord.Counts)
> LE.write<uint64_t>(I);
>
> - // Compute the number of value kinds with value sites.
> - uint64_t NumValueKinds = ProfRecord.getNumValueKinds();
> - LE.write<uint64_t>(NumValueKinds);
> -
> // Write value data
> - for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind) {
> - uint32_t NumValueSites = ProfRecord.getNumValueSites(Kind);
> - if (NumValueSites == 0)
> - continue;
> - LE.write<uint64_t>(Kind); // Write value kind
> - // Write number of value sites for current value kind
> - LE.write<uint64_t>(NumValueSites);
> -
> - for (uint32_t I = 0; I < NumValueSites; I++) {
> - // Write number of value data pairs at this value site
> - uint64_t NumValueDataForSite =
> - ProfRecord.getNumValueDataForSite(Kind, I);
> - LE.write<uint64_t>(NumValueDataForSite);
> - std::unique_ptr<InstrProfValueData[]> VD =
> - ProfRecord.getValueForSite(Kind, I);
> -
> - for (uint32_t V = 0; V < NumValueDataForSite; V++) {
> - if (Kind == IPVK_IndirectCallTarget)
> - LE.write<uint64_t>(ComputeHash((const char *)VD[V].Value));
> - else
> - LE.write<uint64_t>(VD[V].Value);
> - LE.write<uint64_t>(VD[V].Count);
> - }
> - }
> + std::unique_ptr<IndexedInstrProf::ValueProfData> VDataPtr =
> + IndexedInstrProf::ValueProfData::serializeFrom(ProfileData.second);
> +
> + uint64_t *RawPtr = reinterpret_cast<uint64_t *>(VDataPtr.get());
> + for (uint32_t I = 0; I < VDataPtr->TotalSize / sizeof(uint64_t); I++)
> + LE.write<uint64_t>(RawPtr[I]);
> }
> }
> - }
> };
> }
>
> Index: lib/ProfileData/InstrProfReader.cpp
> ===================================================================
> --- lib/ProfileData/InstrProfReader.cpp
> +++ lib/ProfileData/InstrProfReader.cpp
> @@ -18,6 +18,169 @@
>
> using namespace llvm;
>
> +namespace llvm {
> +namespace IndexedInstrProf {
> +
> +uint32_t ValueProfRecord::getHeaderSize(uint32_t NumValueSites) {
> + uint32_t Size = offsetof(ValueProfRecord, SiteCountArray) +
> + sizeof(uint8_t) * NumValueSites;
> + // Round the size to multiple of 8 bytes.
> + Size = (Size + 7) & ~7;
> + return Size;
> +}
> +
> +uint32_t ValueProfRecord::getSize(uint32_t NumValueSites,
> + uint32_t NumValueData) {
> + return getHeaderSize(NumValueSites) +
> + sizeof(InstrProfValueData) * NumValueData;
> +}
> +
> +void ValueProfRecord::deserializeTo(InstrProfRecord &Record,
> + InstrProfRecord::ValueMapType *VMap) {
> + Record.reserveSites(Kind, NumValueSites);
> +
> + InstrProfValueData *ValueData = this->getValueData();
> + for (uint64_t VSite = 0; VSite < NumValueSites; ++VSite) {
> + uint8_t ValueDataCount = this->SiteCountArray[VSite];
> + Record.addValueData(Kind, VSite, ValueData, ValueDataCount, VMap);
> + ValueData += ValueDataCount;
> + }
> +}
> +
> +void ValueProfRecord::serializeFrom(const InstrProfRecord &Record,
> + uint32_t ValueKind,
> + uint32_t NumValueSites) {
> + Kind = ValueKind;
> + this->NumValueSites = NumValueSites;
> + InstrProfValueData *DstVD = getValueData();
> + for (uint32_t S = 0; S < NumValueSites; S++) {
> + uint32_t ND = Record.getNumValueDataForSite(ValueKind, S);
> + SiteCountArray[S] = ND;
> + std::unique_ptr<InstrProfValueData[]> SrcVD =
> + Record.getValueForSite(ValueKind, S);
> + for (uint32_t I = 0; I < ND; I++) {
> + DstVD[I] = SrcVD[I];
> + if (ValueKind == IPVK_IndirectCallTarget)
> + DstVD[I].Value = ComputeHash(HashType, (const char *)DstVD[I].Value);
Please switch over the value kinds here - that way we'll get a warning
for missing switch cases if we add kinds and don't update this code.
> + }
> + DstVD += ND;
> + }
> +}
> +
> +uint32_t ValueProfData::getSize(const InstrProfRecord &Record) {
> + uint32_t TotalSize = sizeof(ValueProfData);
> + uint32_t NumValueKinds = Record.getNumValueKinds();
> + if (NumValueKinds == 0)
> + return TotalSize;
> +
> + for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; Kind++) {
> + uint32_t NumValueSites = Record.getNumValueSites(Kind);
> + if (!NumValueSites)
> + continue;
> + TotalSize +=
> + ValueProfRecord::getSize(NumValueSites, Record.getNumValueData(Kind));
> + }
> + return TotalSize;
> +}
> +
> +void ValueProfData::deserializeTo(InstrProfRecord &Record,
> + InstrProfRecord::ValueMapType *VMap) {
> + if (NumValueKinds == 0)
> + return;
> +
> + ValueProfRecord *VR = getFirstValueProfRecord();
> + for (uint32_t K = 0; K < NumValueKinds; K++) {
> + VR->deserializeTo(Record, VMap);
> + VR = VR->getNext();
> + }
> +}
> +
> +std::unique_ptr<ValueProfData>
> +ValueProfData::serializeFrom(const InstrProfRecord &Record) {
> + uint32_t TotalSize = getSize(Record);
> + std::unique_ptr<ValueProfData> VPD(
> + reinterpret_cast<ValueProfData *>(new char[TotalSize]));
> +
> + VPD->TotalSize = TotalSize;
> + VPD->NumValueKinds = Record.getNumValueKinds();
> + ValueProfRecord *VR = VPD->getFirstValueProfRecord();
> + for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; Kind++) {
> + uint32_t NumValueSites = Record.getNumValueSites(Kind);
> + if (!NumValueSites)
> + continue;
> + VR->serializeFrom(Record, Kind, NumValueSites);
> + VR = VR->getNext();
> + }
> + return VPD;
> +}
> +
> +ErrorOr<std::unique_ptr<ValueProfData>>
> +ValueProfData::swapBytes(const unsigned char *D,
> + const unsigned char *const BufferEnd) {
> + using namespace support;
> + if (D + sizeof(ValueProfData) > BufferEnd)
> + return instrprof_error::truncated;
> +
> + // Use a union to allow easy 64 bit byte swap of the
> + // Header structure.
> + union {
> + ValueProfData Header;
> + uint64_t RawHeader;
> + };
> + RawHeader = endian::byte_swap<uint64_t, little>(*D);
> + uint32_t TotalSize = Header.TotalSize;
> + if (D + TotalSize > BufferEnd)
> + return instrprof_error::too_large;
> + if (Header.NumValueKinds > IPVK_Last)
> + return instrprof_error::malformed;
> + if (TotalSize % sizeof(uint64_t))
> + return instrprof_error::malformed;
> +
> + std::unique_ptr<ValueProfData> VPD(
> + reinterpret_cast<ValueProfData *>(new char[TotalSize]));
> +
> + for (uint32_t I = 0; I < TotalSize / sizeof(uint64_t); I++)
> + reinterpret_cast<uint64_t *>(VPD.get())[I] =
> + endian::byte_swap<uint64_t, little>(
> + reinterpret_cast<const uint64_t *>(D)[I]);
> +
> + // Data integrety check:
> + ValueProfRecord *VR = VPD->getFirstValueProfRecord();
> + for (uint32_t K = 0; K < VPD->NumValueKinds; K++) {
> + if (VR->Kind > IPVK_Last)
> + return instrprof_error::malformed;
> + VR = VR->getNext();
> + if ((char *)VR - (char *)VPD.get() > TotalSize)
> + return instrprof_error::malformed;
> + }
> +
> + return std::move(VPD);
> +}
> +
> +ValueProfRecord *ValueProfData::getFirstValueProfRecord() {
> + return reinterpret_cast<ValueProfRecord *>((char *)this +
> + sizeof(ValueProfData));
> +}
> +
> +uint32_t ValueProfRecord::getNumValueData() const {
> + uint32_t NumValueData = 0;
> + for (uint32_t I = 0; I < NumValueSites; I++)
> + NumValueData += SiteCountArray[I];
> + return NumValueData;
> +}
> +
> +ValueProfRecord *ValueProfRecord::getNext() {
> + return reinterpret_cast<ValueProfRecord *>((char *)this + getSize());
> +}
> +
> +InstrProfValueData *ValueProfRecord::getValueData() {
> + return reinterpret_cast<InstrProfValueData *>((char *)this +
> + getHeaderSize(NumValueSites));
> +}
> +
> +} // End of IndexedInstrProf namespace.
> +} // End of llvm namespace.
> +
> static ErrorOr<std::unique_ptr<MemoryBuffer>>
> setupMemoryBuffer(std::string Path) {
> ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrErr =
> @@ -298,51 +461,15 @@
>
> bool InstrProfLookupTrait::ReadValueProfilingData(
> const unsigned char *&D, const unsigned char *const End) {
> + ErrorOr<std::unique_ptr<IndexedInstrProf::ValueProfData>> VDataPtrOrErr =
> + IndexedInstrProf::ValueProfData::swapBytes(D, End);
>
> - using namespace support;
> - // Read number of value kinds with value sites.
> - if (D + sizeof(uint64_t) > End)
> + if (VDataPtrOrErr.getError())
> return false;
> - uint64_t ValueKindCount = endian::readNext<uint64_t, little, unaligned>(D);
> -
> - InstrProfRecord &ProfRecord = DataBuffer.back();
> - for (uint32_t Kind = 0; Kind < ValueKindCount; ++Kind) {
> -
> - // Read value kind and number of value sites for kind.
> - if (D + 2 * sizeof(uint64_t) > End)
> - return false;
> -
> - uint64_t ValueKind = endian::readNext<uint64_t, little, unaligned>(D);
> - uint64_t ValueSiteCount = endian::readNext<uint64_t, little, unaligned>(D);
>
> - ProfRecord.reserveSites(ValueKind, ValueSiteCount);
> + VDataPtrOrErr.get()->deserializeTo(DataBuffer.back(), &HashKeys);
> + D += VDataPtrOrErr.get()->TotalSize;
>
> - for (uint64_t VSite = 0; VSite < ValueSiteCount; ++VSite) {
> - // Read number of value data pairs at value site.
> - if (D + sizeof(uint64_t) > End)
> - return false;
> -
> - uint64_t ValueDataCount =
> - endian::readNext<uint64_t, little, unaligned>(D);
> -
> - // Check if there are as many ValueDataPairs as ValueDataCount in memory.
> - if (D + (ValueDataCount << 1) * sizeof(uint64_t) > End)
> - return false;
> -
> - std::unique_ptr<InstrProfValueData[]> VDataPtr(
> - ValueDataCount == 0 ? nullptr
> - : new InstrProfValueData[ValueDataCount]);
> -
> - for (uint64_t VCount = 0; VCount < ValueDataCount; ++VCount) {
> - VDataPtr[VCount].Value =
> - endian::readNext<uint64_t, little, unaligned>(D);
> - VDataPtr[VCount].Count =
> - endian::readNext<uint64_t, little, unaligned>(D);
> - }
> - ProfRecord.addValueData(ValueKind, VSite, VDataPtr.get(), ValueDataCount,
> - &HashKeys);
> - }
> - }
> return true;
> }
>
> Index: include/llvm/ProfileData/InstrProf.h
> ===================================================================
> --- include/llvm/ProfileData/InstrProf.h
> +++ include/llvm/ProfileData/InstrProf.h
> @@ -20,6 +20,7 @@
> #include "llvm/ADT/StringSet.h"
> #include "llvm/Support/Endian.h"
> #include "llvm/Support/ErrorHandling.h"
> +#include "llvm/Support/ErrorOr.h"
> #include "llvm/Support/MD5.h"
> #include <cstdint>
> #include <list>
> @@ -288,6 +289,16 @@
> return NumValueKinds;
> }
>
> +uint32_t InstrProfRecord::getNumValueData(uint32_t ValueKind) const {
> + uint32_t N = 0;
> + const std::vector<InstrProfValueSiteRecord> &SiteRecords =
> + getValueSitesForKind(ValueKind);
> + for (auto &SR : SiteRecords) {
> + N += SR.ValueData.size();
> + }
> + return N;
> +}
> +
> uint32_t InstrProfRecord::getNumValueSites(uint32_t ValueKind) const {
> return getValueSitesForKind(ValueKind).size();
> }
> @@ -389,14 +400,106 @@
> const uint64_t Version = 3;
> const HashT HashType = HashT::MD5;
>
> +// This structure defines the file header of the LLVM profile
> +// data file in indexed-format.
Might as well use /// so doxygen picks these comments up. Applies to the
new structs as well.
> struct Header {
> uint64_t Magic;
> uint64_t Version;
> uint64_t MaxFunctionCount;
> uint64_t HashType;
> uint64_t HashOffset;
> };
>
> +// This is the header of the data structure that defines the on-disk
> +// layout of the value profile data of a particular kind for one function.
> +struct ValueProfRecord {
> + // The kind of the value profile record.
> + uint32_t Kind;
> + // The number of value profile sites. It is guaranteed to be non-zero;
> + // otherwise the record for this kind won't be emitted.
> + uint32_t NumValueSites;
> + // The first element of the array that stores the number of profiled
> + // values for each value site. The size of the array is NumValueSites.
> + // Since NumValueSites is greater than zero, there is at least one
> + // element inthe array.
> + uint8_t SiteCountArray[1];
> +
> +#if 0
> + // The fake declaration is for documentation purpose only.
Please don't do this. Documentation in a comment is clearer than dead
#if'd out code. OTOH, can this be simplified with the TrailingObjects
template James introduced a while ago?
That's in include/llvm/Support/TrailingObjects.h, I believe.
> + // Align the start of next field to be on 8 byte boundaries.
> + uint8_t Padding[X];
> +
> + // The array of value profile data. The size of the array is the sum
> + // of all elements in SiteCountArray[].
> + InstrProfValueData ValueData[];
> +#endif
> +
> + /// Return the ValueProfRecord header size including the padding bytes.
> + static uint32_t getHeaderSize(uint32_t NumValueSites);
> + /// Return the total size of the value profile record including the
> + /// header and the value data.
> + static uint32_t getSize(uint32_t NumValueSites, uint32_t NumValueData);
> + /// Return the total size of the value profile record including the
> + /// header and the value data.
> + uint32_t getSize() const { return getSize(NumValueSites, getNumValueData()); }
> + /// Use this method to advance to the next ValueProfRecord.
> + ValueProfRecord *getNext();
> + /// Return the pointer to the first value profile data.
> + InstrProfValueData *getValueData();
> + /// Return the number of value sites.
> + uint32_t getNumValueSites() const { return NumValueSites; }
> + /// Return the number of value data.
> + uint32_t getNumValueData() const;
> + /// Read data from this record and save it to Record.
> + void deserializeTo(InstrProfRecord &Record,
> + InstrProfRecord::ValueMapType *VMap);
> + /// Extract data from Record and serialize into this instance.
> + void serializeFrom(const InstrProfRecord &Record, uint32_t ValueKind,
> + uint32_t NumValueSites);
> +};
> +
> +// Per-function header/control data structure for value profiling
> +// data in indexed-format.
> +struct ValueProfData {
> + // Total size in bytes including this field. It must be a multiple
> + // of sizeof(uint64_t).
> + uint32_t TotalSize;
> + // The number of value profile kinds that has value profile data.
> + // In this implementation, a value profile kind is considered to
> + // have profile data if the number of value profile sites for the
> + // kind is not zero. More aggressively, the implemnetation can
> + // choose to check the actual data value: if none of the value sites
> + // has any profiled values, the kind can be skipped.
> + uint32_t NumValueKinds;
> +
> +#if 0
> + // Following are a sequence of variable length records. The prefix/header
> + // of each record is defined by ValueProfRecord type. The number of
> + // records is NumValueKinds.
> + ValueProfRecord Record_1;
> + ..ValueProfRecord Record_N;
> +#endif
> +
> + /// Return the total size in bytes of the on-disk value profile data
> + /// given the data stored in Record.
> + static uint32_t getSize(const InstrProfRecord &Record);
> + /// Return a pointer to ValueProfData instance ready to be streamed.
> + static std::unique_ptr<ValueProfData>
> + serializeFrom(const InstrProfRecord &Record);
> + /// Return a pointer to ValueProfileData instance ready to be read.
> + /// All data in the instance are properly swapped.
> + static ErrorOr<std::unique_ptr<ValueProfData>>
> + swapBytes(const unsigned char *D, const unsigned char *const BufferEnd);
> +
> + /// Return the total size of the ValueProfileData.
> + uint32_t getSize() const { return TotalSize; }
> + /// Read data from this data and save it to Record.
> + void deserializeTo(InstrProfRecord &Record,
> + InstrProfRecord::ValueMapType *VMap);
> + /// Return the first ValueProfRecord instance.
> + ValueProfRecord *getFirstValueProfRecord();
> +};
> +
> } // end namespace IndexedInstrProf
>
> namespace RawInstrProf {
> @@ -428,20 +531,20 @@
> uint64_t('R') << 8 | uint64_t(129);
> }
>
> +// Per-function profile data header/control structure.
> // The definition should match the structure defined in
> // compiler-rt/lib/profile/InstrProfiling.h.
> // It should also match the synthesized type in
> // Transforms/Instrumentation/InstrProfiling.cpp:getOrCreateRegionCounters.
> -
> template <class IntPtrT> struct ProfileData {
> #define INSTR_PROF_DATA(Type, LLVMType, Name, Init) Type Name;
> #include "llvm/ProfileData/InstrProfData.inc"
> };
>
> +// File header structure of the LLVM profile data in raw format.
> // The definition should match the header referenced in
> // compiler-rt/lib/profile/InstrProfilingFile.c and
> // InstrProfilingBuffer.c.
> -
> struct Header {
> const uint64_t Magic;
> const uint64_t Version;
> @@ -456,6 +559,14 @@
>
> namespace coverage {
>
> +// Profile coverage map has the following layout:
> +// [CoverageMapFileHeader]
> +// [ArrayStart]
> +// [CovMapFunctionRecord]
> +// [CovMapFunctionRecord]
> +// ...
> +// [ArrayEnd]
> +// [Encoded Region Mapping Data]
> LLVM_PACKED_START
> template <class IntPtrT> struct CovMapFunctionRecord {
> #define COVMAP_FUNC_RECORD(Type, LLVMType, Name, Init) Type Name;
More information about the llvm-commits
mailing list