[PATCH] D109551: [AutoFDO][llvm-profgen] Profile generation for LBR(non-CS) sample

[Lei Wang via Phabricator via llvm-commits]

wlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:253
+    if (Ret.second) {
+      Ret.first->second.setName(FrameVec[I].CallerName);
+    }
----------------
hoy wrote:
> nit: use `setContext`. It's also consistent with `getTopLevelFunctionProfile`.
Done!


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:280
+    // can be Addr1+1 to Addr2-1. We should ignore such range.
+    if (IP.Address > RangeEnd)
+      continue;
----------------
hoy wrote:
> This is covered by the following while loop?
Good catch, also remove the one in CSProfileGenerator.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:308
+    StringRef CalleeName = FunctionSamples::getCanonicalFnName(
+        Binary->getFuncFromStartOffset(TargetOffset));
+    if (CalleeName.size() == 0)
----------------
hoy wrote:
> I just noticed that this call may result in empty entries populated in `FuncStartAddrMap`. Maybe refactor it to use `find`?
> 
>      StringRef getFuncFromStartOffset(uint64_t Offset) {
>         return FuncStartAddrMap[Offset];
>      }
Good catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109551