[PATCH] D10674: Value profiling - patchset 3

Betul Buyukkurt via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 28 13:06:54 PDT 2015


>>>>>      DataBuffer.push_back(InstrProfRecord(K, Hash,
>>>> std::move(CounterBuffer)));
>>>>> +
>>>>> +    // Read value profiling data
>>>>> +    for (uint32_t Kind = instrprof_value_kind::first;
>>>>> +         Kind < instrprof_value_kind::size; ++Kind) {
>>>
>>>> I still think we should store the kind in the data here and then read
>>> the following data based on that. There's no guarantee that all kinds
>>> that
>>> aren't indirect_call_target will behave as they do in this loop as is,
>>> so
>>> I'd rather just reject outright kinds we don't understand. If the type
>>> is
>>> in the data we can do that, and it also saves a few bytes for functions
>>> that don't have any value profiling data (and more bytes when we
> support
>>> more kinds of value profiling).
>>>
>>> I do not think this is possible. The format indicates that multiple
> hash
>>> values and data may be stored per name entry. Given that there is no
>>> indicator in the format that tells whether the next bytes after counter
>>> values would belong to a value kind or a hash value for the next data
>>> for
>>> the same key.
>>>
>>
>> Sorry I missed this one. This seems to be the last main thing to be
>> addressed/resolved/agreed upon for the indexed format.
>>
>> I think Justin's suggestion is possible. We just need one mandatory
>> field to be emitted before the value profile data.
>> value_profile_section:
>>           .quad  NUM_VALUE_KIND
>>
>> This field indicates how many value profile kinds have profile data.
>> When value profile data are missing, this format has storage
>> advantage. For instance, when the function has no value profile data,
>> The value profile section is just
>>
>> value_profile_section:
>>           .quad 0
>>
>> With Betuls patch, it will be (assuming 2 kinds)
>>
>> value_profile_section:
>>          .quad 0           // num of sites for kind 1 is 0
>>          .quad 0           // num of sites for kind 2 is 0
>>
>> However if value profile data exists for all value kinds, Betul's
>> suggested format is slightly better -- it does not need to store the
>> mandatory NUM_VALUE_KIND field, nor the value kind field it self.
>>
>> Since I believe that value profile data will be sparse (especially
>> when we add more kinds in the future), my vote is on Justin's format
>> which is more flexible.
>
> I'll make sure to address this item as well. I should respond in a day or
> so.

I've a patch that outputs the value kind into the indexed profile file
along with the value data. However, I'd like to clarify one thing. Value
kind should not be emitted when there is no value data or when there are
no value sites.

If a function has no value sites for a particular value kind, no value
kind/site/data should be outputted. However, if the function does have
value sites but no value data was recorded into any of them, in that case
value kind, value site count, and value data count for each site should
still be emitted. Is this the intended behavior? During merges we do check
on the value site counts. This is done in the combineInstrProfRecords
routine, the value site counts are compared and error is emitted if value
site counts mismatch.

if (Dest.ValueSites[Kind].size() != Source.ValueSites[Kind].size())
  return instrprof_error::value_site_count_mismatch;

Thanks,
-Betul

