From 5391d5c278137c4423981271bbfbfc1fa9eb0acb Mon Sep 17 00:00:00 2001 From: Justin Bogner Date: Sat, 20 Jun 2015 01:37:56 +0000 Subject: [PATCH] Revert "InstrProf: When reading, copy the data instead of taking a reference. NFC" Seems like MSVC doesn't like this: InstrProf.h(49) : error C2614: 'llvm::InstrProfRecord' : illegal member initialization: 'Hash' is not a base or member This reverts r240206. llvm-svn: 240208 --- include/llvm/ProfileData/InstrProf.h | 12 --- include/llvm/ProfileData/InstrProfReader.h | 50 +++++++-- lib/ProfileData/InstrProfReader.cpp | 114 +++++++++------------ 3 files changed, 88 insertions(+), 88 deletions(-) diff --git a/include/llvm/ProfileData/InstrProf.h b/include/llvm/ProfileData/InstrProf.h index 5c9a5b693dd..eafb76886c8 100644 --- a/include/llvm/ProfileData/InstrProf.h +++ b/include/llvm/ProfileData/InstrProf.h @@ -16,9 +16,7 @@ #ifndef LLVM_PROFILEDATA_INSTRPROF_H_ #define LLVM_PROFILEDATA_INSTRPROF_H_ -#include "llvm/ADT/StringRef.h" #include -#include namespace llvm { const std::error_category &instrprof_category(); @@ -43,16 +41,6 @@ inline std::error_code make_error_code(instrprof_error E) { return std::error_code(static_cast(E), instrprof_category()); } -/// Profiling information for a single function. -struct InstrProfRecord { - InstrProfRecord() {} - InstrProfRecord(StringRef Name, uint64_t Hash, std::vector &Counts) - : Name(Name), Hash(Hash), Counts(std::move(Counts)) {} - StringRef Name; - uint64_t Hash; - std::vector Counts; -}; - } // end namespace llvm namespace std { diff --git a/include/llvm/ProfileData/InstrProfReader.h b/include/llvm/ProfileData/InstrProfReader.h index f937e7d08d5..63a6ac671f2 100644 --- a/include/llvm/ProfileData/InstrProfReader.h +++ b/include/llvm/ProfileData/InstrProfReader.h @@ -29,6 +29,16 @@ namespace llvm { class InstrProfReader; +/// Profiling information for a single function. +struct InstrProfRecord { + InstrProfRecord() {} + InstrProfRecord(StringRef Name, uint64_t Hash, ArrayRef Counts) + : Name(Name), Hash(Hash), Counts(Counts) {} + StringRef Name; + uint64_t Hash; + ArrayRef Counts; +}; + /// A file format agnostic iterator over profiling data. class InstrProfIterator : public std::iterator { @@ -104,6 +114,8 @@ private: std::unique_ptr DataBuffer; /// Iterator over the profile data. line_iterator Line; + /// The current set of counter values. + std::vector Counts; TextInstrProfReader(const TextInstrProfReader &) = delete; TextInstrProfReader &operator=(const TextInstrProfReader &) = delete; @@ -129,6 +141,8 @@ class RawInstrProfReader : public InstrProfReader { private: /// The profile data file contents. std::unique_ptr DataBuffer; + /// The current set of counter values. + std::vector Counts; struct ProfileData { const uint32_t NameSize; const uint32_t NumCounters; @@ -192,16 +206,17 @@ enum class HashT : uint32_t; /// Trait for lookups into the on-disk hash table for the binary instrprof /// format. class InstrProfLookupTrait { - std::vector DataBuffer; + std::vector DataBuffer; IndexedInstrProf::HashT HashType; - unsigned FormatVersion; - public: - InstrProfLookupTrait(IndexedInstrProf::HashT HashType, unsigned FormatVersion) - : HashType(HashType), FormatVersion(FormatVersion) {} - - typedef ArrayRef data_type; + InstrProfLookupTrait(IndexedInstrProf::HashT HashType) : HashType(HashType) {} + struct data_type { + data_type(StringRef Name, ArrayRef Data) + : Name(Name), Data(Data) {} + StringRef Name; + ArrayRef Data; + }; typedef StringRef internal_key_type; typedef StringRef external_key_type; typedef uint64_t hash_value_type; @@ -224,9 +239,22 @@ public: return StringRef((const char *)D, N); } - data_type ReadData(StringRef K, const unsigned char *D, offset_type N); -}; + data_type ReadData(StringRef K, const unsigned char *D, offset_type N) { + DataBuffer.clear(); + if (N % sizeof(uint64_t)) + // The data is corrupt, don't try to read it. + return data_type("", DataBuffer); + using namespace support; + // We just treat the data as opaque here. It's simpler to handle in + // IndexedInstrProfReader. + unsigned NumEntries = N / sizeof(uint64_t); + DataBuffer.reserve(NumEntries); + for (unsigned I = 0; I < NumEntries; ++I) + DataBuffer.push_back(endian::readNext(D)); + return data_type(K, DataBuffer); + } +}; typedef OnDiskIterableChainedHashTable InstrProfReaderIndex; @@ -239,6 +267,8 @@ private: std::unique_ptr Index; /// Iterator over the profile data. InstrProfReaderIndex::data_iterator RecordIterator; + /// Offset into our current data set. + size_t CurrentOffset; /// The file format version of the profile data. uint64_t FormatVersion; /// The maximal execution count among all functions. @@ -248,7 +278,7 @@ private: IndexedInstrProfReader &operator=(const IndexedInstrProfReader &) = delete; public: IndexedInstrProfReader(std::unique_ptr DataBuffer) - : DataBuffer(std::move(DataBuffer)), Index(nullptr) {} + : DataBuffer(std::move(DataBuffer)), Index(nullptr), CurrentOffset(0) {} /// Return true if the given buffer is in an indexed instrprof format. static bool hasFormat(const MemoryBuffer &DataBuffer); diff --git a/lib/ProfileData/InstrProfReader.cpp b/lib/ProfileData/InstrProfReader.cpp index b8bdbc57af8..3a5b266016c 100644 --- a/lib/ProfileData/InstrProfReader.cpp +++ b/lib/ProfileData/InstrProfReader.cpp @@ -15,6 +15,7 @@ #include "llvm/ProfileData/InstrProfReader.h" #include "InstrProfIndexed.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ProfileData/InstrProf.h" #include using namespace llvm; @@ -125,16 +126,18 @@ std::error_code TextInstrProfReader::readNextRecord(InstrProfRecord &Record) { return error(instrprof_error::malformed); // Read each counter and fill our internal storage with the values. - Record.Counts.clear(); - Record.Counts.reserve(NumCounters); + Counts.clear(); + Counts.reserve(NumCounters); for (uint64_t I = 0; I < NumCounters; ++I) { if (Line.is_at_end()) return error(instrprof_error::truncated); uint64_t Count; if ((Line++)->getAsInteger(10, Count)) return error(instrprof_error::malformed); - Record.Counts.push_back(Count); + Counts.push_back(Count); } + // Give the record a reference to our internal counter storage. + Record.Counts = Counts; return success(); } @@ -277,10 +280,11 @@ RawInstrProfReader::readNextRecord(InstrProfRecord &Record) { Record.Hash = swap(Data->FuncHash); Record.Name = RawName; if (ShouldSwapBytes) { - Record.Counts.clear(); - Record.Counts.reserve(RawCounts.size()); + Counts.clear(); + Counts.reserve(RawCounts.size()); for (uint64_t Count : RawCounts) - Record.Counts.push_back(swap(Count)); + Counts.push_back(swap(Count)); + Record.Counts = Counts; } else Record.Counts = RawCounts; @@ -299,49 +303,6 @@ InstrProfLookupTrait::ComputeHash(StringRef K) { return IndexedInstrProf::ComputeHash(HashType, K); } -typedef InstrProfLookupTrait::data_type data_type; -typedef InstrProfLookupTrait::offset_type offset_type; - -data_type InstrProfLookupTrait::ReadData(StringRef K, const unsigned char *D, - offset_type N) { - - // Check if the data is corrupt. If so, don't try to read it. - if (N % sizeof(uint64_t)) - return data_type(); - - DataBuffer.clear(); - uint64_t NumCounts; - uint64_t NumEntries = N / sizeof(uint64_t); - std::vector CounterBuffer; - for (uint64_t I = 0; I < NumEntries; I += NumCounts) { - using namespace support; - // The function hash comes first. - uint64_t Hash = endian::readNext(D); - - if (++I >= NumEntries) - return data_type(); - - // In v1, we have at least one count. - // Later, we have the number of counts. - NumCounts = (1 == FormatVersion) - ? NumEntries - I - : endian::readNext(D); - if (1 != FormatVersion) - ++I; - - // If we have more counts than data, this is bogus. - if (I + NumCounts > NumEntries) - return data_type(); - - CounterBuffer.clear(); - for (unsigned J = 0; J < NumCounts; ++J) - CounterBuffer.push_back(endian::readNext(D)); - - DataBuffer.push_back(InstrProfRecord(K, Hash, CounterBuffer)); - } - return DataBuffer; -} - bool IndexedInstrProfReader::hasFormat(const MemoryBuffer &DataBuffer) { if (DataBuffer.getBufferSize() < 8) return false; @@ -381,9 +342,8 @@ std::error_code IndexedInstrProfReader::readHeader() { uint64_t HashOffset = endian::readNext(Cur); // The rest of the file is an on disk hash table. - Index.reset(InstrProfReaderIndex::Create( - Start + HashOffset, Cur, Start, - InstrProfLookupTrait(HashType, FormatVersion))); + Index.reset(InstrProfReaderIndex::Create(Start + HashOffset, Cur, Start, + InstrProfLookupTrait(HashType))); // Set up our iterator for readNextRecord. RecordIterator = Index->data_begin(); @@ -397,14 +357,21 @@ std::error_code IndexedInstrProfReader::getFunctionCounts( return error(instrprof_error::unknown_function); // Found it. Look for counters with the right hash. - ArrayRef Data = (*Iter); - if (Data.empty()) - return error(instrprof_error::malformed); - - for (unsigned I = 0, E = Data.size(); I < E; ++I) { + ArrayRef Data = (*Iter).Data; + uint64_t NumCounts; + for (uint64_t I = 0, E = Data.size(); I != E; I += NumCounts) { + // The function hash comes first. + uint64_t FoundHash = Data[I++]; + // In v1, we have at least one count. Later, we have the number of counts. + if (I == E) + return error(instrprof_error::malformed); + NumCounts = FormatVersion == 1 ? E - I : Data[I++]; + // If we have more counts than data, this is bogus. + if (I + NumCounts > E) + return error(instrprof_error::malformed); // Check for a match and fill the vector if there is one. - if (Data[I].Hash == FuncHash) { - Counts = Data[I].Counts; + if (FoundHash == FuncHash) { + Counts = Data.slice(I, NumCounts); return success(); } } @@ -417,15 +384,30 @@ IndexedInstrProfReader::readNextRecord(InstrProfRecord &Record) { if (RecordIterator == Index->data_end()) return error(instrprof_error::eof); - if ((*RecordIterator).empty()) - return error(instrprof_error::malformed); + // Record the current function name. + Record.Name = (*RecordIterator).Name; - static unsigned RecordIndex = 0; - ArrayRef Data = (*RecordIterator); - Record = Data[RecordIndex++]; - if (RecordIndex >= Data.size()) { + ArrayRef Data = (*RecordIterator).Data; + // Valid data starts with a hash and either a count or the number of counts. + if (CurrentOffset + 1 > Data.size()) + return error(instrprof_error::malformed); + // First we have a function hash. + Record.Hash = Data[CurrentOffset++]; + // In version 1 we knew the number of counters implicitly, but in newer + // versions we store the number of counters next. + uint64_t NumCounts = + FormatVersion == 1 ? Data.size() - CurrentOffset : Data[CurrentOffset++]; + if (CurrentOffset + NumCounts > Data.size()) + return error(instrprof_error::malformed); + // And finally the counts themselves. + Record.Counts = Data.slice(CurrentOffset, NumCounts); + + // If we've exhausted this function's data, increment the record. + CurrentOffset += NumCounts; + if (CurrentOffset == Data.size()) { ++RecordIterator; - RecordIndex = 0; + CurrentOffset = 0; } + return success(); }