From 023e1a4afb1315b4632f2beffe8f0a3d81f201fb Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Sat, 10 Aug 2024 21:34:02 -0700 Subject: [PATCH 1/4] =?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/lib/Rewrite/PseudoProbeRewriter.cpp | 1 - llvm/include/llvm/MC/MCPseudoProbe.h | 6 + llvm/lib/MC/MCPseudoProbe.cpp | 147 ++++++++++++++----- llvm/tools/llvm-profgen/ProfileGenerator.cpp | 2 +- llvm/tools/llvm-profgen/ProfiledBinary.cpp | 3 +- llvm/tools/llvm-profgen/ProfiledBinary.h | 7 +- 6 files changed, 122 insertions(+), 44 deletions(-) diff --git a/bolt/lib/Rewrite/PseudoProbeRewriter.cpp b/bolt/lib/Rewrite/PseudoProbeRewriter.cpp index 886bbdbf9d686e..37a5b937ebcaa3 100644 --- a/bolt/lib/Rewrite/PseudoProbeRewriter.cpp +++ b/bolt/lib/Rewrite/PseudoProbeRewriter.cpp @@ -143,7 +143,6 @@ void PseudoProbeRewriter::parsePseudoProbe() { if (!ProbeDecoder.buildAddress2ProbeMap( reinterpret_cast(Contents.data()), Contents.size(), GuidFilter, FuncStartAddrs)) { - ProbeDecoder.getAddress2ProbesMap().clear(); errs() << "BOLT-WARNING: fail in building Address2ProbeMap\n"; return; } diff --git a/llvm/include/llvm/MC/MCPseudoProbe.h b/llvm/include/llvm/MC/MCPseudoProbe.h index 54a7f832e50081..5c536a8ec0b551 100644 --- a/llvm/include/llvm/MC/MCPseudoProbe.h +++ b/llvm/include/llvm/MC/MCPseudoProbe.h @@ -241,6 +241,7 @@ class MCPseudoProbeInlineTreeBase { InlinedProbeTreeMap &getChildren() { return Children; } const InlinedProbeTreeMap &getChildren() const { return Children; } std::vector &getProbes() { return Probes; } + const std::vector &getProbes() const { return Probes; } void addProbes(ProbeType Probe) { Probes.push_back(Probe); } // Caller node of the inline site MCPseudoProbeInlineTreeBase *Parent = @@ -369,6 +370,11 @@ class MCPseudoProbeDecoder { // Decode pseudo_probe_desc section to build GUID to PseudoProbeFuncDesc map. bool buildGUID2FuncDescMap(const uint8_t *Start, std::size_t Size); + // Decode pseudo_probe section to count the number of probes and inlined + // function records for each function record. + bool countRecords(bool IsTopLevelFunc, bool &Discard, uint32_t &ProbeCount, + uint32_t &InlinedCount, const Uint64Set &GuidFilter); + // Decode pseudo_probe section to build address to probes map for specifed // functions only. bool buildAddress2ProbeMap(const uint8_t *Start, std::size_t Size, diff --git a/llvm/lib/MC/MCPseudoProbe.cpp b/llvm/lib/MC/MCPseudoProbe.cpp index a5a030e19b849e..e9e391c8eadd70 100644 --- a/llvm/lib/MC/MCPseudoProbe.cpp +++ b/llvm/lib/MC/MCPseudoProbe.cpp @@ -18,6 +18,7 @@ #include "llvm/MC/MCObjectStreamer.h" #include "llvm/MC/MCSymbol.h" #include "llvm/Support/Endian.h" +#include "llvm/Support/Error.h" #include "llvm/Support/LEB128.h" #include "llvm/Support/MD5.h" #include "llvm/Support/raw_ostream.h" @@ -429,17 +430,11 @@ bool MCPseudoProbeDecoder::buildAddress2ProbeMap( Index = Cur->getChildren().size(); } else { // Read inline site for inlinees - auto ErrorOrIndex = readUnsignedNumber(); - if (!ErrorOrIndex) - return false; - Index = std::move(*ErrorOrIndex); + Index = cantFail(errorOrToExpected(readUnsignedNumber())); } // Read guid - auto ErrorOrCurGuid = readUnencodedNumber(); - if (!ErrorOrCurGuid) - return false; - uint64_t Guid = std::move(*ErrorOrCurGuid); + uint64_t Guid = cantFail(errorOrToExpected(readUnencodedNumber())); // Decide if top-level node should be disgarded. if (IsTopLevelFunc && !GuidFilter.empty() && !GuidFilter.count(Guid)) @@ -457,41 +452,27 @@ bool MCPseudoProbeDecoder::buildAddress2ProbeMap( } // Read number of probes in the current node. - auto ErrorOrNodeCount = readUnsignedNumber(); - if (!ErrorOrNodeCount) - return false; - uint32_t NodeCount = std::move(*ErrorOrNodeCount); + uint32_t NodeCount = + cantFail(errorOrToExpected(readUnsignedNumber())); // Read number of direct inlinees - auto ErrorOrCurChildrenToProcess = readUnsignedNumber(); - if (!ErrorOrCurChildrenToProcess) - return false; + uint32_t ChildrenToProcess = + cantFail(errorOrToExpected(readUnsignedNumber())); // Read all probes in this node for (std::size_t I = 0; I < NodeCount; I++) { // Read index - auto ErrorOrIndex = readUnsignedNumber(); - if (!ErrorOrIndex) - return false; - uint32_t Index = std::move(*ErrorOrIndex); + uint32_t Index = + cantFail(errorOrToExpected(readUnsignedNumber())); // Read type | flag. - auto ErrorOrValue = readUnencodedNumber(); - if (!ErrorOrValue) - return false; - uint8_t Value = std::move(*ErrorOrValue); + uint8_t Value = cantFail(errorOrToExpected(readUnencodedNumber())); uint8_t Kind = Value & 0xf; uint8_t Attr = (Value & 0x70) >> 4; // Read address uint64_t Addr = 0; if (Value & 0x80) { - auto ErrorOrOffset = readSignedNumber(); - if (!ErrorOrOffset) - return false; - int64_t Offset = std::move(*ErrorOrOffset); + int64_t Offset = cantFail(errorOrToExpected(readSignedNumber())); Addr = LastAddr + Offset; } else { - auto ErrorOrAddr = readUnencodedNumber(); - if (!ErrorOrAddr) - return false; - Addr = std::move(*ErrorOrAddr); + Addr = cantFail(errorOrToExpected(readUnencodedNumber())); if (isSentinelProbe(Attr)) { // For sentinel probe, the addr field actually stores the GUID of the // split function. Convert it to the real address. @@ -508,10 +489,8 @@ bool MCPseudoProbeDecoder::buildAddress2ProbeMap( uint32_t Discriminator = 0; if (hasDiscriminator(Attr)) { - auto ErrorOrDiscriminator = readUnsignedNumber(); - if (!ErrorOrDiscriminator) - return false; - Discriminator = std::move(*ErrorOrDiscriminator); + Discriminator = + cantFail(errorOrToExpected(readUnsignedNumber())); } if (Cur && !isSentinelProbe(Attr)) { @@ -524,17 +503,109 @@ bool MCPseudoProbeDecoder::buildAddress2ProbeMap( LastAddr = Addr; } - uint32_t ChildrenToProcess = std::move(*ErrorOrCurChildrenToProcess); for (uint32_t I = 0; I < ChildrenToProcess; I++) { buildAddress2ProbeMap(Cur, LastAddr, GuidFilter, FuncStartAddrs); } + return true; +} + +bool MCPseudoProbeDecoder::countRecords(bool IsTopLevelFunc, bool &Discard, + uint32_t &ProbeCount, + uint32_t &InlinedCount, + const Uint64Set &GuidFilter) { + if (!IsTopLevelFunc) + // Read inline site for inlinees + if (!readUnsignedNumber()) + return false; + + // Read guid + auto ErrorOrCurGuid = readUnencodedNumber(); + if (!ErrorOrCurGuid) + return false; + uint64_t Guid = std::move(*ErrorOrCurGuid); + + // Decide if top-level node should be disgarded. + if (IsTopLevelFunc) { + Discard = !GuidFilter.empty() && !GuidFilter.count(Guid); + if (!Discard) + // Allocate an entry for top-level function record. + ++InlinedCount; + } + // Read number of probes in the current node. + auto ErrorOrNodeCount = readUnsignedNumber(); + if (!ErrorOrNodeCount) + return false; + uint32_t NodeCount = std::move(*ErrorOrNodeCount); + uint32_t CurrentProbeCount = 0; + + // Read number of direct inlinees + auto ErrorOrCurChildrenToProcess = readUnsignedNumber(); + if (!ErrorOrCurChildrenToProcess) + return false; + uint32_t ChildrenToProcess = std::move(*ErrorOrCurChildrenToProcess); + + // Read all probes in this node + for (std::size_t I = 0; I < NodeCount; I++) { + // Read index + if (!readUnsignedNumber()) + return false; + + // Read type | flag. + auto ErrorOrValue = readUnencodedNumber(); + if (!ErrorOrValue) + return false; + uint8_t Value = std::move(*ErrorOrValue); + + uint8_t Attr = (Value & 0x70) >> 4; + if (Value & 0x80) { + // Offset + if (!readSignedNumber()) + return false; + } else { + // Addr + if (!readUnencodedNumber()) + return false; + } + + if (hasDiscriminator(Attr)) + // Discriminator + if (!readUnsignedNumber()) + return false; + + if (!Discard && !isSentinelProbe(Attr)) + ++CurrentProbeCount; + } + + if (!Discard) { + ProbeCount += CurrentProbeCount; + InlinedCount += ChildrenToProcess; + } + + for (uint32_t I = 0; I < ChildrenToProcess; I++) + if (!countRecords(false, Discard, ProbeCount, InlinedCount, GuidFilter)) + return false; return true; } bool MCPseudoProbeDecoder::buildAddress2ProbeMap( const uint8_t *Start, std::size_t Size, const Uint64Set &GuidFilter, const Uint64Map &FuncStartAddrs) { + // For function records in the order of their appearance in the encoded data + // (DFS), count the number of contained probes and inlined function records. + uint32_t ProbeCount = 0; + uint32_t InlinedCount = 0; + uint32_t TopLevelFuncs = 0; + Data = Start; + End = Data + Size; + bool Discard = false; + while (Data < End) { + if (!countRecords(true, Discard, ProbeCount, InlinedCount, GuidFilter)) + return false; + TopLevelFuncs += !Discard; + } + assert(Data == End && "Have unprocessed data in pseudo_probe section"); + Data = Start; End = Data + Size; uint64_t LastAddr = 0; @@ -559,7 +630,7 @@ void MCPseudoProbeDecoder::printProbeForAddress(raw_ostream &OS, uint64_t Address) { auto It = Address2ProbesMap.find(Address); if (It != Address2ProbesMap.end()) { - for (auto &Probe : It->second) { + for (const MCDecodedPseudoProbe &Probe : It->second) { OS << " [Probe]:\t"; Probe.print(OS, GUID2FuncDescMap, true); } @@ -586,7 +657,7 @@ MCPseudoProbeDecoder::getCallProbeForAddr(uint64_t Address) const { const auto &Probes = It->second; const MCDecodedPseudoProbe *CallProbe = nullptr; - for (const auto &Probe : Probes) { + for (const MCDecodedPseudoProbe &Probe : Probes) { if (Probe.isCall()) { // Disabling the assert and returning first call probe seen so far. // Subsequent call probes, if any, are ignored. Due to the the way diff --git a/llvm/tools/llvm-profgen/ProfileGenerator.cpp b/llvm/tools/llvm-profgen/ProfileGenerator.cpp index 175556c2220e6d..c840e7789edfe2 100644 --- a/llvm/tools/llvm-profgen/ProfileGenerator.cpp +++ b/llvm/tools/llvm-profgen/ProfileGenerator.cpp @@ -1194,7 +1194,7 @@ void ProfileGeneratorBase::extractProbesFromRange( Binary->getAddress2ProbesMap(); auto It = Address2ProbesMap.find(IP.Address); if (It != Address2ProbesMap.end()) { - for (const auto &Probe : It->second) { + for (const MCDecodedPseudoProbe &Probe : It->second) { ProbeCounter[&Probe] += Count; } } diff --git a/llvm/tools/llvm-profgen/ProfiledBinary.cpp b/llvm/tools/llvm-profgen/ProfiledBinary.cpp index 632ddc7b50f54a..574a9c9f52bf18 100644 --- a/llvm/tools/llvm-profgen/ProfiledBinary.cpp +++ b/llvm/tools/llvm-profgen/ProfiledBinary.cpp @@ -137,7 +137,8 @@ void BinarySizeContextTracker::trackInlineesOptimizedAway( void BinarySizeContextTracker::trackInlineesOptimizedAway( MCPseudoProbeDecoder &ProbeDecoder, - MCDecodedPseudoProbeInlineTree &ProbeNode, ProbeFrameStack &ProbeContext) { + const MCDecodedPseudoProbeInlineTree &ProbeNode, + ProbeFrameStack &ProbeContext) { StringRef FuncName = ProbeDecoder.getFuncDescForGUID(ProbeNode.Guid)->FuncName; ProbeContext.emplace_back(FuncName, 0); diff --git a/llvm/tools/llvm-profgen/ProfiledBinary.h b/llvm/tools/llvm-profgen/ProfiledBinary.h index f2eeca45454592..0588cb48b2af62 100644 --- a/llvm/tools/llvm-profgen/ProfiledBinary.h +++ b/llvm/tools/llvm-profgen/ProfiledBinary.h @@ -167,9 +167,10 @@ class BinarySizeContextTracker { void trackInlineesOptimizedAway(MCPseudoProbeDecoder &ProbeDecoder); using ProbeFrameStack = SmallVector>; - void trackInlineesOptimizedAway(MCPseudoProbeDecoder &ProbeDecoder, - MCDecodedPseudoProbeInlineTree &ProbeNode, - ProbeFrameStack &Context); + void + trackInlineesOptimizedAway(MCPseudoProbeDecoder &ProbeDecoder, + const MCDecodedPseudoProbeInlineTree &ProbeNode, + ProbeFrameStack &Context); void dump() { RootContext.dumpTree(); } From ddcbb593f72ca47acaa82f9c14a7fd2c4e30903b Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Tue, 13 Aug 2024 03:51:31 -0700 Subject: [PATCH 2/4] Pass CurChildIndex by value Created using spr 1.3.4 --- llvm/include/llvm/MC/MCPseudoProbe.h | 6 ++++-- llvm/lib/MC/MCPseudoProbe.cpp | 26 +++++++++++--------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/llvm/include/llvm/MC/MCPseudoProbe.h b/llvm/include/llvm/MC/MCPseudoProbe.h index a46188e565c7e8..32d7a4e9129eca 100644 --- a/llvm/include/llvm/MC/MCPseudoProbe.h +++ b/llvm/include/llvm/MC/MCPseudoProbe.h @@ -474,11 +474,13 @@ class MCPseudoProbeDecoder { } private: + // Recursively parse an inlining tree encoded in pseudo_probe section. Returns + // whether the the top-level node should be skipped. template - void buildAddress2ProbeMap(MCDecodedPseudoProbeInlineTree *Cur, + bool buildAddress2ProbeMap(MCDecodedPseudoProbeInlineTree *Cur, uint64_t &LastAddr, const Uint64Set &GuildFilter, const Uint64Map &FuncStartAddrs, - uint32_t &CurChild); + const uint32_t CurChildIndex); }; } // end namespace llvm diff --git a/llvm/lib/MC/MCPseudoProbe.cpp b/llvm/lib/MC/MCPseudoProbe.cpp index c4c2dfcec40564..e6f6e797b4ee71 100644 --- a/llvm/lib/MC/MCPseudoProbe.cpp +++ b/llvm/lib/MC/MCPseudoProbe.cpp @@ -420,17 +420,17 @@ bool MCPseudoProbeDecoder::buildGUID2FuncDescMap(const uint8_t *Start, } template -void MCPseudoProbeDecoder::buildAddress2ProbeMap( +bool MCPseudoProbeDecoder::buildAddress2ProbeMap( MCDecodedPseudoProbeInlineTree *Cur, uint64_t &LastAddr, const Uint64Set &GuidFilter, const Uint64Map &FuncStartAddrs, - uint32_t &CurChild) { + const uint32_t CurChildIndex) { // The pseudo_probe section encodes an inline forest and each tree has a // format defined in MCPseudoProbe.h uint32_t Index = 0; if (IsTopLevelFunc) { // Use a sequential id for top level inliner. - Index = CurChild; + Index = CurChildIndex; } else { // Read inline site for inlinees Index = cantFail(errorOrToExpected(readUnsignedNumber())); @@ -446,19 +446,14 @@ void MCPseudoProbeDecoder::buildAddress2ProbeMap( // If the incoming node is null, all its children nodes should be disgarded. if (Cur) { // Switch/add to a new tree node(inlinee) - Cur->Children[CurChild] = MCDecodedPseudoProbeInlineTree(Guid, Index, Cur); - Cur = &Cur->Children[CurChild]; + Cur->Children[CurChildIndex] = + MCDecodedPseudoProbeInlineTree(Guid, Index, Cur); + Cur = &Cur->Children[CurChildIndex]; if (IsTopLevelFunc && !EncodingIsAddrBased) { if (auto V = FuncStartAddrs.lookup(Guid)) LastAddr = V; } } - // Advance CurChild for non-skipped top-level functions and unconditionally - // for inlined functions. - if (IsTopLevelFunc) - CurChild += !!Cur; - else - ++CurChild; // Read number of probes in the current node. uint32_t NodeCount = @@ -519,9 +514,10 @@ void MCPseudoProbeDecoder::buildAddress2ProbeMap( InlineTreeVec.resize(InlineTreeVec.size() + ChildrenToProcess); Cur->Children = MutableArrayRef(InlineTreeVec).take_back(ChildrenToProcess); } - for (uint32_t I = 0; I < ChildrenToProcess;) { + for (uint32_t I = 0; I < ChildrenToProcess; I++) { buildAddress2ProbeMap(Cur, LastAddr, GuidFilter, FuncStartAddrs, I); } + return Cur; } template @@ -630,10 +626,10 @@ bool MCPseudoProbeDecoder::buildAddress2ProbeMap( Data = Start; End = Data + Size; uint64_t LastAddr = 0; - uint32_t Child = 0; + uint32_t CurChildIndex = 0; while (Data < End) - buildAddress2ProbeMap(&DummyInlineRoot, LastAddr, GuidFilter, - FuncStartAddrs, Child); + CurChildIndex += buildAddress2ProbeMap( + &DummyInlineRoot, LastAddr, GuidFilter, FuncStartAddrs, CurChildIndex); assert(Data == End && "Have unprocessed data in pseudo_probe section"); return true; } From 73d808abad1e66b6d7f5a9a52f9617b5267ee4c0 Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Wed, 14 Aug 2024 07:50:31 -0700 Subject: [PATCH 3/4] s/ChildrenType/InlinedProbeTreeMap Created using spr 1.3.4 --- llvm/include/llvm/MC/MCPseudoProbe.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/llvm/include/llvm/MC/MCPseudoProbe.h b/llvm/include/llvm/MC/MCPseudoProbe.h index 0f21d89971f7ab..c21aff7b277aa6 100644 --- a/llvm/include/llvm/MC/MCPseudoProbe.h +++ b/llvm/include/llvm/MC/MCPseudoProbe.h @@ -214,11 +214,11 @@ class MCDecodedPseudoProbe : public MCPseudoProbeBase { }; template + typename InlinedProbeTreeMap> class MCPseudoProbeInlineTreeBase { protected: // Track children (e.g. inlinees) of current context - ChildrenType Children; + InlinedProbeTreeMap Children; // Set of probes that come with the function. ProbesType Probes; MCPseudoProbeInlineTreeBase() { @@ -233,13 +233,13 @@ class MCPseudoProbeInlineTreeBase { // Root node has a GUID 0. bool isRoot() const { return Guid == 0; } - ChildrenType &getChildren() { return Children; } - const ChildrenType &getChildren() const { return Children; } + InlinedProbeTreeMap &getChildren() { return Children; } + const InlinedProbeTreeMap &getChildren() const { return Children; } ProbesType &getProbes() { return Probes; } const ProbesType &getProbes() const { return Probes; } // Caller node of the inline site MCPseudoProbeInlineTreeBase *Parent = nullptr; + InlinedProbeTreeMap> *Parent = nullptr; DerivedProbeInlineTreeType *getOrAddNode(const InlineSite &Site) { auto Ret = Children.emplace( Site, std::make_unique(Site)); @@ -284,6 +284,7 @@ class MCDecodedPseudoProbeInlineTree MutableArrayRef> { uint32_t NumProbes = 0; uint32_t ProbeId = 0; + public: MCDecodedPseudoProbeInlineTree() = default; MCDecodedPseudoProbeInlineTree(const InlineSite &Site, From 800dfcdc3b540ffe54ded40a23d46d854f7bd6aa Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Thu, 15 Aug 2024 11:20:52 -0700 Subject: [PATCH 4/4] Add asserts, address comments Created using spr 1.3.4 --- llvm/include/llvm/MC/MCPseudoProbe.h | 1 - llvm/lib/MC/MCPseudoProbe.cpp | 4 ++++ llvm/tools/llvm-profgen/ProfileGenerator.cpp | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/llvm/include/llvm/MC/MCPseudoProbe.h b/llvm/include/llvm/MC/MCPseudoProbe.h index c21aff7b277aa6..66ad9db4860d8a 100644 --- a/llvm/include/llvm/MC/MCPseudoProbe.h +++ b/llvm/include/llvm/MC/MCPseudoProbe.h @@ -235,7 +235,6 @@ class MCPseudoProbeInlineTreeBase { bool isRoot() const { return Guid == 0; } InlinedProbeTreeMap &getChildren() { return Children; } const InlinedProbeTreeMap &getChildren() const { return Children; } - ProbesType &getProbes() { return Probes; } const ProbesType &getProbes() const { return Probes; } // Caller node of the inline site MCPseudoProbeInlineTreeBase( &DummyInlineRoot, LastAddr, GuidFilter, FuncStartAddrs, CurChildIndex); assert(Data == End && "Have unprocessed data in pseudo_probe section"); + assert(PseudoProbeVec.size() == ProbeCount && + "Mismatching probe count pre- and post-parsing"); + assert(InlineTreeVec.size() == InlinedCount && + "Mismatching function records count pre- and post-parsing"); return true; } diff --git a/llvm/tools/llvm-profgen/ProfileGenerator.cpp b/llvm/tools/llvm-profgen/ProfileGenerator.cpp index 1081ccfa8c25b0..ea7b9b9c7bd528 100644 --- a/llvm/tools/llvm-profgen/ProfileGenerator.cpp +++ b/llvm/tools/llvm-profgen/ProfileGenerator.cpp @@ -1293,7 +1293,7 @@ void CSProfileGenerator::populateBodySamplesWithProbes( // and will be inferred by the compiler. for (auto &I : FrameSamples) { for (auto *FunctionProfile : I.second) { - for (auto &Probe : I.first->getProbes()) { + for (const MCDecodedPseudoProbe &Probe : I.first->getProbes()) { FunctionProfile->addBodySamples(Probe.getIndex(), Probe.getDiscriminator(), 0); }