[PATCH] D159141: [memprof] Add a MemProfReader base class.
Snehasish Kumar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 30 13:06:03 PDT 2023
snehasish marked an inline comment as done.
snehasish added a comment.
In D159141#4626637 <https://reviews.llvm.org/D159141#4626637>, @tejohnson wrote:
> Should there be a test to cover the new base class? In particular the constructor isn't called and the readNextRecord with a null Callback case.
Added a test which uses the constructor and relies on the default null Callback behaviour.
================
Comment at: llvm/include/llvm/ProfileData/RawMemProfReader.h:86
+ // Initialize the MemProfReader with the frame mappings and profile contents.
+ MemProfReader(
+ llvm::DenseMap<FrameId, Frame> FrameIdMap,
----------------
tejohnson wrote:
> I don't see this constructor being called anywhere in this patch?
Added a test which uses this constructor.
================
Comment at: llvm/include/llvm/ProfileData/RawMemProfReader.h:147
// Constructor for unittests only.
RawMemProfReader(std::unique_ptr<llvm::symbolize::SymbolizableModule> Sym,
llvm::SmallVectorImpl<SegmentEntry> &Seg,
----------------
tejohnson wrote:
> Should this call the base class constructor?
No we can't call the base class constructor from here since we need to do additional processing (`symbolizeAndFilterStackFrames` and `mapRawProfileToRecords`) before populating the function profile data structure.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D159141/new/
https://reviews.llvm.org/D159141
More information about the llvm-commits
mailing list