-
Notifications
You must be signed in to change notification settings - Fork 15.2k
Insert symbols for prefetch targets read from basic blocks section profile. #168439
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
base: main
Are you sure you want to change the base?
Changes from all commits
49c5f22
b25adef
bbfb7ba
3e6b04f
9967360
a08b65a
d988a3c
4008445
e3b501f
715f1b8
a1e1e00
717e6fe
3605b0d
6408bd7
a06cb9d
ceefc56
639efd7
cc4e333
6d8bdb1
d93a5ec
7cb4f6b
9fdf7d0
0c17e45
500b536
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 |
|---|---|---|
|
|
@@ -1985,7 +1985,33 @@ void AsmPrinter::emitFunctionBody() { | |
| // Print a label for the basic block. | ||
| emitBasicBlockStart(MBB); | ||
| DenseMap<StringRef, unsigned> MnemonicCounts; | ||
|
|
||
| SmallVector<unsigned> PrefetchTargets = | ||
| MBB.getPrefetchTargetCallsiteIndexes(); | ||
| auto PrefetchTargetIt = PrefetchTargets.begin(); | ||
| unsigned LastCallsiteIndex = 0; | ||
| // Helper to emit a symbol for the prefetch target and proceed to the next | ||
| // one. | ||
| auto EmitPrefetchTargetSymbolIfNeeded = [&]() { | ||
| if (PrefetchTargetIt != PrefetchTargets.end() && | ||
| *PrefetchTargetIt == LastCallsiteIndex) { | ||
| MCSymbol *PrefetchTargetSymbol = OutContext.getOrCreateSymbol( | ||
| Twine("__llvm_prefetch_target_") + MF->getName() + Twine("_") + | ||
| utostr(MBB.getBBID()->BaseID) + Twine("_") + | ||
| utostr(static_cast<unsigned>(*PrefetchTargetIt))); | ||
| // If the function is weak-linkage it may be replaced by a strong | ||
| // version, in which case the prefetch targets should also be replaced. | ||
| OutStreamer->emitSymbolAttribute( | ||
| PrefetchTargetSymbol, | ||
| MF->getFunction().isWeakForLinker() ? MCSA_Weak : MCSA_Global); | ||
| OutStreamer->emitLabel(PrefetchTargetSymbol); | ||
| ++PrefetchTargetIt; | ||
| } | ||
| }; | ||
|
|
||
| for (auto &MI : MBB) { | ||
| EmitPrefetchTargetSymbolIfNeeded(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would have EmitPrefetchTargetIt take a parameter which is the unsigned callsiteindex. I would remove the updates to the PrefetchTargets from the lambda and do it within the loop and replace this line with: if (PrefetchTargetIt != PrefetchTargets.end() &&
*PrefetchTargetIt == LastCallSiteIndex) {
EmitPrefetchTargetSymbolIfNeeded(*PrefetchTargetIt);
++PrefetchTargetIt;
} |
||
|
|
||
| // Print the assembly for the instruction. | ||
| if (!MI.isPosition() && !MI.isImplicitDef() && !MI.isKill() && | ||
| !MI.isDebugInstr()) { | ||
|
|
@@ -2123,8 +2149,11 @@ void AsmPrinter::emitFunctionBody() { | |
| break; | ||
| } | ||
|
|
||
| if (MI.isCall() && MF->getTarget().Options.BBAddrMap) | ||
| OutStreamer->emitLabel(createCallsiteEndSymbol(MBB)); | ||
| if (MI.isCall()) { | ||
| if (MF->getTarget().Options.BBAddrMap) | ||
| OutStreamer->emitLabel(createCallsiteEndSymbol(MBB)); | ||
| LastCallsiteIndex++; | ||
| } | ||
|
|
||
| if (TM.Options.EmitCallGraphSection && MI.isCall()) | ||
| handleCallsiteForCallgraph(FuncCGInfo, CallSitesInfoMap, MI); | ||
|
|
@@ -2136,6 +2165,8 @@ void AsmPrinter::emitFunctionBody() { | |
| for (auto &Handler : Handlers) | ||
| Handler->endInstruction(); | ||
| } | ||
| // Emit the last prefetch target in case the last instruction was a call. | ||
| EmitPrefetchTargetSymbolIfNeeded(); | ||
|
|
||
| // We must emit temporary symbol for the end of this basic block, if either | ||
| // we have BBLabels enabled or if this basic blocks marks the end of a | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| //===-- InsertCodePrefetch.cpp ---=========--------------------------------===// | ||
| // | ||
| // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
| // See https://llvm.org/LICENSE.txt for license information. | ||
| // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
| // | ||
| /// \file | ||
| /// Code Prefetch Insertion Pass. | ||
| //===----------------------------------------------------------------------===// | ||
| /// This pass inserts code prefetch instructions according to the prefetch | ||
| /// directives in the basic block section profile. The target of a prefetch can | ||
| /// be the beginning of any dynamic basic block, that is the beginning of a | ||
| /// machine basic block, or immediately after a callsite. A global symbol is | ||
| /// emitted at the position of the target so it can be addressed from the | ||
| /// prefetch instruction from any module. | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #include "llvm/ADT/SmallVector.h" | ||
| #include "llvm/ADT/StringExtras.h" | ||
| #include "llvm/ADT/StringRef.h" | ||
| #include "llvm/CodeGen/BasicBlockSectionUtils.h" | ||
| #include "llvm/CodeGen/BasicBlockSectionsProfileReader.h" | ||
| #include "llvm/CodeGen/MachineBasicBlock.h" | ||
| #include "llvm/CodeGen/MachineFunction.h" | ||
| #include "llvm/CodeGen/MachineFunctionPass.h" | ||
| #include "llvm/CodeGen/Passes.h" | ||
| #include "llvm/InitializePasses.h" | ||
|
|
||
| using namespace llvm; | ||
| #define DEBUG_TYPE "insert-code-prefetch" | ||
|
|
||
| namespace { | ||
| class InsertCodePrefetch : public MachineFunctionPass { | ||
| public: | ||
| static char ID; | ||
|
|
||
| InsertCodePrefetch() : MachineFunctionPass(ID) { | ||
| initializeInsertCodePrefetchPass(*PassRegistry::getPassRegistry()); | ||
| } | ||
|
|
||
| StringRef getPassName() const override { | ||
| return "Code Prefetch Inserter Pass"; | ||
| } | ||
|
|
||
| void getAnalysisUsage(AnalysisUsage &AU) const override; | ||
|
|
||
| // Sets prefetch targets based on the bb section profile. | ||
| bool runOnMachineFunction(MachineFunction &MF) override; | ||
| }; | ||
|
|
||
| } // end anonymous namespace | ||
|
|
||
| //===----------------------------------------------------------------------===// | ||
| // Implementation | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| char InsertCodePrefetch::ID = 0; | ||
| INITIALIZE_PASS_BEGIN(InsertCodePrefetch, DEBUG_TYPE, "Code prefetch insertion", | ||
| true, false) | ||
| INITIALIZE_PASS_DEPENDENCY(BasicBlockSectionsProfileReaderWrapperPass) | ||
| INITIALIZE_PASS_END(InsertCodePrefetch, DEBUG_TYPE, "Code prefetch insertion", | ||
| true, false) | ||
|
|
||
| bool InsertCodePrefetch::runOnMachineFunction(MachineFunction &MF) { | ||
| assert(MF.getTarget().getBBSectionsType() == BasicBlockSection::List && | ||
| "BB Sections list not enabled!"); | ||
| if (hasInstrProfHashMismatch(MF)) | ||
| return false; | ||
| // Set each block's prefetch targets so AsmPrinter can emit a special symbol | ||
| // there. | ||
| SmallVector<CallsiteID> PrefetchTargets = | ||
| getAnalysis<BasicBlockSectionsProfileReaderWrapperPass>() | ||
| .getPrefetchTargetsForFunction(MF.getName()); | ||
| DenseMap<UniqueBBID, SmallVector<unsigned>> PrefetchTargetsByBBID; | ||
| for (const auto &Target : PrefetchTargets) | ||
| PrefetchTargetsByBBID[Target.BBID].push_back(Target.CallsiteIndex); | ||
| // Sort and uniquify the callsite indices for every block. | ||
| for (auto &[K, V] : PrefetchTargetsByBBID) { | ||
| llvm::sort(V); | ||
| V.erase(llvm::unique(V), V.end()); | ||
| } | ||
| for (auto &MBB : MF) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possible to iterate the BBIDs and use MF->getBlockNumbered(ID) method to get MBB?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't store a map from BBIDs to MBBs. So there is no way around once iterating over MBBs if that's what you mean (We can create the map here but it would still require an iteration). |
||
| auto R = PrefetchTargetsByBBID.find(*MBB.getBBID()); | ||
| if (R == PrefetchTargetsByBBID.end()) | ||
| continue; | ||
| MBB.setPrefetchTargetCallsiteIndexes(R->second); | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| void InsertCodePrefetch::getAnalysisUsage(AnalysisUsage &AU) const { | ||
| AU.setPreservesAll(); | ||
| AU.addRequired<BasicBlockSectionsProfileReaderWrapperPass>(); | ||
| MachineFunctionPass::getAnalysisUsage(AU); | ||
| } | ||
|
|
||
| MachineFunctionPass *llvm::createInsertCodePrefetchPass() { | ||
| return new InsertCodePrefetch(); | ||
| } | ||
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.
What happens to internal linkage? Can you make this symbol a global, that would conflict.
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.
Right. We are relying on funique as mentioned in the RFC. Should we add a check here to ensure funique has been used (it's possible that funique is not applied on some functions).