-
Notifications
You must be signed in to change notification settings - Fork 12k
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
[BOLT][NFC] Make estimateEdgeCounts a BinaryFunctionPass #93074
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Eliminate the dependence of Profile on estimateEdgeCounts defined in Passes/MCF.
aaupov
requested review from
maksfb,
rafaelauler,
ayermolo and
dcci
as code owners
May 22, 2024 17:19
@llvm/pr-subscribers-bolt Author: Amir Ayupov (aaupov) ChangesEliminate the dependence of Profile on estimateEdgeCounts defined in Full diff: https://github.com/llvm/llvm-project/pull/93074.diff 5 Files Affected:
diff --git a/bolt/include/bolt/Passes/MCF.h b/bolt/include/bolt/Passes/MCF.h
index 4b87401498fa5..3fe674463bf13 100644
--- a/bolt/include/bolt/Passes/MCF.h
+++ b/bolt/include/bolt/Passes/MCF.h
@@ -9,10 +9,12 @@
#ifndef BOLT_PASSES_MCF_H
#define BOLT_PASSES_MCF_H
+#include "bolt/Passes/BinaryPasses.h"
+#include "llvm/Support/CommandLine.h"
+
namespace llvm {
namespace bolt {
-class BinaryFunction;
class DataflowInfoManager;
/// Implement the idea in "SamplePGO - The Power of Profile Guided Optimizations
@@ -23,7 +25,18 @@ void equalizeBBCounts(DataflowInfoManager &Info, BinaryFunction &BF);
/// Fill edge counts based on the basic block count. Used in nonLBR mode when
/// we only have bb count.
-void estimateEdgeCounts(BinaryFunction &BF);
+class EstimateEdgeCounts : public BinaryFunctionPass {
+ void runOnFunction(BinaryFunction &BF);
+
+public:
+ explicit EstimateEdgeCounts(const cl::opt<bool> &PrintPass)
+ : BinaryFunctionPass(PrintPass) {}
+
+ const char *getName() const override { return "estimate-edge-counts"; }
+
+ /// Pass entry point
+ Error runOnFunctions(BinaryContext &BC) override;
+};
} // end namespace bolt
} // end namespace llvm
diff --git a/bolt/lib/Passes/MCF.cpp b/bolt/lib/Passes/MCF.cpp
index b2723cd8dcb8b..77dea7369140e 100644
--- a/bolt/lib/Passes/MCF.cpp
+++ b/bolt/lib/Passes/MCF.cpp
@@ -12,9 +12,11 @@
#include "bolt/Passes/MCF.h"
#include "bolt/Core/BinaryFunction.h"
+#include "bolt/Core/ParallelUtilities.h"
#include "bolt/Passes/DataflowInfoManager.h"
#include "bolt/Utils/CommandLineOpts.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/CommandLine.h"
#include <algorithm>
#include <vector>
@@ -432,7 +434,7 @@ void equalizeBBCounts(DataflowInfoManager &Info, BinaryFunction &BF) {
}
}
-void estimateEdgeCounts(BinaryFunction &BF) {
+void EstimateEdgeCounts::runOnFunction(BinaryFunction &BF) {
EdgeWeightMap PredEdgeWeights;
EdgeWeightMap SuccEdgeWeights;
if (!opts::IterativeGuess) {
@@ -453,5 +455,25 @@ void estimateEdgeCounts(BinaryFunction &BF) {
recalculateBBCounts(BF, /*AllEdges=*/false);
}
+Error EstimateEdgeCounts::runOnFunctions(BinaryContext &BC) {
+ if (llvm::none_of(llvm::make_second_range(BC.getBinaryFunctions()),
+ [](const BinaryFunction &BF) {
+ return BF.getProfileFlags() == BinaryFunction::PF_SAMPLE;
+ }))
+ return Error::success();
+
+ ParallelUtilities::WorkFuncTy WorkFun = [&](BinaryFunction &BF) {
+ runOnFunction(BF);
+ };
+ ParallelUtilities::PredicateTy SkipFunc = [&](const BinaryFunction &BF) {
+ return BF.getProfileFlags() != BinaryFunction::PF_SAMPLE;
+ };
+
+ ParallelUtilities::runOnEachFunction(
+ BC, ParallelUtilities::SchedulingPolicy::SP_BB_QUADRATIC, WorkFun,
+ SkipFunc, "EstimateEdgeCounts");
+ return Error::success();
+}
+
} // namespace bolt
} // namespace llvm
diff --git a/bolt/lib/Profile/DataReader.cpp b/bolt/lib/Profile/DataReader.cpp
index 06c5e96b78064..f2e999bbfdc6d 100644
--- a/bolt/lib/Profile/DataReader.cpp
+++ b/bolt/lib/Profile/DataReader.cpp
@@ -598,8 +598,6 @@ void DataReader::readSampleData(BinaryFunction &BF) {
}
BF.ExecutionCount = TotalEntryCount;
-
- estimateEdgeCounts(BF);
}
void DataReader::convertBranchData(BinaryFunction &BF) const {
diff --git a/bolt/lib/Profile/YAMLProfileReader.cpp b/bolt/lib/Profile/YAMLProfileReader.cpp
index 29d94067f459f..aa38dda47eb56 100644
--- a/bolt/lib/Profile/YAMLProfileReader.cpp
+++ b/bolt/lib/Profile/YAMLProfileReader.cpp
@@ -253,10 +253,8 @@ bool YAMLProfileReader::parseFunctionProfile(
if (BB.getExecutionCount() == BinaryBasicBlock::COUNT_NO_PROFILE)
BB.setExecutionCount(0);
- if (YamlBP.Header.Flags & BinaryFunction::PF_SAMPLE) {
+ if (YamlBP.Header.Flags & BinaryFunction::PF_SAMPLE)
BF.setExecutionCount(FunctionExecutionCount);
- estimateEdgeCounts(BF);
- }
ProfileMatched &= !MismatchedBlocks && !MismatchedCalls && !MismatchedEdges;
diff --git a/bolt/lib/Rewrite/BinaryPassManager.cpp b/bolt/lib/Rewrite/BinaryPassManager.cpp
index cbb7199a53ddd..0712330a524b0 100644
--- a/bolt/lib/Rewrite/BinaryPassManager.cpp
+++ b/bolt/lib/Rewrite/BinaryPassManager.cpp
@@ -23,6 +23,7 @@
#include "bolt/Passes/JTFootprintReduction.h"
#include "bolt/Passes/LongJmp.h"
#include "bolt/Passes/LoopInversionPass.h"
+#include "bolt/Passes/MCF.h"
#include "bolt/Passes/PLTCall.h"
#include "bolt/Passes/PatchEntries.h"
#include "bolt/Passes/RegReAssign.h"
@@ -90,6 +91,11 @@ PrintAfterLowering("print-after-lowering",
cl::desc("print function after instruction lowering"),
cl::Hidden, cl::cat(BoltOptCategory));
+static cl::opt<bool> PrintEstimateEdgeCounts(
+ "print-estimate-edge-counts",
+ cl::desc("print function after edge counts are set for no-LBR profile"),
+ cl::Hidden, cl::cat(BoltOptCategory));
+
cl::opt<bool>
PrintFinalized("print-finalized",
cl::desc("print function after CFG is finalized"),
@@ -334,6 +340,9 @@ Error BinaryFunctionPassManager::runPasses() {
Error BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) {
BinaryFunctionPassManager Manager(BC);
+ Manager.registerPass(
+ std::make_unique<EstimateEdgeCounts>(PrintEstimateEdgeCounts));
+
const DynoStats InitialDynoStats =
getDynoStats(BC.getBinaryFunctions(), BC.isAArch64());
|
rafaelauler
approved these changes
May 22, 2024
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.
LGTM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Eliminate the dependence of Profile on estimateEdgeCounts defined in
Passes/MCF.