[PATCH] D70653: [AutoFDO] Properly merge context-sensitive profile of inlinee back to outlined function

Wed Nov 27 10:18:24 PST 2019

wenlei marked an inline comment as done.
wenlei added a comment.

In D70653#1761978 <https://reviews.llvm.org/D70653#1761978>, @wmi wrote:

> I did performance test combing D70653 <https://reviews.llvm.org/D70653> and D70655 <https://reviews.llvm.org/D70655>, there was ~0.2% latency improvement in a benchmark, so that is good.

Thanks for code review and measurement. Glad to know that it helped your workload too.

================
Comment at: llvm/include/llvm/ProfileData/SampleProfReader.h:363
+  /// FunctionSamples if it doesn't exist.
+  FunctionSamples *getOrCreateSamplesFor(const Function &F) {
+    std::string FGUID;
----------------
wmi wrote:
> I found a potential problem here. When we create a new profile for a function when merging inline instance profile back, if the function somehow has no debug information, the annotation will fail and a warning will be issued. We may want to skip those functions without debug information (The warning is issued in SampleProfileLoader::getFunctionLoc)  
For these cases, if the function without debug info wasn't inlined, we would still get the warning, right? I though the warning in this case is no different and as helpful comparing to other cases, or did I miss anything? 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70653