[PATCH] D121655: [llvm-profgen] Read sample profiles for post-processing.

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.h:135-137
+                   const ContextSampleCounterMap &&Counters)
+      : ProfileGeneratorBase(Binary, std::move(Counters)){};
+  ProfileGenerator(ProfiledBinary *Binary, const SampleProfileMap &&Profiles)
----------------
hoy wrote:
> wenlei wrote:
> > Both `Reader->getSampleCounters()` and `Reader->getProfiles()` return by reference so there should be no copy, why do we need move ctor here? 
> This is because previously the `SampleCounters` field is a reference and it must be initialized in constructors. I'm now changing it to a value field and using the move constructor to initialize it when needed.
Ok, move still has some cost as a new object is constructed. In this case can we just use pointer type instead? 

IIUC, the change is to allow them to be "optional" depending on whether input is perf profile or llvm profile, for that it seems pointer which is nullable fits well.. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121655/new/

https://reviews.llvm.org/D121655