Use profile info to set function section prefix to group hot/cold functions.

[danielcdh]

Summary

The original implementation is in r261607, which was reverted in r269726 to accomendate the ProfileSummaryInfo analysis pass. The new implementation:

1. add a new metadata for function section prefix
2. query against ProfileSummaryInfo in CGP to set the correct section prefix for each function
3. output the section prefix set by CGP

[eraman]

lib/CodeGen/CodeGenPrepare.cpp

124

Option name misspelt: guideded -> guided

246

isHotFunction and isColdFunction do not use the histogram based hotness/coldness information. It's probably fine to use this now and later change their implementation.

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

305

Remove the FIXME

test/Transforms/CodeGenPrepare/section.ll

7

You don't need the PROF variable since you don't use it later

[danielcdh]

update

[danielcdh]

ping...

[vsk]

lib/Analysis/ProfileSummaryInfo.cpp

116

Please spell the method name out to be explicit.

120

Why aren't the Hot/Cold count thresholds cleared?

139

How is the call to "resetM" being tested by your test?

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

301

IMO this makes more sense as "auto OptionalPrefix =" or "Optional<StringRef> Prefix =".

test/Transforms/CodeGenPrepare/section.ll

6

Please move your checks closer to the relevant "source" IR. Checks for module-level metadata should go at the end of the test, and checks related to a function should be near its definition.

7

"[0-9]*" should be "[0-9]+" in these tests.

[danielcdh]

rebase and update

[danielcdh]

lib/Analysis/ProfileSummaryInfo.cpp

116

after rebasing, these changes are not necessary.

120

after rebasing, these changes are not necessary.

139

after rebasing, these changes are not necessary.

[vsk]

No objections from me! It'd be better if Easwaran or David sign off on this though.

[danielcdh]

rebase

[eraman]

I've some minor comments, but otherwise LGTM.

lib/CodeGen/CodeGenPrepare.cpp

216

This is not part of your change, but why are the other analysis passes (LoopInfoWrapper etc) already specified in INITIALIZE_PASS_DEPENDENCY?

lib/IR/Function.cpp

1280

This seems to exceed 80 char limit. Did you run clang-format?

1285

if MD is non nullptr, then you shouldn't have to check for MD->getOperand(0) to be non-null right? Similarly, below, you don't have to check that operand 0 equals "function_section_prefix". Removing the ifs will make this clean. Or may be add asserts?

[danielcdh]

update

lib/CodeGen/CodeGenPrepare.cpp

216

In theory, all analysis pass it depends on should be declared here. But other analysis pass it depends on have all been declared as dependency of something initialized by LLVMInitializeXXXXXXTarget, but ProfileSummaryInfo is not such case, thus needs to be explicitly declared here.

[danielcdh]

ping...

[eraman]

LGTM

lib/IR/Function.cpp

1285

Weird formatting here - space between * and MD.

include/llvm/Analysis/ProfileSummaryInfo.h

 /// isColdCount that tells whether a given profile count is considered hot/cold
 /// based on the profile summary. This also provides convenience methods to
 /// check whether a function is hot or cold.
 
 // FIXME: Provide convenience methods to determine hotness/coldness of other IR
 // units. This would require making this depend on BFI.
 class ProfileSummaryInfo {
 private:
-  Module &M;
+  Module *M;
   std::unique_ptr<ProfileSummary> Summary;
   void computeSummary();
   void computeThresholds();
   // Count thresholds to answer isHotCount and isColdCount queries.
   Optional<uint64_t> HotCountThreshold, ColdCountThreshold;
 
 public:
-  ProfileSummaryInfo(Module &M) : M(M) {}
+  ProfileSummaryInfo(Module *M) : M(M) {}
   ProfileSummaryInfo(ProfileSummaryInfo &&Arg)
       : M(Arg.M), Summary(std::move(Arg.Summary)) {}
   /// \brief Returns true if \p F is a hot function.
   bool isHotFunction(const Function *F);
   /// \brief Returns true if \p F is a cold function.
   bool isColdFunction(const Function *F);
   /// \brief Returns true if count \p C is considered hot.
   bool isHotCount(uint64_t C);
   /// \brief Returns true if count \p C is considered cold.
   bool isColdCount(uint64_t C);
+  /// \brief Checks if \p newM is up-to-date, if not, invalidate Summary.
+  void resetM(Module *NewM);
 };
 
 /// An analysis pass based on legacy pass manager to deliver ProfileSummaryInfo.
 class ProfileSummaryInfoWrapperPass : public ImmutablePass {
   std::unique_ptr<ProfileSummaryInfo> PSI;
 
 public:
   static char ID;

include/llvm/IR/Function.h

public:
   }
 
   /// Set the entry count for this function.
   void setEntryCount(uint64_t Count);
 
   /// Get the entry count for this function.
   Optional<uint64_t> getEntryCount() const;
 
