[PATCH] D110466: [llvm-profgen][CSSPGO] On-demand function size computation for preinliner

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:430
+  if (Binary->getTrackFuncContextSize())
+    computeFuncSizeForSampleHitFunction();
+
----------------
wenlei wrote:
> hoy wrote:
> > Can this be done when preinliner is enabled?
> See ctor of ProfiledBinary, TrackFuncContextSize = EnableCSPreInliner && UseContextCostForPreInliner, 
Sounds good, thanks for pointing it out.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:447
+      uint64_t RangeStartOffset = Item.first.first;
+      auto FuncRange = Binary->findFuncOffsetRange(RangeStartOffset);
+      if (FuncRange.second != 0)
----------------
wenlei wrote:
> hoy wrote:
> > Does this include ranges not executed? Those ranges should also be counted for size calculation.
> Yes, not executed range is included by findFuncOffsetRange - that gives the function start, end for the containing top level inliner.
I see. This converts each lbr range to its corresponding function range, so should cover the whole function.




================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.cpp:554
+  InstructionPointer IP(this, StartAddr, true);
+  while (IP.Address < EndAddr) {
+    uint64_t Offset = virtualAddrToOffset(IP.Address);
----------------
wenlei wrote:
> Is the EndAddr inclusive for function end? What is we have a 1-byte instr at EndAddr?
EndAddr should be the start of the next function, as computed in ProfiledBinary::dissassembleSymbol. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110466