snehasish updated this revision to Diff 392927.
snehasish added a comment.
Cleanup for review -
- Added some more descriptive comments.
- Removed a couple of unintentional blank lines.
- Removed a couple of commented lines of code.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115393/new/
https://reviews.llvm.org/D115393
Files:
clang/lib/Frontend/CompilerInvocation.cpp
llvm/include/llvm/ProfileData/InstrProf.h
llvm/include/llvm/ProfileData/InstrProfReader.h
llvm/include/llvm/ProfileData/InstrProfWriter.h
llvm/lib/ProfileData/InstrProfReader.cpp
llvm/lib/ProfileData/InstrProfWriter.cpp
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
llvm/tools/llvm-profdata/llvm-profdata.cpp
Index: llvm/tools/llvm-profdata/llvm-profdata.cpp
===================================================================
--- llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -251,9 +251,7 @@
}
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>(
@@ -262,7 +260,6 @@
Filename);
return;
}
- WC->Writer.setInstrEntryBBEnabled(Reader->instrEntryBBEnabled());
for (auto &I : *Reader) {
if (Remapper)
@@ -2078,7 +2075,8 @@
exitWithError(std::move(E), Filename);
auto Reader = std::move(ReaderOrErr.get());
- bool IsIRInstr = Reader->isIRLevelProfile();
+ bool IsIRInstr =
+ static_cast<bool>(Reader->getProfileKind() & InstrProfKind::IR);
size_t ShownFunctions = 0;
size_t BelowCutoffFunctions = 0;
int NumVPKind = IPVK_Last - IPVK_First + 1;
@@ -2104,7 +2102,7 @@
OS << ":ir\n";
for (const auto &Func : *Reader) {
- if (Reader->isIRLevelProfile()) {
+ if (static_cast<bool>(Reader->getProfileKind() & InstrProfKind::IR)) {
bool FuncIsCS = NamedInstrProfRecord::hasCSFlagInHash(Func.Hash);
if (FuncIsCS != ShowCS)
continue;
@@ -2203,10 +2201,11 @@
if (TextFormat)
return 0;
std::unique_ptr<ProfileSummary> PS(Builder.getSummary());
- bool IsIR = Reader->isIRLevelProfile();
+ bool IsIR = static_cast<bool>(Reader->getProfileKind() & InstrProfKind::IR);
OS << "Instrumentation level: " << (IsIR ? "IR" : "Front-end");
if (IsIR)
- OS << " entry_first = " << Reader->instrEntryBBEnabled();
+ OS << " entry_first = "
+ << static_cast<bool>(Reader->getProfileKind() & InstrProfKind::BB);
OS << "\n";
if (ShowAllFunctions || !ShowFunction.empty())
OS << "Functions shown: " << ShownFunctions << "\n";
Index: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -1827,11 +1827,13 @@
StringRef("Cannot get PGOReader")));
return false;
}
- if (!PGOReader->hasCSIRLevelProfile() && IsCS)
+
+ const InstrProfKind ProfileKind = PGOReader->getProfileKind();
+ if (!static_cast<bool>(ProfileKind & InstrProfKind::CS) && IsCS)
return false;
// TODO: might need to change the warning once the clang option is finalized.
- if (!PGOReader->isIRLevelProfile()) {
+ if (!static_cast<bool>(ProfileKind & InstrProfKind::IR)) {
Ctx.diagnose(DiagnosticInfoPGOProfile(
ProfileFileName.data(), "Not an IR level instrumentation profile"));
return false;
@@ -1852,7 +1854,7 @@
// If the profile marked as always instrument the entry BB, do the
// same. Note this can be overwritten by the internal option in CFGMST.h
- bool InstrumentFuncEntry = PGOReader->instrEntryBBEnabled();
+ bool InstrumentFuncEntry = static_cast<bool>(ProfileKind & InstrProfKind::BB);
if (PGOInstrumentEntry.getNumOccurrences() > 0)
InstrumentFuncEntry = PGOInstrumentEntry;
for (auto &F : M) {
Index: llvm/lib/ProfileData/InstrProfWriter.cpp
===================================================================
--- llvm/lib/ProfileData/InstrProfWriter.cpp
+++ llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -165,9 +165,8 @@
} // 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; }
@@ -302,13 +301,11 @@
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;
@@ -336,7 +333,7 @@
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++)
@@ -357,7 +354,7 @@
// 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);
@@ -469,11 +466,13 @@
}
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;
Index: llvm/lib/ProfileData/InstrProfReader.cpp
===================================================================
--- llvm/lib/ProfileData/InstrProfReader.cpp
+++ llvm/lib/ProfileData/InstrProfReader.cpp
@@ -151,30 +151,24 @@
// 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();
}
@@ -1015,7 +1009,7 @@
void InstrProfReader::accumulateCounts(CountSumOrPercent &Sum, bool IsCS) {
uint64_t NumFuncs = 0;
for (const auto &Func : *this) {
- if (isIRLevelProfile()) {
+ if (static_cast<bool>(getProfileKind() & InstrProfKind::IR)) {
bool FuncIsCS = NamedInstrProfRecord::hasCSFlagInHash(Func.Hash);
if (FuncIsCS != IsCS)
continue;
Index: llvm/include/llvm/ProfileData/InstrProfWriter.h
===================================================================
--- llvm/include/llvm/ProfileData/InstrProfWriter.h
+++ llvm/include/llvm/ProfileData/InstrProfWriter.h
@@ -33,19 +33,17 @@
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; }
@@ -79,30 +77,28 @@
/// 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);
Index: llvm/include/llvm/ProfileData/InstrProfReader.h
===================================================================
--- llvm/include/llvm/ProfileData/InstrProfReader.h
+++ llvm/include/llvm/ProfileData/InstrProfReader.h
@@ -90,11 +90,8 @@
InstrProfIterator begin() { return InstrProfIterator(this); }
InstrProfIterator end() { return InstrProfIterator(); }
- virtual bool isIRLevelProfile() const = 0;
-
- virtual bool hasCSIRLevelProfile() const = 0;
-
- virtual bool instrEntryBBEnabled() const = 0;
+ /// Returns a BitsetEnum describing the attributes of the profile.
+ virtual InstrProfKind getProfileKind() = 0;
/// Return the PGO symtab. There are three different readers:
/// Raw, Text, and Indexed profile readers. The first two types
@@ -170,9 +167,8 @@
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);
@@ -185,11 +181,8 @@
/// Return true if the given buffer is in text instrprof format.
static bool hasFormat(const MemoryBuffer &Buffer);
- bool isIRLevelProfile() const override { return IsIRLevelProfile; }
-
- bool hasCSIRLevelProfile() const override { return HasCSIRLevelProfile; }
-
- bool instrEntryBBEnabled() const override { return InstrEntryBBEnabled; }
+ /// Returns a BitsetEnum describing the attributes of the text format profile.
+ InstrProfKind getProfileKind() override { return ProfileKind; }
/// Read the header.
Error readHeader() override;
@@ -247,16 +240,19 @@
Error readNextRecord(NamedInstrProfRecord &Record) override;
Error printBinaryIds(raw_ostream &OS) override;
- bool isIRLevelProfile() const override {
- return (Version & VARIANT_MASK_IR_PROF) != 0;
- }
-
- bool hasCSIRLevelProfile() const override {
- return (Version & VARIANT_MASK_CSIR_PROF) != 0;
- }
-
- bool instrEntryBBEnabled() const override {
- return (Version & VARIANT_MASK_INSTR_ENTRY) != 0;
+ /// Returns a BitsetEnum describing the attributes of the raw instr profile.
+ InstrProfKind getProfileKind() 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 {
@@ -393,9 +389,7 @@
virtual bool atEnd() const = 0;
virtual void setValueProfDataEndianness(support::endianness Endianness) = 0;
virtual uint64_t getVersion() const = 0;
- 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;
};
@@ -436,16 +430,18 @@
uint64_t getVersion() const override { return GET_VERSION(FormatVersion); }
- bool isIRLevelProfile() const override {
- return (FormatVersion & VARIANT_MASK_IR_PROF) != 0;
- }
-
- bool hasCSIRLevelProfile() const override {
- return (FormatVersion & VARIANT_MASK_CSIR_PROF) != 0;
- }
-
- bool instrEntryBBEnabled() const override {
- 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 {
@@ -497,14 +493,10 @@
/// Return the profile version.
uint64_t getVersion() const { return Index->getVersion(); }
- bool isIRLevelProfile() const override { return Index->isIRLevelProfile(); }
- bool hasCSIRLevelProfile() const override {
- return Index->hasCSIRLevelProfile();
- }
- bool instrEntryBBEnabled() const override {
- return Index->instrEntryBBEnabled();
- }
+ /// Returns a BitsetEnum describing the attributes of the indexed instr
+ /// profile.
+ InstrProfKind getProfileKind() override { return Index->getProfileKind(); }
/// Return true if the given buffer is in an indexed instrprof format.
static bool hasFormat(const MemoryBuffer &DataBuffer);
Index: llvm/include/llvm/ProfileData/InstrProf.h
===================================================================
--- llvm/include/llvm/ProfileData/InstrProf.h
+++ llvm/include/llvm/ProfileData/InstrProf.h
@@ -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"
@@ -277,6 +278,16 @@
/// 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 {
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1300,8 +1300,9 @@
}
std::unique_ptr<llvm::IndexedInstrProfReader> PGOReader =
std::move(ReaderOrErr.get());
- if (PGOReader->isIRLevelProfile()) {
- if (PGOReader->hasCSIRLevelProfile())
+ const llvm::InstrProfKind ProfileKind = PGOReader->getProfileKind();
+ if (static_cast<bool>(ProfileKind & llvm::InstrProfKind::IR)) {
+ if (static_cast<bool>(ProfileKind & llvm::InstrProfKind::CS))
Opts.setProfileUse(CodeGenOptions::ProfileCSIRInstr);
else
Opts.setProfileUse(CodeGenOptions::ProfileIRInstr);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
