[profile] Remove unnecessary field in raw profile reader

[davidxl]

Summary

This is a preparation patch that allows us to get rid of the ValueDataSize field in the raw header. By removing that field, the value profile writer code can be greatly simplified without the need to be split into two phases : data gathering and writing, which will also reduce the memory usage in profile runtime (no need to buffer all value data before writing out).

[vsk]

Looks good with a nit.

lib/ProfileData/InstrProfReader.cpp

431 ↗

(On Diff #56240)

I suggest adding something like getNextHeaderPos() (which asserts atEnd()) to RawInstrProfReader for clarity.

[davidxl]

Done.

[silvas]

This patch LGTM too.

[vitalybuka]

This CL probably breaks this bot
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/23853/steps/test%20standalone%20compiler-rt/logs/stdio

[vitalybuka]

Actually I am not sure. Next one
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/23854 has
that step green, but I see no CL fixing this.

llvm/trunk/include/llvm/ProfileData/InstrProfReader.h

private:
   uint64_t Version;
   uint64_t CountersDelta;
   uint64_t NamesDelta;
   const RawInstrProf::ProfileData<IntPtrT> *Data;
   const RawInstrProf::ProfileData<IntPtrT> *DataEnd;
   const uint64_t *CountersStart;
   const char *NamesStart;
   uint64_t NamesSize;
+  // After value profile is all read, this pointer points to
+  // the header of next profile data (if exists)
   const uint8_t *ValueDataStart;
-  const char *ProfileEnd;
   uint32_t ValueKindLast;
   uint32_t CurValueDataSize;
 
   InstrProfRecord::ValueMapType FunctionPtrToNameMap;
 
   RawInstrProfReader(const RawInstrProfReader &) = delete;
   RawInstrProfReader &operator=(const RawInstrProfReader &) = delete;
 public:
private:
   std::error_code readFuncHash(InstrProfRecord &Record);
   std::error_code readRawCounts(InstrProfRecord &Record);
   std::error_code readValueProfilingData(InstrProfRecord &Record);
   bool atEnd() const { return Data == DataEnd; }
   void advanceData() {
     Data++;
     ValueDataStart += CurValueDataSize;
   }
+  const char *getNextHeaderPos() const {
+      assert(atEnd());
+      return (const char *)ValueDataStart;
+  }
 
   const uint64_t *getCounter(IntPtrT CounterPtr) const {
     ptrdiff_t Offset = (swap(CounterPtr) - CountersDelta) / sizeof(uint64_t);
     return CountersStart + Offset;
   }
   StringRef getName(uint64_t NameRef) const {
     return Symtab->getFuncName(swap(NameRef));
   }

llvm/trunk/lib/ProfileData/InstrProfReader.cpp

RawInstrProfReader<IntPtrT>::readHeader(const RawInstrProf::Header &Header) {
   if (GET_VERSION(Version) != RawInstrProf::Version)
     return error(instrprof_error::unsupported_version);
 
   CountersDelta = swap(Header.CountersDelta);
   NamesDelta = swap(Header.NamesDelta);
   auto DataSize = swap(Header.DataSize);
   auto CountersSize = swap(Header.CountersSize);
   NamesSize = swap(Header.NamesSize);
-  auto ValueDataSize = swap(Header.ValueDataSize);
   ValueKindLast = swap(Header.ValueKindLast);
 
   auto DataSizeInBytes = DataSize * sizeof(RawInstrProf::ProfileData<IntPtrT>);
   auto PaddingSize = getNumPaddingBytes(NamesSize);
 
   ptrdiff_t DataOffset = sizeof(RawInstrProf::Header);
   ptrdiff_t CountersOffset = DataOffset + DataSizeInBytes;
   ptrdiff_t NamesOffset = CountersOffset + sizeof(uint64_t) * CountersSize;
   ptrdiff_t ValueDataOffset = NamesOffset + NamesSize + PaddingSize;
-  size_t ProfileSize = ValueDataOffset + ValueDataSize;
 
   auto *Start = reinterpret_cast<const char *>(&Header);
-  if (Start + ProfileSize > DataBuffer->getBufferEnd())
+  if (Start + ValueDataOffset > DataBuffer->getBufferEnd())
     return error(instrprof_error::bad_header);
 
   Data = reinterpret_cast<const RawInstrProf::ProfileData<IntPtrT> *>(
       Start + DataOffset);
   DataEnd = Data + DataSize;
   CountersStart = reinterpret_cast<const uint64_t *>(Start + CountersOffset);
   NamesStart = Start + NamesOffset;
   ValueDataStart = reinterpret_cast<const uint8_t *>(Start + ValueDataOffset);
-  ProfileEnd = Start + ProfileSize;
 
   std::unique_ptr<InstrProfSymtab> NewSymtab = make_unique<InstrProfSymtab>();
   if (auto EC = createSymtab(*NewSymtab.get()))
     return EC;
 
   Symtab = std::move(NewSymtab);
   return success();
 }
RawInstrProfReader<IntPtrT>::readValueProfilingData(InstrProfRecord &Record) {
   uint32_t NumValueKinds = 0;
   for (uint32_t I = 0; I < IPVK_Last + 1; I++)
     NumValueKinds += (Data->NumValueSites[I] != 0);
 
   if (!NumValueKinds)
     return success();
 
   ErrorOr<std::unique_ptr<ValueProfData>> VDataPtrOrErr =
-      ValueProfData::getValueProfData(ValueDataStart,
-                                      (const unsigned char *)ProfileEnd,
-                                      getDataEndianness());
+      ValueProfData::getValueProfData(
+          ValueDataStart, (const unsigned char *)DataBuffer->getBufferEnd(),
 
   if (VDataPtrOrErr.getError())
     return VDataPtrOrErr.getError();
 
   // Note that besides deserialization, this also performs the conversion for
   // indirect call targets.  The function pointers from the raw profile are
   // remapped into function name hashes.
   VDataPtrOrErr.get()->deserializeTo(Record, &Symtab->getAddrHashMap());
   CurValueDataSize = VDataPtrOrErr.get()->getSize();
   return success();
 }
 
 template <class IntPtrT>
 std::error_code
 RawInstrProfReader<IntPtrT>::readNextRecord(InstrProfRecord &Record) {
   if (atEnd())
-    if (std::error_code EC = readNextHeader(ProfileEnd))
+    // At this point, ValueDataStart field points to the next header.
+    if (std::error_code EC = readNextHeader(getNextHeaderPos()))
       return EC;
 
   // Read name ad set it in Record.
   if (std::error_code EC = readName(Record))
     return EC;
 
   // Read FuncHash and set it in Record.
   if (std::error_code EC = readFuncHash(Record))