[StaticDataLayout] Reconcile two sources of variable hotness

llvm/include/llvm/Analysis/StaticDataProfileInfo.h

@@ -14,8 +14,10 @@ namespace llvm {
 /// profile information and provides methods to operate on them.
 class StaticDataProfileInfo {
 public:
-  /// Accummulate the profile count of a constant that will be lowered to static
-  /// data sections.
+  /// A constant and its profile count.
+  /// A constant is tracked if both conditions are met:
+  ///   1) It has local (i.e., private or internal) linkage.
+  //    2) Its data kind is one of {.rodata, .data, .bss, .data.rel.ro}.
   DenseMap<const Constant *, uint64_t> ConstantProfileCounts;
 
   /// Keeps track of the constants that are seen at least once without profile
@@ -26,7 +28,24 @@ class StaticDataProfileInfo {
   LLVM_ABI std::optional<uint64_t>
   getConstantProfileCount(const Constant *C) const;
 
+  enum class StaticDataHotness : uint8_t {
+    Cold = 0,
+    LukewarmOrUnknown = 1,
+    Hot = 2,
+  };
+
+  LLVM_ABI StaticDataHotness getSectionHotnessUsingProfileCount(
+      const Constant *C, const ProfileSummaryInfo *PSI, uint64_t Count) const;
+  LLVM_ABI StaticDataHotness
+  getSectionHotnessUsingDAP(std::optional<StringRef> SectionPrefix) const;
+
+  LLVM_ABI StringRef hotnessToStr(StaticDataHotness Hotness) const;
+
+  bool HasDataAccessProf = false;
+
 public:
+  StaticDataProfileInfo(bool HasDataAccessProf)
+      : HasDataAccessProf(HasDataAccessProf) {}
   StaticDataProfileInfo() = default;
 
   /// If \p Count is not nullopt, add it to the profile count of the constant \p
@@ -36,14 +55,10 @@ class StaticDataProfileInfo {
   LLVM_ABI void addConstantProfileCount(const Constant *C,
                                         std::optional<uint64_t> Count);
 
-  /// Return a section prefix for the constant \p C based on its profile count.
-  /// - If a constant doesn't have a counter, return an empty string.
-  /// - Otherwise,
-  ///   - If it has a hot count, return "hot".
-  ///   - If it is seen by unprofiled function, return an empty string.
-  ///   - If it has a cold count, return "unlikely".
-  ///   - Otherwise (e.g. it's used by lukewarm functions), return an empty
-  ///     string.
+  /// Given a constant \p C, returns a section prefix.
+  /// If \p C is a global variable, the section prefix is the bigger one
+  /// between its existing section prefix and its use profile count. Otherwise,
+  /// the section prefix is based on its use profile count.
   LLVM_ABI StringRef getConstantSectionPrefix(
       const Constant *C, const ProfileSummaryInfo *PSI) const;
 };

llvm/include/llvm/IR/GlobalObject.h

@@ -121,9 +121,13 @@ class GlobalObject : public GlobalValue {
   /// appropriate default object file section.
   LLVM_ABI void setSection(StringRef S);
 
-  /// Set the section prefix for this global object.
+  /// Set the section prefix for this global object. If \p Prefix is empty,
+  /// the section prefix metadata will be cleared if it exists.
   LLVM_ABI void setSectionPrefix(StringRef Prefix);
 
+  /// If \p Prefix is different from existing prefix, update section prefix. Returns true if an update happens and false otherwise.
+  LLVM_ABI bool updateSectionPrefix(StringRef Prefix);
+
   /// Get the section prefix for this global object.
   LLVM_ABI std::optional<StringRef> getSectionPrefix() const;
 

llvm/lib/Analysis/StaticDataProfileInfo.cpp

@@ -1,11 +1,17 @@
 #include "llvm/Analysis/StaticDataProfileInfo.h"
 #include "llvm/Analysis/ProfileSummaryInfo.h"
 #include "llvm/IR/Constant.h"
+#include "llvm/IR/Constants.h"
 #include "llvm/IR/GlobalVariable.h"
+#include "llvm/IR/Metadata.h"
+#include "llvm/IR/Module.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/ProfileData/InstrProf.h"
 
 using namespace llvm;
+
+extern cl::opt<bool> AnnotateStaticDataSectionPrefix;
+
 void StaticDataProfileInfo::addConstantProfileCount(
     const Constant *C, std::optional<uint64_t> Count) {
   if (!Count) {
@@ -28,29 +34,79 @@ StaticDataProfileInfo::getConstantProfileCount(const Constant *C) const {
   return I->second;
 }
 
-StringRef StaticDataProfileInfo::getConstantSectionPrefix(
-    const Constant *C, const ProfileSummaryInfo *PSI) const {
-  auto Count = getConstantProfileCount(C);
-  if (!Count)
-    return "";
+StaticDataProfileInfo::StaticDataHotness
+StaticDataProfileInfo::getSectionHotnessUsingProfileCount(
+    const Constant *C, const ProfileSummaryInfo *PSI, uint64_t Count) const {
   // The accummulated counter shows the constant is hot. Return 'hot' whether
   // this variable is seen by unprofiled functions or not.
-  if (PSI->isHotCount(*Count))
-    return "hot";
+  if (PSI->isHotCount(Count))
+    return StaticDataHotness::Hot;
   // The constant is not hot, and seen by unprofiled functions. We don't want to
   // assign it to unlikely sections, even if the counter says 'cold'. So return
   // an empty prefix before checking whether the counter is cold.
   if (ConstantWithoutCounts.count(C))
-    return "";
+    return StaticDataHotness::LukewarmOrUnknown;
   // The accummulated counter shows the constant is cold. Return 'unlikely'.
-  if (PSI->isColdCount(*Count))
+  if (PSI->isColdCount(Count))
+    return StaticDataHotness::Cold;
+
+  return StaticDataHotness::LukewarmOrUnknown;
+}
+
+StringRef StaticDataProfileInfo::hotnessToStr(
+    StaticDataProfileInfo::StaticDataHotness Hotness) const {
+  switch (Hotness) {
+  case StaticDataProfileInfo::StaticDataHotness::Cold:
     return "unlikely";
-  // The counter says lukewarm. Return an empty prefix.
-  return "";
+  case StaticDataProfileInfo::StaticDataHotness::Hot:
+    return "hot";
+  default:
+    return "";
+  }
+}
+
+StaticDataProfileInfo::StaticDataHotness
+StaticDataProfileInfo::getSectionHotnessUsingDAP(
+    std::optional<StringRef> MaybeSectionPrefix) const {
+  if (!MaybeSectionPrefix)
+    return StaticDataProfileInfo::StaticDataHotness::LukewarmOrUnknown;
+  StringRef Prefix = *MaybeSectionPrefix;
+  assert((Prefix == "hot" || Prefix == "unlikely") &&
+         "Expect section_prefix to be one of hot or unlikely");
+  return Prefix == "hot" ? StaticDataProfileInfo::StaticDataHotness::Hot
+                         : StaticDataProfileInfo::StaticDataHotness::Cold;
+}
+
+StringRef StaticDataProfileInfo::getConstantSectionPrefix(
+    const Constant *C, const ProfileSummaryInfo *PSI) const {
+  auto Count = getConstantProfileCount(C);
+
+  if (HasDataAccessProf) {
+    // Module flag `HasDataAccessProf` is 1 -> empty section prefix means uknown
+    // hotness except for string literals.
+    if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(C);
+        GV && !GV->getName().starts_with(".str")) {
+      auto HotnessFromDAP = getSectionHotnessUsingDAP(GV->getSectionPrefix());
+
+      if (!Count)
+        return hotnessToStr(HotnessFromDAP);
+
+      auto HotnessFromPGO = getSectionHotnessUsingProfileCount(C, PSI, *Count);
+      return hotnessToStr(std::max(HotnessFromDAP, HotnessFromPGO));
+    }
+  }
+
+  if (!Count)
+    return "";
+  return hotnessToStr(getSectionHotnessUsingProfileCount(C, PSI, *Count));
 }
 
 bool StaticDataProfileInfoWrapperPass::doInitialization(Module &M) {
-  Info.reset(new StaticDataProfileInfo());
+  bool HasDataAccessProf = false;
+  if (auto *MD = mdconst::extract_or_null<ConstantInt>(
+          M.getModuleFlag("HasDataAccessProf")))
+    HasDataAccessProf = MD->getZExtValue();
+  Info.reset(new StaticDataProfileInfo(HasDataAccessProf));
   return false;
 }
 

llvm/lib/CodeGen/StaticDataAnnotator.cpp

@@ -31,6 +31,8 @@
 #include "llvm/Analysis/StaticDataProfileInfo.h"
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/IR/Analysis.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/InitializePasses.h"
@@ -78,21 +80,8 @@ bool StaticDataAnnotator::runOnModule(Module &M) {
     if (GV.isDeclarationForLinker())
       continue;
 
-    // The implementation below assumes prior passes don't set section prefixes,
-    // and specifically do 'assign' rather than 'update'. So report error if a
-    // section prefix is already set.
-    if (auto maybeSectionPrefix = GV.getSectionPrefix();
-        maybeSectionPrefix && !maybeSectionPrefix->empty())
-      llvm::report_fatal_error("Global variable " + GV.getName() +
-                               " already has a section prefix " +
-                               *maybeSectionPrefix);
-
     StringRef SectionPrefix = SDPI->getConstantSectionPrefix(&GV, PSI);
-    if (SectionPrefix.empty())
-      continue;
-
-    GV.setSectionPrefix(SectionPrefix);
-    Changed = true;
+    Changed |= GV.updateSectionPrefix(SectionPrefix);
   }
 
   return Changed;

llvm/lib/IR/Globals.cpp

@@ -289,11 +289,28 @@ void GlobalObject::setSection(StringRef S) {
 }
 
 void GlobalObject::setSectionPrefix(StringRef Prefix) {
+  if (Prefix.empty()) {
+    setMetadata(LLVMContext::MD_section_prefix, nullptr);
+    return;
+  }
   MDBuilder MDB(getContext());
   setMetadata(LLVMContext::MD_section_prefix,
               MDB.createGlobalObjectSectionPrefix(Prefix));
 }
 
+bool GlobalObject::updateSectionPrefix(StringRef Prefix) {
+  auto MD = getMetadata(LLVMContext::MD_section_prefix);
+  StringRef ExistingPrefix; // Empty by default.
+  if (MD != nullptr)
+    ExistingPrefix = cast<MDString>(MD->getOperand(1))->getString();
+
+  if (ExistingPrefix != Prefix) {
+    setSectionPrefix(Prefix);
+    return true;
+  }
+  return false;
+}
+
 std::optional<StringRef> GlobalObject::getSectionPrefix() const {
   if (MDNode *MD = getMetadata(LLVMContext::MD_section_prefix)) {
     [[maybe_unused]] StringRef MDName =

llvm/lib/ProfileData/DataAccessProf.cpp

@@ -1,6 +1,7 @@
 #include "llvm/ProfileData/DataAccessProf.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Errc.h"
@@ -9,6 +10,9 @@
 #include "llvm/Support/raw_ostream.h"
 
 namespace llvm {
+cl::opt<bool> AnnotateStaticDataSectionPrefix(
+    "memprof-annotate-static-data-prefix", cl::init(false), cl::Hidden,
+    cl::desc("If true, annotate the static data section prefix"));
 namespace memprof {
 
 // If `Map` has an entry keyed by `Str`, returns the entry iterator. Otherwise,

llvm/lib/Transforms/Instrumentation/MemProfUse.cpp

@@ -41,6 +41,7 @@ using namespace llvm::memprof;
 #define DEBUG_TYPE "memprof"
 
 namespace llvm {
+extern cl::opt<bool> AnnotateStaticDataSectionPrefix;
 extern cl::opt<bool> PGOWarnMissing;
 extern cl::opt<bool> NoPGOWarnMismatch;
 extern cl::opt<bool> NoPGOWarnMismatchComdatWeak;
@@ -76,10 +77,6 @@ static cl::opt<unsigned> MinMatchedColdBytePercent(
     "memprof-matching-cold-threshold", cl::init(100), cl::Hidden,
     cl::desc("Min percent of cold bytes matched to hint allocation cold"));
 
-static cl::opt<bool> AnnotateStaticDataSectionPrefix(
-    "memprof-annotate-static-data-prefix", cl::init(false), cl::Hidden,
-    cl::desc("If true, annotate the static data section prefix"));
-
 // Matching statistics
 STATISTIC(NumOfMemProfMissing, "Number of functions without memory profile.");
 STATISTIC(NumOfMemProfMismatch,
@@ -797,7 +794,11 @@ bool MemProfUsePass::annotateGlobalVariables(
   if (!AnnotateStaticDataSectionPrefix || M.globals().empty())
     return false;
 
+  // The module flag helps codegen passes interpret empty section prefix:
+  // - 0 : empty section prefix is expected for each GV.
+  // - 1 : empty section prefix means the GV has unknown hotness.
   if (!DataAccessProf) {
+    M.addModuleFlag(Module::Warning, "HasDataAccessProf", 0U);
     M.getContext().diagnose(DiagnosticInfoPGOProfile(
         MemoryProfileFileName.data(),
         StringRef("Data access profiles not found in memprof. Ignore "
@@ -806,6 +807,8 @@ bool MemProfUsePass::annotateGlobalVariables(
     return false;
   }
 
+  M.addModuleFlag(Module::Warning, "HasDataAccessProf", 1);
+
   bool Changed = false;
   // Iterate all global variables in the module and annotate them based on
   // data access profiles. Note it's up to the linker to decide how to map input