https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/102789
>From ddcbb593f72ca47acaa82f9c14a7fd2c4e30903b Mon Sep 17 00:00:00 2001 From: Amir Ayupov <aau...@fb.com> Date: Tue, 13 Aug 2024 03:51:31 -0700 Subject: [PATCH 1/3] 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 <bool IsTopLevelFunc> - 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 <bool IsTopLevelFunc> -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<uint32_t>())); @@ -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<false>(Cur, LastAddr, GuidFilter, FuncStartAddrs, I); } + return Cur; } template <bool IsTopLevelFunc> @@ -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<true>(&DummyInlineRoot, LastAddr, GuidFilter, - FuncStartAddrs, Child); + CurChildIndex += buildAddress2ProbeMap<true>( + &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 <aau...@fb.com> Date: Wed, 14 Aug 2024 07:50:31 -0700 Subject: [PATCH 2/3] 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 ProbesType, typename DerivedProbeInlineTreeType, - typename ChildrenType> + 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<ProbesType, DerivedProbeInlineTreeType, - ChildrenType> *Parent = nullptr; + InlinedProbeTreeMap> *Parent = nullptr; DerivedProbeInlineTreeType *getOrAddNode(const InlineSite &Site) { auto Ret = Children.emplace( Site, std::make_unique<DerivedProbeInlineTreeType>(Site)); @@ -284,6 +284,7 @@ class MCDecodedPseudoProbeInlineTree MutableArrayRef<MCDecodedPseudoProbeInlineTree>> { 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 <aau...@fb.com> Date: Thu, 15 Aug 2024 11:20:52 -0700 Subject: [PATCH 3/3] 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<ProbesType, DerivedProbeInlineTreeType, diff --git a/llvm/lib/MC/MCPseudoProbe.cpp b/llvm/lib/MC/MCPseudoProbe.cpp index b44f9f844437eb..1031dac331bb1c 100644 --- a/llvm/lib/MC/MCPseudoProbe.cpp +++ b/llvm/lib/MC/MCPseudoProbe.cpp @@ -631,6 +631,10 @@ bool MCPseudoProbeDecoder::buildAddress2ProbeMap( CurChildIndex += buildAddress2ProbeMap<true>( &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); } _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits