Skip to content

Commit

Permalink
[InstrProf][NFC] Refactor Profile kind into a bitset enum.
Browse files Browse the repository at this point in the history
This change refactors the ProfileKind enum into a bitset enum to
represent the different attributes a profile can have. This change
simplifies the logic in the instrprof writer when multiple profiles are
merged together. In the future we plan on introducing a new memory
profile section which will extend the enum by one additional entry.
Without this change when accounting for memory profiles will have to be
maintained separately and will make the logic more complex.

Differential Revision: https://reviews.llvm.org/D115393
  • Loading branch information
snehasish committed Jan 27, 2022
1 parent 1e12156 commit 13d8947
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 57 deletions.
11 changes: 11 additions & 0 deletions llvm/include/llvm/ProfileData/InstrProf.h
Expand Up @@ -16,6 +16,7 @@
#define LLVM_PROFILEDATA_INSTRPROF_H

#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/BitmaskEnum.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
Expand Down Expand Up @@ -277,6 +278,16 @@ void createPGOFuncNameMetadata(Function &F, StringRef PGOFuncName);
/// the duplicated profile variables for Comdat functions.
bool needsComdatForCounter(const Function &F, const Module &M);

/// An enum describing the attributes of an instrumented profile.
enum class InstrProfKind {
Unknown = 0x0,
FE = 0x1, // A frontend clang profile, incompatible with other attrs.
IR = 0x2, // An IR-level profile (default when -fprofile-generate is used).
BB = 0x4, // A profile with entry basic block instrumentation.
CS = 0x8, // A context sensitive IR-level profile.
LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/CS)
};

const std::error_category &instrprof_category();

