https://github.com/shawbyoung updated https://github.com/llvm/llvm-project/pull/99891
>From 0274f697376264c2d77816190f9a434f64e79089 Mon Sep 17 00:00:00 2001 From: shawbyoung <shawbyo...@gmail.com> Date: Mon, 22 Jul 2024 11:56:23 -0700 Subject: [PATCH 1/3] Changed assignment of profiles with pseudo probe index Created using spr 1.3.4 --- bolt/lib/Profile/StaleProfileMatching.cpp | 85 +++++++++++++++---- .../X86/match-blocks-with-pseudo-probes.test | 25 ++---- 2 files changed, 78 insertions(+), 32 deletions(-) diff --git a/bolt/lib/Profile/StaleProfileMatching.cpp b/bolt/lib/Profile/StaleProfileMatching.cpp index 4105f626fb5b6..c135ee5ff4837 100644 --- a/bolt/lib/Profile/StaleProfileMatching.cpp +++ b/bolt/lib/Profile/StaleProfileMatching.cpp @@ -195,11 +195,15 @@ class StaleMatcher { void init(const std::vector<FlowBlock *> &Blocks, const std::vector<BlendedBlockHash> &Hashes, const std::vector<uint64_t> &CallHashes, - std::optional<uint64_t> YamlBFGUID) { + const std::unordered_map<uint64_t, + std::vector<const MCDecodedPseudoProbe *>> + IndexToBinaryPseudoProbes, + const std::unordered_map<const MCDecodedPseudoProbe *, FlowBlock *> + BinaryPseudoProbeToBlock, + const uint64_t YamlBFGUID) { assert(Blocks.size() == Hashes.size() && Hashes.size() == CallHashes.size() && "incorrect matcher initialization"); - for (size_t I = 0; I < Blocks.size(); I++) { FlowBlock *Block = Blocks[I]; uint16_t OpHash = Hashes[I].OpcodeHash; @@ -209,6 +213,8 @@ class StaleMatcher { std::make_pair(Hashes[I], Block)); this->Blocks.push_back(Block); } + this->IndexToBinaryPseudoProbes = IndexToBinaryPseudoProbes; + this->BinaryPseudoProbeToBlock = BinaryPseudoProbeToBlock; this->YamlBFGUID = YamlBFGUID; } @@ -234,10 +240,14 @@ class StaleMatcher { using HashBlockPairType = std::pair<BlendedBlockHash, FlowBlock *>; std::unordered_map<uint16_t, std::vector<HashBlockPairType>> OpHashToBlocks; std::unordered_map<uint64_t, std::vector<HashBlockPairType>> CallHashToBlocks; - std::vector<FlowBlock *> Blocks; + std::unordered_map<uint64_t, std::vector<const MCDecodedPseudoProbe *>> + IndexToBinaryPseudoProbes; + std::unordered_map<const MCDecodedPseudoProbe *, FlowBlock *> + BinaryPseudoProbeToBlock; + std::vector<const FlowBlock *> Blocks; // If the pseudo probe checksums of the profiled and binary functions are // equal, then the YamlBF's GUID is defined and used to match blocks. - std::optional<uint64_t> YamlBFGUID; + uint64_t YamlBFGUID; // Uses OpcodeHash to find the most similar block for a given hash. const FlowBlock *matchWithOpcodes(BlendedBlockHash BlendedHash) const { @@ -284,7 +294,7 @@ class StaleMatcher { // Searches for the pseudo probe attached to the matched function's block, // ignoring pseudo probes attached to function calls and inlined functions' // blocks. - outs() << "match with pseudo probes\n"; + std::vector<const yaml::bolt::PseudoProbeInfo *> BlockPseudoProbes; for (const auto &PseudoProbe : PseudoProbes) { // Ensures that pseudo probe information belongs to the appropriate // function and not an inlined function. @@ -293,11 +303,30 @@ class StaleMatcher { // Skips pseudo probes attached to function calls. if (PseudoProbe.Type != static_cast<uint8_t>(PseudoProbeType::Block)) continue; - assert(PseudoProbe.Index < Blocks.size() && - "pseudo probe index out of range"); - return Blocks[PseudoProbe.Index]; + + BlockPseudoProbes.push_back(&PseudoProbe); } - return nullptr; + + // Returns nullptr if there is not a 1:1 mapping of the yaml block pseudo + // probe and binary pseudo probe. + if (BlockPseudoProbes.size() == 0 || BlockPseudoProbes.size() > 1) + return nullptr; + + uint64_t Index = BlockPseudoProbes[0]->Index; + assert(Index < Blocks.size() && "Invalid pseudo probe index"); + + auto It = IndexToBinaryPseudoProbes.find(Index); + assert(It != IndexToBinaryPseudoProbes.end() && + "All blocks should have a pseudo probe"); + if (It->second.size() > 1) + return nullptr; + + const MCDecodedPseudoProbe *BinaryPseudoProbe = It->second[0]; + auto BinaryPseudoProbeIt = BinaryPseudoProbeToBlock.find(BinaryPseudoProbe); + assert(BinaryPseudoProbeIt != BinaryPseudoProbeToBlock.end() && + "All binary pseudo probes should belong a binary basic block"); + + return BinaryPseudoProbeIt->second; } }; @@ -491,6 +520,11 @@ size_t matchWeightsByHashes( std::vector<uint64_t> CallHashes; std::vector<FlowBlock *> Blocks; std::vector<BlendedBlockHash> BlendedHashes; + std::unordered_map<uint64_t, std::vector<const MCDecodedPseudoProbe *>> + IndexToBinaryPseudoProbes; + std::unordered_map<const MCDecodedPseudoProbe *, FlowBlock *> + BinaryPseudoProbeToBlock; + const MCPseudoProbeDecoder *PseudoProbeDecoder = BC.getPseudoProbeDecoder(); for (uint64_t I = 0; I < BlockOrder.size(); I++) { const BinaryBasicBlock *BB = BlockOrder[I]; assert(BB->getHash() != 0 && "empty hash of BinaryBasicBlock"); @@ -510,9 +544,27 @@ size_t matchWeightsByHashes( Blocks.push_back(&Func.Blocks[I + 1]); BlendedBlockHash BlendedHash(BB->getHash()); BlendedHashes.push_back(BlendedHash); + if (PseudoProbeDecoder) { + const AddressProbesMap &ProbeMap = + PseudoProbeDecoder->getAddress2ProbesMap(); + const uint64_t FuncAddr = BF.getAddress(); + const std::pair<uint64_t, uint64_t> &BlockRange = + BB->getInputAddressRange(); + const auto &BlockProbes = + llvm::make_range(ProbeMap.lower_bound(FuncAddr + BlockRange.first), + ProbeMap.lower_bound(FuncAddr + BlockRange.second)); + for (const auto &[_, Probes] : BlockProbes) { + for (const MCDecodedPseudoProbe &Probe : Probes) { + IndexToBinaryPseudoProbes[Probe.getIndex()].push_back(&Probe); + BinaryPseudoProbeToBlock[&Probe] = Blocks[I]; + } + } + } + LLVM_DEBUG(dbgs() << "BB with index " << I << " has hash = " << Twine::utohexstr(BB->getHash()) << "\n"); } + uint64_t BFPseudoProbeDescHash = 0; if (BF.hasPseudoProbe()) { const MCPseudoProbeDecoder *PseudoProbeDecoder = BC.getPseudoProbeDecoder(); @@ -521,14 +573,15 @@ size_t matchWeightsByHashes( BFPseudoProbeDescHash = PseudoProbeDecoder->getFuncDescForGUID(BF.getGUID())->FuncHash; } - bool MatchWithPseudoProbes = - BFPseudoProbeDescHash && YamlBF.PseudoProbeDescHash - ? BFPseudoProbeDescHash == YamlBF.PseudoProbeDescHash - : false; + uint64_t YamlBFGUID = + BFPseudoProbeDescHash && YamlBF.PseudoProbeDescHash && + BFPseudoProbeDescHash == YamlBF.PseudoProbeDescHash + ? static_cast<uint64_t>(YamlBF.GUID) + : 0; + StaleMatcher Matcher; - Matcher.init(Blocks, BlendedHashes, CallHashes, - MatchWithPseudoProbes ? std::make_optional(YamlBF.GUID) - : std::nullopt); + Matcher.init(Blocks, BlendedHashes, CallHashes, IndexToBinaryPseudoProbes, + BinaryPseudoProbeToBlock, YamlBFGUID); // Index in yaml profile => corresponding (matched) block DenseMap<uint64_t, const FlowBlock *> MatchedBlocks; diff --git a/bolt/test/X86/match-blocks-with-pseudo-probes.test b/bolt/test/X86/match-blocks-with-pseudo-probes.test index e0adb6948e206..1d74b92a11c56 100644 --- a/bolt/test/X86/match-blocks-with-pseudo-probes.test +++ b/bolt/test/X86/match-blocks-with-pseudo-probes.test @@ -5,7 +5,7 @@ # RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %t/main.s -o %t.o # RUN: %clang %cflags %t.o -o %t.exe -Wl,-q -nostdlib # RUN: llvm-bolt %t.exe -o %t.out --data %t/yaml -v=2 \ -# RUN: --print-cfg --funcs=main --profile-ignore-hash=0 2>&1 | FileCheck %s +# RUN: --print-cfg --funcs=main --profile-ignore-hash=0 --infer-stale-profile 2>&1 | FileCheck %s # CHECK: BOLT-INFO: matched 0 functions with similar names @@ -47,23 +47,16 @@ header: dfs-order: false hash-func: xxh3 functions: - - name: main - fid: 0 - hash: 0x0000000000000001 - exec: 1 - nblocks: 6 + - name: main + fid: 0 + hash: 0x0000000000000001 + exec: 1 + nblocks: 6 + guid: 0xDB956436E78DD5FA + pseudo_probe_desc_hash: 15822663052811949562 #lookup in code in a second blocks: - bid: 1 hash: 0x0000000000000001 insns: 1 succ: [ { bid: 3, cnt: 1} ] - - name: foo - fid: 1 - hash: 0x0000000000000002 - exec: 1 - nblocks: 6 - blocks: - - bid: 1 - hash: 0x0000000000000002 - insns: 1 - succ: [ { bid: 3, cnt: 1} ] + pseudo_probes: [ { guid: 0xDB956436E78DD5FA, id: 0, type: 0 } ] >From 7e3d8d6b171954836c858f0814befc54f70bd3aa Mon Sep 17 00:00:00 2001 From: shawbyoung <shawbyo...@gmail.com> Date: Mon, 22 Jul 2024 14:27:44 -0700 Subject: [PATCH 2/3] Edit test and assert Created using spr 1.3.4 --- bolt/lib/Profile/StaleProfileMatching.cpp | 2 +- bolt/test/X86/match-blocks-with-pseudo-probes.test | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bolt/lib/Profile/StaleProfileMatching.cpp b/bolt/lib/Profile/StaleProfileMatching.cpp index c135ee5ff4837..71e0579415fc6 100644 --- a/bolt/lib/Profile/StaleProfileMatching.cpp +++ b/bolt/lib/Profile/StaleProfileMatching.cpp @@ -313,7 +313,7 @@ class StaleMatcher { return nullptr; uint64_t Index = BlockPseudoProbes[0]->Index; - assert(Index < Blocks.size() && "Invalid pseudo probe index"); + assert(Index <= Blocks.size() && "Invalid pseudo probe index"); auto It = IndexToBinaryPseudoProbes.find(Index); assert(It != IndexToBinaryPseudoProbes.end() && diff --git a/bolt/test/X86/match-blocks-with-pseudo-probes.test b/bolt/test/X86/match-blocks-with-pseudo-probes.test index 1d74b92a11c56..6dc01eb492eae 100644 --- a/bolt/test/X86/match-blocks-with-pseudo-probes.test +++ b/bolt/test/X86/match-blocks-with-pseudo-probes.test @@ -53,10 +53,10 @@ functions: exec: 1 nblocks: 6 guid: 0xDB956436E78DD5FA - pseudo_probe_desc_hash: 15822663052811949562 #lookup in code in a second + pseudo_probe_desc_hash: 4294967295 #lookup in code in a second blocks: - bid: 1 hash: 0x0000000000000001 insns: 1 succ: [ { bid: 3, cnt: 1} ] - pseudo_probes: [ { guid: 0xDB956436E78DD5FA, id: 0, type: 0 } ] + pseudo_probes: [ { guid: 0xDB956436E78DD5FA, id: 1, type: 0 } ] >From 780a07ee5a4b2bc3f5bd6e33fb072d67d1113c89 Mon Sep 17 00:00:00 2001 From: shawbyoung <shawbyo...@gmail.com> Date: Tue, 23 Jul 2024 11:37:14 -0700 Subject: [PATCH 3/3] Fixed failing asserts, pruned prospective pseudo probes for matching Created using spr 1.3.4 --- bolt/lib/Profile/StaleProfileMatching.cpp | 56 ++++++++++++++++------- 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/bolt/lib/Profile/StaleProfileMatching.cpp b/bolt/lib/Profile/StaleProfileMatching.cpp index 71e0579415fc6..d45066ed66ef2 100644 --- a/bolt/lib/Profile/StaleProfileMatching.cpp +++ b/bolt/lib/Profile/StaleProfileMatching.cpp @@ -45,6 +45,7 @@ namespace opts { extern cl::opt<bool> TimeRewrite; extern cl::OptionCategory BoltOptCategory; +extern cl::opt<unsigned> Verbosity; cl::opt<bool> InferStaleProfile("infer-stale-profile", @@ -197,9 +198,9 @@ class StaleMatcher { const std::vector<uint64_t> &CallHashes, const std::unordered_map<uint64_t, std::vector<const MCDecodedPseudoProbe *>> - IndexToBinaryPseudoProbes, + &IndexToBinaryPseudoProbes, const std::unordered_map<const MCDecodedPseudoProbe *, FlowBlock *> - BinaryPseudoProbeToBlock, + &BinaryPseudoProbeToBlock, const uint64_t YamlBFGUID) { assert(Blocks.size() == Hashes.size() && Hashes.size() == CallHashes.size() && @@ -294,6 +295,9 @@ class StaleMatcher { // Searches for the pseudo probe attached to the matched function's block, // ignoring pseudo probes attached to function calls and inlined functions' // blocks. + if (opts::Verbosity >= 2) + outs() << "BOLT-INFO: attempting to match block with pseudo probes\n"; + std::vector<const yaml::bolt::PseudoProbeInfo *> BlockPseudoProbes; for (const auto &PseudoProbe : PseudoProbes) { // Ensures that pseudo probe information belongs to the appropriate @@ -306,26 +310,41 @@ class StaleMatcher { BlockPseudoProbes.push_back(&PseudoProbe); } - // Returns nullptr if there is not a 1:1 mapping of the yaml block pseudo // probe and binary pseudo probe. - if (BlockPseudoProbes.size() == 0 || BlockPseudoProbes.size() > 1) + if (BlockPseudoProbes.size() == 0) { + if (opts::Verbosity >= 2) + errs() << "BOLT-WARNING: no pseudo probes in profile block\n"; return nullptr; - + } + if (BlockPseudoProbes.size() > 1) { + if (opts::Verbosity >= 2) + errs() << "BOLT-WARNING: more than 1 pseudo probes in profile block\n"; + return nullptr; + } uint64_t Index = BlockPseudoProbes[0]->Index; - assert(Index <= Blocks.size() && "Invalid pseudo probe index"); - + if (Index > Blocks.size()) { + if (opts::Verbosity >= 2) + errs() << "BOLT-WARNING: invalid index block pseudo probe index\n"; + return nullptr; + } auto It = IndexToBinaryPseudoProbes.find(Index); - assert(It != IndexToBinaryPseudoProbes.end() && - "All blocks should have a pseudo probe"); - if (It->second.size() > 1) + if (It == IndexToBinaryPseudoProbes.end()) { + if (opts::Verbosity >= 2) + errs() << "BOLT-WARNING: no block pseudo probes found within binary " + "block at index\n"; return nullptr; - + } + if (It->second.size() > 1) { + if (opts::Verbosity >= 2) + errs() << "BOLT-WARNING: more than 1 block pseudo probes in binary " + "block at index\n"; + return nullptr; + } const MCDecodedPseudoProbe *BinaryPseudoProbe = It->second[0]; auto BinaryPseudoProbeIt = BinaryPseudoProbeToBlock.find(BinaryPseudoProbe); assert(BinaryPseudoProbeIt != BinaryPseudoProbeToBlock.end() && "All binary pseudo probes should belong a binary basic block"); - return BinaryPseudoProbeIt->second; } }; @@ -555,6 +574,10 @@ size_t matchWeightsByHashes( ProbeMap.lower_bound(FuncAddr + BlockRange.second)); for (const auto &[_, Probes] : BlockProbes) { for (const MCDecodedPseudoProbe &Probe : Probes) { + if (Probe.getInlineTreeNode()->hasInlineSite()) + continue; + if (Probe.getType() != static_cast<uint8_t>(PseudoProbeType::Block)) + continue; IndexToBinaryPseudoProbes[Probe.getIndex()].push_back(&Probe); BinaryPseudoProbeToBlock[&Probe] = Blocks[I]; } @@ -566,12 +589,13 @@ size_t matchWeightsByHashes( } uint64_t BFPseudoProbeDescHash = 0; - if (BF.hasPseudoProbe()) { - const MCPseudoProbeDecoder *PseudoProbeDecoder = BC.getPseudoProbeDecoder(); + if (BF.getGUID() != 0) { assert(PseudoProbeDecoder && "If BF has pseudo probe, BC should have a pseudo probe decoder"); - BFPseudoProbeDescHash = - PseudoProbeDecoder->getFuncDescForGUID(BF.getGUID())->FuncHash; + auto &GUID2FuncDescMap = PseudoProbeDecoder->getGUID2FuncDescMap(); + auto It = GUID2FuncDescMap.find(BF.getGUID()); + if (It != GUID2FuncDescMap.end()) + BFPseudoProbeDescHash = It->second.FuncHash; } uint64_t YamlBFGUID = BFPseudoProbeDescHash && YamlBF.PseudoProbeDescHash && _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits