[PATCH] D135914: [PseudoProbe] Decode offset based pseudo probe.
Hongtao Yu via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 25 09:34:41 PDT 2022
hoy added a comment.
In D135914#3881607 <https://reviews.llvm.org/D135914#3881607>, @wenlei wrote:
> Sentinel probe case isn't covered for both encoding and decoding in any test cases, would be good to have some coverage.
Forgot to mention that the updated binary, i.e, inline-cs-pseudoprobe.perfbin, is built with the new encoding. The binary supports a dozen llvm-profgen tests and they all pass with the current decoding changes.
================
Comment at: llvm/lib/MC/MCPseudoProbe.cpp:418
// Decide if top-level node should be disgarded.
- if (Cur == &DummyInlineRoot && !GuildFilter.empty() &&
- !GuildFilter.count(Guid))
+ if (Cur == &DummyInlineRoot && !GuidFilter.empty() && !GuidFilter.count(Guid))
Cur = nullptr;
----------------
wenlei wrote:
> nit: `Cur == &DummyInlineRoot` -> `IsTopLevelFunc`
Done.
================
Comment at: llvm/lib/MC/MCPseudoProbe.cpp:476
+ // leading probe address or function start address.
+ EncodingIsAddrBased = true;
+ }
----------------
wenlei wrote:
> Maybe I'm asking the same question as the one on previous patch, but why do we still have cases with absolute address encoded? I thought for main function body we have GUID -> symbol (reverse lookup) -> address; and we have similar way to get address for sentinel for split binary function. The rest should all be offset/delta.
This is mainly for downwards-compatibility, i.e. to have new llvm-profgen handle old binaries.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135914/new/
https://reviews.llvm.org/D135914
More information about the llvm-commits
mailing list