+  /// Set the section prefix for this function.
+  void setSectionPrefix(StringRef Prefix);
+
+  /// Get the section prefix for this function.
+  Optional<StringRef> getSectionPrefix() const;
+
   /// @brief Return true if the function has the attribute.
   bool hasFnAttribute(Attribute::AttrKind Kind) const {
     return AttributeSets.hasFnAttribute(Kind);
   }
   bool hasFnAttribute(StringRef Kind) const {
     return AttributeSets.hasFnAttribute(Kind);
   }
 

include/llvm/IR/LLVMContext.h

enum {
     MD_dereferenceable = 12,          // "dereferenceable"
     MD_dereferenceable_or_null = 13,  // "dereferenceable_or_null"
     MD_make_implicit = 14,            // "make.implicit"
     MD_unpredictable = 15,            // "unpredictable"
     MD_invariant_group = 16,          // "invariant.group"
     MD_align = 17,                    // "align"
     MD_loop = 18,                     // "llvm.loop"
     MD_type = 19,                     // "type"
+    MD_section_prefix = 20,           // "section_prefix"
   };
 
   /// Known operand bundle tag IDs, which always have the same value.  All
   /// operand bundle tags that LLVM has special knowledge of are listed here.
   /// Additionally, this scheme allows LLVM to efficiently check for specific
   /// operand bundle tags without comparing strings.
   enum {
     OB_deopt = 0,         // "deopt"

include/llvm/IR/MDBuilder.h

public:
   MDNode *createBranchWeights(ArrayRef<uint32_t> Weights);
 
   /// Return metadata specifying that a branch or switch is unpredictable.
   MDNode *createUnpredictable();
 
   /// Return metadata containing the entry count for a function.
   MDNode *createFunctionEntryCount(uint64_t Count);
 
+  /// Return metadata containing the section prefix for a function.
+  MDNode *createFunctionSectionPrefix(StringRef Prefix);
+
   //===------------------------------------------------------------------===//
   // Range metadata.
   //===------------------------------------------------------------------===//
 
   /// \brief Return metadata describing the range [Lo, Hi).
   MDNode *createRange(const APInt &Lo, const APInt &Hi);
 
   /// \brief Return metadata describing the range [Lo, Hi).

lib/Analysis/ProfileSummaryInfo.cpp

 
 // The profile summary metadata may be attached either by the frontend or by
 // any backend passes (IR level instrumentation, for example). This method
 // checks if the Summary is null and if so checks if the summary metadata is now
 // available in the module and parses it to get the Summary object.
 void ProfileSummaryInfo::computeSummary() {
   if (Summary)
     return;
-  auto *SummaryMD = M.getProfileSummary();
+  auto *SummaryMD = M->getProfileSummary();
   if (!SummaryMD)
     return;
   Summary.reset(ProfileSummary::getFromMD(SummaryMD));
 }
 
 // Returns true if the function is a hot function. If it returns false, it
 // either means it is not hot or it is unknown whether F is hot or not (for
 // example, no profile data is available).
if (!Summary)
     return;
   auto &DetailedSummary = Summary->getDetailedSummary();
   HotCountThreshold =
       getMinCountForPercentile(DetailedSummary, ProfileSummaryCutoffHot);
   ColdCountThreshold =
       getMinCountForPercentile(DetailedSummary, ProfileSummaryCutoffCold);
 }
 
