[PGO] Supporting code for always instrumenting entry block

This patch includes the supporting code that enables always instrumenting the function entry block by default. This patch will NOT the default behavior. It adds a variant bit in the profile version, adds new directives in text profile format, and changes llvm-profdata tool accordingly. This patch is a split of D83024 (https://reviews.llvm.org/D83024) Many test changes from D83024 are also included. Differential Revision: https://reviews.llvm.org/D84261
llvm · Jul 22, 2020 · 50da55a · 50da55a
1 parent e0ee228
commit 50da55a
Show file tree

Hide file tree

Showing 47 changed files with 545 additions and 138 deletions.
diff --git a/clang/test/CodeGenCXX/Inputs/profile-remap_entry.proftext b/clang/test/CodeGenCXX/Inputs/profile-remap_entry.proftext
@@ -0,0 +1,8 @@
+:ir
+:entry_first
+_ZN3Foo8functionENS_1XE
+29667547796
+2
+100
+90
+
diff --git a/clang/test/CodeGenCXX/profile-remap.cpp b/clang/test/CodeGenCXX/profile-remap.cpp
@@ -3,6 +3,8 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fprofile-sample-use=%S/Inputs/profile-remap.samples -fprofile-remapping-file=%S/Inputs/profile-remap.map -fexperimental-new-pass-manager -O2 %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-SAMPLES
 // RUN: llvm-profdata merge -output %T.profdata %S/Inputs/profile-remap.proftext
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fprofile-instrument-use-path=%T.profdata -fprofile-remapping-file=%S/Inputs/profile-remap.map -fexperimental-new-pass-manager -O2 %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-INSTR
+// RUN: llvm-profdata merge -output %T.profdata %S/Inputs/profile-remap_entry.proftext
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fprofile-instrument-use-path=%T.profdata -fprofile-remapping-file=%S/Inputs/profile-remap.map -fexperimental-new-pass-manager -O2 %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-INSTR
 
 namespace Foo {
   struct X {};

diff --git a/clang/test/Profile/Inputs/gcc-flag-compatibility_IR_entry.proftext b/clang/test/Profile/Inputs/gcc-flag-compatibility_IR_entry.proftext
@@ -0,0 +1,12 @@
+# IR level Instrumentation Flag
+:ir
+:entry_first
+main
+# Func Hash:
+34137660316
+# Num Counters:
+2
+# Counter Values:
+1
+100
+
diff --git a/clang/test/Profile/gcc-flag-compatibility.c b/clang/test/Profile/gcc-flag-compatibility.c
@@ -49,6 +49,10 @@
 // RUN: llvm-profdata merge %S/Inputs/gcc-flag-compatibility_IR.proftext -o %t.dir/some/path/file.prof
 // RUN: %clang %s -o - -emit-llvm -S -fprofile-use=%t.dir/some/path/file.prof -fno-experimental-new-pass-manager | FileCheck -check-prefix=PROFILE-USE-IR %s
 // RUN: %clang %s -o - -emit-llvm -S -fprofile-use=%t.dir/some/path/file.prof -fexperimental-new-pass-manager | FileCheck -check-prefix=PROFILE-USE-IR %s
+//
+// RUN: llvm-profdata merge %S/Inputs/gcc-flag-compatibility_IR_entry.proftext -o %t.dir/some/path/file.prof
+// RUN: %clang %s -o - -emit-llvm -S -fprofile-use=%t.dir/some/path/file.prof -fno-experimental-new-pass-manager | FileCheck -check-prefix=PROFILE-USE-IR %s
+// RUN: %clang %s -o - -emit-llvm -S -fprofile-use=%t.dir/some/path/file.prof -fexperimental-new-pass-manager | FileCheck -check-prefix=PROFILE-USE-IR %s
 
 // PROFILE-USE-IR: = !{!"branch_weights", i32 100, i32 1}
 

diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -1146,7 +1146,8 @@ void getMemOPSizeRangeFromOption(StringRef Str, int64_t &RangeStart,
 
 // Create a COMDAT variable INSTR_PROF_RAW_VERSION_VAR to make the runtime
 // aware this is an ir_level profile so it can set the version flag.
-void createIRLevelProfileFlagVar(Module &M, bool IsCS);
+void createIRLevelProfileFlagVar(Module &M, bool IsCS,
+                                 bool InstrEntryBBEnabled);
 
 // Create the variable for the profile file name.
 void createProfileFileNameVar(Module &M, StringRef InstrProfileOutput);

diff --git a/llvm/include/llvm/ProfileData/InstrProfData.inc b/llvm/include/llvm/ProfileData/InstrProfData.inc
@@ -673,6 +673,7 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
 #define GET_VERSION(V) ((V) & ~VARIANT_MASKS_ALL)
 #define VARIANT_MASK_IR_PROF (0x1ULL << 56)
 #define VARIANT_MASK_CSIR_PROF (0x1ULL << 57)
+#define VARIANT_MASK_INSTR_ENTRY (0x1ULL << 58)
 #define INSTR_PROF_RAW_VERSION_VAR __llvm_profile_raw_version
 #define INSTR_PROF_PROFILE_RUNTIME_VAR __llvm_profile_runtime
 

diff --git a/llvm/include/llvm/ProfileData/InstrProfReader.h b/llvm/include/llvm/ProfileData/InstrProfReader.h
@@ -79,6 +79,8 @@ class InstrProfReader {
 
   virtual bool hasCSIRLevelProfile() const = 0;
 
+  virtual bool instrEntryBBEnabled() 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
@@ -148,6 +150,7 @@ class TextInstrProfReader : public InstrProfReader {
   line_iterator Line;
   bool IsIRLevelProfile = false;
   bool HasCSIRLevelProfile = false;
+  bool InstrEntryBBEnabled = false;
 
   Error readValueProfileData(InstrProfRecord &Record);
 
@@ -164,6 +167,8 @@ class TextInstrProfReader : public InstrProfReader {
 
   bool hasCSIRLevelProfile() const override { return HasCSIRLevelProfile; }
 
+  bool instrEntryBBEnabled() const override { return InstrEntryBBEnabled; }
+
   /// Read the header.
   Error readHeader() override;
 
@@ -224,6 +229,10 @@ class RawInstrProfReader : public InstrProfReader {
     return (Version & VARIANT_MASK_CSIR_PROF) != 0;
   }
 
+  bool instrEntryBBEnabled() const override {
+    return (Version & VARIANT_MASK_INSTR_ENTRY) != 0;
+  }
+
   InstrProfSymtab &getSymtab() override {
     assert(Symtab.get());
     return *Symtab.get();
@@ -360,6 +369,7 @@ struct InstrProfReaderIndexBase {
   virtual uint64_t getVersion() const = 0;
   virtual bool isIRLevelProfile() const = 0;
   virtual bool hasCSIRLevelProfile() const = 0;
+  virtual bool instrEntryBBEnabled() const = 0;
   virtual Error populateSymtab(InstrProfSymtab &) = 0;
 };
 
@@ -408,6 +418,10 @@ class InstrProfReaderIndex : public InstrProfReaderIndexBase {
     return (FormatVersion & VARIANT_MASK_CSIR_PROF) != 0;
   }
 
+  bool instrEntryBBEnabled() const override {
+    return (FormatVersion & VARIANT_MASK_INSTR_ENTRY) != 0;
+  }
+
   Error populateSymtab(InstrProfSymtab &Symtab) override {
     return Symtab.create(HashTable->keys());
   }
@@ -462,6 +476,10 @@ class IndexedInstrProfReader : public InstrProfReader {
     return Index->hasCSIRLevelProfile();
   }
 
+  bool instrEntryBBEnabled() const override {
+    return Index->instrEntryBBEnabled();
+  }
+
   /// Return true if the given buffer is in an indexed instrprof format.
   static bool hasFormat(const MemoryBuffer &DataBuffer);
 

diff --git a/llvm/include/llvm/ProfileData/InstrProfWriter.h b/llvm/include/llvm/ProfileData/InstrProfWriter.h
@@ -40,11 +40,12 @@ class InstrProfWriter {
   bool Sparse;
   StringMap<ProfilingData> FunctionData;
   ProfKind ProfileKind = PF_Unknown;
+  bool InstrEntryBBEnabled;
   // Use raw pointer here for the incomplete type object.
   InstrProfRecordWriterTrait *InfoObj;
 
 public:
-  InstrProfWriter(bool Sparse = false);
+  InstrProfWriter(bool Sparse = false, bool InstrEntryBBEnabled = false);
   ~InstrProfWriter();
 
   /// Add function counts for the given function. If there are already counts
@@ -97,6 +98,7 @@ class InstrProfWriter {
     return Error::success();
   }
 
+  void setInstrEntryBBEnabled(bool Enabled) { InstrEntryBBEnabled = Enabled; }
   // Internal interface for testing purpose only.
   void setValueProfDataEndianness(support::endianness Endianness);
   void setOutputSparse(bool Sparse);

diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
@@ -1136,12 +1136,15 @@ void getMemOPSizeRangeFromOption(StringRef MemOPSizeRange, int64_t &RangeStart,
 
 // Create a COMDAT variable INSTR_PROF_RAW_VERSION_VAR to make the runtime
 // aware this is an ir_level profile so it can set the version flag.
-void createIRLevelProfileFlagVar(Module &M, bool IsCS) {
+void createIRLevelProfileFlagVar(Module &M, bool IsCS,
+                                 bool InstrEntryBBEnabled) {
   const StringRef VarName(INSTR_PROF_QUOTE(INSTR_PROF_RAW_VERSION_VAR));
   Type *IntTy64 = Type::getInt64Ty(M.getContext());
   uint64_t ProfileVersion = (INSTR_PROF_RAW_VERSION | VARIANT_MASK_IR_PROF);
   if (IsCS)
     ProfileVersion |= VARIANT_MASK_CSIR_PROF;
+  if (InstrEntryBBEnabled)
+    ProfileVersion |= VARIANT_MASK_INSTR_ENTRY;
   auto IRLevelVersionVariable = new GlobalVariable(
       M, IntTy64, true, GlobalValue::WeakAnyLinkage,
       Constant::getIntegerValue(IntTy64, APInt(64, ProfileVersion)), VarName);

diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp
@@ -154,23 +154,29 @@ bool TextInstrProfReader::hasFormat(const MemoryBuffer &Buffer) {
 Error TextInstrProfReader::readHeader() {
   Symtab.reset(new InstrProfSymtab());
   bool IsIRInstr = false;
-  if (!Line->startswith(":")) {
-    IsIRLevelProfile = false;
-    return success();
+  bool IsEntryFirst = false;
+  bool IsCS = false;
+
+  while (Line->startswith(":")) {
+    StringRef Str = Line->substr(1);
+    if (Str.equals_lower("ir"))
+      IsIRInstr = true;
+    else if (Str.equals_lower("fe"))
+      IsIRInstr = false;
+    else if (Str.equals_lower("csir")) {
+      IsIRInstr = true;
+      IsCS = true;
+    } else if (Str.equals_lower("entry_first"))
+      IsEntryFirst = true;
+    else if (Str.equals_lower("not_entry_first"))
+      IsEntryFirst = false;
+    else
+      return error(instrprof_error::bad_header);
+    ++Line;
   }
-  StringRef Str = (Line)->substr(1);
-  if (Str.equals_lower("ir"))
-    IsIRInstr = true;
-  else if (Str.equals_lower("fe"))
-    IsIRInstr = false;
-  else if (Str.equals_lower("csir")) {
-    IsIRInstr = true;
-    HasCSIRLevelProfile = true;
-  } else
-    return error(instrprof_error::bad_header);
-
-  ++Line;
   IsIRLevelProfile = IsIRInstr;
+  InstrEntryBBEnabled = IsEntryFirst;
+  HasCSIRLevelProfile = IsCS;
   return success();
 }
 

diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -165,8 +165,9 @@ class InstrProfRecordWriterTrait {
 
 } // end namespace llvm
 
-InstrProfWriter::InstrProfWriter(bool Sparse)
-    : Sparse(Sparse), InfoObj(new InstrProfRecordWriterTrait()) {}
+InstrProfWriter::InstrProfWriter(bool Sparse, bool InstrEntryBBEnabled)
+    : Sparse(Sparse), InstrEntryBBEnabled(InstrEntryBBEnabled),
+      InfoObj(new InstrProfRecordWriterTrait()) {}
 
 InstrProfWriter::~InstrProfWriter() { delete InfoObj; }
 
@@ -308,6 +309,9 @@ void InstrProfWriter::writeImpl(ProfOStream &OS) {
     Header.Version |= VARIANT_MASK_IR_PROF;
     Header.Version |= VARIANT_MASK_CSIR_PROF;
   }
+  if (InstrEntryBBEnabled)
+    Header.Version |= VARIANT_MASK_INSTR_ENTRY;
+
   Header.Unused = 0;
   Header.HashType = static_cast<uint64_t>(IndexedInstrProf::HashType);
   Header.HashOffset = 0;
@@ -441,6 +445,8 @@ Error InstrProfWriter::writeText(raw_fd_ostream &OS) {
     OS << "# IR level Instrumentation Flag\n:ir\n";
   else if (ProfileKind == PF_IRLevelWithCS)
     OS << "# CSIR level Instrumentation Flag\n:csir\n";
+  if (InstrEntryBBEnabled)
+    OS << "# Always instrument the function entry block\n:entry_first\n";
   InstrProfSymtab Symtab;
 
   using FuncPair = detail::DenseMapPair<uint64_t, InstrProfRecord>;

diff --git a/llvm/lib/Transforms/Instrumentation/CFGMST.h b/llvm/lib/Transforms/Instrumentation/CFGMST.h
@@ -20,7 +20,6 @@
 #include "llvm/Analysis/BranchProbabilityInfo.h"
 #include "llvm/Analysis/CFG.h"
 #include "llvm/Support/BranchProbability.h"
-#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
@@ -30,9 +29,6 @@
 #define DEBUG_TYPE "cfgmst"
 
 using namespace llvm;
-static cl::opt<bool> PGOInstrumentEntry(
-    "pgo-instrument-entry", cl::init(false), cl::Hidden,
-    cl::desc("Force to instrument function entry basicblock."));
 
 namespace llvm {
 
@@ -107,7 +103,7 @@ template <class Edge, class BBInfo> class CFGMST {
     const BasicBlock *Entry = &(F.getEntryBlock());
     uint64_t EntryWeight = (BFI != nullptr ? BFI->getEntryFreq() : 2);
     // If we want to instrument the entry count, lower the weight to 0.
-    if (PGOInstrumentEntry)
+    if (InstrumentFuncEntry)
       EntryWeight = 0;
     Edge *EntryIncoming = nullptr, *EntryOutgoing = nullptr,
          *ExitOutgoing = nullptr, *ExitIncoming = nullptr;
@@ -282,14 +278,19 @@ template <class Edge, class BBInfo> class CFGMST {
   BranchProbabilityInfo *BPI;
   BlockFrequencyInfo *BFI;
 
+  // If function entry will be always instrumented.
+  bool InstrumentFuncEntry;
+
 public:
-  CFGMST(Function &Func, BranchProbabilityInfo *BPI_ = nullptr,
+  CFGMST(Function &Func, bool InstrumentFuncEntry_,
+         BranchProbabilityInfo *BPI_ = nullptr,
          BlockFrequencyInfo *BFI_ = nullptr)
-      : F(Func), BPI(BPI_), BFI(BFI_) {
+      : F(Func), BPI(BPI_), BFI(BFI_),
+        InstrumentFuncEntry(InstrumentFuncEntry_) {
     buildEdges();
     sortEdgesByWeight();
     computeMinimumSpanningTree();
-    if (PGOInstrumentEntry && (AllEdges.size() > 1))
+    if (AllEdges.size() > 1 && InstrumentFuncEntry)
       std::iter_swap(std::move(AllEdges.begin()),
                      std::move(AllEdges.begin() + AllEdges.size() - 1));
   }