[PATCH] LLVM changes for indirect call target profiling support
Justin Bogner
mail at justinbogner.com
Thu May 21 17:19:44 PDT 2015
Betul Buyukkurt <betulb at codeaurora.org> writes:
> New fields are reserved in enum instr_value_prof_kind : uint32_t {
> ... } for future value profiling types.
>
>
> http://reviews.llvm.org/D8908
>
> Files:
> docs/LangRef.rst
> include/llvm/IR/IntrinsicInst.h
> include/llvm/IR/Intrinsics.td
> include/llvm/ProfileData/InstrProf.h
> include/llvm/ProfileData/InstrProfReader.h
> include/llvm/ProfileData/InstrProfRecord.h
> include/llvm/ProfileData/InstrProfWriter.h
> lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> lib/ProfileData/InstrProfIndexed.h
> lib/ProfileData/InstrProfReader.cpp
> lib/ProfileData/InstrProfWriter.cpp
> lib/Transforms/Instrumentation/InstrProfiling.cpp
> test/Instrumentation/InstrProfiling/linkage.ll
> test/Instrumentation/InstrProfiling/platform.ll
> test/Instrumentation/InstrProfiling/profiling.ll
> tools/llvm-profdata/llvm-profdata.cpp
> unittests/ProfileData/CoverageMappingTest.cpp
> unittests/ProfileData/InstrProfTest.cpp
>
> EMAIL PREFERENCES
> http://reviews.llvm.org/settings/panel/emailpreferences/
>
> Index: docs/LangRef.rst
> ===================================================================
> --- docs/LangRef.rst
> +++ docs/LangRef.rst
> @@ -8064,6 +8064,53 @@
> format that can be written out by a compiler runtime and consumed via
> the ``llvm-profdata`` tool.
>
> +'``llvm.instrprof_instrument_target``' Intrinsic
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Syntax:
> +"""""""
> +
> +::
> +
> + declare void @llvm.instrprof_instrument_target(i8* <name>, i64 <hash>,
> + i32 <value_kind>,
> + i64 <value>,
> + i32 <index>)
> +
> +Overview:
> +"""""""""
> +
> +The '``llvm.instrprof_instrument_target``' intrinsic can be emitted by a
> +frontend for use with instrumentation based profiling. This will be
> +lowered by the ``-instrprof`` pass to find out the target values,
> +instrumented expressions take in a program at runtime.
> +
> +Arguments:
> +""""""""""
> +
> +The first argument is a pointer to a global variable containing the
> +name of the entity being instrumented. ``name`` should generally be the
> +(mangled) function name for a set of counters.
> +
> +The second argument is a hash value that can be used by the consumer
> +of the profile data to detect changes to the instrumented source. It
> +is an error if ``hash`` differs between two instances of
> +``instrprof_instrument_target`` that refer to the same name.
> +
> +The third argument represents the kind of the value profiling that is done.
> +The fourth argument is the value of the expression being profiled. The last
> +argument is the index of the instrumented expression within ``name``.
> +It should be >= 0.
These arguments need further explanation.
> +
> +Semantics:
> +""""""""""
> +
> +This intrinsic represents the point where a call to a runtime routine
> +should be inserted for profiling of values at target expressions. It will
> +cause the ``-instrprof`` pass to generate the appropriate data
> +structures and the call to the profile runtime library with proper
> +arguments.
> +
> Standard C Library Intrinsics
> -----------------------------
>
> Index: include/llvm/IR/IntrinsicInst.h
> ===================================================================
> --- include/llvm/IR/IntrinsicInst.h
> +++ include/llvm/IR/IntrinsicInst.h
> @@ -372,6 +372,38 @@
> return cast<ConstantInt>(const_cast<Value *>(getArgOperand(3)));
> }
> };
> +
> + /// This represents the llvm.instrprof_instrument_target intrinsic.
> + class InstrProfInstrumentTargetInst : public IntrinsicInst {
> + public:
> + static inline bool classof(const IntrinsicInst *I) {
> + return I->getIntrinsicID() == Intrinsic::instrprof_instrument_target;
> + }
> + static inline bool classof(const Value *V) {
> + return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
> + }
> +
> + GlobalVariable *getName() const {
> + return cast<GlobalVariable>(
> + const_cast<Value *>(getArgOperand(0))->stripPointerCasts());
> + }
> +
> + ConstantInt *getHash() const {
> + return cast<ConstantInt>(const_cast<Value *>(getArgOperand(1)));
> + }
> +
> + ConstantInt *getValueKind() const {
> + return cast<ConstantInt>(const_cast<Value *>(getArgOperand(2)));
> + }
> +
> + Value *getTargetValue() const {
> + return cast<Value>(const_cast<Value *>(getArgOperand(3)));
> + }
> +
> + ConstantInt *getIndex() const {
> + return cast<ConstantInt>(const_cast<Value *>(getArgOperand(4)));
> + }
> + };
> }
>
> #endif
> Index: include/llvm/IR/Intrinsics.td
> ===================================================================
> --- include/llvm/IR/Intrinsics.td
> +++ include/llvm/IR/Intrinsics.td
> @@ -304,6 +304,13 @@
> llvm_i32_ty, llvm_i32_ty],
> []>;
>
> +// A call to profile runtime for value profiling of target expressions
> +// through instrumentation based profiling.
> +def int_instrprof_instrument_target : Intrinsic<[],
> + [llvm_ptr_ty, llvm_i64_ty, llvm_i32_ty,
> + llvm_i64_ty, llvm_i32_ty],
> + []>;
> +
> //===------------------- Standard C Library Intrinsics --------------------===//
> //
>
> Index: include/llvm/ProfileData/InstrProf.h
> ===================================================================
> --- include/llvm/ProfileData/InstrProf.h
> +++ include/llvm/ProfileData/InstrProf.h
> @@ -16,9 +16,19 @@
> #ifndef LLVM_PROFILEDATA_INSTRPROF_H_
> #define LLVM_PROFILEDATA_INSTRPROF_H_
>
> +#include <cstdint>
> #include <system_error>
>
> namespace llvm {
> +
> +enum instr_value_prof_kind : uint32_t {
> + first = 0,
> + indirect_call_target = 0,
> + reserved_1 = 1,
> + reserved_2 = 2,
> + last = 3
> +};
> +
> const std::error_category &instrprof_category();
>
> enum class instrprof_error {
> Index: include/llvm/ProfileData/InstrProfReader.h
> ===================================================================
> --- include/llvm/ProfileData/InstrProfReader.h
> +++ include/llvm/ProfileData/InstrProfReader.h
> @@ -17,28 +17,21 @@
>
> #include "llvm/ADT/ArrayRef.h"
> #include "llvm/ADT/StringExtras.h"
> -#include "llvm/ProfileData/InstrProf.h"
> +#include "llvm/ProfileData/InstrProfRecord.h"
> #include "llvm/Support/EndianStream.h"
> #include "llvm/Support/ErrorOr.h"
> +#include "llvm/Support/raw_ostream.h"
> #include "llvm/Support/LineIterator.h"
> #include "llvm/Support/MemoryBuffer.h"
> #include "llvm/Support/OnDiskHashTable.h"
> #include <iterator>
> +#include <map>
> +#include <string>
>
> namespace llvm {
>
> class InstrProfReader;
>
> -/// Profiling information for a single function.
> -struct InstrProfRecord {
> - InstrProfRecord() {}
> - InstrProfRecord(StringRef Name, uint64_t Hash, ArrayRef<uint64_t> Counts)
> - : Name(Name), Hash(Hash), Counts(Counts) {}
> - StringRef Name;
> - uint64_t Hash;
> - ArrayRef<uint64_t> Counts;
> -};
> -
> /// A file format agnostic iterator over profiling data.
> class InstrProfIterator : public std::iterator<std::input_iterator_tag,
> InstrProfRecord> {
> @@ -138,7 +131,7 @@
> /// that wrote the profile.
> template <class IntPtrT>
> class RawInstrProfReader : public InstrProfReader {
> -private:
> +protected:
> /// The profile data file contents.
> std::unique_ptr<MemoryBuffer> DataBuffer;
> /// The current set of counter values.
> @@ -159,8 +152,8 @@
> const uint64_t CountersDelta;
> const uint64_t NamesDelta;
> };
> -
> bool ShouldSwapBytes;
> + uint64_t FormatVersion;
> uint64_t CountersDelta;
> uint64_t NamesDelta;
> const ProfileData *Data;
> @@ -171,17 +164,19 @@
>
> RawInstrProfReader(const RawInstrProfReader &) = delete;
> RawInstrProfReader &operator=(const RawInstrProfReader &) = delete;
> +
> public:
> RawInstrProfReader(std::unique_ptr<MemoryBuffer> DataBuffer)
> : DataBuffer(std::move(DataBuffer)) { }
>
> static bool hasFormat(const MemoryBuffer &DataBuffer);
> std::error_code readHeader() override;
> std::error_code readNextRecord(InstrProfRecord &Record) override;
>
> -private:
> +protected:
> std::error_code readNextHeader(const char *CurrentPos);
> std::error_code readHeader(const RawHeader &Header);
> + std::error_code readData(InstrProfRecord &Record);
> template <class IntT>
> IntT swap(IntT Int) const {
> return ShouldSwapBytes ? sys::getSwappedBytes(Int) : Int;
> @@ -199,24 +194,82 @@
> typedef RawInstrProfReader<uint32_t> RawInstrProfReader32;
> typedef RawInstrProfReader<uint64_t> RawInstrProfReader64;
>
> +/// Reader for the raw instrumented value profile binary format from runtime.
> +///
> +/// This format is a raw memory dump of the instrumentation-based value profiling
> +/// data from the runtime. It has no index.
> +///
> +/// Templated on the unsigned type whose size matches pointers on the platform
> +/// that wrote the profile.
> +template <class IntPtrT>
> +class RawInstrValueProfReader : public RawInstrProfReader<IntPtrT> {
As discussed in the other email, we don't need this class.
> +private:
> + using Base = RawInstrProfReader<IntPtrT>;
> + using RawHeader = struct Base::RawHeader;
> + using ProfileData = struct Base::ProfileData;
> + using Base::Base;
> + using Base::swap;
> + using Base::getName;
> +
> + struct ValueData {
> + ProfileData Data;
> + const IntPtrT FunctionPointer;
> + const uint32_t NumValueSites[instr_value_prof_kind::last];
> + const IntPtrT Values[instr_value_prof_kind::last]; // NULL, used at runtime
> + };
> + struct ValueHeader {
> + RawHeader Header;
> + const uint64_t ValuesSize;
> + const uint64_t ValuesDelta;
> + };
> + struct ValueEntry {
> + const uint64_t Value;
> + const uint64_t NumTaken;
> + const uint32_t ValueKind;
> + const uint32_t CounterIndex;
> + const IntPtrT VDataIndex;
> + };
> + const ValueData *VData;
> + const ValueData *VDataEnd;
> + const ValueEntry *CurrentVEntry;
> + uint64_t CurrentVDataIndex;
> + std::map<const IntPtrT, std::string> FunctionPtrToNameMap;
> +
> + RawInstrValueProfReader(const RawInstrValueProfReader &) = delete;
> + RawInstrValueProfReader &operator=(const RawInstrValueProfReader &) = delete;
> +
> +public:
> + RawInstrValueProfReader(std::unique_ptr<MemoryBuffer> DataBuffer)
> + : Base(std::move(DataBuffer)) { }
> +
> + static bool hasVPFormat(const MemoryBuffer &DataBuffer);
> + std::error_code readHeader() override;
> + std::error_code readNextRecord(InstrProfRecord &Record) override;
> +
> +protected:
> + std::error_code readNextHeader(const char *CurrentPos);
> + std::error_code readHeader(const ValueHeader &VHeader);
> +};
> +
> +typedef RawInstrValueProfReader<uint32_t> RawInstrValueProfReader32;
> +typedef RawInstrValueProfReader<uint64_t> RawInstrValueProfReader64;
> +
> namespace IndexedInstrProf {
> enum class HashT : uint32_t;
> }
>
> /// Trait for lookups into the on-disk hash table for the binary instrprof
> /// format.
> class InstrProfLookupTrait {
> - std::vector<uint64_t> DataBuffer;
> + std::vector<InstrProfRecord> DataBuffer;
This means we're copying a lot more data around - we need to allocate
std::vectors for each set of counters and for each set of value
profiling data, rather than not copying any data at all. I'm not sure if
it's a problem in practice, but we should be aware of the tradeoffs here.
> IndexedInstrProf::HashT HashType;
> + unsigned FormatVersion;
> public:
> - InstrProfLookupTrait(IndexedInstrProf::HashT HashType) : HashType(HashType) {}
> + InstrProfLookupTrait(IndexedInstrProf::HashT HashType, unsigned FormatVersion)
> + : HashType(HashType), FormatVersion(FormatVersion) {}
> +
> + typedef ArrayRef<InstrProfRecord> data_type;
>
> - struct data_type {
> - data_type(StringRef Name, ArrayRef<uint64_t> Data)
> - : Name(Name), Data(Data) {}
> - StringRef Name;
> - ArrayRef<uint64_t> Data;
> - };
> typedef StringRef internal_key_type;
> typedef StringRef external_key_type;
> typedef uint64_t hash_value_type;
> @@ -240,21 +293,110 @@
> }
>
> data_type ReadData(StringRef K, const unsigned char *D, offset_type N) {
> - DataBuffer.clear();
> + switch (FormatVersion) {
> + case 1:
> + case 2: return ReadDataV1V2(K, D, N);
> + case 3: return ReadDataV3(K, D, N);
> + }
> + return data_type();
> + }
> +
> + data_type ReadDataV1V2(StringRef K, const unsigned char *D, offset_type N) {
These are too complex to leave in the header. Please move them to
InstrProfReader.cpp.
> + // Check if the data is corrupt. If so, don't try to read it.
> if (N % sizeof(uint64_t))
> - // The data is corrupt, don't try to read it.
> - return data_type("", DataBuffer);
> + return data_type();
> +
> + DataBuffer.clear();
> + uint64_t NumCounts;
> + uint64_t NumEntries = N / sizeof(uint64_t);
> + std::vector<uint64_t> CounterBuffer;
> + for (uint64_t I = 0; I < NumEntries; I += NumCounts) {
> + using namespace support;
> + // The function hash comes first.
> + uint64_t Hash = endian::readNext<uint64_t, little, unaligned>(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<uint64_t, little, unaligned>(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<uint64_t, little, unaligned>(D));
> +
> + DataBuffer.push_back(InstrProfRecord(K, Hash, CounterBuffer));
> + }
> + return DataBuffer;
> + }
>
> + data_type ReadDataV3(StringRef K, const unsigned char *D, offset_type N) {
> 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<uint64_t, little, unaligned>(D));
> - return data_type(K, DataBuffer);
> + const unsigned char *Start = D;
> + if (D - Start + sizeof (uint64_t) > N)
> + return data_type();
> + uint64_t DataBufferSize = endian::readNext<uint64_t, little, unaligned>(D);
> + DataBuffer.clear();
> + DataBuffer.reserve(DataBufferSize);
> + std::vector<uint64_t> CounterBuffer;
> + std::vector<InstrProfValueRecord> ValueBuffer;
> + for (unsigned I = 0; I < DataBufferSize; ++I) {
> + if (D - Start + sizeof (uint64_t) + sizeof(uint32_t) >= N)
> + return data_type();
> + uint64_t Hash = endian::readNext<uint64_t, little, unaligned>(D);
> + uint32_t CountsSize = endian::readNext<uint32_t, little, unaligned>(D);
> + if (D - Start + CountsSize * sizeof(uint64_t) >= N)
> + return data_type();
> + CounterBuffer.clear();
> + for (unsigned I = 0; I < CountsSize; ++I)
> + CounterBuffer.push_back(endian::readNext<uint64_t, little, unaligned>(D));
> +
> + DataBuffer.push_back(InstrProfRecord(K, Hash, CounterBuffer));
> +
> + for (uint32_t Kind = instr_value_prof_kind::first;
> + Kind < instr_value_prof_kind::last; ++Kind) {
> + if (D - Start + 2 * sizeof(uint32_t) > N)
> + return data_type();
> + uint32_t ValueKind = endian::readNext<uint32_t, little, unaligned>(D);
> + if (ValueKind != Kind)
> + return data_type();
> + uint32_t ValuesSize = endian::readNext<uint32_t, little, unaligned>(D);
> + ValueBuffer.clear();
> + for (unsigned I = 0; I < ValuesSize; ++I) {
> + if (D - Start + sizeof(uint32_t) >= N) return data_type();
> + uint32_t NameSize = endian::readNext<uint32_t, little, unaligned>(D);
> + if (D - Start + NameSize * sizeof(char)>= N)
> + return data_type();
> +
> + std::string Name((const char *)D, NameSize);
> + D += NameSize;
> +
> + if (D - Start + 2 * sizeof(uint64_t) >= N)
> + return data_type();
> + uint64_t Value = endian::readNext<uint64_t, little, unaligned>(D);
> + uint64_t NumTaken = endian::readNext<uint64_t, little, unaligned>(D);
> +
> + if (D - Start + sizeof(uint32_t) > N)
> + return data_type();
> + uint32_t CounterIndex = endian::readNext<uint32_t, little, unaligned>(D);
> + ValueBuffer.push_back(
> + InstrProfValueRecord(Name, Value, NumTaken, CounterIndex));
> + }
> + DataBuffer[DataBuffer.size()-1].Values[Kind] = ValueBuffer;
> + }
I guess this'll be simpler without having to deal with the kinds, but
there's also a lot of "D - Start + ..." math in here that I think would
be easier to understand with a few more named variables.
> + }
> + return DataBuffer;
> }
> };
> +
> typedef OnDiskIterableChainedHashTable<InstrProfLookupTrait>
> InstrProfReaderIndex;
>
> @@ -267,8 +409,6 @@
> std::unique_ptr<InstrProfReaderIndex> Index;
> /// Iterator over the profile data.
> InstrProfReaderIndex::data_iterator RecordIterator;
This name is a bit confusing now - it iterates over arrays of records,
not individual ones.
> - /// 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.
> @@ -278,7 +418,7 @@
> IndexedInstrProfReader &operator=(const IndexedInstrProfReader &) = delete;
> public:
> IndexedInstrProfReader(std::unique_ptr<MemoryBuffer> DataBuffer)
> - : DataBuffer(std::move(DataBuffer)), Index(nullptr), CurrentOffset(0) {}
> + : DataBuffer(std::move(DataBuffer)), Index(nullptr) {}
>
> /// Return true if the given buffer is in an indexed instrprof format.
> static bool hasFormat(const MemoryBuffer &DataBuffer);
> @@ -288,9 +428,16 @@
> /// Read a single record.
> std::error_code readNextRecord(InstrProfRecord &Record) override;
>
> - /// Fill Counts with the profile data for the given function name.
> + /// Return profile data for the given function name and hash
> std::error_code getFunctionCounts(StringRef FuncName, uint64_t FuncHash,
> - std::vector<uint64_t> &Counts);
> + std::vector<uint64_t> &Counts);
It really does fill "Counts". It doesn't return anything, so the old doc
comment was better.
Also the formatting is funny here and for the function below -
clang-format will fix that for you.
> +
> + /// 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<InstrProfValueRecord> &Values);
> +
> /// Return the maximum of all known function counts.
> uint64_t getMaximumFunctionCount() { return MaxFunctionCount; }
>
> Index: include/llvm/ProfileData/InstrProfRecord.h
> ===================================================================
> --- /dev/null
> +++ include/llvm/ProfileData/InstrProfRecord.h
> @@ -0,0 +1,67 @@
> +//=-- InstrProfRecord.h - Instrumented profiling format support ---*- C++ -*-=//
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +//
> +// Instrumentation-based profiling data is generated by instrumented
> +// binaries through library functions in compiler-rt, and read by the clang
> +// frontend to feed PGO.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#ifndef LLVM_PROFILEDATA_INSTRPROFRECORD_H_
> +#define LLVM_PROFILEDATA_INSTRPROFRECORD_H_
We don't need this header - just put these definitions in InstrProf.h
> +
> +#include "llvm/ADT/SmallString.h"
> +#include "llvm/ProfileData/InstrProf.h"
> +
> +namespace llvm {
> +
> +// Profiling information for a single target value
> +struct InstrProfValueRecord {
> + InstrProfValueRecord() {};
> + InstrProfValueRecord(uint64_t Value, uint64_t NumTaken, uint32_t CounterIndex)
> + : Value(Value), NumTaken(NumTaken), CounterIndex(CounterIndex) {}
> + InstrProfValueRecord(StringRef Name, uint64_t NumTaken, uint32_t CounterIndex)
> + : Name(Name), NumTaken(NumTaken), CounterIndex(CounterIndex) {}
What are these two constructors that don't initialize all of the fields
for? If we use them we're just asking for memory problems.
> + InstrProfValueRecord(StringRef Name, uint64_t Value, uint64_t NumTaken,
> + uint32_t CounterIndex) : Name(Name), Value(Value), NumTaken(NumTaken),
> + CounterIndex(CounterIndex) {}
> + SmallString<32> Name;
> + uint64_t Value;
> + uint64_t NumTaken;
> + uint32_t CounterIndex;
> +};
> +
> +/// Profiling information for a single function.
> +struct InstrProfRecord {
> + InstrProfRecord() {}
> + InstrProfRecord(StringRef Name, uint64_t Hash, ArrayRef<uint64_t> Counts)
> + : Name(Name), Hash(Hash), Counts(Counts) {}
> + void clearValues() {
> + uint32_t i = instr_value_prof_kind::first;
> + for ( ; i < instr_value_prof_kind::last; ++i)
> + Values[i].clear();
> + }
> + StringRef Name;
> + uint64_t Hash;
> + std::vector<uint64_t> Counts;
> + std::vector<InstrProfValueRecord> Values[instr_value_prof_kind::last];
> +};
> +
> +/// Profiling information for a single function for outputting through the
> +/// IndexedInstrProfWriter
> +struct IndexedInstrProfRecord {
This only used in the writer - better to define it there locally.
> + IndexedInstrProfRecord() {}
> + IndexedInstrProfRecord(ArrayRef<uint64_t> Counts) : Counts(Counts) {}
> + std::vector<uint64_t> Counts;
> + std::vector<InstrProfValueRecord> Values[instr_value_prof_kind::last];
> +};
> +
> +} // end namespace llvm
> +
> +#endif // LLVM_PROFILEDATA_INSTRPROFRECORD_H_
> Index: include/llvm/ProfileData/InstrProfWriter.h
> ===================================================================
> --- include/llvm/ProfileData/InstrProfWriter.h
> +++ include/llvm/ProfileData/InstrProfWriter.h
> @@ -19,6 +19,7 @@
> #include "llvm/ADT/DenseMap.h"
> #include "llvm/ADT/StringMap.h"
> #include "llvm/ProfileData/InstrProf.h"
> +#include "llvm/ProfileData/InstrProfRecord.h"
> #include "llvm/Support/DataTypes.h"
> #include "llvm/Support/MemoryBuffer.h"
> #include "llvm/Support/raw_ostream.h"
> @@ -29,20 +30,18 @@
> /// Writer for instrumentation based profile data.
> class InstrProfWriter {
> public:
> - typedef SmallDenseMap<uint64_t, std::vector<uint64_t>, 1> CounterData;
> + typedef SmallDenseMap<uint64_t, IndexedInstrProfRecord, 1> ProfilingData;
> private:
> - StringMap<CounterData> FunctionData;
> + 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);
> - /// Write the profile to \c OS
> + std::error_code addFunctionCounts(const InstrProfRecord &I);
Might as well rename this to addRecord - it does more than function
counts now.
> + /// Ensure that all data is written to disk.
> void write(raw_fd_ostream &OS);
> /// Write the profile, returning the raw data. For testing.
> std::unique_ptr<MemoryBuffer> writeBuffer();
> Index: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> ===================================================================
> --- lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> +++ lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> @@ -4896,7 +4896,8 @@
> }
> case Intrinsic::instrprof_increment:
> llvm_unreachable("instrprof failed to lower an increment");
> -
> + case Intrinsic::instrprof_instrument_target:
> + llvm_unreachable("instrprof failed to lower an value profiling call");
> case Intrinsic::frameescape: {
> MachineFunction &MF = DAG.getMachineFunction();
> const TargetInstrInfo *TII = DAG.getSubtarget().getInstrInfo();
> 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/InstrProfReader.cpp
> ===================================================================
> --- lib/ProfileData/InstrProfReader.cpp
> +++ lib/ProfileData/InstrProfReader.cpp
> @@ -52,6 +52,10 @@
> // Create the reader.
> if (IndexedInstrProfReader::hasFormat(*Buffer))
> Result.reset(new IndexedInstrProfReader(std::move(Buffer)));
> + else if (RawInstrValueProfReader64::hasVPFormat(*Buffer))
> + Result.reset(new RawInstrValueProfReader64(std::move(Buffer)));
> + else if (RawInstrValueProfReader32::hasVPFormat(*Buffer))
> + Result.reset(new RawInstrValueProfReader32(std::move(Buffer)));
> else if (RawInstrProfReader64::hasFormat(*Buffer))
> Result.reset(new RawInstrProfReader64(std::move(Buffer)));
> else if (RawInstrProfReader32::hasFormat(*Buffer))
> @@ -183,8 +187,6 @@
>
> template <class IntPtrT>
> std::error_code RawInstrProfReader<IntPtrT>::readHeader() {
> - if (!hasFormat(*DataBuffer))
> - return error(instrprof_error::bad_magic);
> if (DataBuffer->getBufferSize() < sizeof(RawHeader))
> return error(instrprof_error::bad_header);
> auto *Header =
> @@ -256,11 +258,7 @@
>
> template <class IntPtrT>
> std::error_code
> -RawInstrProfReader<IntPtrT>::readNextRecord(InstrProfRecord &Record) {
> - if (Data == DataEnd)
> - if (std::error_code EC = readNextHeader(ProfileEnd))
> - return EC;
> -
> +RawInstrProfReader<IntPtrT>::readData(InstrProfRecord &Record) {
> // Get the raw data.
> StringRef RawName(getName(Data->NamePtr), swap(Data->NameSize));
> uint32_t NumCounters = swap(Data->NumCounters);
> @@ -288,6 +286,20 @@
> } else
> Record.Counts = RawCounts;
>
> + return success();
> +}
> +
> +template <class IntPtrT>
> +std::error_code
> +RawInstrProfReader<IntPtrT>::readNextRecord(InstrProfRecord &Record) {
> +
> + if (Data == DataEnd)
> + if (std::error_code EC = readNextHeader(ProfileEnd))
> + return EC;
> +
> + if (std::error_code EC = readData(Record))
> + return EC;
> +
> // Iterate.
> ++Data;
> return success();
> @@ -298,6 +310,174 @@
> template class RawInstrProfReader<uint64_t>;
> }
>
> +template <class IntPtrT>
> +static uint64_t getRawVPMagic();
> +
> +template <>
> +uint64_t getRawVPMagic<uint64_t>() {
> + return
> + uint64_t(255) << 56 |
> + uint64_t('v') << 48 |
> + uint64_t('p') << 40 |
> + uint64_t('r') << 32 |
> + uint64_t('o') << 24 |
> + uint64_t('f') << 16 |
> + uint64_t('r') << 8 |
> + uint64_t(129);
> +}
> +
> +template <>
> +uint64_t getRawVPMagic<uint32_t>() {
> + return
> + uint64_t(255) << 56 |
> + uint64_t('v') << 48 |
> + uint64_t('p') << 40 |
> + uint64_t('r') << 32 |
> + uint64_t('o') << 24 |
> + uint64_t('f') << 16 |
> + uint64_t('R') << 8 |
> + uint64_t(129);
> +}
> +
> +template <class IntPtrT>
> +bool RawInstrValueProfReader<IntPtrT>::hasVPFormat(
> + const MemoryBuffer &DataBuffer) {
> +
> + if (DataBuffer.getBufferSize() < sizeof(uint64_t))
> + return false;
> + uint64_t Magic =
> + *reinterpret_cast<const uint64_t *>(DataBuffer.getBufferStart());
> + return getRawVPMagic<IntPtrT>() == Magic ||
> + sys::getSwappedBytes(getRawVPMagic<IntPtrT>()) == Magic;
> +}
> +
> +template <class IntPtrT>
> +std::error_code RawInstrValueProfReader<IntPtrT>::readHeader() {
> + if (Base::DataBuffer->getBufferSize() < sizeof(ValueHeader))
> + return Base::error(instrprof_error::bad_header);
> + auto *VHeader =
> + reinterpret_cast<const ValueHeader *>(Base::DataBuffer->getBufferStart());
> + Base::ShouldSwapBytes = VHeader->Header.Magic != getRawVPMagic<IntPtrT>();
> + return readHeader(*VHeader);
> +}
> +
> +template <class IntPtrT>
> +std::error_code
> +RawInstrValueProfReader<IntPtrT>::readNextHeader(const char *CurrentPos) {
> + const char *End = Base::DataBuffer->getBufferEnd();
> + // Skip zero padding between profiles.
> + while (CurrentPos != End && *CurrentPos == 0)
> + ++CurrentPos;
> + // If there's nothing left, we're done.
> + if (CurrentPos == End)
> + return instrprof_error::eof;
> + // If there isn't enough space for another header, this is probably just
> + // garbage at the end of the file.
> + if (CurrentPos + sizeof(ValueHeader) > End)
> + return instrprof_error::malformed;
> + // The writer ensures each profile is padded to start at an aligned address.
> + if (reinterpret_cast<size_t>(CurrentPos) % alignOf<uint64_t>())
> + return instrprof_error::malformed;
> + // The magic should have the same byte order as in the previous header.
> + uint64_t Magic = *reinterpret_cast<const uint64_t *>(CurrentPos);
> + if (Magic != swap(getRawVPMagic<IntPtrT>()))
> + return instrprof_error::bad_magic;
> +
> + // There's another profile to read, so we need to process the header.
> + auto *VHeader = reinterpret_cast<const ValueHeader *>(CurrentPos);
> + return readHeader(*VHeader);
> +}
> +
> +template <class IntPtrT>
> +std::error_code
> +RawInstrValueProfReader<IntPtrT>::readHeader(const ValueHeader &VHeader) {
> + const RawHeader &Header = VHeader.Header;
> + Base::CountersDelta = swap(Header.CountersDelta);
> + Base::NamesDelta = swap(Header.NamesDelta);
> + auto DataSize = swap(Header.DataSize);
> + auto CountersSize = swap(Header.CountersSize);
> + auto NamesSize = swap(Header.NamesSize);
> + auto PaddingSize = sizeof(uint64_t) - NamesSize % sizeof(uint64_t);
> + auto ValuesSize = swap(VHeader.ValuesSize);
> + auto ValuesSizeInBytes = ValuesSize * sizeof(ValueEntry);
> +
> + ptrdiff_t DataOffset = sizeof(ValueHeader);
> + ptrdiff_t CountersOffset = DataOffset + sizeof(ValueData) * DataSize;
> + ptrdiff_t NamesOffset = CountersOffset + sizeof(uint64_t) * CountersSize;
> + ptrdiff_t ValuesOffset = NamesOffset +
> + sizeof(char) * (NamesSize + PaddingSize);
> + size_t ProfileSize = ValuesOffset + ValuesSizeInBytes;
> +
> + auto *Start = reinterpret_cast<const char *>(&VHeader);
> + if (Start + ProfileSize > Base::DataBuffer->getBufferEnd())
> + return Base::error(instrprof_error::bad_header);
> +
> + CurrentVDataIndex = 0;
> + VData = reinterpret_cast<const ValueData *>(Start + DataOffset);
> + VDataEnd = VData + DataSize;
> + Base::CountersStart =
> + reinterpret_cast<const uint64_t *>(Start + CountersOffset);
> + Base::NamesStart = Start + NamesOffset;
> + CurrentVEntry = reinterpret_cast<const ValueEntry *>(Start + ValuesOffset);
> + Base::ProfileEnd = Start + ProfileSize;
> +
> + FunctionPtrToNameMap.clear();
> + for (const ValueData *I = VData; I != VDataEnd; ++I) {
> + std::string RawFunctionName(getName(I->Data.NamePtr), swap(I->Data.NameSize));
> + FunctionPtrToNameMap.insert(std::pair<const IntPtrT, std::string>
> + (swap(I->FunctionPointer), RawFunctionName));
> + }
> + return Base::success();
> +}
> +
> +template <class IntPtrT>
> +std::error_code
> +RawInstrValueProfReader<IntPtrT>::readNextRecord(InstrProfRecord &Record) {
> + if (VData == VDataEnd)
> + if (std::error_code EC = readNextHeader(Base::ProfileEnd))
> + return EC;
> +
> + Base::Data = &(VData->Data);
> + if (std::error_code EC = Base::readData(Record))
> + if (EC != Base::success())
> + return EC;
> +
> + // Read value profiling data.
> + auto *BufferEnd = Base::DataBuffer->getBufferEnd();
> + Record.clearValues();
> + while ((const char*)(CurrentVEntry+1) <= BufferEnd) {
> + if (CurrentVEntry->VDataIndex != CurrentVDataIndex)
> + break;
> + std::string TargetFunctionName;
> + if (CurrentVEntry->ValueKind ==
> + instr_value_prof_kind::indirect_call_target) {
> + typename std::map<const IntPtrT, std::string>::iterator I =
> + FunctionPtrToNameMap.find(swap(CurrentVEntry->Value));
> + if (I == FunctionPtrToNameMap.end()) {
> + ++CurrentVEntry;
> + continue;
> + }
> + TargetFunctionName = I->second;
> + }
> + InstrProfValueRecord NewVEntry;
> + NewVEntry.Name = TargetFunctionName;
> + NewVEntry.Value = swap(CurrentVEntry->Value);
> + NewVEntry.NumTaken = swap(CurrentVEntry->NumTaken);
> + NewVEntry.CounterIndex = swap(CurrentVEntry->CounterIndex);
> + Record.Values[CurrentVEntry->ValueKind].push_back(NewVEntry);
> + ++CurrentVEntry;
> + }
> + // Iterate.
> + ++CurrentVDataIndex;
> + ++VData;
> + return Base::success();
> +}
> +
> +namespace llvm {
> +template class RawInstrValueProfReader<uint32_t>;
> +template class RawInstrValueProfReader<uint64_t>;
> +}
> +
> InstrProfLookupTrait::hash_value_type
> InstrProfLookupTrait::ComputeHash(StringRef K) {
> return IndexedInstrProf::ComputeHash(HashType, K);
> @@ -340,38 +520,55 @@
> 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)));
> + InstrProfLookupTrait(HashType, FormatVersion)));
> // 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);
>
> // Found it. Look for counters with the right hash.
> - ArrayRef<uint64_t> 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);
> + 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) {
> + Counts = Data[I].Counts;
> + return success();
> + }
> + }
> + return error(instrprof_error::hash_mismatch);
> +}
> +
> +std::error_code IndexedInstrProfReader::getFunctionValuesForKind(
> + StringRef FuncName, uint64_t FuncHash, uint32_t ValueKind,
> + std::vector<InstrProfValueRecord> &Values) {
> +
> + 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 (FoundHash == FuncHash) {
> - Counts = Data.slice(I, NumCounts);
> + if (Data[I].Hash == FuncHash) {
> + Values = Data[I].Values[ValueKind];
> return success();
> }
> }
> @@ -384,30 +581,14 @@
> if (RecordIterator == Index->data_end())
> return error(instrprof_error::eof);
>
> - // Record the current function name.
> - Record.Name = (*RecordIterator).Name;
> -
> - ArrayRef<uint64_t> 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())
> + if ((*RecordIterator).empty())
> 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()) {
> + static unsigned RecordIndex = 0;
> + Record = (*RecordIterator)[RecordIndex++];
> + if (RecordIndex >= (*RecordIterator).size()) {
There're a lot of uses of (*RecordIterator) in here, which is awkward
for reading - a named variable would be nice.
> ++RecordIterator;
> - CurrentOffset = 0;
> + RecordIndex = 0;
> }
> -
> return success();
> }
> Index: lib/ProfileData/InstrProfWriter.cpp
> ===================================================================
> --- lib/ProfileData/InstrProfWriter.cpp
> +++ lib/ProfileData/InstrProfWriter.cpp
> @@ -26,8 +26,8 @@
> typedef StringRef key_type;
> typedef StringRef key_type_ref;
>
> - typedef const InstrProfWriter::CounterData *const data_type;
> - typedef const InstrProfWriter::CounterData *const data_type_ref;
> + typedef const InstrProfWriter::ProfilingData *const data_type;
> + typedef const InstrProfWriter::ProfilingData *const data_type_ref;
>
> typedef uint64_t hash_value_type;
> typedef uint64_t offset_type;
> @@ -44,9 +44,25 @@
> offset_type N = K.size();
> LE.write<offset_type>(N);
>
> - offset_type M = 0;
> - for (const auto &Counts : *V)
> - M += (2 + Counts.second.size()) * sizeof(uint64_t);
> + offset_type M = sizeof(uint64_t); // size of ProfileData
> + for (const auto &ProfileData : *V) {
> + M += sizeof(uint64_t); // size of ProfileData.first
> + M += sizeof(uint32_t); // size of ProfileData.second.Counts.size()
> + M += ProfileData.second.Counts.size() * sizeof(uint64_t);
> + for (uint32_t Kind = instr_value_prof_kind::first;
> + Kind < instr_value_prof_kind::last; ++Kind) {
> + M += sizeof(uint32_t); // size of Kind
> + M += sizeof(uint32_t); // size of ProfileData.second.Values[Kind].size()
> + for (InstrProfValueRecord I : ProfileData.second.Values[Kind]) {
> + M += sizeof(uint32_t); // size of I.Name
> + M += I.Name.str().size() * sizeof(char);
> + M += sizeof(uint64_t); // size of I.Value
> + M += sizeof(uint64_t); // size of I.NumTaken
> + M += sizeof(uint32_t); // size of I.CounterIndex
> + }
> + }
> + }
> +
> LE.write<offset_type>(M);
>
> return std::make_pair(N, M);
> @@ -61,51 +77,98 @@
> using namespace llvm::support;
> endian::Writer<little> LE(Out);
>
> - for (const auto &Counts : *V) {
> - LE.write<uint64_t>(Counts.first);
> - LE.write<uint64_t>(Counts.second.size());
> - for (uint64_t I : Counts.second)
> + LE.write<uint64_t>(V->size());
> + for (const auto &ProfileData : *V) {
> + LE.write<uint64_t>(ProfileData.first);
> + LE.write<uint32_t>(ProfileData.second.Counts.size());
> + for (uint64_t I : ProfileData.second.Counts)
> LE.write<uint64_t>(I);
> + for (uint32_t Kind = instr_value_prof_kind::first;
> + Kind < instr_value_prof_kind::last; ++Kind) {
> + LE.write<uint32_t>(Kind);
> + LE.write<uint32_t>(ProfileData.second.Values[Kind].size());
> + for (InstrProfValueRecord I : ProfileData.second.Values[Kind]) {
> + LE.write<uint32_t>(I.Name.str().size());
> + Out.write(I.Name.c_str(), I.Name.str().size());
> + LE.write<uint64_t>(I.Value);
> + LE.write<uint64_t>(I.NumTaken);
> + LE.write<uint32_t>(I.CounterIndex);
> + }
> + }
> }
> }
> };
> }
>
> -std::error_code
> -InstrProfWriter::addFunctionCounts(StringRef FunctionName,
> - uint64_t FunctionHash,
> - ArrayRef<uint64_t> Counters) {
> - auto &CounterData = FunctionData[FunctionName];
> -
> - auto Where = CounterData.find(FunctionHash);
> - if (Where == CounterData.end()) {
> - // We've never seen a function with this name and hash, add it.
> - CounterData[FunctionHash] = Counters;
> - // We keep track of the max function count as we go for simplicity.
> - if (Counters[0] > MaxFunctionCount)
> - MaxFunctionCount = Counters[0];
> - return instrprof_error::success;
> - }
> +static std::error_code combineInstrProfRecords(
> + IndexedInstrProfRecord &Dest, const InstrProfRecord &Source,
> + uint64_t &MaxFunctionCount) {
>
> - // We're updating a function we've seen before.
> - auto &FoundCounters = Where->second;
> - // If the number of counters doesn't match we either have bad data or a hash
> - // collision.
> - if (FoundCounters.size() != Counters.size())
> + // If the number of counters doesn't match we either have bad data
> + // or a hash collision.
> + if (Dest.Counts.size() != Source.Counts.size())
> return instrprof_error::count_mismatch;
>
> - for (size_t I = 0, E = Counters.size(); I < E; ++I) {
> - if (FoundCounters[I] + Counters[I] < FoundCounters[I])
> + for (size_t I = 0, E = Source.Counts.size(); I < E; ++I) {
> + if (Dest.Counts[I] + Source.Counts[I] < Dest.Counts[I])
> return instrprof_error::counter_overflow;
> - FoundCounters[I] += Counters[I];
> + Dest.Counts[I] += Source.Counts[I];
> + }
> +
> + for (uint32_t Kind = instr_value_prof_kind::first;
> + Kind < instr_value_prof_kind::last; ++Kind) {
> + std::vector<InstrProfValueRecord>::const_iterator I =
> + Source.Values[Kind].begin();
> + for ( ; I != Source.Values[Kind].end(); ++I) {
> + bool Found = false;
> + std::vector<InstrProfValueRecord>::iterator J = Dest.Values[Kind].begin();
> + for ( ; J != Dest.Values[Kind].end(); ++J) {
> + if (J->CounterIndex == I->CounterIndex) {
> + if ((Kind == instr_value_prof_kind::indirect_call_target) &&
> + (0 == J->Name.compare(I->Name)))
> + Found = true;
> + else if (J->Value == I->Value)
> + Found = true;
> +
> + if (Found) {
> + J->NumTaken += I->NumTaken;
> + break;
> + }
> + }
> + }
> + if (!Found) Dest.Values[Kind].push_back(*I);
> + }
> }
> +
> // We keep track of the max function count as we go for simplicity.
> - if (FoundCounters[0] > MaxFunctionCount)
> - MaxFunctionCount = FoundCounters[0];
> + if (Dest.Counts[0] > MaxFunctionCount)
> + MaxFunctionCount = Dest.Counts[0];
>
> return instrprof_error::success;
> }
>
> +std::error_code
> +InstrProfWriter::addFunctionCounts(const InstrProfRecord &I) {
> + auto &ProfileDataMap = FunctionData[I.Name];
> +
> + auto Where = ProfileDataMap.find(I.Hash);
> + if (Where == ProfileDataMap.end()) {
> + // We've never seen a function with this name and hash, add it.
> + ProfileDataMap[I.Hash] = IndexedInstrProfRecord(I.Counts);
> + for (uint32_t Kind = instr_value_prof_kind::first;
> + Kind < instr_value_prof_kind::last; ++Kind)
> + ProfileDataMap[I.Hash].Values[Kind] = I.Values[Kind];
> +
> + // We keep track of the max function count as we go for simplicity.
> + if (I.Counts[0] > MaxFunctionCount)
> + MaxFunctionCount = I.Counts[0];
> + return instrprof_error::success;
> + }
> +
> + // We're updating a function we've seen before.
> + return combineInstrProfRecords(Where->second, I, MaxFunctionCount);
> +}
> +
> std::pair<uint64_t, uint64_t> InstrProfWriter::writeImpl(raw_ostream &OS) {
> OnDiskChainedHashTableGenerator<InstrProfRecordTrait> Generator;
>
> Index: lib/Transforms/Instrumentation/InstrProfiling.cpp
> ===================================================================
> --- lib/Transforms/Instrumentation/InstrProfiling.cpp
> +++ lib/Transforms/Instrumentation/InstrProfiling.cpp
> @@ -7,9 +7,9 @@
> //
> //===----------------------------------------------------------------------===//
> //
> -// This pass lowers instrprof_increment intrinsics emitted by a frontend for
> -// profiling. It also builds the data structures and initialization code needed
> -// for updating execution counts and emitting the profile at runtime.
> +// This pass lowers instrprof_* intrinsics emitted by a frontend for profiling.
> +// It also builds the data structures and initialization code needed for
> +// updating execution counts and emitting the profile at runtime.
> //
> //===----------------------------------------------------------------------===//
>
> @@ -19,6 +19,7 @@
> #include "llvm/IR/IRBuilder.h"
> #include "llvm/IR/IntrinsicInst.h"
> #include "llvm/IR/Module.h"
> +#include "llvm/ProfileData/InstrProf.h"
> #include "llvm/Transforms/Utils/ModuleUtils.h"
>
> using namespace llvm;
> @@ -49,7 +50,15 @@
> private:
> InstrProfOptions Options;
> Module *M;
> - DenseMap<GlobalVariable *, GlobalVariable *> RegionCounters;
> + typedef struct PerFunctionProfileData {
> + uint32_t NumValueSites[instr_value_prof_kind::last];
> + GlobalVariable* RegionCounters;
> + GlobalVariable* DataVar;
> + PerFunctionProfileData() : RegionCounters(nullptr), DataVar(nullptr) {
> + memset(NumValueSites, 0, instr_value_prof_kind::last);
> + }
> + } PerFunctionProfileData;
> + DenseMap<GlobalVariable *, PerFunctionProfileData> ProfileDataMap;
> std::vector<Value *> UsedVars;
>
> bool isMachO() const {
> @@ -76,6 +85,12 @@
> return isMachO() ? "__DATA,__llvm_covmap" : "__llvm_covmap";
> }
>
> + /// Count the number of instrumented value sites for the function.
> + void computeNumValueSiteCounts(InstrProfInstrumentTargetInst *Ins);
> +
> + /// Replace instrprof_instrument_target with a call to runtime library.
> + void lowerInstrumentTargetInst(InstrProfInstrumentTargetInst *Ins);
> +
> /// Replace instrprof_increment with an increment of the appropriate value.
> void lowerIncrement(InstrProfIncrementInst *Inc);
>
> @@ -117,20 +132,39 @@
> bool MadeChange = false;
>
> this->M = &M;
> - RegionCounters.clear();
> + ProfileDataMap.clear();
> UsedVars.clear();
>
> + // We did not know how many value sites there would be inside
> + // the instrumented function. This is counting the number of instrumented
> + // target value sites to enter it as field in the profile data variable.
> + for (Function &F : M)
> + for (BasicBlock &BB : F)
> + for (auto I = BB.begin(), E = BB.end(); I != E;)
> + if (auto *Ind = dyn_cast<InstrProfInstrumentTargetInst>(I++))
> + computeNumValueSiteCounts(Ind);
The frontend should pass the number of value sites in the function in
the arguments to the intrinsic, like we do for the number of counters in
the increment intrinsic. In addition to being simpler, it's more robust
against things like dead code elimination which could remove the call to
an intrinsic completely.
> +
> for (Function &F : M)
> for (BasicBlock &BB : F)
> for (auto I = BB.begin(), E = BB.end(); I != E;)
> if (auto *Inc = dyn_cast<InstrProfIncrementInst>(I++)) {
> lowerIncrement(Inc);
> MadeChange = true;
> }
> +
> + for (Function &F : M)
> + for (BasicBlock &BB : F)
> + for (auto I = BB.begin(), E = BB.end(); I != E;)
> + if (auto *Ind = dyn_cast<InstrProfInstrumentTargetInst>(I++)) {
> + lowerInstrumentTargetInst(Ind);
> + MadeChange = true;
> + }
It seems like we could do both of these in one loop, no?
> +
> if (GlobalVariable *Coverage = M.getNamedGlobal("__llvm_coverage_mapping")) {
> lowerCoverageData(Coverage);
> MadeChange = true;
> }
> +
> if (!MadeChange)
> return false;
>
> @@ -141,13 +175,62 @@
> return true;
> }
>
> +static Constant *getOrInsertValueProfilingCall(Module &M) {
> + auto *VoidTy = Type::getVoidTy(M.getContext());
> + auto *VoidPtrTy = Type::getInt8PtrTy(M.getContext());
> + auto *Int32Ty = Type::getInt32Ty(M.getContext());
> + auto *Int64Ty = Type::getInt64Ty(M.getContext());
> + Type *ArgTypes[] = {Int32Ty, Int64Ty, VoidPtrTy, Int32Ty};
> + auto *ValueProfilingCallTy = FunctionType::get(VoidTy,
> + makeArrayRef(ArgTypes), false);
> + return M.getOrInsertFunction("__llvm_profile_instrument_target",
> + ValueProfilingCallTy);
> +}
> +
> +void InstrProfiling::computeNumValueSiteCounts(
> + InstrProfInstrumentTargetInst *Ind) {
> +
> + GlobalVariable *Name = Ind->getName();
> + uint32_t ValueKind = Ind->getValueKind()->getZExtValue();
> + uint64_t Index = Ind->getIndex()->getZExtValue();
> + auto It = ProfileDataMap.find(Name);
> + if (It == ProfileDataMap.end()) {
> + PerFunctionProfileData PD;
> + PD.NumValueSites[ValueKind] = Index + 1;
> + ProfileDataMap[Name] = PD;
> + }
> + else if (It->second.NumValueSites[ValueKind] <= Index)
> + It->second.NumValueSites[ValueKind] = Index + 1;
> +}
> +
> +void InstrProfiling::lowerInstrumentTargetInst(
> + InstrProfInstrumentTargetInst *Ind) {
> +
> + GlobalVariable *Name = Ind->getName();
> + auto It = ProfileDataMap.find(Name);
> + assert(It != ProfileDataMap.end() && It->second.DataVar &&
> + "value profiling detected in function with no counter incerement");
> +
> + GlobalVariable *DataVar = It->second.DataVar;
> + uint32_t ValueKind = Ind->getValueKind()->getZExtValue();
> + uint64_t Index = Ind->getIndex()->getZExtValue();
> + IRBuilder<> Builder(Ind->getParent(), *Ind);
> + Value* Args[4] = {Builder.getInt32(ValueKind),
> + Ind->getTargetValue(),
> + Builder.CreateBitCast(DataVar, Builder.getInt8PtrTy()),
Presumably we'll pass the actual address of the first
__llvm_profile_value_data here if we allocate that statically.
> + Builder.getInt32(Index)};
> + Ind->replaceAllUsesWith(
> + Builder.CreateCall(getOrInsertValueProfilingCall(*M), Args));
> + Ind->eraseFromParent();
> +}
> +
> void InstrProfiling::lowerIncrement(InstrProfIncrementInst *Inc) {
> GlobalVariable *Counters = getOrCreateRegionCounters(Inc);
>
> IRBuilder<> Builder(Inc->getParent(), *Inc);
> uint64_t Index = Inc->getIndex()->getZExtValue();
> - llvm::Value *Addr = Builder.CreateConstInBoundsGEP2_64(Counters, 0, Index);
> - llvm::Value *Count = Builder.CreateLoad(Addr, "pgocount");
> + Value *Addr = Builder.CreateConstInBoundsGEP2_64(Counters, 0, Index);
> + Value *Count = Builder.CreateLoad(Addr, "pgocount");
> Count = Builder.CreateAdd(Count, Builder.getInt64(1));
> Inc->replaceAllUsesWith(Builder.CreateStore(Count, Addr));
> Inc->eraseFromParent();
> @@ -172,9 +255,10 @@
> GlobalVariable *Name = cast<GlobalVariable>(V);
>
> // If we have region counters for this name, we've already handled it.
> - auto It = RegionCounters.find(Name);
> - if (It != RegionCounters.end())
> - continue;
> + auto It = ProfileDataMap.find(Name);
> + if (It != ProfileDataMap.end())
> + if (It->second.RegionCounters)
> + continue;
>
> // Move the name variable to the right section.
> Name->setSection(getNameSection());
> @@ -192,9 +276,13 @@
> GlobalVariable *
> InstrProfiling::getOrCreateRegionCounters(InstrProfIncrementInst *Inc) {
> GlobalVariable *Name = Inc->getName();
> - auto It = RegionCounters.find(Name);
> - if (It != RegionCounters.end())
> - return It->second;
> + auto It = ProfileDataMap.find(Name);
> + PerFunctionProfileData PD;
> + if (It != ProfileDataMap.end()) {
> + if (It->second.RegionCounters)
> + return It->second.RegionCounters;
> + PD = It->second;
> + }
>
> // Move the name variable to the right section.
> Name->setSection(getNameSection());
> @@ -212,30 +300,46 @@
> Counters->setSection(getCountersSection());
> Counters->setAlignment(8);
>
> - RegionCounters[Inc->getName()] = Counters;
> -
> // Create data variable.
> auto *NameArrayTy = Name->getType()->getPointerElementType();
> auto *Int32Ty = Type::getInt32Ty(Ctx);
> auto *Int64Ty = Type::getInt64Ty(Ctx);
> auto *Int8PtrTy = Type::getInt8PtrTy(Ctx);
> auto *Int64PtrTy = Type::getInt64PtrTy(Ctx);
> + auto *Int32ArrayTy = ArrayType::get(Int32Ty, instr_value_prof_kind::last);
> + auto *Int8PtrArrayTy = ArrayType::get(Int8PtrTy, instr_value_prof_kind::last);
> +
> + Constant *Int32ArrayVals[instr_value_prof_kind::last];
> + Constant *Int8PtrArrayVals[instr_value_prof_kind::last];
> + for (uint32_t Kind = instr_value_prof_kind::first;
> + Kind < instr_value_prof_kind::last; ++Kind) {
> + Int32ArrayVals[Kind] = ConstantInt::get(Int32Ty, PD.NumValueSites[Kind]);
> + Int8PtrArrayVals[Kind] = ConstantPointerNull::get(Int8PtrTy);
> + }
>
> - Type *DataTypes[] = {Int32Ty, Int32Ty, Int64Ty, Int8PtrTy, Int64PtrTy};
> + Type *DataTypes[] = {Int32Ty, Int32Ty, Int64Ty, Int8PtrTy, Int64PtrTy,
> + Int8PtrTy, Int32ArrayTy, Int8PtrArrayTy};
> auto *DataTy = StructType::get(Ctx, makeArrayRef(DataTypes));
> Constant *DataVals[] = {
> ConstantInt::get(Int32Ty, NameArrayTy->getArrayNumElements()),
> ConstantInt::get(Int32Ty, NumCounters),
> ConstantInt::get(Int64Ty, Inc->getHash()->getZExtValue()),
> ConstantExpr::getBitCast(Name, Int8PtrTy),
> - ConstantExpr::getBitCast(Counters, Int64PtrTy)};
> - auto *Data = new GlobalVariable(*M, DataTy, true, Name->getLinkage(),
> + ConstantExpr::getBitCast(Counters, Int64PtrTy),
> + ConstantExpr::getBitCast(Inc->getParent()->getParent(), Int8PtrTy),
> + ConstantArray::get(Int32ArrayTy, Int32ArrayVals),
> + ConstantArray::get(Int8PtrArrayTy,Int8PtrArrayVals)};
> + auto *Data = new GlobalVariable(*M, DataTy, false, Name->getLinkage(),
> ConstantStruct::get(DataTy, DataVals),
> getVarName(Inc, "data"));
> Data->setVisibility(Name->getVisibility());
> Data->setSection(getDataSection());
> Data->setAlignment(8);
>
> + PD.RegionCounters = Counters;
> + PD.DataVar = Data;
> + ProfileDataMap[Name] = PD;
> +
> // Mark the data variable as used so that it isn't stripped out.
> UsedVars.push_back(Data);
>
> @@ -257,7 +361,7 @@
> if (Options.NoRedZone)
> RegisterF->addFnAttr(Attribute::NoRedZone);
>
> - auto *RuntimeRegisterTy = llvm::FunctionType::get(VoidTy, VoidPtrTy, false);
> + auto *RuntimeRegisterTy = FunctionType::get(VoidTy, VoidPtrTy, false);
Please don't include unrelated cleanups - they make patch review
harder. This can happen separately.
> auto *RuntimeRegisterF =
> Function::Create(RuntimeRegisterTy, GlobalVariable::ExternalLinkage,
> "__llvm_profile_register_function", M);
> @@ -317,13 +421,13 @@
> // Add uses for our data.
> for (auto *Value : UsedVars)
> MergedVars.push_back(
> - ConstantExpr::getBitCast(cast<llvm::Constant>(Value), i8PTy));
> + ConstantExpr::getBitCast(cast<Constant>(Value), i8PTy));
>
> // Recreate llvm.used.
> ArrayType *ATy = ArrayType::get(i8PTy, MergedVars.size());
> - LLVMUsed = new llvm::GlobalVariable(
> - *M, ATy, false, llvm::GlobalValue::AppendingLinkage,
> - llvm::ConstantArray::get(ATy, MergedVars), "llvm.used");
> + LLVMUsed = new GlobalVariable(
> + *M, ATy, false, GlobalValue::AppendingLinkage,
> + ConstantArray::get(ATy, MergedVars), "llvm.used");
>
> LLVMUsed->setSection("llvm.metadata");
> }
> Index: test/Instrumentation/InstrProfiling/linkage.ll
> ===================================================================
> --- test/Instrumentation/InstrProfiling/linkage.ll
> +++ test/Instrumentation/InstrProfiling/linkage.ll
> @@ -9,28 +9,28 @@
> @__llvm_profile_name_foo_inline = linkonce_odr hidden constant [10 x i8] c"foo_inline"
>
> ; CHECK: @__llvm_profile_counters_foo = hidden global
> -; CHECK: @__llvm_profile_data_foo = hidden constant
> +; CHECK: @__llvm_profile_data_foo = hidden
> define void @foo() {
> call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__llvm_profile_name_foo, i32 0, i32 0), i64 0, i32 1, i32 0)
> ret void
> }
>
> ; CHECK: @__llvm_profile_counters_foo_weak = weak hidden global
> -; CHECK: @__llvm_profile_data_foo_weak = weak hidden constant
> +; CHECK: @__llvm_profile_data_foo_weak = weak hidden
> define weak void @foo_weak() {
> call void @llvm.instrprof.increment(i8* getelementptr inbounds ([8 x i8], [8 x i8]* @__llvm_profile_name_foo_weak, i32 0, i32 0), i64 0, i32 1, i32 0)
> ret void
> }
>
> ; CHECK: @"__llvm_profile_counters_linkage.ll:foo_internal" = internal global
> -; CHECK: @"__llvm_profile_data_linkage.ll:foo_internal" = internal constant
> +; CHECK: @"__llvm_profile_data_linkage.ll:foo_internal" = internal
> define internal void @foo_internal() {
> call void @llvm.instrprof.increment(i8* getelementptr inbounds ([23 x i8], [23 x i8]* @"__llvm_profile_name_linkage.ll:foo_internal", i32 0, i32 0), i64 0, i32 1, i32 0)
> ret void
> }
>
> ; CHECK: @__llvm_profile_counters_foo_inline = linkonce_odr hidden global
> -; CHECK: @__llvm_profile_data_foo_inline = linkonce_odr hidden constant
> +; CHECK: @__llvm_profile_data_foo_inline = linkonce_odr hidden
> define linkonce_odr void @foo_inline() {
> call void @llvm.instrprof.increment(i8* getelementptr inbounds ([10 x i8], [10 x i8]* @__llvm_profile_name_foo_inline, i32 0, i32 0), i64 0, i32 1, i32 0)
> ret void
> Index: test/Instrumentation/InstrProfiling/platform.ll
> ===================================================================
> --- test/Instrumentation/InstrProfiling/platform.ll
> +++ test/Instrumentation/InstrProfiling/platform.ll
> @@ -10,8 +10,8 @@
> ; MACHO: @__llvm_profile_counters_foo = hidden global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
> ; ELF: @__llvm_profile_counters_foo = hidden global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", align 8
>
> -; MACHO: @__llvm_profile_data_foo = hidden constant {{.*}}, section "__DATA,__llvm_prf_data", align 8
> -; ELF: @__llvm_profile_data_foo = hidden constant {{.*}}, section "__llvm_prf_data", align 8
> +; MACHO: @__llvm_profile_data_foo = hidden {{.*}}, section "__DATA,__llvm_prf_data", align 8
> +; ELF: @__llvm_profile_data_foo = hidden {{.*}}, section "__llvm_prf_data", align 8
> define void @foo() {
> call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__llvm_profile_name_foo, i32 0, i32 0), i64 0, i32 1, i32 0)
> ret void
> Index: test/Instrumentation/InstrProfiling/profiling.ll
> ===================================================================
> --- test/Instrumentation/InstrProfiling/profiling.ll
> +++ test/Instrumentation/InstrProfiling/profiling.ll
> @@ -10,21 +10,21 @@
> ; CHECK: @baz_prof_name = hidden constant [3 x i8] c"baz", section "__DATA,__llvm_prf_names", align 1
>
> ; CHECK: @__llvm_profile_counters_foo = hidden global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
> -; CHECK: @__llvm_profile_data_foo = hidden constant {{.*}}, section "__DATA,__llvm_prf_data", align 8
> +; CHECK: @__llvm_profile_data_foo = hidden {{.*}}, section "__DATA,__llvm_prf_data", align 8
> define void @foo() {
> call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__llvm_profile_name_foo, i32 0, i32 0), i64 0, i32 1, i32 0)
> ret void
> }
>
> ; CHECK: @__llvm_profile_counters_bar = hidden global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
> -; CHECK: @__llvm_profile_data_bar = hidden constant {{.*}}, section "__DATA,__llvm_prf_data", align 8
> +; CHECK: @__llvm_profile_data_bar = hidden {{.*}}, section "__DATA,__llvm_prf_data", align 8
> define void @bar() {
> call void @llvm.instrprof.increment(i8* getelementptr inbounds ([4 x i8], [4 x i8]* @__llvm_profile_name_bar, i32 0, i32 0), i64 0, i32 1, i32 0)
> ret void
> }
>
> ; CHECK: @__llvm_profile_counters_baz = hidden global [3 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
> -; CHECK: @__llvm_profile_data_baz = hidden constant {{.*}}, section "__DATA,__llvm_prf_data", align 8
> +; CHECK: @__llvm_profile_data_baz = hidden {{.*}}, section "__DATA,__llvm_prf_data", align 8
> define void @baz() {
> call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @baz_prof_name, i32 0, i32 0), i64 0, i32 3, i32 0)
> call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @baz_prof_name, i32 0, i32 0), i64 0, i32 3, i32 1)
> Index: tools/llvm-profdata/llvm-profdata.cpp
> ===================================================================
> --- tools/llvm-profdata/llvm-profdata.cpp
> +++ tools/llvm-profdata/llvm-profdata.cpp
> @@ -57,8 +57,7 @@
>
> auto Reader = std::move(ReaderOrErr.get());
> for (const auto &I : *Reader)
> - if (std::error_code EC =
> - Writer.addFunctionCounts(I.Name, I.Hash, I.Counts))
> + if (std::error_code EC = Writer.addFunctionCounts(I))
> errs() << Filename << ": " << I.Name << ": " << EC.message() << "\n";
> if (Reader->hasError())
> exitWithError(Reader->getError().message(), Filename);
> @@ -132,8 +131,8 @@
> }
>
> static int showInstrProfile(std::string Filename, bool ShowCounts,
> - bool ShowAllFunctions, std::string ShowFunction,
> - raw_fd_ostream &OS) {
> + bool ShowIndirectCallTargets, bool ShowAllFunctions,
> + std::string ShowFunction, raw_fd_ostream &OS) {
> auto ReaderOrErr = InstrProfReader::create(Filename);
> if (std::error_code EC = ReaderOrErr.getError())
> exitWithError(EC.message(), Filename);
> @@ -160,6 +159,9 @@
> << " Hash: " << format("0x%016" PRIx64, Func.Hash) << "\n"
> << " Counters: " << Func.Counts.size() << "\n"
> << " Function count: " << Func.Counts[0] << "\n";
> + if (ShowIndirectCallTargets)
> + OS << " Indirect Target Data: "
> + << Func.Values[instr_value_prof_kind::indirect_call_target].size() << "\n";
> }
>
> if (Show && ShowCounts)
> @@ -172,6 +174,15 @@
> }
> if (Show && ShowCounts)
> OS << "]\n";
> +
> + if (Show && ShowIndirectCallTargets) {
> + OS << " Indirect Target Results: \n";
> + for (size_t I = 0, E = Func.Values[0].size(); I < E; ++I) {
> + OS << "\t[ " << Func.Values[0][I].CounterIndex << ", ";
> + OS << Func.Values[0][I].Name << ", ";
> + OS << Func.Values[0][I].NumTaken << " ]\n";
> + }
> + }
> }
> if (Reader->hasError())
> exitWithError(Reader->getError().message(), Filename);
> @@ -208,6 +219,8 @@
>
> cl::opt<bool> ShowCounts("counts", cl::init(false),
> cl::desc("Show counter values for shown functions"));
> + cl::opt<bool> ShowIndirectCallTargets("ic-targets", cl::init(false),
> + cl::desc("Show indirect call site target values for shown functions"));
> cl::opt<bool> ShowAllFunctions("all-functions", cl::init(false),
> cl::desc("Details for every function"));
> cl::opt<std::string> ShowFunction("function",
> @@ -236,8 +249,8 @@
> errs() << "warning: -function argument ignored: showing all functions\n";
>
> if (ProfileKind == instr)
> - return showInstrProfile(Filename, ShowCounts, ShowAllFunctions,
> - ShowFunction, OS);
> + return showInstrProfile(Filename, ShowCounts, ShowIndirectCallTargets,
> + ShowAllFunctions, ShowFunction, OS);
> else
> return showSampleProfile(Filename, ShowCounts, ShowAllFunctions,
> ShowFunction, OS);
> Index: unittests/ProfileData/CoverageMappingTest.cpp
> ===================================================================
> --- unittests/ProfileData/CoverageMappingTest.cpp
> +++ unittests/ProfileData/CoverageMappingTest.cpp
> @@ -188,7 +188,8 @@
> }
>
> TEST_F(CoverageMappingTest, basic_coverage_iteration) {
> - ProfileWriter.addFunctionCounts("func", 0x1234, {30, 20, 10, 0});
> + InstrProfRecord Record("func", 0x1234, {30, 20, 10, 0});
> + ProfileWriter.addFunctionCounts(Record);
> readProfCounts();
>
> addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
> @@ -238,7 +239,8 @@
> }
>
> TEST_F(CoverageMappingTest, combine_regions) {
> - ProfileWriter.addFunctionCounts("func", 0x1234, {10, 20, 30});
> + InstrProfRecord Record("func", 0x1234, {10, 20, 30});
> + ProfileWriter.addFunctionCounts(Record);
> readProfCounts();
>
> addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
> @@ -256,7 +258,8 @@
> }
>
> TEST_F(CoverageMappingTest, dont_combine_expansions) {
> - ProfileWriter.addFunctionCounts("func", 0x1234, {10, 20});
> + InstrProfRecord Record("func", 0x1234, {10, 20});
> + ProfileWriter.addFunctionCounts(Record);
> readProfCounts();
>
> addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
> @@ -275,7 +278,8 @@
> }
>
> TEST_F(CoverageMappingTest, strip_filename_prefix) {
> - ProfileWriter.addFunctionCounts("file1:func", 0x1234, {10});
> + InstrProfRecord Record("file1:func", 0x1234, {10});
> + ProfileWriter.addFunctionCounts(Record);
> readProfCounts();
>
> addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
> Index: unittests/ProfileData/InstrProfTest.cpp
> ===================================================================
> --- unittests/ProfileData/InstrProfTest.cpp
> +++ unittests/ProfileData/InstrProfTest.cpp
> @@ -50,7 +50,8 @@
> }
>
> TEST_F(InstrProfTest, write_and_read_one_function) {
> - Writer.addFunctionCounts("foo", 0x1234, {1, 2, 3, 4});
> + InstrProfRecord Record("foo", 0x1234, {1, 2, 3, 4});
> + Writer.addFunctionCounts(Record);
> auto Profile = Writer.writeBuffer();
> readProfile(std::move(Profile));
>
> @@ -67,12 +68,14 @@
> }
>
> TEST_F(InstrProfTest, get_function_counts) {
> - Writer.addFunctionCounts("foo", 0x1234, {1, 2});
> + InstrProfRecord Record("foo", 0x1234, {1, 2});
> + Writer.addFunctionCounts(Record);
> auto Profile = Writer.writeBuffer();
> readProfile(std::move(Profile));
>
> std::vector<uint64_t> Counts;
> - ASSERT_TRUE(NoError(Reader->getFunctionCounts("foo", 0x1234, Counts)));
> + ASSERT_TRUE(NoError(
> + Reader->getFunctionCounts("foo", 0x1234, Counts)));
> ASSERT_EQ(2U, Counts.size());
> ASSERT_EQ(1U, Counts[0]);
> ASSERT_EQ(2U, Counts[1]);
> @@ -86,9 +89,12 @@
> }
>
> TEST_F(InstrProfTest, get_max_function_count) {
> - Writer.addFunctionCounts("foo", 0x1234, {1ULL << 31, 2});
> - Writer.addFunctionCounts("bar", 0, {1ULL << 63});
> - Writer.addFunctionCounts("baz", 0x5678, {0, 0, 0, 0});
> + InstrProfRecord Record1("foo", 0x1234, {1ULL << 31, 2});
> + InstrProfRecord Record2("bar", 0, {1ULL << 63});
> + InstrProfRecord Record3("baz", 0x5678, {0, 0, 0, 0});
> + Writer.addFunctionCounts(Record1);
> + Writer.addFunctionCounts(Record2);
> + Writer.addFunctionCounts(Record3);
> auto Profile = Writer.writeBuffer();
> readProfile(std::move(Profile));
>
More information about the llvm-commits
mailing list