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

[Wenlei He via Phabricator via llvm-commits]

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


================
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:
> wmi wrote:
> > wenlei wrote:
> > > 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? 
> > Although without the patch, we may also see the warning because of the source drift or compiler upgrade, in reality we   rarely see that happens. I am worried that the patch may increase the chance of such warning and break many builds.
> > 
> > So I will look into it and see what causes the warning and whether it is a rare case or not.      
> I looked into it and found the case was not rare.
> 
> For the function without debug information, in afdo profile there was no inline instance. The function was added to localNotInlinedCallSites because findFunctionSamplesAt returned a function sample as long as the location matched (the imprecise matching is designed for indirect promotion). In this case, the FunctionSamples returned didn't belong to the function without debug information.
> 
> I applied file specific -Wno-error to disable the warning whenever I saw a case like this in a large benchmark build, and I found the same problem in at least three files (If I continue the process, I may find more).
> 
> To solve the problem, maybe check debug information in findFunctionSamplesAt before return a FunctionSamples when callee name doesn't match the name in FunctionSamples.  If debug information doesn't exists for a function, inlining it won't bring more context sensitive information because no profile annotation is possible. 
Thanks for looking! We only call `getOrCreateSamplesFor` when a callee is found from call instruction, i.e., it's a direct call. So in this case, it seems a direct call's callee got samples from a different function (inlinee)? Sounds like it can be caused by source change? 

If imprecise matching is only meant for indirect call, I can turn it off for direct call - inside `findFunctionSamplesAt`, only do the fuzzy matching when `CalleeName` is empty string. 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70653