-
Notifications
You must be signed in to change notification settings - Fork 15.2k
Adding Matching and Inference Functionality to Propeller-PR4: Implement matching and inference and create clusters #165868
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
Conversation
|
@llvm/pr-subscribers-pgo @llvm/pr-subscribers-backend-x86 Author: None (wdx727) ChangesAdding Matching and Inference Functionality to Propeller. For detailed information, please refer to the following RFC: https://discourse.llvm.org/t/rfc-adding-matching-and-inference-functionality-to-propeller/86238. co-authors: lifengxiang1025 [[email protected]](mailto:[email protected]); zcfh [[email protected]](mailto:[email protected]) Patch is 36.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/165868.diff 14 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/BasicBlockMatchingAndInference.h b/llvm/include/llvm/CodeGen/BasicBlockMatchingAndInference.h
new file mode 100644
index 0000000000000..6e9bbb969a445
--- /dev/null
+++ b/llvm/include/llvm/CodeGen/BasicBlockMatchingAndInference.h
@@ -0,0 +1,62 @@
+//===- llvm/CodeGen/BasicBlockMatchingAndInference.h ------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+// Infer weights for all basic blocks using matching and inference.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CODEGEN_BASIC_BLOCK_AND_INFERENCE_H
+#define LLVM_CODEGEN_BASIC_BLOCK_AND_INFERENCE_H
+
+#include "llvm/CodeGen/BasicBlockSectionsProfileReader.h"
+#include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/Transforms/Utils/SampleProfileInference.h"
+
+namespace llvm {
+
+class BasicBlockMatchingAndInference : public MachineFunctionPass {
+private:
+ using Edge = std::pair<const MachineBasicBlock *, const MachineBasicBlock *>;
+ using BlockWeightMap = DenseMap<const MachineBasicBlock *, uint64_t>;
+ using EdgeWeightMap = DenseMap<Edge, uint64_t>;
+ using BlockEdgeMap = DenseMap<const MachineBasicBlock *,
+ SmallVector<const MachineBasicBlock *, 8>>;
+
+ struct WeightInfo {
+ // Weight of basic blocks.
+ BlockWeightMap BlockWeights;
+ // Weight of edges.
+ EdgeWeightMap EdgeWeights;
+ };
+
+public:
+ static char ID;
+ BasicBlockMatchingAndInference();
+
+ StringRef getPassName() const override {
+ return "Basic Block Matching and Inference";
+ }
+
+ void getAnalysisUsage(AnalysisUsage &AU) const override;
+
+ bool runOnMachineFunction(MachineFunction &F) override;
+
+ std::optional<WeightInfo> getWeightInfo(StringRef FuncName) const;
+
+private:
+ StringMap<WeightInfo> ProgramWeightInfo;
+
+ WeightInfo initWeightInfoByMatching(MachineFunction &MF);
+
+ void generateWeightInfoByInference(MachineFunction &MF,
+ WeightInfo &MatchWeight);
+};
+
+} // end namespace llvm
+
+#endif // LLVM_CODEGEN_BASIC_BLOCK_AND_INFERENCE_H
diff --git a/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h b/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
index 823753021ff74..e8422e22aca0e 100644
--- a/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
+++ b/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
@@ -90,6 +90,10 @@ class BasicBlockSectionsProfileReader {
uint64_t getEdgeCount(StringRef FuncName, const UniqueBBID &SrcBBID,
const UniqueBBID &SinkBBID) const;
+ // Return the complete function path and cluster info for the given function.
+ std::pair<bool, FunctionPathAndClusterInfo>
+ getFunctionPathAndClusterInfo(StringRef FuncName) const;
+
private:
StringRef getAliasName(StringRef FuncName) const {
auto R = FuncAliasMap.find(FuncName);
@@ -199,6 +203,9 @@ class BasicBlockSectionsProfileReaderWrapperPass : public ImmutablePass {
uint64_t getEdgeCount(StringRef FuncName, const UniqueBBID &SrcBBID,
const UniqueBBID &DestBBID) const;
+ std::pair<bool, FunctionPathAndClusterInfo>
+ getFunctionPathAndClusterInfo(StringRef FuncName) const;
+
// Initializes the FunctionNameToDIFilename map for the current module and
// then reads the profile for the matching functions.
bool doInitialization(Module &M) override;
diff --git a/llvm/include/llvm/CodeGen/MachineBlockHashInfo.h b/llvm/include/llvm/CodeGen/MachineBlockHashInfo.h
index d044d5f940b75..3a4027aea3853 100644
--- a/llvm/include/llvm/CodeGen/MachineBlockHashInfo.h
+++ b/llvm/include/llvm/CodeGen/MachineBlockHashInfo.h
@@ -21,50 +21,55 @@ namespace llvm {
/// (blended) hash is represented and stored as one uint64_t, while individual
/// components are of smaller size (e.g., uint16_t or uint8_t).
struct BlendedBlockHash {
+private:
+ static uint64_t combineHashes(uint16_t Hash1, uint16_t Hash2, uint16_t Hash3,
+ uint16_t Hash4) {
+ uint64_t Hash = 0;
+
+ Hash |= uint64_t(Hash4);
+ Hash <<= 16;
+
+ Hash |= uint64_t(Hash3);
+ Hash <<= 16;
+
+ Hash |= uint64_t(Hash2);
+ Hash <<= 16;
+
+ Hash |= uint64_t(Hash1);
+
+ return Hash;
+ }
+
+ static void parseHashes(uint64_t Hash, uint16_t &Hash1, uint16_t &Hash2,
+ uint16_t &Hash3, uint16_t &Hash4) {
+ Hash1 = Hash & 0xffff;
+ Hash >>= 16;
+
+ Hash2 = Hash & 0xffff;
+ Hash >>= 16;
+
+ Hash3 = Hash & 0xffff;
+ Hash >>= 16;
+
+ Hash4 = Hash & 0xffff;
+ Hash >>= 16;
+ }
+
public:
- explicit BlendedBlockHash(uint16_t Offset, uint16_t OpcodeHash,
- uint16_t InstrHash, uint16_t NeighborHash)
- : Offset(Offset), OpcodeHash(OpcodeHash), InstrHash(InstrHash),
- NeighborHash(NeighborHash) {}
+ explicit BlendedBlockHash() {}
explicit BlendedBlockHash(uint64_t CombinedHash) {
- Offset = CombinedHash & 0xffff;
- CombinedHash >>= 16;
- OpcodeHash = CombinedHash & 0xffff;
- CombinedHash >>= 16;
- InstrHash = CombinedHash & 0xffff;
- CombinedHash >>= 16;
- NeighborHash = CombinedHash & 0xffff;
+ parseHashes(CombinedHash, Offset, OpcodeHash, InstrHash, NeighborHash);
}
/// Combine the blended hash into uint64_t.
uint64_t combine() const {
- uint64_t Hash = 0;
- Hash |= uint64_t(NeighborHash);
- Hash <<= 16;
- Hash |= uint64_t(InstrHash);
- Hash <<= 16;
- Hash |= uint64_t(OpcodeHash);
- Hash <<= 16;
- Hash |= uint64_t(Offset);
- return Hash;
+ return combineHashes(Offset, OpcodeHash, InstrHash, NeighborHash);
}
/// Compute a distance between two given blended hashes. The smaller the
/// distance, the more similar two blocks are. For identical basic blocks,
/// the distance is zero.
- /// Since OpcodeHash is highly stable, we consider a match good only if
- /// the OpcodeHashes are identical. Mismatched OpcodeHashes lead to low
- /// matching accuracy, and poor matches undermine the quality of final
- /// inference. Notably, during inference, we also consider the matching
- /// ratio of basic blocks. For MachineFunctions with a low matching
- /// ratio, we directly skip optimization to reduce the impact of
- /// mismatches. This ensures even very poor profiles won’t cause negative
- /// optimization.
- /// In the context of matching, we consider NeighborHash to be more
- /// important. This is especially true when accounting for inlining
- /// scenarios, where the position of a basic block in the control
- /// flow graph is more critical.
uint64_t distance(const BlendedBlockHash &BBH) const {
assert(OpcodeHash == BBH.OpcodeHash &&
"incorrect blended hash distance computation");
@@ -80,21 +85,20 @@ struct BlendedBlockHash {
return Dist;
}
-private:
/// The offset of the basic block from the function start.
uint16_t Offset{0};
- /// Hash of the basic block instructions, excluding operands.
+ /// (Loose) Hash of the basic block instructions, excluding operands.
uint16_t OpcodeHash{0};
- /// Hash of the basic block instructions, including opcodes and
+ /// (Strong) Hash of the basic block instructions, including opcodes and
/// operands.
uint16_t InstrHash{0};
- /// OpcodeHash of the basic block together with OpcodeHashes of its
+ /// Hash of the (loose) basic block together with (loose) hashes of its
/// successors and predecessors.
uint16_t NeighborHash{0};
};
class MachineBlockHashInfo : public MachineFunctionPass {
- DenseMap<const MachineBasicBlock *, uint64_t> MBBHashInfo;
+ DenseMap<unsigned, uint64_t> MBBHashInfo;
public:
static char ID;
diff --git a/llvm/include/llvm/CodeGen/Passes.h b/llvm/include/llvm/CodeGen/Passes.h
index a8525554b142e..2bf83cfa655b6 100644
--- a/llvm/include/llvm/CodeGen/Passes.h
+++ b/llvm/include/llvm/CodeGen/Passes.h
@@ -69,6 +69,10 @@ LLVM_ABI MachineFunctionPass *createBasicBlockSectionsPass();
LLVM_ABI MachineFunctionPass *createBasicBlockPathCloningPass();
+/// createBasicBlockMatchingAndInferencePass - This pass enables matching
+/// and inference when using propeller.
+LLVM_ABI MachineFunctionPass *createBasicBlockMatchingAndInferencePass();
+
/// createMachineBlockHashInfoPass - This pass computes basic block hashes.
LLVM_ABI MachineFunctionPass *createMachineBlockHashInfoPass();
diff --git a/llvm/include/llvm/InitializePasses.h b/llvm/include/llvm/InitializePasses.h
index 581b4ad161daa..9360550875219 100644
--- a/llvm/include/llvm/InitializePasses.h
+++ b/llvm/include/llvm/InitializePasses.h
@@ -55,6 +55,7 @@ LLVM_ABI void initializeAlwaysInlinerLegacyPassPass(PassRegistry &);
LLVM_ABI void initializeAssignmentTrackingAnalysisPass(PassRegistry &);
LLVM_ABI void initializeAssumptionCacheTrackerPass(PassRegistry &);
LLVM_ABI void initializeAtomicExpandLegacyPass(PassRegistry &);
+LLVM_ABI void initializeBasicBlockMatchingAndInferencePass(PassRegistry &);
LLVM_ABI void initializeBasicBlockPathCloningPass(PassRegistry &);
LLVM_ABI void
initializeBasicBlockSectionsProfileReaderWrapperPassPass(PassRegistry &);
diff --git a/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h b/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
index 7231e45fe8eb7..e1663d29c1e3c 100644
--- a/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
+++ b/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
@@ -130,6 +130,11 @@ template <typename FT> class SampleProfileInference {
SampleProfileInference(FunctionT &F, BlockEdgeMap &Successors,
BlockWeightMap &SampleBlockWeights)
: F(F), Successors(Successors), SampleBlockWeights(SampleBlockWeights) {}
+ SampleProfileInference(FunctionT &F, BlockEdgeMap &Successors,
+ BlockWeightMap &SampleBlockWeights,
+ EdgeWeightMap &SampleEdgeWeights)
+ : F(F), Successors(Successors), SampleBlockWeights(SampleBlockWeights),
+ SampleEdgeWeights(SampleEdgeWeights) {}
/// Apply the profile inference algorithm for a given function
void apply(BlockWeightMap &BlockWeights, EdgeWeightMap &EdgeWeights);
@@ -157,6 +162,9 @@ template <typename FT> class SampleProfileInference {
/// Map basic blocks to their sampled weights.
BlockWeightMap &SampleBlockWeights;
+
+ /// Map edges to their sampled weights.
+ EdgeWeightMap SampleEdgeWeights;
};
template <typename BT>
@@ -266,6 +274,14 @@ FlowFunction SampleProfileInference<BT>::createFlowFunction(
FlowJump Jump;
Jump.Source = BlockIndex[BB];
Jump.Target = BlockIndex[Succ];
+ auto It = SampleEdgeWeights.find(std::make_pair(BB, Succ));
+ if (It != SampleEdgeWeights.end()) {
+ Jump.HasUnknownWeight = false;
+ Jump.Weight = It->second;
+ } else {
+ Jump.HasUnknownWeight = true;
+ Jump.Weight = 0;
+ }
Func.Jumps.push_back(Jump);
}
}
diff --git a/llvm/lib/CodeGen/BasicBlockMatchingAndInference.cpp b/llvm/lib/CodeGen/BasicBlockMatchingAndInference.cpp
new file mode 100644
index 0000000000000..85a9cfde16508
--- /dev/null
+++ b/llvm/lib/CodeGen/BasicBlockMatchingAndInference.cpp
@@ -0,0 +1,187 @@
+//===- llvm/CodeGen/BasicBlockMatchingAndInference.cpp ----------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+// Infer weights for all basic blocks using matching and inference.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/CodeGen/BasicBlockMatchingAndInference.h"
+#include "llvm/CodeGen/BasicBlockSectionsProfileReader.h"
+#include "llvm/CodeGen/MachineBlockHashInfo.h"
+#include "llvm/CodeGen/Passes.h"
+#include "llvm/InitializePasses.h"
+#include <llvm/Support/CommandLine.h>
+
+using namespace llvm;
+
+static cl::opt<float>
+ PropellerInferThreshold("propeller-infer-threshold",
+ cl::desc("Threshold for infer stale profile"),
+ cl::init(0.6), cl::Optional);
+
+/// The object is used to identify and match basic blocks given their hashes.
+class StaleMatcher {
+public:
+ /// Initialize stale matcher.
+ void init(const std::vector<MachineBasicBlock *> &Blocks,
+ const std::vector<BlendedBlockHash> &Hashes) {
+ assert(Blocks.size() == Hashes.size() &&
+ "incorrect matcher initialization");
+ for (size_t I = 0; I < Blocks.size(); I++) {
+ MachineBasicBlock *Block = Blocks[I];
+ uint16_t OpHash = Hashes[I].OpcodeHash;
+ OpHashToBlocks[OpHash].push_back(std::make_pair(Hashes[I], Block));
+ }
+ }
+
+ /// Find the most similar block for a given hash.
+ MachineBasicBlock *matchBlock(BlendedBlockHash BlendedHash) const {
+ auto BlockIt = OpHashToBlocks.find(BlendedHash.OpcodeHash);
+ if (BlockIt == OpHashToBlocks.end()) {
+ return nullptr;
+ }
+ MachineBasicBlock *BestBlock = nullptr;
+ uint64_t BestDist = std::numeric_limits<uint64_t>::max();
+ for (auto It : BlockIt->second) {
+ MachineBasicBlock *Block = It.second;
+ BlendedBlockHash Hash = It.first;
+ uint64_t Dist = Hash.distance(BlendedHash);
+ if (BestBlock == nullptr || Dist < BestDist) {
+ BestDist = Dist;
+ BestBlock = Block;
+ }
+ }
+ return BestBlock;
+ }
+
+private:
+ using HashBlockPairType = std::pair<BlendedBlockHash, MachineBasicBlock *>;
+ std::unordered_map<uint16_t, std::vector<HashBlockPairType>> OpHashToBlocks;
+};
+
+INITIALIZE_PASS_BEGIN(BasicBlockMatchingAndInference,
+ "machine-block-match-infer",
+ "Machine Block Matching and Inference Analysis", true,
+ true)
+INITIALIZE_PASS_DEPENDENCY(MachineBlockHashInfo)
+INITIALIZE_PASS_DEPENDENCY(BasicBlockSectionsProfileReaderWrapperPass)
+INITIALIZE_PASS_END(BasicBlockMatchingAndInference, "machine-block-match-infer",
+ "Machine Block Matching and Inference Analysis", true, true)
+
+char BasicBlockMatchingAndInference::ID = 0;
+
+BasicBlockMatchingAndInference::BasicBlockMatchingAndInference()
+ : MachineFunctionPass(ID) {
+ initializeBasicBlockMatchingAndInferencePass(
+ *PassRegistry::getPassRegistry());
+}
+
+void BasicBlockMatchingAndInference::getAnalysisUsage(AnalysisUsage &AU) const {
+ AU.addRequired<MachineBlockHashInfo>();
+ AU.addRequired<BasicBlockSectionsProfileReaderWrapperPass>();
+ AU.setPreservesAll();
+ MachineFunctionPass::getAnalysisUsage(AU);
+}
+
+std::optional<BasicBlockMatchingAndInference::WeightInfo>
+BasicBlockMatchingAndInference::getWeightInfo(StringRef FuncName) const {
+ auto It = ProgramWeightInfo.find(FuncName);
+ if (It == ProgramWeightInfo.end()) {
+ return std::nullopt;
+ }
+ return It->second;
+}
+
+BasicBlockMatchingAndInference::WeightInfo
+BasicBlockMatchingAndInference::initWeightInfoByMatching(MachineFunction &MF) {
+ std::vector<MachineBasicBlock *> Blocks;
+ std::vector<BlendedBlockHash> Hashes;
+ auto BSPR = &getAnalysis<BasicBlockSectionsProfileReaderWrapperPass>();
+ auto MBHI = &getAnalysis<MachineBlockHashInfo>();
+ for (auto &Block : MF) {
+ Blocks.push_back(&Block);
+ Hashes.push_back(BlendedBlockHash(MBHI->getMBBHash(Block)));
+ }
+ StaleMatcher Matcher;
+ Matcher.init(Blocks, Hashes);
+ BasicBlockMatchingAndInference::WeightInfo MatchWeight;
+ auto [Flag, PathAndClusterInfo] =
+ BSPR->getFunctionPathAndClusterInfo(MF.getName());
+ if (!Flag)
+ return MatchWeight;
+ for (auto &BlockCount : PathAndClusterInfo.NodeCounts) {
+ if (PathAndClusterInfo.BBHashes.count(BlockCount.first.BaseID)) {
+ auto Hash = PathAndClusterInfo.BBHashes[BlockCount.first.BaseID];
+ MachineBasicBlock *Block = Matcher.matchBlock(BlendedBlockHash(Hash));
+ // When a basic block has clone copies, sum their counts.
+ if (Block != nullptr)
+ MatchWeight.BlockWeights[Block] += BlockCount.second;
+ }
+ }
+ for (auto &PredItem : PathAndClusterInfo.EdgeCounts) {
+ auto PredID = PredItem.first.BaseID;
+ if (!PathAndClusterInfo.BBHashes.count(PredID))
+ continue;
+ auto PredHash = PathAndClusterInfo.BBHashes[PredID];
+ MachineBasicBlock *PredBlock =
+ Matcher.matchBlock(BlendedBlockHash(PredHash));
+ if (PredBlock == nullptr)
+ continue;
+ for (auto &SuccItem : PredItem.second) {
+ auto SuccID = SuccItem.first.BaseID;
+ auto EdgeWeight = SuccItem.second;
+ if (PathAndClusterInfo.BBHashes.count(SuccID)) {
+ auto SuccHash = PathAndClusterInfo.BBHashes[SuccID];
+ MachineBasicBlock *SuccBlock =
+ Matcher.matchBlock(BlendedBlockHash(SuccHash));
+ // When an edge has clone copies, sum their counts.
+ if (SuccBlock != nullptr)
+ MatchWeight.EdgeWeights[std::make_pair(PredBlock, SuccBlock)] +=
+ EdgeWeight;
+ }
+ }
+ }
+ return MatchWeight;
+}
+
+void BasicBlockMatchingAndInference::generateWeightInfoByInference(
+ MachineFunction &MF,
+ BasicBlockMatchingAndInference::WeightInfo &MatchWeight) {
+ BlockEdgeMap Successors;
+ for (auto &Block : MF) {
+ for (auto *Succ : Block.successors())
+ Successors[&Block].push_back(Succ);
+ }
+ SampleProfileInference<MachineFunction> SPI(
+ MF, Successors, MatchWeight.BlockWeights, MatchWeight.EdgeWeights);
+ BlockWeightMap BlockWeights;
+ EdgeWeightMap EdgeWeights;
+ SPI.apply(BlockWeights, EdgeWeights);
+ ProgramWeightInfo.try_emplace(
+ MF.getName(), BasicBlockMatchingAndInference::WeightInfo{
+ std::move(BlockWeights), std::move(EdgeWeights)});
+}
+
+bool BasicBlockMatchingAndInference::runOnMachineFunction(MachineFunction &MF) {
+ if (MF.empty())
+ return false;
+ auto MatchWeight = initWeightInfoByMatching(MF);
+ // If the ratio of the number of MBBs in matching to the total number of MBBs
+ // in the function is less than the threshold value, the processing should be
+ // abandoned.
+ if (static_cast<float>(MatchWeight.BlockWeights.size()) / MF.size() <
+ PropellerInferThreshold) {
+ return false;
+ }
+ generateWeightInfoByInference(MF, MatchWeight);
+ return false;
+}
+
+MachineFunctionPass *llvm::createBasicBlockMatchingAndInferencePass() {
+ return new BasicBlockMatchingAndInference();
+}
diff --git a/llvm/lib/CodeGen/BasicBlockSections.cpp b/llvm/lib/CodeGen/BasicBlockSections.cpp
index e317e1c06741f..a86ac6d6eab23 100644
--- a/llvm/lib/CodeGen/BasicBlockSections.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSections.cpp
@@ -70,6 +70,7 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/CodeGen/BasicBlockMatchingAndInference.h"
#include "llvm/CodeGen/BasicBlockSectionUtils.h"
#include "llvm/CodeGen/BasicBlockSectionsProfileReader.h"
#include "llvm/CodeGen/MachineDominators.h"
@@ -81,6 +82,7 @@
#include "llvm/InitializePasses.h"
#include "llvm/Support/UniqueBBID.h"
#include "llvm/Target/TargetMachine.h"
+#include "llvm/Transforms/Utils/CodeLayout.h"
#include <optional>
using namespace llvm;
@@ -175,6 +177,77 @@ updateBranches(MachineFunction &MF,
}
}
+// This function generates the machine basic block clusters of "hot" blocks.
+// Currently, only support one cluster creation.
+// TODO: Support multi-cluster creation and path cloning.
+static std::pair<bool, SmallVector<BBClusterInfo>>
+createBBClusterInfoForFunction(const MachineFunction &MF,
+ BasicBlockMatchingAndInference *BMI) {
+ unsigned CurrentCluster = 0;
+ auto OptWeightInfo = BMI->getWeightInfo(MF.getName());
+ if (!OptWeightInfo)
+ return std::pair(false, SmallVector<BBClusterInfo>{});
+ auto BlockWeights = OptWeightInfo->BlockWeights;
+ auto EdgeWeights = OptWeightInfo->EdgeWeights;
+
+ SmallVector<const MachineBasicBlock *, 4> HotMBBs;
+ if (MF.size() <= 2) {
+ for (auto &MBB : MF) {
+ if (MBB.isEntryBlock() ...
[truncated]
|
813b362 to
a0b8804
Compare
| using namespace llvm; | ||
|
|
||
| static cl::opt<float> | ||
| PropellerInferThreshold("propeller-infer-threshold", |
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.
Why is this in llvm/lib/CodeGen? Is this implementation tied to the Propeller use case, or a general one?
I'm confused because we have a very similar code for BOLT in bolt/lib/Profile/StaleProfileMatching.cpp.
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.
We currently plan to reserve this file exclusively for Propeller. A better approach would be to integrate the implementations here with those in BOLT into the LLVM infrastructure, enabling reuse by both Propeller and BOLT. For now, we aim to first add matching and inference support to Propeller, with the code integration work scheduled for a separate PR in the future.
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.
Should this be indicated in the filename or comments?
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.
We have added a TODO item in the comments of this file.
| using namespace llvm; | ||
|
|
||
| static cl::opt<float> | ||
| PropellerInferThreshold("propeller-infer-threshold", |
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.
Should this be indicated in the filename or comments?
| StaleMatcher Matcher; | ||
| Matcher.init(Blocks, Hashes); | ||
| BasicBlockMatchingAndInference::WeightInfo MatchWeight; | ||
| auto [Flag, PathAndClusterInfo] = |
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.
It's unclear what this Flag is. Can you find a more descriptive name?
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.
Done.
| // | ||
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // Infer weights for all basic blocks using matching and inference. |
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.
I didn't check your earlier commits but is there somewhere an extend comments, explaining what this pass is about? I don't think this one sentence is sufficient
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.
Done. We have added a more detailed functional description in the comments of this file.
| CostInc = Params.CostJumpUnknownInc; | ||
| CostDec = 0; | ||
| } else { | ||
| assert(Jump.Weight > 0 && "found zero-weight jump with a positive weight"); |
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.
Why removing this?
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.
In Propeller's profile, edges with a weight of 0 have a clear meaning: they indicate the edge is "cold". Therefore, if an edge is matched during the matching process and has a weight of 0 in the Propeller profile, we will set HasUnknownWeight for this edge to false and its weight to 0. The validation here would cause this scenario to fail.
| for (auto *Succ : Block.successors()) | ||
| Successors[&Block].push_back(Succ); | ||
| } | ||
| SampleProfileInference<MachineFunction> SPI( |
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.
iirc SampleProfileInference has a bunch of parameters that might affect the quality of the inference. In BOLT we override those to get better results.
I assume default values work well for your use case?
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 default values work well for our use case. The experimental results can be found in our RFC (https://discourse.llvm.org/t/rfc-adding-matching-and-inference-functionality-to-propeller/86238/9), so we directly use the default parameters of the inference algorithm in LLVM here.
a0b8804 to
50d9c6a
Compare
spupyrev
left a comment
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.
looks good to me but maybe let's wait for other reviews?
| /// Enable matching and inference when using propeller. | ||
| static cl::opt<bool> | ||
| PropellerMatchInfer("propeller-match-infer", | ||
| cl::desc("Use match&infer to evaluate stale profile"), |
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.
The description ("...evaluate...") doesn't match the comment ("enable"); adjust maybe?
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.
Done.
50d9c6a to
072a672
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
072a672 to
274e0fb
Compare
Okay, appreciate your review! |
|
@spupyrev My permission is still pending. Could you help merge this PR? |
7b8dfba to
11405ae
Compare
…nt matching and inference and create clusters. Co-authored-by: lifengxiang1025 <[email protected]> Co-authored-by: zcfh <[email protected]>
11405ae to
4781fcd
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/163/builds/29681 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/39877 Here is the relevant piece of the build log for the reference |
|
@wdx727 please let me know what you think about the build failures |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/17102 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/8/builds/23397 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/206/builds/8813 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/30471 Here is the relevant piece of the build log for the reference |
|
Hi! This PR breaks several bots. If it might take longer for investigation, could we please revert it to unblock bots? Thx! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/18048 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/205/builds/27567 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/117/builds/14977 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/203/builds/28776 Here is the relevant piece of the build log for the reference |
Revert: #167559 |
… Implement matching and inference and create clusters" (#167559) Reverts #165868 due to buildbot failures Co-authored-by: spupyrev <[email protected]>
…peller-PR4: Implement matching and inference and create clusters" (#167559) Reverts llvm/llvm-project#165868 due to buildbot failures Co-authored-by: spupyrev <[email protected]>
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/113/builds/9480 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/119/builds/7284 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/151/builds/7376 Here is the relevant piece of the build log for the reference |
Looking at the error, it's caused by a return type mismatch, and we'll fix it. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/95/builds/19075 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/76/builds/13373 Here is the relevant piece of the build log for the reference |
Adding Matching and Inference Functionality to Propeller. For detailed information, please refer to the following RFC: https://discourse.llvm.org/t/rfc-adding-matching-and-inference-functionality-to-propeller/86238.
This is the fourth PR, which is used to implement matching and inference and create the clusters. The associated PRs are:
PR1: #160706
PR2: #162963
PR3: #164223
co-authors: lifengxiang1025 [email protected]; zcfh [email protected]