From 68765b3f37f0aafa018d9e47d6adc55cdf68461b Mon Sep 17 00:00:00 2001 From: mingmingl 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 e5bf0a5a3a255..5d481c7d13e6c 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().getMBFI(); PSI = &getAnalysis().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 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 5d481c7d13e6c..0965fe85acfc7 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;