+void ProfileSummaryInfo::resetM(Module *newM) {
+  if (newM == M)
+    return;
+  M = newM;
+  delete Summary.release();
+}
+
 bool ProfileSummaryInfo::isHotCount(uint64_t C) {
   if (!HotCountThreshold)
     computeThresholds();
   return HotCountThreshold && C >= HotCountThreshold.getValue();
 }
 
 bool ProfileSummaryInfo::isColdCount(uint64_t C) {
   if (!ColdCountThreshold)
     computeThresholds();
   return ColdCountThreshold && C <= ColdCountThreshold.getValue();
 }
 
 ProfileSummaryInfo *ProfileSummaryInfoWrapperPass::getPSI(Module &M) {
   if (!PSI)
-    PSI.reset(new ProfileSummaryInfo(M));
+    PSI.reset(new ProfileSummaryInfo(&M));
+  else
+    PSI->resetM(&M);
   return PSI.get();
 }
 
 INITIALIZE_PASS(ProfileSummaryInfoWrapperPass, "profile-summary-info",
                 "Profile summary info", false, true)
 
 ProfileSummaryInfoWrapperPass::ProfileSummaryInfoWrapperPass()
     : ImmutablePass(ID) {
   initializeProfileSummaryInfoWrapperPassPass(*PassRegistry::getPassRegistry());
 }
 
 char ProfileSummaryAnalysis::PassID;
 ProfileSummaryInfo ProfileSummaryAnalysis::run(Module &M,
                                                ModuleAnalysisManager &) {
-  return ProfileSummaryInfo(M);
+  return ProfileSummaryInfo(&M);
 }
 
 // FIXME: This only tests isHotFunction and isColdFunction and not the
 // isHotCount and isColdCount calls.
 PreservedAnalyses ProfileSummaryPrinterPass::run(Module &M,
                                                  ModuleAnalysisManager &AM) {
   ProfileSummaryInfo &PSI = AM.getResult<ProfileSummaryAnalysis>(M);
 

lib/CodeGen/CodeGenPrepare.cpp

 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/InstructionSimplify.h"
 #include "llvm/Analysis/LoopInfo.h"
+#include "llvm/Analysis/ProfileSummaryInfo.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/Analysis/MemoryBuiltins.h"
 #include "llvm/CodeGen/Analysis.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
static cl::opt<bool> StressExtLdPromotion(
     "stress-cgp-ext-ld-promotion", cl::Hidden, cl::init(false),
     cl::desc("Stress test ext(promotable(ld)) -> promoted(ext(ld)) "
              "optimization in CodeGenPrepare"));
 
 static cl::opt<bool> DisablePreheaderProtect(
     "disable-preheader-prot", cl::Hidden, cl::init(false),
     cl::desc("Disable protection against removing loop preheaders"));
 
+static cl::opt<bool> ProfileGuidedSectionPrefix(
+    "profile-guided-section-prefix", cl::Hidden, cl::init(true),
+    cl::desc("Use profile info to add section prefix for hot/cold functions"));
+
 namespace {
 typedef SmallPtrSet<Instruction *, 16> SetOfInstrs;
 typedef PointerIntPair<Type *, 1, bool> TypeIsSExt;
 typedef DenseMap<Instruction *, TypeIsSExt> InstrToOrigTy;
 class TypePromotionTransaction;
 
   class CodeGenPrepare : public FunctionPass {
     const TargetMachine *TM;
explicit CodeGenPrepare(const TargetMachine *TM = nullptr)
         initializeCodeGenPreparePass(*PassRegistry::getPassRegistry());
       }
     bool runOnFunction(Function &F) override;
 
     const char *getPassName() const override { return "CodeGen Prepare"; }
 
     void getAnalysisUsage(AnalysisUsage &AU) const override {
       // FIXME: When we can selectively preserve passes, preserve the domtree.
+      AU.addRequired<ProfileSummaryInfoWrapperPass>();
       AU.addRequired<TargetLibraryInfoWrapperPass>();
       AU.addRequired<TargetTransformInfoWrapperPass>();
       AU.addRequired<LoopInfoWrapperPass>();
     }
 
   private:
     bool eliminateFallThrough(Function &F);
     bool eliminateMostlyEmptyBlocks(Function &F);
bool extLdPromotion(TypePromotionTransaction &TPT, LoadInst *&LI,
                         unsigned CreatedInstCost);
     bool splitBranchCondition(Function &F);
     bool simplifyOffsetableRelocate(Instruction &I);
     void stripInvariantGroupMetadata(Instruction &I);
   };
 }
 
 char CodeGenPrepare::ID = 0;
