[PATCH] D159169: [CSSPGO] Refactoring findIRAnchors

[Lei Wang via Phabricator via llvm-commits]

wlei added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:2162-2164
+          LineLocation IRCallsite = FunctionSamples::getCallSiteIdentifier(DIL);
+          StringRef CalleeName = FindCalleeName(dyn_cast<CallBase>(&I));
+          IRAnchors.emplace(IRCallsite, CalleeName);
----------------
wlei wrote:
> wenlei wrote:
> > Actually can this be unified with `FindTopLevelFrame`? For not inlined calls, `DIL->getSubprogramLinkageName` should be equivalent to `FindCalleeName(dyn_cast<CallBase>(&I))`?
> Good point, but to clarify:
> `DIL->getSubprogramLinkageName` is the current/top-level Frame. 
> `FindCalleeName` is the current callee name, it's the second top Frame.
> 
> Maybe my variable name is misleading in the new patch. Here is actually not `FindTopLevelFrame`, For example: "main:1 @ foo", the term "top-level" frame is "main:1", but here what it returns is {1, foo} , i,e {the top-level frame's location, the second top's frame's func name}
> 
> uh..I need to fix it.
> 
> That said, maybe we still can merge `FindTopLevelFrame` and `FindCalleeName `, like check if it's inlined code inside of the lambda, let me think.
> 
OK.. I'm done one version, please take a look https://reviews.llvm.org/D159221

I ran into some bugs and spent some times debugging, the tricky parts are :

1) For pseudo-probe mode, a) for inline code, we need to use `getCallSiteIdentifier` to extract the callsite but b) for non-inlined code, we need to use the probe id.

2) The pseudo instruction is a `CallBase` and the `getCalledFunction` will return a non-empty callee name "llvm.pseudoprobe" which actually should be a empty StringRef.

I then made it work, but as it introduces more complexity, maybe it's still not what we want, any thoughts on this?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159169