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

[Lei Wang via Phabricator via llvm-commits]

wlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:443
+void CSProfileGenerator::computeFuncSizeForSampleHitFunction() {
+  std::unordered_map<uint64_t, uint64_t> FuncRanges;
+  for (const auto &CI : SampleCounters) {
----------------
wenlei wrote:
> Some comment here would be helpful. Specifically what's in the map, and what computeFuncSizeForRange does..  I think probably computeInlinedContextSizeForRange is probably better name for computeFuncSizeForRange. (saying this because if the map contains start and end offset for a function, then compute size from given start and end offset seems weird..)
comments added.

> computeInlinedContextSizeForRange

That's better name, thanks for the suggestion!


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.cpp:554
+  InstructionPointer IP(this, StartAddr, true);
+  while (IP.Address < EndAddr) {
+    uint64_t Offset = virtualAddrToOffset(IP.Address);
----------------
hoy wrote:
> 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. 
Yeah, in `dissassembleSymbol ` the logic is like

```
  uint64_t Offset = StartOffset;
  while (Offset < EndOffset) { 
    ...
    Offset += Size;
   }

```

so it guarantees that the end `Offset` in this function(the last `IP.Address`) won't be `EndOffset`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110466