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

298–299

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 ↗

(On Diff #72749)

Please spell the method name out to be explicit.

120 ↗

(On Diff #72749)

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

139 ↗

(On Diff #72749)

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

7

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.

8

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

[danielcdh]

rebase and update

[danielcdh]

lib/Analysis/ProfileSummaryInfo.cpp

116 ↗

(On Diff #72749)

after rebasing, these changes are not necessary.

120 ↗

(On Diff #72749)

after rebasing, these changes are not necessary.

139 ↗

(On Diff #72749)

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/IR/Function.h

public:
   void setEntryCount(uint64_t Count);
 
   /// \brief Get the entry count for this function.
   ///
   /// Entry count is the number of times the function was executed based on
   /// pgo data.
   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/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;
 
     StringRef 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();
+    if (PSI->isFunctionEntryHot(&F))
+      F.setSectionPrefix(".hot");
+    else if (PSI->isFunctionEntryCold(&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);
   }
-  // FIXME: Extend the section prefix to include hotness catagories such as .hot
-  //  or .unlikely for functions.
+
+  if (const Function *F = dyn_cast<Function>(GV)) {
+    const auto &OptionalPrefix = F->getSectionPrefix();
+    if (OptionalPrefix)
+      Name += *OptionalPrefix;
+  }
 
   if (EmitUniqueSection && UniqueSectionNames) {
     Name.push_back('.');
     TM.getNameWithPrefix(Name, GV, Mang, true);
   }
   unsigned UniqueID = MCContext::GenericSectionID;
   if (EmitUniqueSection && !UniqueSectionNames) {
     UniqueID = *NextUniqueID;

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 {
+  if (MDNode * MD = getMetadata(LLVMContext::MD_section_prefix)) {
+    assert(dyn_cast<MDString>(MD->getOperand(0))
+               ->getString()
+               .equals("function_section_prefix") &&
+           "Metadata not match");
+    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-LLC: .section .text.hot
+; CHECK-LLC-NEXT: .globl hot_func
+define void @hot_func() !prof !15 {
+  ret void
+}
+
+; CHECK-OPT: cold_func{{.*}}!section_prefix ![[COLD_ID:[0-9]+]]
+; CHECK-LLC: .section .text.cold
+; CHECK-LLC-NEXT: .globl cold_func
+define void @cold_func() !prof !16 {
+  ret void
+}
+
+; CHECK-OPT: ![[HOT_ID]] = !{!"function_section_prefix", !".hot"}
+; CHECK-OPT: ![[COLD_ID]] = !{!"function_section_prefix", !".cold"}
+!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}