snehasish created this revision.
snehasish added reviewers: xur, davidxl.
Herald added subscribers: wenlei, hiraditya.
snehasish requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

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.


Repository:
  rG LLVM Github Monorepo

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,13 +260,13 @@
         Filename);
     return;
   }
-  WC->Writer.setInstrEntryBBEnabled(Reader->instrEntryBBEnabled());
 
   for (auto &I : *Reader) {
     if (Remapper)
       I.Name = (*Remapper)(I.Name);
     const StringRef FuncName = I.Name;
     bool Reported = false;
+
     WC->Writer.addRecord(std::move(I), Input.Weight, [&](Error E) {
       if (Reported) {
         consumeError(std::move(E));
@@ -2078,7 +2076,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 +2103,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 +2202,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,10 @@
 
 } // end namespace llvm
 
-InstrProfWriter::InstrProfWriter(bool Sparse, bool InstrEntryBBEnabled)
-    : Sparse(Sparse), InstrEntryBBEnabled(InstrEntryBBEnabled),
-      InfoObj(new InstrProfRecordWriterTrait()) {}
+// InstrProfWriter::InstrProfWriter(bool Sparse, bool InstrEntryBBEnabled)
+InstrProfWriter::InstrProfWriter(bool Sparse)
+    // : Sparse(Sparse), InstrEntryBBEnabled(InstrEntryBBEnabled),
+    : Sparse(Sparse), InfoObj(new InstrProfRecordWriterTrait()) {}
 
 InstrProfWriter::~InstrProfWriter() { delete InfoObj; }
 
@@ -302,13 +303,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 +335,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 +356,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 +468,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
@@ -146,35 +146,32 @@
                      [](char c) { return isPrint(c) || isSpace(c); });
 }
 
+InstrProfKind TextInstrProfReader::getProfileKind() { return ProfileKind; }
+
 // Read the profile variant flag from the header: ":FE" means this is a FE
 // generated profile. ":IR" means this is an IR level profile. Other strings
 // 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 +1012,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,16 @@
 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;
+  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 +76,26 @@
   /// 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;
+  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,7 @@
   InstrProfIterator begin() { return InstrProfIterator(this); }
   InstrProfIterator end() { return InstrProfIterator(); }
 
-  virtual bool isIRLevelProfile() const = 0;
-
-  virtual bool hasCSIRLevelProfile() const = 0;
-
-  virtual bool instrEntryBBEnabled() const = 0;
+  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 +166,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 +180,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 the enum describing the profile kind.
+  InstrProfKind getProfileKind() override;
 
   /// Read the header.
   Error readHeader() override;
@@ -247,16 +239,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 the attributes of the current 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 +388,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 +429,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 +492,8 @@
 
   /// 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();
-  }
+  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,15 @@
 /// the duplicated profile variables for Comdat functions.
 bool needsComdatForCounter(const Function &F, const Module &M);
 
+enum class InstrProfKind {
+  Unknown = 0x0,
+  FE = 0x1, // A frontend clang generated profile.
+  IR = 0x2, // An IR-level profile.
+  BB = 0x4, // A profile with entry basic block instrumentation.
+  CS = 0x8, // Whether the IR-level profile is context sensitive.
+  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

Reply via email to