-INITIALIZE_TM_PASS(CodeGenPrepare, "codegenprepare",
+INITIALIZE_TM_PASS_BEGIN(CodeGenPrepare, "codegenprepare",
+                         "Optimize for code generation", false, false)
+INITIALIZE_PASS_DEPENDENCY(ProfileSummaryInfoWrapperPass)
+INITIALIZE_TM_PASS_END(CodeGenPrepare, "codegenprepare",
                        "Optimize for code generation", false, false)
 
 FunctionPass *llvm::createCodeGenPreparePass(const TargetMachine *TM) {
   return new CodeGenPrepare(TM);
 }
 
 bool CodeGenPrepare::runOnFunction(Function &F) {
   if (skipFunction(F))
     return false;
 
   DL = &F.getParent()->getDataLayout();
 
   bool EverMadeChange = false;
   // Clear per function information.
   InsertedInsts.clear();
   PromotedInsts.clear();
 
   ModifiedDT = false;
   if (TM)
     TLI = TM->getSubtargetImpl(F)->getTargetLowering();
   TLInfo = &getAnalysis<TargetLibraryInfoWrapperPass>().getTLI();
   TTI = &getAnalysis<TargetTransformInfoWrapperPass>().getTTI(F);
   LI = &getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
   OptSize = F.optForSize();
 
+  if (ProfileGuidedSectionPrefix) {
+    ProfileSummaryInfo *PSI =
+        getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI(*F.getParent());;
+    if (PSI->isHotFunction(&F))
+      F.setSectionPrefix(".hot");
+    else if (PSI->isColdFunction(&F))
+      F.setSectionPrefix(".cold");
+  }
+
   /// This optimization identifies DIV instructions that can be
   /// profitably bypassed and carried out with a shorter, faster divide.
   if (!OptSize && TLI && TLI->isSlowDivBypassed()) {
     const DenseMap<unsigned int, unsigned int> &BypassWidths =
        TLI->getBypassSlowDivWidths();
     BasicBlock* BB = &*F.begin();
     while (BB != nullptr) {
       // bypassSlowDivision may create new BBs, but we don't want to reapply the

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

if (Kind.isMergeableCString()) {
     std::string SizeSpec = ".rodata.str" + utostr(EntrySize) + ".";
     Name = SizeSpec + utostr(Align);
   } else if (Kind.isMergeableConst()) {
     Name = ".rodata.cst";
     Name += utostr(EntrySize);
   } else {
     Name = getSectionPrefixForGlobal(Kind);
   }
+
+  if (const Function *F = dyn_cast<Function>(GV)) {
+    const auto &Prefix = F->getSectionPrefix();
+    if (Prefix)
+      Name += *Prefix;
+  }
   // FIXME: Extend the section prefix to include hotness catagories such as .hot
   //  or .unlikely for functions.
 
   if (EmitUniqueSection && UniqueSectionNames) {
     Name.push_back('.');
     TM.getNameWithPrefix(Name, GV, Mang, true);
   }
   unsigned UniqueID = MCContext::GenericSectionID;
   if (EmitUniqueSection && !UniqueSectionNames) {

lib/IR/Function.cpp

if (MDString *MDS = dyn_cast<MDString>(MD->getOperand(0)))
         ConstantInt *CI = mdconst::extract<ConstantInt>(MD->getOperand(1));
         uint64_t Count = CI->getValue().getZExtValue();
         if (Count == 0)
           return None;
         return Count;
       }
   return None;
 }
+
+void Function::setSectionPrefix(StringRef Prefix) {
+  MDBuilder MDB(getContext());
+  setMetadata(LLVMContext::MD_section_prefix, MDB.createFunctionSectionPrefix(Prefix));
+}
+
+Optional<StringRef> Function::getSectionPrefix() const {
+  MDNode *MD = getMetadata(LLVMContext::MD_section_prefix);
+  if (MD && MD->getOperand(0))
+    if (MDString *MDS = dyn_cast<MDString>(MD->getOperand(0)))
+      if (MDS->getString().equals("function_section_prefix"))
+        return dyn_cast<MDString>(MD->getOperand(1))->getString();
+  return None;
+}

lib/IR/LLVMContext.cpp

LLVMContext::LLVMContext() : pImpl(new LLVMContextImpl(*this)) {
   unsigned LoopID = getMDKindID("llvm.loop");
   assert(LoopID == MD_loop && "llvm.loop kind id drifted");
   (void)LoopID;
 
   unsigned TypeID = getMDKindID("type");
   assert(TypeID == MD_type && "type kind id drifted");
   (void)TypeID;
 
+  unsigned SectionPrefixID = getMDKindID("section_prefix");
+  assert(SectionPrefixID == MD_section_prefix &&
+         "section_prefix kind id drifted");
+  (void)SectionPrefixID;
+
   auto *DeoptEntry = pImpl->getOrInsertBundleTag("deopt");
   assert(DeoptEntry->second == LLVMContext::OB_deopt &&
          "deopt operand bundle id drifted!");
   (void)DeoptEntry;
 
   auto *FuncletEntry = pImpl->getOrInsertBundleTag("funclet");
   assert(FuncletEntry->second == LLVMContext::OB_funclet &&
          "funclet operand bundle id drifted!");

lib/IR/MDBuilder.cpp

 
 MDNode *MDBuilder::createFunctionEntryCount(uint64_t Count) {
   Type *Int64Ty = Type::getInt64Ty(Context);
   return MDNode::get(Context,
                      {createString("function_entry_count"),
                       createConstant(ConstantInt::get(Int64Ty, Count))});
 }
 
+MDNode *MDBuilder::createFunctionSectionPrefix(StringRef Prefix) {
+  return MDNode::get(Context,
+                     {createString("function_section_prefix"),
+                      createString(Prefix)});
+}
+
 MDNode *MDBuilder::createRange(const APInt &Lo, const APInt &Hi) {
   assert(Lo.getBitWidth() == Hi.getBitWidth() && "Mismatched bitwidths!");
 
   Type *Ty = IntegerType::get(Context, Lo.getBitWidth());
   return createRange(ConstantInt::get(Ty, Lo), ConstantInt::get(Ty, Hi));
 }
 
 MDNode *MDBuilder::createRange(Constant *Lo, Constant *Hi) {

test/Transforms/CodeGenPrepare/section.ll

+; RUN: opt < %s -codegenprepare -S | FileCheck --check-prefixes=CHECK-OPT %s
+; RUN: llc < %s -mtriple=x86_64-pc-linux-gnu -o - | FileCheck --check-prefixes=CHECK-LLC %s
+
+; This tests that hot/cold functions get correct section prefix assigned
+
+; CHECK-OPT: hot_func{{.*}}!section_prefix ![[HOT_ID:[0-9]*]]
+; CHECK-OPT: cold_func{{.*}}!section_prefix ![[COLD_ID:[0-9]*]]
+; CHECK-OPT: ![[HOT_ID]] = !{!"function_section_prefix", !".hot"}
+; CHECK-OPT: ![[COLD_ID]] = !{!"function_section_prefix", !".cold"}
+
+; CHECK-LLC: .section .text.hot
+; CHECK-LLC-NEXT: .globl hot_func
+; CHECK-LLC: .section .text.cold
+; CHECK-LLC-NEXT: .globl cold_func
+
+define void @hot_func() !prof !15 {
+  ret void
+}
+
+define void @cold_func() !prof !16 {
+  ret void
+}
+
+!llvm.module.flags = !{!1}
+!1 = !{i32 1, !"ProfileSummary", !2}
+!2 = !{!3, !4, !5, !6, !7, !8, !9, !10}
+!3 = !{!"ProfileFormat", !"InstrProf"}
+!4 = !{!"TotalCount", i64 10000}
+!5 = !{!"MaxCount", i64 1000}
+!6 = !{!"MaxInternalCount", i64 1}
+!7 = !{!"MaxFunctionCount", i64 1000}
+!8 = !{!"NumCounts", i64 3}
+!9 = !{!"NumFunctions", i64 3}
+!10 = !{!"DetailedSummary", !11}
+!11 = !{!12, !13, !14}
+!12 = !{i32 10000, i64 100, i32 1}
+!13 = !{i32 999000, i64 100, i32 1}
+!14 = !{i32 999999, i64 1, i32 2}
+!15 = !{!"function_entry_count", i64 1000}
+!16 = !{!"function_entry_count", i64 1}