resolve comments

Author: mingmingl-llvm

llvm/include/llvm/CodeGen/AsmPrinter.h

@@ -338,6 +338,10 @@ class AsmPrinter : public MachineFunctionPass {
     DwarfUsesRelocationsAcrossSections = Enable;
   }
 
+  // Returns a section suffix (hot or unlikely) for the constant if profiles
+  // are available. Returns empty string otherwise.
+  StringRef getConstantSectionSuffix(const Constant *C) const;
+
   //===------------------------------------------------------------------===//
   // XRay instrumentation implementation.
   //===------------------------------------------------------------------===//

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

@@ -2769,6 +2769,23 @@ namespace {
 
 } // end anonymous namespace
 
+StringRef AsmPrinter::getConstantSectionSuffix(const Constant *C) const {
+  SmallString<8> SectionNameSuffix;
+  if (TM.Options.EnableStaticDataPartitioning) {
+    if (C && SDPI && PSI) {
+      auto Count = SDPI->getConstantProfileCount(C);
+      if (Count) {
+        if (PSI->isHotCount(*Count)) {
+          SectionNameSuffix.append("hot");
+        } else if (PSI->isColdCount(*Count) && !SDPI->hasUnknownCount(C)) {
+          SectionNameSuffix.append("unlikely");
+        }
+      }
+    }
+  }
+  return SectionNameSuffix.str();
+}
+
 /// EmitConstantPool - Print to the current output stream assembly
 /// representations of the constants in the constant pool MCP. This is
 /// used to print out constants which have been "spilled to memory" by
