From 7ef36d8ad42c1d9a77fc61df553af38b64c7ee30 Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Tue, 3 Sep 2024 11:14:41 -0700 Subject: [PATCH 1/8] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20ch?= =?UTF-8?q?anges=20to=20main=20this=20commit=20is=20based=20on?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Created using spr 1.3.4 [skip ci] --- bolt/include/bolt/Utils/Utils.h | 5 ++ bolt/lib/Profile/DataAggregator.cpp | 4 +- bolt/lib/Profile/YAMLProfileReader.cpp | 5 -- bolt/lib/Profile/YAMLProfileWriter.cpp | 11 +++- bolt/lib/Rewrite/PseudoProbeRewriter.cpp | 63 ++++++++++++++----- bolt/lib/Utils/Utils.cpp | 12 +++- .../test/X86/pseudoprobe-decoding-inline.test | 6 +- 7 files changed, 75 insertions(+), 31 deletions(-) diff --git a/bolt/include/bolt/Utils/Utils.h b/bolt/include/bolt/Utils/Utils.h index 3886c5f8757c0..9baee7d94066d 100644 --- a/bolt/include/bolt/Utils/Utils.h +++ b/bolt/include/bolt/Utils/Utils.h @@ -41,6 +41,11 @@ std::string getEscapedName(const StringRef &Name); /// Return the unescaped name std::string getUnescapedName(const StringRef &Name); +/// Return a common part for a given \p Name wrt a given \p Suffixes list. +/// Preserve the suffix if \p KeepSuffix is set, only dropping characters +/// following it, otherwise drop the suffix as well. +std::optional getCommonName(const StringRef Name, bool KeepSuffix, + ArrayRef Suffixes); /// LTO-generated function names take a form: /// /// .lto_priv./... diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp index 813d825f8b570..10d745cc69824 100644 --- a/bolt/lib/Profile/DataAggregator.cpp +++ b/bolt/lib/Profile/DataAggregator.cpp @@ -88,7 +88,7 @@ MaxSamples("max-samples", cl::cat(AggregatorCategory)); extern cl::opt ProfileFormat; -extern cl::opt ProfileUsePseudoProbes; +extern cl::opt ProfileWritePseudoProbes; extern cl::opt SaveProfile; cl::opt ReadPreAggregated( @@ -2300,7 +2300,7 @@ std::error_code DataAggregator::writeBATYAML(BinaryContext &BC, yaml::bolt::BinaryProfile BP; const MCPseudoProbeDecoder *PseudoProbeDecoder = - opts::ProfileUsePseudoProbes ? BC.getPseudoProbeDecoder() : nullptr; + opts::ProfileWritePseudoProbes ? BC.getPseudoProbeDecoder() : nullptr; // Fill out the header info. BP.Header.Version = 1; diff --git a/bolt/lib/Profile/YAMLProfileReader.cpp b/bolt/lib/Profile/YAMLProfileReader.cpp index 3eca5e972fa5b..604a9fb4813be 100644 --- a/bolt/lib/Profile/YAMLProfileReader.cpp +++ b/bolt/lib/Profile/YAMLProfileReader.cpp @@ -49,11 +49,6 @@ llvm::cl::opt llvm::cl::opt ProfileUseDFS("profile-use-dfs", cl::desc("use DFS order for YAML profile"), cl::Hidden, cl::cat(BoltOptCategory)); - -llvm::cl::opt ProfileUsePseudoProbes( - "profile-use-pseudo-probes", - cl::desc("Use pseudo probes for profile generation and matching"), - cl::Hidden, cl::cat(BoltOptCategory)); } // namespace opts namespace llvm { diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp index f74cf60e076d0..ffbf2388e912f 100644 --- a/bolt/lib/Profile/YAMLProfileWriter.cpp +++ b/bolt/lib/Profile/YAMLProfileWriter.cpp @@ -13,6 +13,7 @@ #include "bolt/Profile/DataAggregator.h" #include "bolt/Profile/ProfileReaderBase.h" #include "bolt/Rewrite/RewriteInstance.h" +#include "bolt/Utils/CommandLineOpts.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/raw_ostream.h" @@ -21,8 +22,12 @@ #define DEBUG_TYPE "bolt-prof" namespace opts { -extern llvm::cl::opt ProfileUseDFS; -extern llvm::cl::opt ProfileUsePseudoProbes; +using namespace llvm; +extern cl::opt ProfileUseDFS; +cl::opt ProfileWritePseudoProbes( + "profile-write-pseudo-probes", + cl::desc("Use pseudo probes in profile generation"), cl::Hidden, + cl::cat(BoltOptCategory)); } // namespace opts namespace llvm { @@ -59,7 +64,7 @@ YAMLProfileWriter::convert(const BinaryFunction &BF, bool UseDFS, yaml::bolt::BinaryFunctionProfile YamlBF; const BinaryContext &BC = BF.getBinaryContext(); const MCPseudoProbeDecoder *PseudoProbeDecoder = - opts::ProfileUsePseudoProbes ? BC.getPseudoProbeDecoder() : nullptr; + opts::ProfileWritePseudoProbes ? BC.getPseudoProbeDecoder() : nullptr; const uint16_t LBRProfile = BF.getProfileFlags() & BinaryFunction::PF_LBR; diff --git a/bolt/lib/Rewrite/PseudoProbeRewriter.cpp b/bolt/lib/Rewrite/PseudoProbeRewriter.cpp index 4925b4b385d9b..492b5300214c0 100644 --- a/bolt/lib/Rewrite/PseudoProbeRewriter.cpp +++ b/bolt/lib/Rewrite/PseudoProbeRewriter.cpp @@ -14,6 +14,7 @@ #include "bolt/Rewrite/MetadataRewriter.h" #include "bolt/Rewrite/MetadataRewriters.h" #include "bolt/Utils/CommandLineOpts.h" +#include "bolt/Utils/Utils.h" #include "llvm/IR/Function.h" #include "llvm/MC/MCPseudoProbe.h" #include "llvm/Support/CommandLine.h" @@ -49,7 +50,7 @@ static cl::opt PrintPseudoProbes( clEnumValN(PPP_All, "all", "enable all debugging printout")), cl::Hidden, cl::cat(BoltCategory)); -extern cl::opt ProfileUsePseudoProbes; +extern cl::opt ProfileWritePseudoProbes; } // namespace opts namespace { @@ -71,7 +72,8 @@ class PseudoProbeRewriter final : public MetadataRewriter { /// Parse .pseudo_probe_desc section and .pseudo_probe section /// Setup Pseudo probe decoder - void parsePseudoProbe(); + /// If \p ProfiledOnly is set, only parse records for functions with profile. + void parsePseudoProbe(bool ProfiledOnly = false); /// PseudoProbe decoder std::shared_ptr ProbeDecoderPtr; @@ -90,21 +92,21 @@ class PseudoProbeRewriter final : public MetadataRewriter { }; Error PseudoProbeRewriter::preCFGInitializer() { - if (opts::ProfileUsePseudoProbes) - parsePseudoProbe(); + if (opts::ProfileWritePseudoProbes) + parsePseudoProbe(true); return Error::success(); } Error PseudoProbeRewriter::postEmitFinalizer() { - if (!opts::ProfileUsePseudoProbes) + if (!opts::ProfileWritePseudoProbes) parsePseudoProbe(); updatePseudoProbes(); return Error::success(); } -void PseudoProbeRewriter::parsePseudoProbe() { +void PseudoProbeRewriter::parsePseudoProbe(bool ProfiledOnly) { MCPseudoProbeDecoder &ProbeDecoder(*ProbeDecoderPtr); PseudoProbeDescSection = BC.getUniqueSectionByName(".pseudo_probe_desc"); PseudoProbeSection = BC.getUniqueSectionByName(".pseudo_probe"); @@ -133,10 +135,29 @@ void PseudoProbeRewriter::parsePseudoProbe() { MCPseudoProbeDecoder::Uint64Set GuidFilter; MCPseudoProbeDecoder::Uint64Map FuncStartAddrs; + SmallVector Suffixes( + {".destroy", ".resume", ".llvm.", ".cold", ".warm"}); for (const BinaryFunction *F : BC.getAllBinaryFunctions()) { + bool HasProfile = F->hasProfileAvailable(); for (const MCSymbol *Sym : F->getSymbols()) { - FuncStartAddrs[Function::getGUID(NameResolver::restore(Sym->getName()))] = - F->getAddress(); + StringRef SymName = Sym->getName(); + for (auto Name : {std::optional(NameResolver::restore(SymName)), + getCommonName(SymName, false, Suffixes)}) { + if (!Name) + continue; + SymName = *Name; + uint64_t GUID = Function::getGUID(SymName); + FuncStartAddrs[GUID] = F->getAddress(); + if (ProfiledOnly && HasProfile) + GuidFilter.insert(GUID); + } + } + } + if (ProfiledOnly) { + for (const auto &FuncDesc : ProbeDecoder.getGUID2FuncDescMap()) { + uint64_t GUID = FuncDesc.FuncGUID; + if (!FuncStartAddrs.contains(GUID)) + GuidFilter.insert(GUID); } } Contents = PseudoProbeSection->getContents(); @@ -155,13 +176,25 @@ void PseudoProbeRewriter::parsePseudoProbe() { ProbeDecoder.printProbesForAllAddresses(outs()); } - for (const auto &FuncDesc : ProbeDecoder.getGUID2FuncDescMap()) { - uint64_t GUID = FuncDesc.FuncGUID; - if (!FuncStartAddrs.contains(GUID)) - continue; - BinaryFunction *BF = BC.getBinaryFunctionAtAddress(FuncStartAddrs[GUID]); - assert(BF); - BF->setGUID(GUID); + const GUIDProbeFunctionMap &GUID2Func = ProbeDecoder.getGUID2FuncDescMap(); + // Checks GUID in GUID2Func and returns it if it's present or null otherwise. + auto checkGUID = [&](StringRef SymName) -> uint64_t { + uint64_t GUID = Function::getGUID(SymName); + if (GUID2Func.find(GUID) == GUID2Func.end()) + return 0; + return GUID; + }; + for (BinaryFunction *F : BC.getAllBinaryFunctions()) { + for (const MCSymbol *Sym : F->getSymbols()) { + StringRef SymName = NameResolver::restore(Sym->getName()); + uint64_t GUID = checkGUID(SymName); + std::optional CommonName = + getCommonName(SymName, false, Suffixes); + if (!GUID && CommonName) + GUID = checkGUID(*CommonName); + if (GUID) + F->setGUID(GUID); + } } } diff --git a/bolt/lib/Utils/Utils.cpp b/bolt/lib/Utils/Utils.cpp index 718e97535fd22..ecc2f1010a985 100644 --- a/bolt/lib/Utils/Utils.cpp +++ b/bolt/lib/Utils/Utils.cpp @@ -66,15 +66,21 @@ std::string getUnescapedName(const StringRef &Name) { return Output; } -std::optional getLTOCommonName(const StringRef Name) { - for (StringRef Suffix : {".__uniq.", ".lto_priv.", ".constprop.", ".llvm."}) { +std::optional getCommonName(const StringRef Name, bool KeepSuffix, + ArrayRef Suffixes) { + for (StringRef Suffix : Suffixes) { size_t LTOSuffixPos = Name.find(Suffix); if (LTOSuffixPos != StringRef::npos) - return Name.substr(0, LTOSuffixPos + Suffix.size()); + return Name.substr(0, LTOSuffixPos + (KeepSuffix ? Suffix.size() : 0)); } return std::nullopt; } +std::optional getLTOCommonName(const StringRef Name) { + return getCommonName(Name, true, + {".__uniq.", ".lto_priv.", ".constprop.", ".llvm."}); +} + std::optional readDWARFExpressionTargetReg(StringRef ExprBytes) { uint8_t Opcode = ExprBytes[0]; if (Opcode == dwarf::DW_CFA_def_cfa_expression) diff --git a/bolt/test/X86/pseudoprobe-decoding-inline.test b/bolt/test/X86/pseudoprobe-decoding-inline.test index b361551e5711e..1fdd00c7ef6c4 100644 --- a/bolt/test/X86/pseudoprobe-decoding-inline.test +++ b/bolt/test/X86/pseudoprobe-decoding-inline.test @@ -6,11 +6,11 @@ # PREAGG: B X:0 #main# 1 0 ## Check pseudo-probes in regular YAML profile (non-BOLTed binary) # RUN: link_fdata %s %S/../../../llvm/test/tools/llvm-profgen/Inputs/inline-cs-pseudoprobe.perfbin %t.preagg PREAGG -# RUN: perf2bolt %S/../../../llvm/test/tools/llvm-profgen/Inputs/inline-cs-pseudoprobe.perfbin -p %t.preagg --pa -w %t.yaml -o %t.fdata --profile-use-pseudo-probes +# RUN: perf2bolt %S/../../../llvm/test/tools/llvm-profgen/Inputs/inline-cs-pseudoprobe.perfbin -p %t.preagg --pa -w %t.yaml -o %t.fdata --profile-write-pseudo-probes # RUN: FileCheck --input-file %t.yaml %s --check-prefix CHECK-YAML ## Check pseudo-probes in BAT YAML profile (BOLTed binary) # RUN: link_fdata %s %t.bolt %t.preagg2 PREAGG -# RUN: perf2bolt %t.bolt -p %t.preagg2 --pa -w %t.yaml2 -o %t.fdata2 --profile-use-pseudo-probes +# RUN: perf2bolt %t.bolt -p %t.preagg2 --pa -w %t.yaml2 -o %t.fdata2 --profile-write-pseudo-probes # RUN: FileCheck --input-file %t.yaml2 %s --check-prefix CHECK-YAML # CHECK-YAML: name: bar # CHECK-YAML: - bid: 0 @@ -30,7 +30,7 @@ # CHECK-YAML: guid: 0xDB956436E78DD5FA # CHECK-YAML: pseudo_probe_desc_hash: 0x10000FFFFFFFF # -## Check that without --profile-use-pseudo-probes option, no pseudo probes are +## Check that without --profile-write-pseudo-probes option, no pseudo probes are ## generated # RUN: perf2bolt %S/../../../llvm/test/tools/llvm-profgen/Inputs/inline-cs-pseudoprobe.perfbin -p %t.preagg --pa -w %t.yaml -o %t.fdata # RUN: FileCheck --input-file %t.yaml %s --check-prefix CHECK-NO-OPT From 50c021b09950cf7d6a8f25b1ac0dec246f2325f5 Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Tue, 3 Sep 2024 11:38:04 -0700 Subject: [PATCH 2/8] update pseudoprobe-decoding-inline.test Created using spr 1.3.4 --- .../test/X86/pseudoprobe-decoding-inline.test | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/bolt/test/X86/pseudoprobe-decoding-inline.test b/bolt/test/X86/pseudoprobe-decoding-inline.test index 1fdd00c7ef6c4..629dd84ab8e1d 100644 --- a/bolt/test/X86/pseudoprobe-decoding-inline.test +++ b/bolt/test/X86/pseudoprobe-decoding-inline.test @@ -14,29 +14,38 @@ # RUN: FileCheck --input-file %t.yaml2 %s --check-prefix CHECK-YAML # CHECK-YAML: name: bar # CHECK-YAML: - bid: 0 -# CHECK-YAML: pseudo_probes: [ { guid: 0xE413754A191DB537, id: 1, type: 0 }, { guid: 0xE413754A191DB537, id: 4, type: 0 } ] -# CHECK-YAML: guid: 0xE413754A191DB537 -# CHECK-YAML: pseudo_probe_desc_hash: 0x10E852DA94 +# CHECK-YAML: pseudo_probes: +# CHECK-YAML-NEXT: - { id: 1, type: 0 +# CHECK-YAML-NEXT: - { id: 4, type: 0 +# CHECK-YAML: inline_tree: +# CHECK-YAML-NEXT: - { guid: 0xE413754A191DB537, hash: 0x10E852DA94, id: 0 } # # CHECK-YAML: name: foo # CHECK-YAML: - bid: 0 -# CHECK-YAML: pseudo_probes: [ { guid: 0x5CF8C24CDB18BDAC, id: 1, type: 0 }, { guid: 0x5CF8C24CDB18BDAC, id: 2, type: 0 } ] -# CHECK-YAML: guid: 0x5CF8C24CDB18BDAC -# CHECK-YAML: pseudo_probe_desc_hash: 0x200205A19C5B4 +# CHECK-YAML: pseudo_probes: +# CHECK-YAML-NEXT: - { id: 1, type: 0 } +# CHECK-YAML-NEXT: - { id: 2, type: 0 } +# CHECK-YAML: inline_tree: +# CHECK-YAML-NEXT: - { guid: 0x5CF8C24CDB18BDAC, hash: 0x200205A19C5B4, id: 0 } +# CHECK-YAML-NEXT: - { guid: 0xE413754A191DB537, hash: 0x10E852DA94, id: 1, callsite: 8 } # # CHECK-YAML: name: main # CHECK-YAML: - bid: 0 -# CHECK-YAML: pseudo_probes: [ { guid: 0xDB956436E78DD5FA, id: 1, type: 0 }, { guid: 0x5CF8C24CDB18BDAC, id: 1, type: 0 }, { guid: 0x5CF8C24CDB18BDAC, id: 2, type: 0 } ] -# CHECK-YAML: guid: 0xDB956436E78DD5FA -# CHECK-YAML: pseudo_probe_desc_hash: 0x10000FFFFFFFF +# CHECK-YAML: pseudo_probes: +# CHECK-YAML-NEXT: - { id: 1, type: 0 } +# CHECK-YAML-NEXT: - { id: 1, type: 0, inline_tree_id: 1 } +# CHECK-YAML-NEXT: - { id: 2, type: 0, inline_tree_id: 1 } +# CHECK-YAML: inline_tree: +# CHECK-YAML-NEXT: - { guid: 0xDB956436E78DD5FA, hash: 0x10000FFFFFFFF, id: 0 } +# CHECK-YAML-NEXT: - { guid: 0x5CF8C24CDB18BDAC, hash: 0x200205A19C5B4, id: 1, callsite: 2 } +# CHECK-YAML-NEXT: - { guid: 0xE413754A191DB537, hash: 0x10E852DA94, id: 2, parent: 1, callsite: 8 } # ## Check that without --profile-write-pseudo-probes option, no pseudo probes are ## generated # RUN: perf2bolt %S/../../../llvm/test/tools/llvm-profgen/Inputs/inline-cs-pseudoprobe.perfbin -p %t.preagg --pa -w %t.yaml -o %t.fdata # RUN: FileCheck --input-file %t.yaml %s --check-prefix CHECK-NO-OPT # CHECK-NO-OPT-NOT: pseudo_probes -# CHECK-NO-OPT-NOT: guid -# CHECK-NO-OPT-NOT: pseudo_probe_desc_hash +# CHECK-NO-OPT-NOT: inline_tree CHECK: Report of decoding input pseudo probe binaries From 90d0d4888954f7642b6d0b84b09088150347a444 Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Mon, 9 Sep 2024 21:33:17 -0700 Subject: [PATCH 3/8] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20ch?= =?UTF-8?q?anges=20introduced=20through=20rebase?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Created using spr 1.3.4 [skip ci] --- bolt/lib/Profile/DataAggregator.cpp | 14 +++++++++----- bolt/lib/Rewrite/PseudoProbeRewriter.cpp | 7 ------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp index 10d745cc69824..4aeeb1daab1b9 100644 --- a/bolt/lib/Profile/DataAggregator.cpp +++ b/bolt/lib/Profile/DataAggregator.cpp @@ -2427,11 +2427,15 @@ std::error_code DataAggregator::writeBATYAML(BinaryContext &BC, } } } - // Drop blocks without a hash, won't be useful for stale matching. - llvm::erase_if(YamlBF.Blocks, - [](const yaml::bolt::BinaryBasicBlockProfile &YamlBB) { - return YamlBB.Hash == (yaml::Hex64)0; - }); + // Skip printing if there's no profile data + llvm::erase_if( + YamlBF.Blocks, [](const yaml::bolt::BinaryBasicBlockProfile &YamlBB) { + auto HasCount = [](const auto &SI) { return SI.Count; }; + bool HasAnyCount = YamlBB.ExecCount || + llvm::any_of(YamlBB.Successors, HasCount) || + llvm::any_of(YamlBB.CallSites, HasCount); + return !HasAnyCount; + }); BP.Functions.emplace_back(YamlBF); } } diff --git a/bolt/lib/Rewrite/PseudoProbeRewriter.cpp b/bolt/lib/Rewrite/PseudoProbeRewriter.cpp index 492b5300214c0..8647df4b0edf8 100644 --- a/bolt/lib/Rewrite/PseudoProbeRewriter.cpp +++ b/bolt/lib/Rewrite/PseudoProbeRewriter.cpp @@ -153,13 +153,6 @@ void PseudoProbeRewriter::parsePseudoProbe(bool ProfiledOnly) { } } } - if (ProfiledOnly) { - for (const auto &FuncDesc : ProbeDecoder.getGUID2FuncDescMap()) { - uint64_t GUID = FuncDesc.FuncGUID; - if (!FuncStartAddrs.contains(GUID)) - GuidFilter.insert(GUID); - } - } Contents = PseudoProbeSection->getContents(); if (!ProbeDecoder.buildAddress2ProbeMap( reinterpret_cast(Contents.data()), Contents.size(), From 6ec4cf6bf05551d02cbf17e9edbe8d6931588ff1 Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Mon, 9 Sep 2024 21:37:28 -0700 Subject: [PATCH 4/8] clang-format Created using spr 1.3.4 --- bolt/lib/Profile/YAMLProfileWriter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp index 70e5e09e2920e..f2609de18ce63 100644 --- a/bolt/lib/Profile/YAMLProfileWriter.cpp +++ b/bolt/lib/Profile/YAMLProfileWriter.cpp @@ -90,7 +90,7 @@ YAMLProfileWriter::convertPseudoProbeDesc(const MCPseudoProbeDecoder &Decoder) { InlineTreeDesc InlineTree; for (const MCDecodedPseudoProbeInlineTree &TopLev : - Decoder.getDummyInlineRoot().getChildren()) + Decoder.getDummyInlineRoot().getChildren()) InlineTree.TopLevelGUIDToInlineTree[TopLev.Guid] = &TopLev; for (const auto &FuncDesc : Decoder.getGUID2FuncDescMap()) From 852eb07f345dd1d9e77a6faead8bf0f73ff64ba7 Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Tue, 10 Sep 2024 12:26:11 -0700 Subject: [PATCH 5/8] Make pseudo_probe_desc optional Created using spr 1.3.4 --- bolt/include/bolt/Profile/ProfileYAMLMapping.h | 9 ++++++++- bolt/test/X86/pseudoprobe-decoding-inline.test | 5 +++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/bolt/include/bolt/Profile/ProfileYAMLMapping.h b/bolt/include/bolt/Profile/ProfileYAMLMapping.h index 588e2f59d67e0..9cc33264d7071 100644 --- a/bolt/include/bolt/Profile/ProfileYAMLMapping.h +++ b/bolt/include/bolt/Profile/ProfileYAMLMapping.h @@ -275,6 +275,12 @@ struct PseudoProbeDesc { std::vector GUID; std::vector Hash; std::vector GUIDHash; // Index of hash for that GUID in Hash + + bool operator==(const PseudoProbeDesc &Other) const { + // Only treat empty Desc as equal + return GUID.empty() && Other.GUID.empty() && Hash.empty() && + Other.Hash.empty() && GUIDHash.empty() && Other.GUIDHash.empty(); + } }; } // end namespace bolt @@ -306,7 +312,8 @@ template <> struct MappingTraits { static void mapping(IO &YamlIO, bolt::BinaryProfile &BP) { YamlIO.mapRequired("header", BP.Header); YamlIO.mapRequired("functions", BP.Functions); - YamlIO.mapOptional("pseudo_probe_desc", BP.PseudoProbeDesc); + YamlIO.mapOptional("pseudo_probe_desc", BP.PseudoProbeDesc, + bolt::PseudoProbeDesc()); } }; diff --git a/bolt/test/X86/pseudoprobe-decoding-inline.test b/bolt/test/X86/pseudoprobe-decoding-inline.test index 0a99795d32935..87e6fa59b0707 100644 --- a/bolt/test/X86/pseudoprobe-decoding-inline.test +++ b/bolt/test/X86/pseudoprobe-decoding-inline.test @@ -36,8 +36,9 @@ ## generated # RUN: perf2bolt %S/../../../llvm/test/tools/llvm-profgen/Inputs/inline-cs-pseudoprobe.perfbin -p %t.preagg --pa -w %t.yaml -o %t.fdata # RUN: FileCheck --input-file %t.yaml %s --check-prefix CHECK-NO-OPT -# CHECK-NO-OPT-NOT: pseudo_probes -# CHECK-NO-OPT-NOT: inline_tree +# CHECK-NO-OPT-NOT: probes: +# CHECK-NO-OPT-NOT: inline_tree: +# CHECK-NO-OPT-NOT: pseudo_probe_desc: CHECK: Report of decoding input pseudo probe binaries From 65c2b99aaaecd272d48e6af6955c439d7d045b70 Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Wed, 11 Sep 2024 19:56:11 -0700 Subject: [PATCH 6/8] Address comments Created using spr 1.3.4 --- .../include/bolt/Profile/ProfileYAMLMapping.h | 35 ++++++++-------- bolt/include/bolt/Profile/YAMLProfileWriter.h | 12 +++--- bolt/lib/Profile/YAMLProfileWriter.cpp | 41 +++++++++---------- .../test/X86/pseudoprobe-decoding-inline.test | 12 +++--- 4 files changed, 49 insertions(+), 51 deletions(-) diff --git a/bolt/include/bolt/Profile/ProfileYAMLMapping.h b/bolt/include/bolt/Profile/ProfileYAMLMapping.h index 9cc33264d7071..ee39051cbc136 100644 --- a/bolt/include/bolt/Profile/ProfileYAMLMapping.h +++ b/bolt/include/bolt/Profile/ProfileYAMLMapping.h @@ -98,7 +98,7 @@ struct PseudoProbeInfo { uint32_t InlineTreeIndex = 0; uint64_t BlockMask = 0; // bitset with probe indices // Assume BlockMask == 1 if no other probes are set - std::vector BlockProbes; + std::vector BlockProbes; // block probes with indices above 64 std::vector CallProbes; std::vector IndCallProbes; std::vector InlineTreeNodes; @@ -113,10 +113,10 @@ struct PseudoProbeInfo { template <> struct MappingTraits { static void mapping(IO &YamlIO, bolt::PseudoProbeInfo &PI) { - YamlIO.mapOptional("blk", PI.BlockMask, 0); - YamlIO.mapOptional("blks", PI.BlockProbes, std::vector()); - YamlIO.mapOptional("calls", PI.CallProbes, std::vector()); - YamlIO.mapOptional("indcalls", PI.IndCallProbes, std::vector()); + YamlIO.mapOptional("blx", PI.BlockMask, 0); + YamlIO.mapOptional("blk", PI.BlockProbes, std::vector()); + YamlIO.mapOptional("call", PI.CallProbes, std::vector()); + YamlIO.mapOptional("icall", PI.IndCallProbes, std::vector()); YamlIO.mapOptional("id", PI.InlineTreeIndex, 0); YamlIO.mapOptional("ids", PI.InlineTreeNodes, std::vector()); } @@ -170,18 +170,18 @@ template <> struct MappingTraits { }; namespace bolt { -struct InlineTreeInfo { +struct InlineTreeNode { uint32_t ParentIndexDelta; uint32_t CallSiteProbe; - // Index in PseudoProbeDesc.GUID + 1, 0 for same as previous + // Index in PseudoProbeDesc.GUID, UINT32_MAX for same as previous (omitted) uint32_t GUIDIndex; - bool operator==(const InlineTreeInfo &) const { return false; } + bool operator==(const InlineTreeNode &) const { return false; } }; } // end namespace bolt -template <> struct MappingTraits { - static void mapping(IO &YamlIO, bolt::InlineTreeInfo &ITI) { - YamlIO.mapOptional("g", ITI.GUIDIndex, 0); +template <> struct MappingTraits { + static void mapping(IO &YamlIO, bolt::InlineTreeNode &ITI) { + YamlIO.mapOptional("g", ITI.GUIDIndex, UINT32_MAX); YamlIO.mapOptional("p", ITI.ParentIndexDelta, 0); YamlIO.mapOptional("cs", ITI.CallSiteProbe, 0); } @@ -192,7 +192,7 @@ template <> struct MappingTraits { } // end namespace llvm LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::yaml::bolt::BinaryBasicBlockProfile) -LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(llvm::yaml::bolt::InlineTreeInfo) +LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(llvm::yaml::bolt::InlineTreeNode) namespace llvm { namespace yaml { @@ -205,7 +205,7 @@ struct BinaryFunctionProfile { llvm::yaml::Hex64 Hash{0}; uint64_t ExecCount{0}; std::vector Blocks; - std::vector InlineTree; + std::vector InlineTree; bool Used{false}; }; } // end namespace bolt @@ -220,7 +220,7 @@ template <> struct MappingTraits { YamlIO.mapOptional("blocks", BFP.Blocks, std::vector()); YamlIO.mapOptional("inline_tree", BFP.InlineTree, - std::vector()); + std::vector()); } }; @@ -274,12 +274,13 @@ namespace bolt { struct PseudoProbeDesc { std::vector GUID; std::vector Hash; - std::vector GUIDHash; // Index of hash for that GUID in Hash + std::vector GUIDHashIdx; // Index of hash for that GUID in Hash bool operator==(const PseudoProbeDesc &Other) const { // Only treat empty Desc as equal return GUID.empty() && Other.GUID.empty() && Hash.empty() && - Other.Hash.empty() && GUIDHash.empty() && Other.GUIDHash.empty(); + Other.Hash.empty() && GUIDHashIdx.empty() && + Other.GUIDHashIdx.empty(); } }; } // end namespace bolt @@ -287,7 +288,7 @@ struct PseudoProbeDesc { template <> struct MappingTraits { static void mapping(IO &YamlIO, bolt::PseudoProbeDesc &PD) { YamlIO.mapRequired("gs", PD.GUID); - YamlIO.mapRequired("gh", PD.GUIDHash); + YamlIO.mapRequired("gh", PD.GUIDHashIdx); YamlIO.mapRequired("hs", PD.Hash); } }; diff --git a/bolt/include/bolt/Profile/YAMLProfileWriter.h b/bolt/include/bolt/Profile/YAMLProfileWriter.h index d691fe6b5a4ce..aec6e47484760 100644 --- a/bolt/include/bolt/Profile/YAMLProfileWriter.h +++ b/bolt/include/bolt/Profile/YAMLProfileWriter.h @@ -43,18 +43,18 @@ class YAMLProfileWriter { GUIDNumMap HashIdxMap; }; - static std::tuple, InlineTreeMapTy> + static std::tuple, InlineTreeMapTy> convertBFInlineTree(const MCPseudoProbeDecoder &Decoder, const InlineTreeDesc &InlineTree, uint64_t GUID); + static std::tuple + convertPseudoProbeDesc(const MCPseudoProbeDecoder &PseudoProbeDecoder); + static yaml::bolt::BinaryFunctionProfile convert(const BinaryFunction &BF, bool UseDFS, const InlineTreeDesc &InlineTree, const BoltAddressTranslation *BAT = nullptr); - static std::tuple - convertPseudoProbeDesc(const MCPseudoProbeDecoder &PseudoProbeDecoder); - /// Set CallSiteInfo destination fields from \p Symbol and return a target /// BinaryFunction for that symbol. static const BinaryFunction * @@ -71,8 +71,8 @@ class YAMLProfileWriter { uint32_t InlineSite; }; static std::vector - getInlineTree(const MCPseudoProbeDecoder &Decoder, - const MCDecodedPseudoProbeInlineTree *Root); + collectInlineTree(const MCPseudoProbeDecoder &Decoder, + const MCDecodedPseudoProbeInlineTree &Root); // 0 - block probe, 1 - indirect call, 2 - direct call using ProbeList = std::array, 3>; diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp index f2609de18ce63..072a23631d04a 100644 --- a/bolt/lib/Profile/YAMLProfileWriter.cpp +++ b/bolt/lib/Profile/YAMLProfileWriter.cpp @@ -18,7 +18,6 @@ #include "llvm/Support/CommandLine.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/raw_ostream.h" -#include #undef DEBUG_TYPE #define DEBUG_TYPE "bolt-prof" @@ -61,23 +60,21 @@ const BinaryFunction *YAMLProfileWriter::setCSIDestination( } std::vector -YAMLProfileWriter::getInlineTree(const MCPseudoProbeDecoder &Decoder, - const MCDecodedPseudoProbeInlineTree *Root) { +YAMLProfileWriter::collectInlineTree( + const MCPseudoProbeDecoder &Decoder, + const MCDecodedPseudoProbeInlineTree &Root) { auto getHash = [&](const MCDecodedPseudoProbeInlineTree &Node) { return Decoder.getFuncDescForGUID(Node.Guid)->FuncHash; }; - assert(Root); - std::vector InlineTree; - InlineTreeNode Node{Root, Root->Guid, getHash(*Root), 0, 0}; - InlineTree.emplace_back(Node); + std::vector InlineTree( + {InlineTreeNode{&Root, Root.Guid, getHash(Root), 0, 0}}); uint32_t ParentId = 0; while (ParentId != InlineTree.size()) { const MCDecodedPseudoProbeInlineTree *Cur = InlineTree[ParentId].InlineTree; - for (const MCDecodedPseudoProbeInlineTree &Child : Cur->getChildren()) { - InlineTreeNode Node{&Child, Child.Guid, getHash(Child), ParentId, - std::get<1>(Child.getInlineSite())}; - InlineTree.emplace_back(Node); - } + for (const MCDecodedPseudoProbeInlineTree &Child : Cur->getChildren()) + InlineTree.emplace_back( + InlineTreeNode{&Child, Child.Guid, getHash(Child), ParentId, + std::get<1>(Child.getInlineSite())}); ++ParentId; } @@ -125,7 +122,7 @@ YAMLProfileWriter::convertPseudoProbeDesc(const MCPseudoProbeDecoder &Decoder) { Desc.GUID.emplace_back(GUID); InlineTree.GUIDIdxMap[GUID] = Index++; uint64_t Hash = Decoder.getFuncDescForGUID(GUID)->FuncHash; - Desc.GUIDHash.emplace_back(InlineTree.HashIdxMap[Hash]); + Desc.GUIDHashIdx.emplace_back(InlineTree.HashIdxMap[Hash]); } return {Desc, InlineTree}; @@ -184,31 +181,31 @@ YAMLProfileWriter::convertNodeProbes(NodeIdToProbes &NodeProbes) { return YamlProbes; } -std::tuple, +std::tuple, YAMLProfileWriter::InlineTreeMapTy> YAMLProfileWriter::convertBFInlineTree(const MCPseudoProbeDecoder &Decoder, const InlineTreeDesc &InlineTree, uint64_t GUID) { DenseMap InlineTreeNodeId; - std::vector YamlInlineTree; + std::vector YamlInlineTree; auto It = InlineTree.TopLevelGUIDToInlineTree.find(GUID); if (It == InlineTree.TopLevelGUIDToInlineTree.end()) return {YamlInlineTree, InlineTreeNodeId}; const MCDecodedPseudoProbeInlineTree *Root = It->second; - assert(Root); + assert(Root && "Malformed TopLevelGUIDToInlineTree"); uint32_t Index = 0; uint32_t PrevParent = 0; - uint32_t PrevGUIDIdx = 0; - for (const auto &Node : getInlineTree(Decoder, Root)) { + uint32_t PrevGUIDIdx = UINT32_MAX; + for (const auto &Node : collectInlineTree(Decoder, *Root)) { InlineTreeNodeId[Node.InlineTree] = Index++; auto GUIDIdxIt = InlineTree.GUIDIdxMap.find(Node.GUID); - assert(GUIDIdxIt != InlineTree.GUIDIdxMap.end()); - uint32_t GUIDIdx = GUIDIdxIt->second + 1; + assert(GUIDIdxIt != InlineTree.GUIDIdxMap.end() && "Malformed GUIDIdxMap"); + uint32_t GUIDIdx = GUIDIdxIt->second; if (GUIDIdx == PrevGUIDIdx) - GUIDIdx = 0; + GUIDIdx = UINT32_MAX; else PrevGUIDIdx = GUIDIdx; - YamlInlineTree.emplace_back(yaml::bolt::InlineTreeInfo{ + YamlInlineTree.emplace_back(yaml::bolt::InlineTreeNode{ Node.ParentId - PrevParent, Node.InlineSite, GUIDIdx}); PrevParent = Node.ParentId; } diff --git a/bolt/test/X86/pseudoprobe-decoding-inline.test b/bolt/test/X86/pseudoprobe-decoding-inline.test index 87e6fa59b0707..c8bdce927ce12 100644 --- a/bolt/test/X86/pseudoprobe-decoding-inline.test +++ b/bolt/test/X86/pseudoprobe-decoding-inline.test @@ -14,18 +14,18 @@ # RUN: FileCheck --input-file %t.yaml2 %s --check-prefix CHECK-YAML # CHECK-YAML: name: bar # CHECK-YAML: - bid: 0 -# CHECK-YAML: probes: [ { blk: 9 } ] -# CHECK-YAML: inline_tree: [ { g: 1 } ] +# CHECK-YAML: probes: [ { blx: 9 } ] +# CHECK-YAML: inline_tree: [ { g: 0 } ] # # CHECK-YAML: name: foo # CHECK-YAML: - bid: 0 -# CHECK-YAML: probes: [ { blk: 3 } ] -# CHECK-YAML: inline_tree: [ { g: 2 }, { g: 1, cs: 8 } ] +# CHECK-YAML: probes: [ { blx: 3 } ] +# CHECK-YAML: inline_tree: [ { g: 1 }, { g: 0, cs: 8 } ] # # CHECK-YAML: name: main # CHECK-YAML: - bid: 0 -# CHECK-YAML: probes: [ { blk: 3, id: 1 }, { } ] -# CHECK-YAML: inline_tree: [ { g: 3 }, { g: 2, cs: 2 }, { g: 1, p: 1, cs: 8 } ] +# CHECK-YAML: probes: [ { blx: 3, id: 1 }, { } ] +# CHECK-YAML: inline_tree: [ { g: 2 }, { g: 1, cs: 2 }, { g: 0, p: 1, cs: 8 } ] # # CHECK-YAML: pseudo_probe_desc: # CHECK-YAML-NEXT: gs: [ 0xE413754A191DB537, 0x5CF8C24CDB18BDAC, 0xDB956436E78DD5FA ] From 7c90ed65e98968f2646a187132bc72962c4f1315 Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Wed, 11 Sep 2024 20:41:17 -0700 Subject: [PATCH 7/8] Drop Block1 optimization Created using spr 1.3.4 --- bolt/include/bolt/Profile/ProfileYAMLMapping.h | 3 +-- bolt/lib/Profile/YAMLProfileWriter.cpp | 6 +----- bolt/test/X86/pseudoprobe-decoding-inline.test | 4 ++-- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/bolt/include/bolt/Profile/ProfileYAMLMapping.h b/bolt/include/bolt/Profile/ProfileYAMLMapping.h index ee39051cbc136..cae00e37bf27e 100644 --- a/bolt/include/bolt/Profile/ProfileYAMLMapping.h +++ b/bolt/include/bolt/Profile/ProfileYAMLMapping.h @@ -96,8 +96,7 @@ template <> struct MappingTraits { namespace bolt { struct PseudoProbeInfo { uint32_t InlineTreeIndex = 0; - uint64_t BlockMask = 0; // bitset with probe indices - // Assume BlockMask == 1 if no other probes are set + uint64_t BlockMask = 0; // bitset with probe indices from 1 to 64 std::vector BlockProbes; // block probes with indices above 64 std::vector CallProbes; std::vector IndCallProbes; diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp index 072a23631d04a..44600c3c5d5ef 100644 --- a/bolt/lib/Profile/YAMLProfileWriter.cpp +++ b/bolt/lib/Profile/YAMLProfileWriter.cpp @@ -173,10 +173,6 @@ YAMLProfileWriter::convertNodeProbes(NodeIdToProbes &NodeProbes) { else YamlBPI.InlineTreeNodes = Nodes; handleMask(BPI.BlockProbes, YamlBPI.BlockProbes, YamlBPI.BlockMask); - // Assume BlockMask == 1 if no other probes are set - if (YamlBPI.BlockMask == 1 && YamlBPI.CallProbes.empty() && - YamlBPI.IndCallProbes.empty()) - YamlBPI.BlockMask = 0; } return YamlProbes; } @@ -195,7 +191,7 @@ YAMLProfileWriter::convertBFInlineTree(const MCPseudoProbeDecoder &Decoder, assert(Root && "Malformed TopLevelGUIDToInlineTree"); uint32_t Index = 0; uint32_t PrevParent = 0; - uint32_t PrevGUIDIdx = UINT32_MAX; + uint32_t PrevGUIDIdx = 0; for (const auto &Node : collectInlineTree(Decoder, *Root)) { InlineTreeNodeId[Node.InlineTree] = Index++; auto GUIDIdxIt = InlineTree.GUIDIdxMap.find(Node.GUID); diff --git a/bolt/test/X86/pseudoprobe-decoding-inline.test b/bolt/test/X86/pseudoprobe-decoding-inline.test index c8bdce927ce12..ec1172573c186 100644 --- a/bolt/test/X86/pseudoprobe-decoding-inline.test +++ b/bolt/test/X86/pseudoprobe-decoding-inline.test @@ -15,7 +15,7 @@ # CHECK-YAML: name: bar # CHECK-YAML: - bid: 0 # CHECK-YAML: probes: [ { blx: 9 } ] -# CHECK-YAML: inline_tree: [ { g: 0 } ] +# CHECK-YAML: inline_tree: [ { } ] # # CHECK-YAML: name: foo # CHECK-YAML: - bid: 0 @@ -24,7 +24,7 @@ # # CHECK-YAML: name: main # CHECK-YAML: - bid: 0 -# CHECK-YAML: probes: [ { blx: 3, id: 1 }, { } ] +# CHECK-YAML: probes: [ { blx: 3, id: 1 }, { blx: 1 } ] # CHECK-YAML: inline_tree: [ { g: 2 }, { g: 1, cs: 2 }, { g: 0, p: 1, cs: 8 } ] # # CHECK-YAML: pseudo_probe_desc: From 7331abae2b65894d2f1e60dea1f385ce8dc85bce Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Wed, 11 Sep 2024 20:47:06 -0700 Subject: [PATCH 8/8] clang-format Created using spr 1.3.4 --- bolt/include/bolt/Profile/ProfileYAMLMapping.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bolt/include/bolt/Profile/ProfileYAMLMapping.h b/bolt/include/bolt/Profile/ProfileYAMLMapping.h index cae00e37bf27e..91955afb186e9 100644 --- a/bolt/include/bolt/Profile/ProfileYAMLMapping.h +++ b/bolt/include/bolt/Profile/ProfileYAMLMapping.h @@ -96,7 +96,7 @@ template <> struct MappingTraits { namespace bolt { struct PseudoProbeInfo { uint32_t InlineTreeIndex = 0; - uint64_t BlockMask = 0; // bitset with probe indices from 1 to 64 + uint64_t BlockMask = 0; // bitset with probe indices from 1 to 64 std::vector BlockProbes; // block probes with indices above 64 std::vector CallProbes; std::vector IndCallProbes;