> Thanks,
> -Betul
>
>>
>> David
>>
>>
>>
>>>> This also has the benefit that the V2 data will be valid for V3, so we
>>> might even be able to get away without adding branches to check the
>>> version difference here, which would be a convenient simplification. I
>>> haven't thought through if that actually works though.
>>>
>>> ReadData can still read V2, therefore I do not understand why it should
>>> not be valid.
>>>
>>>>> +      // Read number of value sites.
>>>>> +      if (D + sizeof(uint32_t) > End)
>>>>> +        return data_type();
>>>>> +      uint32_t ValueSiteCount = endian::readNext<uint32_t, little,
>>>> unaligned>(D);
>>>>> +
>>>>> +      for (uint32_t VSite = 0; VSite < ValueSiteCount; ++VSite) { +
>>>     // Read number of value data pairs at value site.
>>>>> +        if (D + sizeof(uint32_t) > End)
>>>>> +          return data_type();
>>>>> +        uint32_t ValueDataCount = endian::readNext<uint32_t, little,
>>>> unaligned>(D);
>>>>> +
>>>>> +        InstrProfValueSiteRecord VSiteRecord;
>>>>> +        for (uint32_t VCount = 0; VCount < ValueDataCount; ++VCount)
>>>>> {
>>> +          if (D + 2 * sizeof(uint64_t) > End)
>>>>> +            return data_type();
>>>>> +          uint64_t Value = endian::readNext<uint64_t, little,
>>>> unaligned>(D);
>>>>> +          if (Kind == instrprof_value_kind::indirect_call_target) {
> +
>>>           auto Result = HashKeyMap.find(Value);
>>>>> +            if (Result != HashKeyMap.end())
>>>>> +              Value = (uint64_t)Result->second;
>>>
>>>> What if it isn't found? As I mentioned earlier, I'm not very
>>>> comfortable
>>> with this approach to storing the strings.
>>>
>>> I'm not storing additional string in the file. I'm reading the existing
>>> set of keys, add them to the string table and create a map from hash
>>> values to const char*'s to string table entries.
>>>
>>>>> +          }
>>>>> +          uint64_t NumTaken = endian::readNext<uint64_t, little,
>>>> unaligned>(D);
>>>>> +          VSiteRecord.ValueData.push_back(std::make_pair(Value,
>>>> NumTaken));
>>>>> +        }
>>>>> +
>>>>
> DataBuffer[DataBuffer.size()-1].ValueSites[Kind].push_back(VSiteRecord);
>>> This is better written as DataBuffer.back(). It also might make sense
> to
>>> sort these here rather than in mergeValueData - more on that later.
>>>
>>> Done.
>>>
>>>>> +      }
>>>>> +    }
>>>>>    }
>>>>>    return DataBuffer;
>>>>>  }
>>>>> @@ -379,19 +444,24 @@
>>>>>    if (HashType > IndexedInstrProf::HashT::Last)
>>>>>      return error(instrprof_error::unsupported_hash_type);
>>>>>    uint64_t HashOffset = endian::readNext<uint64_t, little,
>>>> unaligned>(Cur);
>>>>> -
>>>>>    // The rest of the file is an on disk hash table.
>>>>>    Index.reset(InstrProfReaderIndex::Create(
>>>>>        Start + HashOffset, Cur, Start,
>>>>> -      InstrProfLookupTrait(HashType, FormatVersion)));
>>>>> +      InstrProfLookupTrait(HashType, FormatVersion, HashKeyMap))); +
>>> for (auto Key : Index->keys()) {
>>>>> +    const char* KeyTableRef = StringTable.insertString(Key);
>>>>> +    HashKeyMap.insert(std::make_pair(ComputeHash(HashType, Key),
>>>> KeyTableRef));
>>>>> +  }
>>>>>    // Set up our iterator for readNextRecord.
>>>>>    RecordIterator = Index->data_begin();
>>>>>    return success();
>>>>>  }
>>>>>  std::error_code IndexedInstrProfReader::getFunctionCounts(
>>>>> -    StringRef FuncName, uint64_t FuncHash, std::vector<uint64_t>
>>>> &Counts) {
>>>>> +  StringRef FuncName, uint64_t FuncHash,
>>>>> +  std::vector<uint64_t> &Counts) {
>>>>> +
>>>>>    auto Iter = Index->find(FuncName);
>>>>>    if (Iter == Index->end())
>>>>>      return error(instrprof_error::unknown_function);
>>>>> @@ -411,6 +481,29 @@
>>>>>    return error(instrprof_error::hash_mismatch);
>>>>>  }
>>>>> +std::error_code IndexedInstrProfReader::getFunctionValuesForKind( +
>>> StringRef FuncName, uint64_t FuncHash, uint32_t ValueKind,
>>>>> +  std::vector<InstrProfValueSiteRecord> &Values) {
>>>
>>>> Does this function work? Won't it return an index into a string table
>>> with no means to turn that into the actual string for indirect call
>>> profiling?
>>>
>>> ValueData for indirect call targets store not indices but char*'s to
>>> string table entries. The function works for that reason and I've
>>> locally
>>> verified that on my end. However, I've removed this function since it's
>>> not in use at this time.
>>>
>>>>> +
>>>>> +  auto Iter = Index->find(FuncName);
>>>>> +  if (Iter == Index->end())
>>>>> +    return error(instrprof_error::unknown_function);
>>>>> +
>>>>> +  // Found it. Look for counters with the right hash.
>>>>> +  ArrayRef<InstrProfRecord> Data = (*Iter);
>>>>> +  if (Data.empty())
>>>>> +    return error(instrprof_error::malformed);
>>>>> +
>>>>> +  for (unsigned I = 0, E = Data.size(); I < E; ++I) {
>>>>> +    // Check for a match and fill the vector if there is one.
>>>>> +    if (Data[I].Hash == FuncHash) {
>>>>> +      Values = Data[I].ValueSites[ValueKind];
>>>>> +      return success();
>>>>> +    }
>>>>> +  }
>>>>> +  return error(instrprof_error::hash_mismatch);
>>>>> +}
>>>>> +
>>>>>  std::error_code
>>>>>  IndexedInstrProfReader::readNextRecord(InstrProfRecord &Record) {
>>>>>    // Are we out of records?
>>>>> Index: lib/ProfileData/InstrProfIndexed.h
>>>>> ===================================================================
>>>>> ---
>>> lib/ProfileData/InstrProfIndexed.h
>>>>> +++ lib/ProfileData/InstrProfIndexed.h
>>>>> @@ -47,7 +47,7 @@
>>>>>  }
>>>>>  const uint64_t Magic = 0x8169666f72706cff; // "\xfflprofi\x81"
>>>>> -const uint64_t Version = 2;
>>>>> +const uint64_t Version = 3;
>>>>>  const HashT HashType = HashT::MD5;
>>>>>  }
>>>>> Index: lib/ProfileData/InstrProf.cpp
>>>>> ===================================================================
>>>>> ---
>>> lib/ProfileData/InstrProf.cpp
>>>>> +++ lib/ProfileData/InstrProf.cpp
>>>>> @@ -50,6 +50,8 @@
>>>>>        return "Function count mismatch";
>>>>>      case instrprof_error::counter_overflow:
>>>>>        return "Counter overflow";
>>>>> +    case instrprof_error::value_site_count_mismatch:
>>>>> +      return "Function's value site counts mismatch";
>>>>>      }
>>>>>      llvm_unreachable("A value of instrprof_error has no message.");
>>>>>    }
>>>>> Index: include/llvm/ProfileData/InstrProfWriter.h
>>>>> ===================================================================
>>>>> ---
>>> include/llvm/ProfileData/InstrProfWriter.h
>>>>> +++ include/llvm/ProfileData/InstrProfWriter.h
>>>>> @@ -15,33 +15,30 @@
>>>>>  #ifndef LLVM_PROFILEDATA_INSTRPROFWRITER_H
>>>>>  #define LLVM_PROFILEDATA_INSTRPROFWRITER_H
>>>>> -#include "llvm/ADT/ArrayRef.h"
>>>>>  #include "llvm/ADT/DenseMap.h"
>>>>>  #include "llvm/ADT/StringMap.h"
>>>>>  #include "llvm/ProfileData/InstrProf.h"
>>>>>  #include "llvm/Support/DataTypes.h"
>>>>>  #include "llvm/Support/MemoryBuffer.h"
>>>>>  #include "llvm/Support/raw_ostream.h"
>>>>> -#include <vector>
>>>>>  namespace llvm {
>>>>>  /// Writer for instrumentation based profile data.
>>>>>  class InstrProfWriter {
>>>>>  public:
>>>>> -  typedef SmallDenseMap<uint64_t, std::vector<uint64_t>, 1>
>>>> CounterData;
>>>>> +  typedef SmallDenseMap<uint64_t, InstrProfRecord, 1> ProfilingData;
>>>>>  private:
>>>>> -  StringMap<CounterData> FunctionData;
>>>>> +  InstrProfStringTable StringTable;
>>>>> +  StringMap<ProfilingData> FunctionData;
>>>>>    uint64_t MaxFunctionCount;
>>>>>  public:
>>>>>    InstrProfWriter() : MaxFunctionCount(0) {}
>>>>>    /// Add function counts for the given function. If there are
>>>>> already
>>>> counts
>>>>>    /// for this function and the hash and number of counts match,
> each
>>>> counter is
>>>>>    /// summed.
>>>>> -  std::error_code addFunctionCounts(StringRef FunctionName,
>>>>> -                                    uint64_t FunctionHash,
>>>>> -                                    ArrayRef<uint64_t> Counters); +
>>> std::error_code addRecord(InstrProfRecord &I);
>>>
>>>> I'm not a fan of dropping const here, but I think it can come back if
>>>> we
>>> move the updateStringTable method anyway.
>>>
>>> Same here. But, I couldn't find a way to get it to work otherwise.
>>>
>>>>>    /// Write the profile to \c OS
>>>>>    void write(raw_fd_ostream &OS);
>>>>>    /// Write the profile, returning the raw data. For testing.
>>>>> Index: include/llvm/ProfileData/InstrProfReader.h
>>>>> ===================================================================
>>>>> ---
>>> include/llvm/ProfileData/InstrProfReader.h
>>>>> +++ include/llvm/ProfileData/InstrProfReader.h
>>>>> @@ -24,6 +24,7 @@
>>>>>  #include "llvm/Support/MemoryBuffer.h"
>>>>>  #include "llvm/Support/OnDiskHashTable.h"
>>>>>  #include <iterator>
>>>>> +#include <map>
>>>>>  namespace llvm {
>>>>> @@ -145,7 +146,6 @@
>>>>>      const uint64_t CountersDelta;
>>>>>      const uint64_t NamesDelta;
>>>>>    };
>>>>> -
>>>>>    bool ShouldSwapBytes;
>>>>>    uint64_t CountersDelta;
>>>>>    uint64_t NamesDelta;
>>>>> @@ -195,10 +195,13 @@
>>>>>    std::vector<InstrProfRecord> DataBuffer;
>>>>>    IndexedInstrProf::HashT HashType;
>>>>>    unsigned FormatVersion;
>>>>> +  const std::map<uint64_t, const char*> &HashKeyMap;
>>>>>  public:
>>>>> -  InstrProfLookupTrait(IndexedInstrProf::HashT HashType, unsigned
>>>> FormatVersion)
>>>>> -      : HashType(HashType), FormatVersion(FormatVersion) {}
>>>>> +  InstrProfLookupTrait(IndexedInstrProf::HashT HashType,
>>>>> +      unsigned FormatVersion, std::map<uint64_t, const char*>
>>>> &HashKeyMap)
>>>>> +      : HashType(HashType), FormatVersion(FormatVersion),
>>>>> +      HashKeyMap(HashKeyMap) {}
>>>>>    typedef ArrayRef<InstrProfRecord> data_type;
>>>>> @@ -209,6 +212,7 @@
>>>>>    static bool EqualKey(StringRef A, StringRef B) { return A == B; }
>>> static StringRef GetInternalKey(StringRef K) { return K; }
>>>>> +  static StringRef GetExternalKey(StringRef K) { return K; }
>>>>>    hash_value_type ComputeHash(StringRef K);
>>>>> @@ -224,7 +228,17 @@
>>>>>      return StringRef((const char *)D, N);
>>>>>    }
>>>>> -  data_type ReadData(StringRef K, const unsigned char *D,
> offset_type
>>>> N);
>>>>> +  data_type ReadDataV1V2(StringRef K, const unsigned char *D,
>>>> offset_type N);
>>>>> +  data_type ReadDataV3(StringRef K, const unsigned char *D,
>>> offset_type
>>>> N);
>>>>> +
>>>>> +  data_type ReadData(StringRef K, const unsigned char *D,
> offset_type
>>>> N) {
>>>>> +    switch (FormatVersion) {
>>>>> +      case 1:
>>>>> +      case 2: return ReadDataV1V2(K, D, N);
>>>>> +      case 3: return ReadDataV3(K, D, N);
>>>>> +    }
>>>>> +    return data_type();
>>>>> +  }
>>>>>  };
>>>>>  typedef OnDiskIterableChainedHashTable<InstrProfLookupTrait>
>>>>> @@ -243,6 +257,10 @@
>>>>>    uint64_t FormatVersion;
>>>>>    /// The maximal execution count among all functions.
>>>>>    uint64_t MaxFunctionCount;
>>>>> +  /// String table.
>>>>> +  InstrProfStringTable StringTable;
>>>>> +  /// Map of hash values to const char* keys in profiling data. +
>>> std::map<uint64_t, const char*> HashKeyMap;
>>>>>    IndexedInstrProfReader(const IndexedInstrProfReader &) = delete;
>>> IndexedInstrProfReader &operator=(const IndexedInstrProfReader &) =
>>>> delete;
>>>>> @@ -261,6 +279,13 @@
>>>>>    /// Fill Counts with the profile data for the given function name.
>>> std::error_code getFunctionCounts(StringRef FuncName, uint64_t
>>>> FuncHash,
>>>>>                                      std::vector<uint64_t> &Counts);
>>>>> +
>>>>> +  /// Return value profile data for the given function name and hash
>>>> and
>>>>> +  /// value profiling kind
>>>>> +  std::error_code getFunctionValuesForKind(StringRef FuncName, +
>>> uint64_t FuncHash, uint32_t ValueKind,
>>>>> +      std::vector<InstrProfValueSiteRecord> &Values);
>>>>> +
>>>>>    /// Return the maximum of all known function counts.
>>>>>    uint64_t getMaximumFunctionCount() { return MaxFunctionCount; }
>>>>> Index: include/llvm/ProfileData/InstrProf.h
>>>>> ===================================================================
>>>>> ---
>>> include/llvm/ProfileData/InstrProf.h
>>>>> +++ include/llvm/ProfileData/InstrProf.h
>>>>> @@ -16,10 +16,11 @@
>>>>>  #ifndef LLVM_PROFILEDATA_INSTRPROF_H_
>>>>>  #define LLVM_PROFILEDATA_INSTRPROF_H_
>>>>> -#include "llvm/ADT/StringRef.h"
>>>>> +#include "llvm/ADT/StringSet.h"
>>>>>  #include <cstdint>
>>>>> -#include <system_error>
>>>>> +#include <list>
>>>>>  #include <vector>
>>>>> +#include <system_error>
>>>>>  namespace llvm {
>>>>>  const std::error_category &instrprof_category();
>>>>> @@ -37,21 +38,99 @@
>>>>>      unknown_function,
>>>>>      hash_mismatch,
>>>>>      count_mismatch,
>>>>> -    counter_overflow
>>>>> +    counter_overflow,
>>>>> +    value_site_count_mismatch
>>>>>  };
>>>>>  inline std::error_code make_error_code(instrprof_error E) {
>>>>>    return std::error_code(static_cast<int>(E), instrprof_category());
>>>>>  }
>>>>> +enum instrprof_value_kind : uint32_t {
>>>>> +    first = 0,
>>>>> +    indirect_call_target = 0,
>>>>> +    size = 1
>>>>> +};
>>>>> +
>>>>> +struct InstrProfStringTable {
>>>>> +  // Set of string values in profiling data.
>>>>> +  StringSet<> StringValueSet;
>>>>> +  InstrProfStringTable() { StringValueSet.clear(); }
>>>>> +  // Get a pointer to internal storage of a string in set
>>>>> +  const char* getStringData(StringRef Str) {
>>>>> +    auto Result = StringValueSet.find(Str);
>>>>> +    return (Result == StringValueSet.end()) ? nullptr :
>>>> Result->first().data();
>>>>> +  }
>>>>> +  // Insert a string to StringTable
>>>>> +  const char* insertString(StringRef Str) {
>>>>> +    auto Result = StringValueSet.insert(Str);
>>>>> +    return Result.first->first().data();
>>>>> +  }
>>>>> +};
>>>> Is this type giving us any value over just using the StringSet<>
>>> directly?
>>>
>>> This is returning const char*'s which is what's stored in the value
> data
>>> entries for indirect call target data. So I found wrapping it like this
>>> very useful.
>>>
>>>>> +
>>>>> +struct InstrProfValueSiteRecord {
>>>>> +  // Typedef for a single TargetValue-NumTaken pair.
>>>>> +  typedef std::pair<uint64_t, uint64_t> ValueDataPair;
>>>>> +  // Value profiling data pairs at a given value site.
>>>>> +  std::list<ValueDataPair> ValueData;
>>>>> +
>>>>> +  InstrProfValueSiteRecord() { ValueData.clear(); }
>>>>> +  InstrProfValueSiteRecord(const InstrProfValueSiteRecord &Rec) +
>>> : ValueData(std::move(Rec.ValueData)) {}
>>>>> +
>>>>> +  // Sort ValueData ascending by TargetValue
>>>>> +  void sortByTargetValues() {
>>>>> +    ValueData.sort([](const ValueDataPair &left, const ValueDataPair
>>>> &right)
>>>>> +                { return left.first < right.first; });
>>>> clang-format and follow LLVM convention for variable names please.
>>>
>>> Done.
>>>
>>>>> +  }
>>>>> +  // Sort ValueData descending by NumTaken
>>>>> +  void sortByNumTaken() {
>>>>> +    ValueData.sort([](const ValueDataPair &left, const ValueDataPair
>>>> &right)
>>>>> +                { return left.second > right.second; });
>>>>> +  }
>>>> This seems to be unused.
>>>
>>> Removed and will be re-introduced w/ the rest of the patches.
>>>
>>>>> +  // Merge data from another InstrProfValueSiteRecord
>>>>> +  void mergeValueData(InstrProfValueSiteRecord &Input) {
>>>>> +    this->sortByTargetValues();
>>>>> +    Input.sortByTargetValues();
>>>> It seems wasteful to re-sort these all the time. Would it make sense
> to
>>> sort at read time, or possibly even canonicalize the on-disk order to
>>> already be sorted?
>>>
>>> I left it as is at this time. Once the API is provided one can not
>>> guarantee the future uses of it. So using the sorting here guarantees
>>> that
>>> future possible usages of the API's  will not break this merge
>>> implementation.
>>>
>>>> I guess it might make sense for the on-disk order to be sorted by
>>> num-taken if that's what the frontend will want, and since merging is
>>> less
>>> common the sort cost is okay. In any case, we should probably set it up
>>> so
>>> that the on disk format is always sorted in a particular way - that
> will
>>> remove some non-determinism and we can use it to speed up whichever
>>> operation we think is more important.
>>>
>>> I'll make sure the on disk format is always sorted.
>>>
>>>>> +    auto I = ValueData.begin();
>>>>> +    auto J = Input.ValueData.begin();
>>>>> +    while (J != Input.ValueData.end()) {
>>>>> +      while (I != ValueData.end() && I->first < J->first)
>>>>> +        ++I;
>>>>> +      if (I != ValueData.end() && I->first == J->first) {
>>>>> +        I->second += J->second;
>>>>> +        ++I;
>>>>> +        ++J;
>>>>> +        continue;
>>>>> +      }
>>>>> +      ValueData.insert(I, *J);
>>>>> +      ++J;
>>>>> +    }
>>>> I suspect a for-loop would read better here. Also please store the
>>> ".end()" values in variables rather than re-evaluating.
>>>
>>> Done.
>>>
>>>>> +  }
>>>>> +};
>>>>> +
>>>>>  /// Profiling information for a single function.
>>>>>  struct InstrProfRecord {
>>>>>    InstrProfRecord() {}
>>>>>    InstrProfRecord(StringRef Name, uint64_t Hash,
>>>>> std::vector<uint64_t>
>>>> Counts)
>>>>>        : Name(Name), Hash(Hash), Counts(std::move(Counts)) {}
>>>>>    StringRef Name;
>>>>>    uint64_t Hash;
>>>>>    std::vector<uint64_t> Counts;
>>>>> +  // Size of vector indicates the number of value sites for a value
>>>> kind
>>>>> +  std::vector<InstrProfValueSiteRecord>
>>>> ValueSites[instrprof_value_kind::size];
>>>> I don't think we're gaining much by having this be an array - I was
>>> thinking it would be more like
>>>>   std::vector<InstrProfValueSiteRecord> IndirectCalls;
>>>> Then when we add more value types, they can have their own variables
>>>> and
>>> be accessed directly. Most of the code that works with these will have
> a
>>> particular kind in mind, and since the value data is dependent on kind
>>> looping over these isn't generally that useful. That is, the looping we
>>> have now is only in the reader and writer, and I can't see the users of
>>> the data ever doing that.
>>>
>>> I think, I'm leaning towards keeping an array of kinds here.
>>>
>>>> For the reader and writer, a switch statement over the kinds will
> allow
>>> us to warn if someone doesn't update somewhere when they add a new
> kind.
>>> For the users of profile data, Data->IndirectCalls reads a lot better
>>> than
>>> Data->ValueSites[instrprof_value_kind::indirect_call_target].
>>>
>>> I'll revisit my latest patch to allow for that.
>>>
>>>> It really seems like the right trade off to me.
>>>>> +  // Clear value data entries
>>>>> +  void clearValueData() {
>>>>> +    for (uint32_t Kind = instrprof_value_kind::first;
>>>>> +         Kind < instrprof_value_kind::size; ++Kind)
>>>>> +      ValueSites[Kind].clear();
>>>>> +  }
>>>
>>>> This is never called, it can be removed.
>>>
>>> Done.
>>>
>>>>> +  void updateStringTableReferences(InstrProfStringTable &StrTable) {
>>>>> +
>>>    Name = StrTable.insertString(Name);
>>>>> +    for (auto VSite :
>>>> ValueSites[instrprof_value_kind::indirect_call_target])
>>>>> +      for (auto VData : VSite.ValueData)
>>>>> +        VData.first = (uint64_t)StrTable.insertString((const char
>>>> *)VData.first);
>>>>> +  }
>>>>>  };
>>>>>  } // end namespace llvm
>>>
>>>
>>>
>>>
>>>
>>>
>>
>
>
>



More information about the llvm-commits mailing list