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

[Lei Wang via Phabricator via llvm-commits]

wlei added a comment.

In D109551#3021298 <https://reviews.llvm.org/D109551#3021298>, @wenlei wrote:

> I just looked through features we have in internal tool, here's a list of things we need in addition to the basic support here and duplication factor. We can port them in later patches.
>
> - profiled symbol list as mentioned.
> - coroutine support to attribute sample on coroutine funclets back to main function.
> - split function support to handle profiling on BOLT binaries.
> - heuristic to compute and report profile density and make profiling suggestion.

Patches are on the way, thanks for the summary.



================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:220
+  // Filter out invalid negative(int type) lineOffset
+  if (LeafLoc.Callsite.LineOffset & 0x80000000)
+    return;
----------------
wenlei wrote:
> This might be a behavior change for AutoFDO. I did see negative offsets in the past - if the code is from macro expansion which is defined before the function start. We would not have any profile if we filter them out. 
The invalid number will be fixed in https://reviews.llvm.org/D110081. As we discuss offline, we might have the invalid case in non-macro case.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.h:34
+      : Binary(Binary), SampleCounters(Counters){};
+  virtual ~ProfileGeneratorBase() = default;
+  static std::unique_ptr<ProfileGeneratorBase>
----------------
wenlei wrote:
> why do we need this? 
We need a virtual destructor for the inheritance class.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109551