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

[Wenlei He via Phabricator via llvm-commits]

wenlei added a comment.

Thanks for refactoring, looks good. and we can deal with other format and profiled symbol list later in separate patch. Two more comments inline.



================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:220
+  // Filter out invalid negative(int type) lineOffset
+  if (LeafLoc.Callsite.LineOffset & 0x80000000)
+    return;
----------------
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. 


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109551