[PATCH] D68601: [SampleFDO] Add indexing for function profiles so they can be loaded on demand in ExtBinary format

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: llvm/include/llvm/ProfileData/SampleProfReader.h:554
+  /// Collect functions to be used when compiling Module \p M.
+  void collectFuncsToUse(const Module &M) override;
 };
----------------
Nit: collectFuncsFrom(const Module &M)


================
Comment at: llvm/lib/ProfileData/SampleProfReader.cpp:507
+  for (auto &F : M) {
+    StringRef CanonName = FunctionSamples::getCanonicalFnName(F);
+    FuncsToUse.insert(CanonName);
----------------
Skip declarations?


================
Comment at: llvm/lib/ProfileData/SampleProfReader.cpp:533
+std::error_code SampleProfileReaderExtBinary::readFuncProfiles(uint64_t Size) {
+  const uint8_t *Start = Data;
+  if (UseAllFuncs) {
----------------
End = Data  + Size


================
Comment at: llvm/lib/ProfileData/SampleProfReader.cpp:536
+    while (Data < Start + Size) {
+      if (std::error_code EC = readFuncProfile())
+        return EC;
----------------
It is more readable if readFuncProfile is taking  the pointer to the data address:
     readFuncProfile(&Data);


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68601