Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NFC]Refactor static data splitter #125758

Merged
merged 2 commits into from
Feb 5, 2025
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 58 additions & 54 deletions llvm/lib/CodeGen/StaticDataSplitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

document the methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

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;
Expand All @@ -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());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be moved up before the partition? With that, an early return can be used when profile is not available.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I revised the change to have updateStatsWithoutProfiles and updateStatsWithProfiles, and moved updateStatsWithoutProfiles up.

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.
Expand All @@ -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;
Expand Down