enum class instrprof_error {
Expand Down
59 changes: 53 additions & 6 deletions llvm/include/llvm/ProfileData/InstrProfReader.h
Expand Up @@ -100,6 +100,10 @@ class InstrProfReader {
/// Return true if we must provide debug info to create PGO profiles.
virtual bool useDebugInfoCorrelate() const { return false; }

/// Returns a BitsetEnum describing the attributes of the profile. To check
/// individual attributes prefer using the helpers above.
virtual InstrProfKind getProfileKind() const = 0;

/// Return the PGO symtab. There are three different readers:
/// Raw, Text, and Indexed profile readers. The first two types
/// of readers are used only by llvm-profdata tool, while the indexed
Expand Down Expand Up @@ -176,9 +180,8 @@ class TextInstrProfReader : public InstrProfReader {
std::unique_ptr<MemoryBuffer> DataBuffer;
/// Iterator over the profile data.
line_iterator Line;
bool IsIRLevelProfile = false;
bool HasCSIRLevelProfile = false;
bool InstrEntryBBEnabled = false;
/// The attributes of the current profile.
InstrProfKind ProfileKind = InstrProfKind::Unknown;

Error readValueProfileData(InstrProfRecord &Record);

Expand All @@ -191,11 +194,19 @@ class TextInstrProfReader : public InstrProfReader {
/// Return true if the given buffer is in text instrprof format.
static bool hasFormat(const MemoryBuffer &Buffer);

bool isIRLevelProfile() const override { return IsIRLevelProfile; }
bool isIRLevelProfile() const override {
return static_cast<bool>(ProfileKind & InstrProfKind::IR);
}

bool hasCSIRLevelProfile() const override {
return static_cast<bool>(ProfileKind & InstrProfKind::CS);
}

bool hasCSIRLevelProfile() const override { return HasCSIRLevelProfile; }
bool instrEntryBBEnabled() const override {
return static_cast<bool>(ProfileKind & InstrProfKind::BB);
}

bool instrEntryBBEnabled() const override { return InstrEntryBBEnabled; }
InstrProfKind getProfileKind() const override { return ProfileKind; }

/// Read the header.
Error readHeader() override;
Expand Down Expand Up @@ -276,6 +287,21 @@ class RawInstrProfReader : public InstrProfReader {
return (Version & VARIANT_MASK_DBG_CORRELATE) != 0;
}

/// Returns a BitsetEnum describing the attributes of the raw instr profile.
InstrProfKind getProfileKind() const override {
InstrProfKind ProfileKind = InstrProfKind::Unknown;
if (Version & VARIANT_MASK_IR_PROF) {
ProfileKind |= InstrProfKind::IR;
}
if (Version & VARIANT_MASK_CSIR_PROF) {
ProfileKind |= InstrProfKind::CS;
}
if (Version & VARIANT_MASK_INSTR_ENTRY) {
ProfileKind |= InstrProfKind::BB;
}
return ProfileKind;
}

InstrProfSymtab &getSymtab() override {
assert(Symtab.get());
return *Symtab.get();
Expand Down Expand Up @@ -413,6 +439,7 @@ struct InstrProfReaderIndexBase {
virtual bool isIRLevelProfile() const = 0;
virtual bool hasCSIRLevelProfile() const = 0;
virtual bool instrEntryBBEnabled() const = 0;
virtual InstrProfKind getProfileKind() const = 0;
virtual Error populateSymtab(InstrProfSymtab &) = 0;
};

Expand Down Expand Up @@ -465,6 +492,20 @@ class InstrProfReaderIndex : public InstrProfReaderIndexBase {
return (FormatVersion & VARIANT_MASK_INSTR_ENTRY) != 0;
}

InstrProfKind getProfileKind() const override {
InstrProfKind ProfileKind = InstrProfKind::Unknown;
if (FormatVersion & VARIANT_MASK_IR_PROF) {
ProfileKind |= InstrProfKind::IR;
}
if (FormatVersion & VARIANT_MASK_CSIR_PROF) {
ProfileKind |= InstrProfKind::CS;
}
if (FormatVersion & VARIANT_MASK_INSTR_ENTRY) {
ProfileKind |= InstrProfKind::BB;
}
return ProfileKind;
}

Error populateSymtab(InstrProfSymtab &Symtab) override {
return Symtab.create(HashTable->keys());
}
Expand Down Expand Up @@ -523,6 +564,12 @@ class IndexedInstrProfReader : public InstrProfReader {
return Index->instrEntryBBEnabled();
}

/// Returns a BitsetEnum describing the attributes of the indexed instr
/// profile.
InstrProfKind getProfileKind() const override {
return Index->getProfileKind();
}

/// Return true if the given buffer is in an indexed instrprof format.
static bool hasFormat(const MemoryBuffer &DataBuffer);

Expand Down
38 changes: 17 additions & 21 deletions llvm/include/llvm/ProfileData/InstrProfWriter.h
Expand Up @@ -33,19 +33,17 @@ class raw_fd_ostream;
class InstrProfWriter {
public:
using ProfilingData = SmallDenseMap<uint64_t, InstrProfRecord>;
// PF_IRLevelWithCS is the profile from context sensitive IR instrumentation.
enum ProfKind { PF_Unknown = 0, PF_FE, PF_IRLevel, PF_IRLevelWithCS };

private:
bool Sparse;
StringMap<ProfilingData> FunctionData;
ProfKind ProfileKind = PF_Unknown;
bool InstrEntryBBEnabled;
// An enum describing the attributes of the profile.
InstrProfKind ProfileKind = InstrProfKind::Unknown;
// Use raw pointer here for the incomplete type object.
InstrProfRecordWriterTrait *InfoObj;

public:
InstrProfWriter(bool Sparse = false, bool InstrEntryBBEnabled = false);
InstrProfWriter(bool Sparse = false);
~InstrProfWriter();

StringMap<ProfilingData> &getProfileData() { return FunctionData; }
Expand Down Expand Up @@ -79,30 +77,28 @@ class InstrProfWriter {
/// Write the profile, returning the raw data. For testing.
std::unique_ptr<MemoryBuffer> writeBuffer();

/// Set the ProfileKind. Report error if mixing FE and IR level profiles.
/// \c WithCS indicates if this is for contenxt sensitive instrumentation.
Error setIsIRLevelProfile(bool IsIRLevel, bool WithCS) {
if (ProfileKind == PF_Unknown) {
if (IsIRLevel)
ProfileKind = WithCS ? PF_IRLevelWithCS : PF_IRLevel;
else
ProfileKind = PF_FE;
/// Update the attributes of the current profile from the attributes
/// specified. An error is returned if IR and FE profiles are mixed.
Error mergeProfileKind(const InstrProfKind Other) {
// If the kind is unset, this is the first profile we are merging so just
// set it to the given type.
if (ProfileKind == InstrProfKind::Unknown) {
ProfileKind = Other;
return Error::success();
}

if (((ProfileKind != PF_FE) && !IsIRLevel) ||
((ProfileKind == PF_FE) && IsIRLevel))
// Check if the profiles are in-compatible. Clang frontend profiles can't be
// merged with other profile types.
if (static_cast<bool>((ProfileKind & InstrProfKind::FE) ^
(Other & InstrProfKind::FE))) {
return make_error<InstrProfError>(instrprof_error::unsupported_version);
}

// When merging a context-sensitive profile (WithCS == true) with an IRLevel
// profile, set the kind to PF_IRLevelWithCS.
if (ProfileKind == PF_IRLevel && WithCS)
ProfileKind = PF_IRLevelWithCS;

// Now we update the profile type with the bits that are set.
ProfileKind |= Other;
return Error::success();
}

void setInstrEntryBBEnabled(bool Enabled) { InstrEntryBBEnabled = Enabled; }
// Internal interface for testing purpose only.
void setValueProfDataEndianness(support::endianness Endianness);
void setOutputSparse(bool Sparse);
Expand Down
18 changes: 6 additions & 12 deletions llvm/lib/ProfileData/InstrProfReader.cpp
Expand Up @@ -154,30 +154,24 @@ bool TextInstrProfReader::hasFormat(const MemoryBuffer &Buffer) {
// with a leading ':' will be reported an error format.
Error TextInstrProfReader::readHeader() {
Symtab.reset(new InstrProfSymtab());
bool IsIRInstr = false;
bool IsEntryFirst = false;
bool IsCS = false;

while (Line->startswith(":")) {
StringRef Str = Line->substr(1);
if (Str.equals_insensitive("ir"))
IsIRInstr = true;
ProfileKind |= InstrProfKind::IR;
else if (Str.equals_insensitive("fe"))
IsIRInstr = false;
ProfileKind |= InstrProfKind::FE;
else if (Str.equals_insensitive("csir")) {
IsIRInstr = true;
IsCS = true;
ProfileKind |= InstrProfKind::IR;
ProfileKind |= InstrProfKind::CS;
} else if (Str.equals_insensitive("entry_first"))
IsEntryFirst = true;
ProfileKind |= InstrProfKind::BB;
else if (Str.equals_insensitive("not_entry_first"))
IsEntryFirst = false;
ProfileKind &= ~InstrProfKind::BB;
else
return error(instrprof_error::bad_header);
++Line;
}
IsIRLevelProfile = IsIRInstr;
InstrEntryBBEnabled = IsEntryFirst;
HasCSIRLevelProfile = IsCS;
return success();
}

Expand Down
27 changes: 13 additions & 14 deletions llvm/lib/ProfileData/InstrProfWriter.cpp
Expand Up @@ -166,9 +166,8 @@ class InstrProfRecordWriterTrait {

} // end namespace llvm

InstrProfWriter::InstrProfWriter(bool Sparse, bool InstrEntryBBEnabled)
: Sparse(Sparse), InstrEntryBBEnabled(InstrEntryBBEnabled),
InfoObj(new InstrProfRecordWriterTrait()) {}
InstrProfWriter::InstrProfWriter(bool Sparse)
: Sparse(Sparse), InfoObj(new InstrProfRecordWriterTrait()) {}

InstrProfWriter::~InstrProfWriter() { delete InfoObj; }

Expand Down Expand Up @@ -303,13 +302,11 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
IndexedInstrProf::Header Header;
Header.Magic = IndexedInstrProf::Magic;
Header.Version = IndexedInstrProf::ProfVersion::CurrentVersion;
if (ProfileKind == PF_IRLevel)
Header.Version |= VARIANT_MASK_IR_PROF;
if (ProfileKind == PF_IRLevelWithCS) {
if (static_cast<bool>(ProfileKind & InstrProfKind::IR))
Header.Version |= VARIANT_MASK_IR_PROF;
if (static_cast<bool>(ProfileKind & InstrProfKind::CS))
Header.Version |= VARIANT_MASK_CSIR_PROF;
}
if (InstrEntryBBEnabled)
if (static_cast<bool>(ProfileKind & InstrProfKind::BB))
Header.Version |= VARIANT_MASK_INSTR_ENTRY;

Header.Unused = 0;
Expand Down Expand Up @@ -337,7 +334,7 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
OS.write(0);
uint64_t CSSummaryOffset = 0;
uint64_t CSSummarySize = 0;
if (ProfileKind == PF_IRLevelWithCS) {
if (static_cast<bool>(ProfileKind & InstrProfKind::CS)) {
CSSummaryOffset = OS.tell();
CSSummarySize = SummarySize / sizeof(uint64_t);
for (unsigned I = 0; I < CSSummarySize; I++)
Expand All @@ -358,7 +355,7 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {

// For Context Sensitive summary.
std::unique_ptr<IndexedInstrProf::Summary> TheCSSummary = nullptr;
if (ProfileKind == PF_IRLevelWithCS) {
if (static_cast<bool>(ProfileKind & InstrProfKind::CS)) {
TheCSSummary = IndexedInstrProf::allocSummary(SummarySize);
std::unique_ptr<ProfileSummary> CSPS = CSISB.getSummary();
setSummary(TheCSSummary.get(), *CSPS);
Expand Down Expand Up @@ -470,11 +467,13 @@ void InstrProfWriter::writeRecordInText(StringRef Name, uint64_t Hash,
}

Error InstrProfWriter::writeText(raw_fd_ostream &OS) {
if (ProfileKind == PF_IRLevel)
OS << "# IR level Instrumentation Flag\n:ir\n";
else if (ProfileKind == PF_IRLevelWithCS)
// Check CS first since it implies an IR level profile.
if (static_cast<bool>(ProfileKind & InstrProfKind::CS))
OS << "# CSIR level Instrumentation Flag\n:csir\n";
if (InstrEntryBBEnabled)
else if (static_cast<bool>(ProfileKind & InstrProfKind::IR))
OS << "# IR level Instrumentation Flag\n:ir\n";

if (static_cast<bool>(ProfileKind & InstrProfKind::BB))
OS << "# Always instrument the function entry block\n:entry_first\n";
InstrProfSymtab Symtab;

Expand Down
5 changes: 1 addition & 4 deletions llvm/tools/llvm-profdata/llvm-profdata.cpp
Expand Up @@ -255,9 +255,7 @@ static void loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
}

auto Reader = std::move(ReaderOrErr.get());
bool IsIRProfile = Reader->isIRLevelProfile();
bool HasCSIRProfile = Reader->hasCSIRLevelProfile();
if (Error E = WC->Writer.setIsIRLevelProfile(IsIRProfile, HasCSIRProfile)) {
if (Error E = WC->Writer.mergeProfileKind(Reader->getProfileKind())) {
consumeError(std::move(E));
WC->Errors.emplace_back(
make_error<StringError>(
Expand All @@ -266,7 +264,6 @@ static void loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
Filename);
return;
}
WC->Writer.setInstrEntryBBEnabled(Reader->instrEntryBBEnabled());

for (auto &I : *Reader) {
if (Remapper)
Expand Down

0 comments on commit 13d8947

Please sign in to comment.