[llvm] [NFC]Refactor static data splitter (PR #125758)

Mingming Liu via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 4 14:40:25 PST 2025


https://github.com/mingmingl-llvm updated https://github.com/llvm/llvm-project/pull/125758

>From 68765b3f37f0aafa018d9e47d6adc55cdf68461b Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Tue, 4 Feb 2025 11:28:09 -0800
Subject: [PATCH 1/2] [NFC]Refactor static data splitter

---
 llvm/lib/CodeGen/StaticDataSplitter.cpp | 112 ++++++++++++------------
 1 file changed, 58 insertions(+), 54 deletions(-)

diff --git a/llvm/lib/CodeGen/StaticDataSplitter.cpp b/llvm/lib/CodeGen/StaticDataSplitter.cpp
index e5bf0a5a3a255f..5d481c7d13e6c3 100644
--- a/llvm/lib/CodeGen/StaticDataSplitter.cpp
+++ b/llvm/lib/CodeGen/StaticDataSplitter.cpp
@@ -46,12 +46,12 @@ class StaticDataSplitter : public MachineFunctionPass {
   const MachineBlockFrequencyInfo *MBFI = nullptr;
   const ProfileSummaryInfo *PSI = nullptr;
 
-  // Returns true iff any jump table is hot-cold categorized.
-  bool splitJumpTables(MachineFunction &MF);
+  void updateStats(bool ProfileAvailable, const MachineJumpTableInfo *MJTI);
+  void updateJumpTableStats(bool ProfileAvailable,
+                            const MachineJumpTableInfo &MJTI);
 
-  // Same as above but works on functions with profile information.
-  bool splitJumpTablesWithProfiles(const MachineFunction &MF,
-                                   MachineJumpTableInfo &MJTI);
+  // Use profiles to partition static data.
+  bool partitionStaticDataWithProfiles(MachineFunction &MF);
 
 public:
   static char ID;
@@ -77,13 +77,22 @@ bool StaticDataSplitter::runOnMachineFunction(MachineFunction &MF) {
   MBFI = &getAnalysis<MachineBlockFrequencyInfoWrapperPass>().getMBFI();
   PSI = &getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI();
 
-  return splitJumpTables(MF);
+  const bool ProfileAvailable = PSI && PSI->hasProfileSummary() && MBFI &&
+                                MF.getFunction().hasProfileData();
+  bool Changed = false;
+
+  if (ProfileAvailable)
+    Changed |= partitionStaticDataWithProfiles(MF);
+
+  updateStats(ProfileAvailable, MF.getJumpTableInfo());
+  return Changed;
 }
 
-bool StaticDataSplitter::splitJumpTablesWithProfiles(
-    const MachineFunction &MF, MachineJumpTableInfo &MJTI) {
+bool StaticDataSplitter::partitionStaticDataWithProfiles(MachineFunction &MF) {
   int NumChangedJumpTables = 0;
 
+  MachineJumpTableInfo *MJTI = MF.getJumpTableInfo();
+
   // Jump table could be used by either terminating instructions or
   // non-terminating ones, so we walk all instructions and use
   // `MachineOperand::isJTI()` to identify jump table operands.
@@ -92,63 +101,58 @@ bool StaticDataSplitter::splitJumpTablesWithProfiles(
   for (const auto &MBB : MF) {
     for (const MachineInstr &I : MBB) {
       for (const MachineOperand &Op : I.operands()) {
-        if (!Op.isJTI())
-          continue;
-        const int JTI = Op.getIndex();
-        // This is not a source block of jump table.
-        if (JTI == -1)
-          continue;
-
-        auto Hotness = MachineFunctionDataHotness::Hot;
-
-        // Hotness is based on source basic block hotness.
-        // TODO: PSI APIs are about instruction hotness. Introduce API for data
-        // access hotness.
-        if (PSI->isColdBlock(&MBB, MBFI))
-          Hotness = MachineFunctionDataHotness::Cold;
-
-        if (MJTI.updateJumpTableEntryHotness(JTI, Hotness))
-          ++NumChangedJumpTables;
+        if (Op.isJTI()) {
+          assert(MJTI != nullptr && "Jump table info is not available.");
+          const int JTI = Op.getIndex();
+          // This is not a source block of jump table.
+          if (JTI == -1)
+            continue;
+
+          auto Hotness = MachineFunctionDataHotness::Hot;
+
+          // Hotness is based on source basic block hotness.
+          // TODO: PSI APIs are about instruction hotness. Introduce API for
+          // data access hotness.
+          if (PSI->isColdBlock(&MBB, MBFI))
+            Hotness = MachineFunctionDataHotness::Cold;
+
+          if (MJTI->updateJumpTableEntryHotness(JTI, Hotness))
+            ++NumChangedJumpTables;
+        }
       }
     }
   }
   return NumChangedJumpTables > 0;
 }
 
-bool StaticDataSplitter::splitJumpTables(MachineFunction &MF) {
-  MachineJumpTableInfo *MJTI = MF.getJumpTableInfo();
-  if (!MJTI || MJTI->getJumpTables().empty())
-    return false;
+void StaticDataSplitter::updateJumpTableStats(
+    bool ProfileAvailable, const MachineJumpTableInfo &MJTI) {
 
-  const bool ProfileAvailable = PSI && PSI->hasProfileSummary() && MBFI &&
-                                MF.getFunction().hasProfileData();
-  auto statOnExit = llvm::make_scope_exit([&] {
-    if (!AreStatisticsEnabled())
-      return;
-
-    if (!ProfileAvailable) {
-      NumUnknownJumpTables += MJTI->getJumpTables().size();
-      return;
-    }
+  if (!ProfileAvailable) {
+    NumUnknownJumpTables += MJTI.getJumpTables().size();
+    return;
+  }
 
-    for (size_t JTI = 0; JTI < MJTI->getJumpTables().size(); JTI++) {
-      auto Hotness = MJTI->getJumpTables()[JTI].Hotness;
-      if (Hotness == MachineFunctionDataHotness::Hot) {
-        ++NumHotJumpTables;
-      } else {
-        assert(Hotness == MachineFunctionDataHotness::Cold &&
-               "A jump table is either hot or cold when profile information is "
-               "available.");
-        ++NumColdJumpTables;
-      }
+  for (size_t JTI = 0; JTI < MJTI.getJumpTables().size(); JTI++) {
+    auto Hotness = MJTI.getJumpTables()[JTI].Hotness;
+    if (Hotness == MachineFunctionDataHotness::Hot) {
+      ++NumHotJumpTables;
+    } else {
+      assert(Hotness == MachineFunctionDataHotness::Cold &&
+             "A jump table is either hot or cold when profile information is "
+             "available.");
+      ++NumColdJumpTables;
     }
-  });
+  }
+}
 
-  // Place jump tables according to block hotness if function has profile data.
-  if (ProfileAvailable)
-    return splitJumpTablesWithProfiles(MF, *MJTI);
+void StaticDataSplitter::updateStats(bool ProfileAvailable,
+                                     const MachineJumpTableInfo *MJTI) {
+  if (!AreStatisticsEnabled())
+    return;
 
-  return true;
+  if (MJTI)
+    updateJumpTableStats(ProfileAvailable, *MJTI);
 }
 
 char StaticDataSplitter::ID = 0;

>From 079e7a61187a0f54fb93397dae08e4c595741f2c Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Tue, 4 Feb 2025 14:34:57 -0800
Subject: [PATCH 2/2] resolve comments

---
 llvm/lib/CodeGen/StaticDataSplitter.cpp | 53 +++++++++++++------------
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/llvm/lib/CodeGen/StaticDataSplitter.cpp b/llvm/lib/CodeGen/StaticDataSplitter.cpp
index 5d481c7d13e6c3..0965fe85acfc78 100644
--- a/llvm/lib/CodeGen/StaticDataSplitter.cpp
+++ b/llvm/lib/CodeGen/StaticDataSplitter.cpp
@@ -46,9 +46,10 @@ class StaticDataSplitter : public MachineFunctionPass {
   const MachineBlockFrequencyInfo *MBFI = nullptr;
   const ProfileSummaryInfo *PSI = nullptr;
 
-  void updateStats(bool ProfileAvailable, const MachineJumpTableInfo *MJTI);
-  void updateJumpTableStats(bool ProfileAvailable,
-                            const MachineJumpTableInfo &MJTI);
+  // Update LLVM statistics for a machine function without profiles.
+  void updateStatsWithoutProfiles(const MachineFunction &MF);
+  // Update LLVM statistics for a machine function with profiles.
+  void updateStatsWithProfiles(const MachineFunction &MF);
 
   // Use profiles to partition static data.
   bool partitionStaticDataWithProfiles(MachineFunction &MF);
@@ -79,12 +80,15 @@ bool StaticDataSplitter::runOnMachineFunction(MachineFunction &MF) {
 
   const bool ProfileAvailable = PSI && PSI->hasProfileSummary() && MBFI &&
                                 MF.getFunction().hasProfileData();
-  bool Changed = false;
 
-  if (ProfileAvailable)
-    Changed |= partitionStaticDataWithProfiles(MF);
+  if (!ProfileAvailable) {
+    updateStatsWithoutProfiles(MF);
+    return false;
+  }
+
+  bool Changed = partitionStaticDataWithProfiles(MF);
 
-  updateStats(ProfileAvailable, MF.getJumpTableInfo());
+  updateStatsWithProfiles(MF);
   return Changed;
 }
 
@@ -125,34 +129,31 @@ bool StaticDataSplitter::partitionStaticDataWithProfiles(MachineFunction &MF) {
   return NumChangedJumpTables > 0;
 }
 
-void StaticDataSplitter::updateJumpTableStats(
-    bool ProfileAvailable, const MachineJumpTableInfo &MJTI) {
-
-  if (!ProfileAvailable) {
-    NumUnknownJumpTables += MJTI.getJumpTables().size();
+void StaticDataSplitter::updateStatsWithProfiles(const MachineFunction &MF) {
+  if (!AreStatisticsEnabled())
     return;
-  }
 
-  for (size_t JTI = 0; JTI < MJTI.getJumpTables().size(); JTI++) {
-    auto Hotness = MJTI.getJumpTables()[JTI].Hotness;
-    if (Hotness == MachineFunctionDataHotness::Hot) {
-      ++NumHotJumpTables;
-    } else {
-      assert(Hotness == MachineFunctionDataHotness::Cold &&
-             "A jump table is either hot or cold when profile information is "
-             "available.");
-      ++NumColdJumpTables;
+  if (const MachineJumpTableInfo *MJTI = MF.getJumpTableInfo()) {
+    for (const auto &JumpTable : MJTI->getJumpTables()) {
+      if (JumpTable.Hotness == MachineFunctionDataHotness::Hot) {
+        ++NumHotJumpTables;
+      } else {
+        assert(JumpTable.Hotness == MachineFunctionDataHotness::Cold &&
+               "A jump table is either hot or cold when profile information is "
+               "available.");
+        ++NumColdJumpTables;
+      }
     }
   }
 }
 
-void StaticDataSplitter::updateStats(bool ProfileAvailable,
-                                     const MachineJumpTableInfo *MJTI) {
+void StaticDataSplitter::updateStatsWithoutProfiles(const MachineFunction &MF) {
   if (!AreStatisticsEnabled())
     return;
 
-  if (MJTI)
-    updateJumpTableStats(ProfileAvailable, *MJTI);
+  if (const MachineJumpTableInfo *MJTI = MF.getJumpTableInfo()) {
+    NumUnknownJumpTables += MJTI->getJumpTables().size();
+  }
 }
 
 char StaticDataSplitter::ID = 0;



More information about the llvm-commits mailing list