@@ -2791,26 +2808,8 @@ void AsmPrinter::emitConstantPool() {
     if (!CPE.isMachineConstantPoolEntry())
       C = CPE.Val.ConstVal;
 
-    MCSection *S = nullptr;
-    if (TM.Options.EnableStaticDataPartitioning) {
-      SmallString<8> SectionNameSuffix;
-      if (C && SDPI && PSI) {
-        auto Count = SDPI->getConstantProfileCount(C);
-        if (Count) {
-          if (PSI->isHotCount(*Count)) {
-            SectionNameSuffix.append("hot");
-          } else if (PSI->isColdCount(*Count) && !SDPI->hasUnknownCount(C)) {
-            SectionNameSuffix.append("unlikely");
-          }
-        }
-      }
-
-      S = getObjFileLowering().getSectionForConstant(
-          getDataLayout(), Kind, C, Alignment, SectionNameSuffix);
-    } else {
-      S = getObjFileLowering().getSectionForConstant(getDataLayout(), Kind, C,
-                                                     Alignment);
-    }
+    MCSection *S = getObjFileLowering().getSectionForConstant(
+        getDataLayout(), Kind, C, Alignment, getConstantSectionSuffix(C));
 
     // The number of sections are small, just do a linear search from the
     // last section to the first.

llvm/lib/CodeGen/StaticDataSplitter.cpp

@@ -69,6 +69,11 @@ class StaticDataSplitter : public MachineFunctionPass {
 
   void annotateStaticDataWithoutProfiles(const MachineFunction &MF);
 
+  // Returns the constant if the operand refers to a global variable or constant
+  // that gets lowered to static data sections. Otherwise, return nullptr.
+  const Constant *getConstant(const MachineOperand &Op, const TargetMachine &TM,
+                              const MachineConstantPool *MCP);
+
 public:
   static char ID;
 
@@ -112,12 +117,42 @@ bool StaticDataSplitter::runOnMachineFunction(MachineFunction &MF) {
   return Changed;
 }
 
+const Constant *
+StaticDataSplitter::getConstant(const MachineOperand &Op,
+                                const TargetMachine &TM,
+                                const MachineConstantPool *MCP) {
+  if (!Op.isGlobal() && !Op.isCPI())
+    return nullptr;
+
+  if (Op.isGlobal()) {
+    // Find global variables with local linkage.
+    const GlobalVariable *GV = getLocalLinkageGlobalVariable(Op.getGlobal());
+    // Skip 'special' global variables conservatively because they are
+    // often handled specially, and skip those not in static data
+    // sections.
+    if (!GV || GV->getName().starts_with("llvm.") ||
+        !inStaticDataSection(GV, TM))
+      return nullptr;
+    return GV;
+  }
+  assert(Op.isCPI() && "Op must be constant pool index in this branch");
+  int CPI = Op.getIndex();
+  if (CPI == -1)
+    return nullptr;
+
+  assert(MCP != nullptr && "Constant pool info is not available.");
+  const MachineConstantPoolEntry &CPE = MCP->getConstants()[CPI];
+
+  if (CPE.isMachineConstantPoolEntry())
+    return nullptr;
+
+  return CPE.Val.ConstVal;
+}
+
 bool StaticDataSplitter::partitionStaticDataWithProfiles(MachineFunction &MF) {
   int NumChangedJumpTables = 0;
 
-  const TargetMachine &TM = MF.getTarget();
   MachineJumpTableInfo *MJTI = MF.getJumpTableInfo();
-  const MachineConstantPool *MCP = MF.getConstantPool();
 
   // Jump table could be used by either terminating instructions or
   // non-terminating ones, so we walk all instructions and use
@@ -149,30 +184,8 @@ bool StaticDataSplitter::partitionStaticDataWithProfiles(MachineFunction &MF) {
 
           if (MJTI->updateJumpTableEntryHotness(JTI, Hotness))
             ++NumChangedJumpTables;
-        } else if (Op.isGlobal()) {
-          // Find global variables with local linkage.
-          const GlobalVariable *GV =
-              getLocalLinkageGlobalVariable(Op.getGlobal());
-          // Skip 'special' global variables conservatively because they are
-          // often handled specially, and skip those not in static data
-          // sections.
-          if (!GV || GV->getName().starts_with("llvm.") ||
-              !inStaticDataSection(GV, TM))
-            continue;
-          SDPI->addConstantProfileCount(GV, Count);
-        } else {
-          assert(Op.isCPI() && "Op must be constant pool index in this branch");
-          int CPI = Op.getIndex();
-          if (CPI == -1)
-            continue;
-
-          assert(MCP != nullptr && "Constant pool info is not available.");
-          const MachineConstantPoolEntry &CPE = MCP->getConstants()[CPI];
-
-          if (CPE.isMachineConstantPoolEntry())
-            continue;
-
-          const Constant *C = CPE.Val.ConstVal;
+        } else if (const Constant *C =
+                       getConstant(Op, MF.getTarget(), MF.getConstantPool())) {
           SDPI->addConstantProfileCount(C, Count);
         }
       }
@@ -218,34 +231,13 @@ void StaticDataSplitter::updateStatsWithProfiles(const MachineFunction &MF) {
 
 void StaticDataSplitter::annotateStaticDataWithoutProfiles(
     const MachineFunction &MF) {
-  const MachineConstantPool *MCP = MF.getConstantPool();
   for (const auto &MBB : MF) {
     for (const MachineInstr &I : MBB) {
       for (const MachineOperand &Op : I.operands()) {
-        if (!Op.isGlobal() && !Op.isCPI())
-          continue;
-        if (Op.isGlobal()) {
-          const GlobalVariable *GV =
-              getLocalLinkageGlobalVariable(Op.getGlobal());
-          if (!GV || GV->getName().starts_with("llvm.") ||
-              !inStaticDataSection(GV, MF.getTarget()))
-            continue;
-          SDPI->addConstantProfileCount(GV, std::nullopt);
-        } else {
-          assert(Op.isCPI() && "Op must be constant pool index in this branch");
-          int CPI = Op.getIndex();
-          if (CPI == -1)
-            continue;
-
-          assert(MCP != nullptr && "Constant pool info is not available.");
-          const MachineConstantPoolEntry &CPE = MCP->getConstants()[CPI];
-
-          if (CPE.isMachineConstantPoolEntry())
-            continue;
-
-          const Constant *C = CPE.Val.ConstVal;
+        const Constant *C =
+            getConstant(Op, MF.getTarget(), MF.getConstantPool());
+        if (C)
           SDPI->addConstantProfileCount(C, std::nullopt);
-        }
       }
     }
   }

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

@@ -1074,35 +1074,35 @@ MCSection *TargetLoweringObjectFileELF::getSectionForConstant(
 
 MCSection *TargetLoweringObjectFileELF::getSectionForConstant(
     const DataLayout &DL, SectionKind Kind, const Constant *C, Align &Alignment,
-    StringRef SectionPrefix) const {
+    StringRef SectionSuffix) const {
   // TODO: Share code between this function and
   // MCObjectInfo::initELFMCObjectFileInfo.
-  if (SectionPrefix.empty())
+  if (SectionSuffix.empty())
     return getSectionForConstant(DL, Kind, C, Alignment);
 
   auto &Context = getContext();
   if (Kind.isMergeableConst4() && MergeableConst4Section)
-    return Context.getELFSection(".rodata.cst4." + SectionPrefix,
+    return Context.getELFSection(".rodata.cst4." + SectionSuffix,
                                  ELF::SHT_PROGBITS,
                                  ELF::SHF_ALLOC | ELF::SHF_MERGE, 4);
   if (Kind.isMergeableConst8() && MergeableConst8Section)
-    return Context.getELFSection(".rodata.cst8." + SectionPrefix,
+    return Context.getELFSection(".rodata.cst8." + SectionSuffix,
                                  ELF::SHT_PROGBITS,
                                  ELF::SHF_ALLOC | ELF::SHF_MERGE, 8);
   if (Kind.isMergeableConst16() && MergeableConst16Section)
-    return Context.getELFSection(".rodata.cst16." + SectionPrefix,
+    return Context.getELFSection(".rodata.cst16." + SectionSuffix,
                                  ELF::SHT_PROGBITS,
                                  ELF::SHF_ALLOC | ELF::SHF_MERGE, 16);
   if (Kind.isMergeableConst32() && MergeableConst32Section)
-    return Context.getELFSection(".rodata.cst32." + SectionPrefix,
+    return Context.getELFSection(".rodata.cst32." + SectionSuffix,
                                  ELF::SHT_PROGBITS,
                                  ELF::SHF_ALLOC | ELF::SHF_MERGE, 32);
   if (Kind.isReadOnly())
-    return Context.getELFSection(".rodata" + SectionPrefix, ELF::SHT_PROGBITS,
+    return Context.getELFSection(".rodata." + SectionSuffix, ELF::SHT_PROGBITS,
                                  ELF::SHF_ALLOC);
 
   assert(Kind.isReadOnlyWithRel() && "Unknown section kind");
-  return Context.getELFSection(".data.rel.ro" + SectionPrefix,
+  return Context.getELFSection(".data.rel.ro." + SectionSuffix,
                                ELF::SHT_PROGBITS,
                                ELF::SHF_ALLOC | ELF::SHF_WRITE);
 }