-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[CodeGen][StaticDataPartitioning]Place local-linkage global variables in hot or unlikely prefixed sections based on profile information #125756
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
Changes from 2 commits
8eea1ea
93d9881
5cbe8f8
8f21570
f07d34d
4e096e9
4a2a881
1f50494
967dc03
9302b2b
97103c6
38c8a03
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,13 +9,13 @@ | |||||||||||||||||||||
| // The pass uses branch profile data to assign hotness based section qualifiers | ||||||||||||||||||||||
| // for the following types of static data: | ||||||||||||||||||||||
| // - Jump tables | ||||||||||||||||||||||
| // - Module-internal global variables | ||||||||||||||||||||||
| // - Constant pools (TODO) | ||||||||||||||||||||||
| // - Other module-internal data (TODO) | ||||||||||||||||||||||
| // | ||||||||||||||||||||||
| // For the original RFC of this pass please see | ||||||||||||||||||||||
| // https://discourse.llvm.org/t/rfc-profile-guided-static-data-partitioning/83744 | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| #include "llvm/ADT/ScopeExit.h" | ||||||||||||||||||||||
| #include "llvm/ADT/APInt.h" | ||||||||||||||||||||||
| #include "llvm/ADT/Statistic.h" | ||||||||||||||||||||||
| #include "llvm/Analysis/ProfileSummaryInfo.h" | ||||||||||||||||||||||
| #include "llvm/CodeGen/MBFIWrapper.h" | ||||||||||||||||||||||
|
|
@@ -27,9 +27,12 @@ | |||||||||||||||||||||
| #include "llvm/CodeGen/MachineFunctionPass.h" | ||||||||||||||||||||||
| #include "llvm/CodeGen/MachineJumpTableInfo.h" | ||||||||||||||||||||||
| #include "llvm/CodeGen/Passes.h" | ||||||||||||||||||||||
| #include "llvm/IR/GlobalVariable.h" | ||||||||||||||||||||||
| #include "llvm/IR/Module.h" | ||||||||||||||||||||||
| #include "llvm/InitializePasses.h" | ||||||||||||||||||||||
| #include "llvm/Pass.h" | ||||||||||||||||||||||
| #include "llvm/Support/CommandLine.h" | ||||||||||||||||||||||
| #include "llvm/Target/TargetLoweringObjectFile.h" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| using namespace llvm; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
@@ -46,12 +49,27 @@ 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); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // If the global value is a local linkage global variable, return it. | ||||||||||||||||||||||
| // Otherwise, return nullptr. | ||||||||||||||||||||||
| const GlobalVariable *getLocalLinkageGlobalVariable(const GlobalValue *GV); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Returns true if the global variable is in one of {.rodata, .bss, .data, | ||||||||||||||||||||||
| // .data.rel.ro} sections | ||||||||||||||||||||||
| bool inStaticDataSection(const GlobalVariable *GV, const TargetMachine &TM); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Iterate all global variables in the module and update the section prefix | ||||||||||||||||||||||
| // of the module-internal data. | ||||||||||||||||||||||
| void updateGlobalVariableSectionPrefix(MachineFunction &MF); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Accummulated data profile count across machine functions in the module. | ||||||||||||||||||||||
| DenseMap<const GlobalVariable *, APInt> DataProfileCounts; | ||||||||||||||||||||||
snehasish marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public: | ||||||||||||||||||||||
| static char ID; | ||||||||||||||||||||||
|
|
@@ -77,13 +95,24 @@ 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); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| updateGlobalVariableSectionPrefix(MF); | ||||||||||||||||||||||
| updateStats(ProfileAvailable, MF.getJumpTableInfo()); | ||||||||||||||||||||||
| return Changed; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| bool StaticDataSplitter::splitJumpTablesWithProfiles( | ||||||||||||||||||||||
| const MachineFunction &MF, MachineJumpTableInfo &MJTI) { | ||||||||||||||||||||||
| bool StaticDataSplitter::partitionStaticDataWithProfiles(MachineFunction &MF) { | ||||||||||||||||||||||
| int NumChangedJumpTables = 0; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const TargetMachine &TM = MF.getTarget(); | ||||||||||||||||||||||
| 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 +121,131 @@ 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) | ||||||||||||||||||||||
| std::optional<uint64_t> Count = std::nullopt; | ||||||||||||||||||||||
| if (!Op.isJTI() && !Op.isGlobal()) | ||||||||||||||||||||||
| continue; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| auto Hotness = MachineFunctionDataHotness::Hot; | ||||||||||||||||||||||
| Count = MBFI->getBlockProfileCount(&MBB); | ||||||||||||||||||||||
|
||||||||||||||||||||||
| std::optional<uint64_t> Count = std::nullopt; | |
| if (!Op.isJTI() && !Op.isGlobal()) | |
| continue; | |
| auto Hotness = MachineFunctionDataHotness::Hot; | |
| Count = MBFI->getBlockProfileCount(&MBB); | |
| if (!Op.isJTI() && !Op.isGlobal()) | |
| continue; | |
| std::optional<uint64_t> Count = MBFI->getBlockProfileCount(&MBB); |
is that OK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes the change is ok.
I kept getting "Applying suggestions on deleted lines is currently not supported." error when trying to commit the suggestion. I'll make the change in the local client..
snehasish marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can "GV->hasLocalLinkage()" cover "GV->isDeclarationForLinker()"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. I haven't thought about it before.
It turns out IR verifier requires a declaration to have one of 'valid' decl linkages, and valid decl linkages must be at least external according to isValidDeclarationLinkage.
I removed GV->isDeclarationForLinker() here and added a comment.
snehasish marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please explain why need to skip LLVM reserved symbols?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. The globals with llvm. as a name prefix are usually handled specially, by many (middle-end and back-end) passes (e.g., AsmPrinter emits special LLVM values specially), and they are skipped in this pass mostly out of conservativeness.
I moved this check before calling inStaticDataSection helper though for readability, and added a comment around the check. What do you think about this?
Uh oh!
There was an error while loading. Please reload this page.