[PATCH] D90125: [CSSPGO] Infrastructure for context-sensitive Sample PGO and Inlining

[Wenlei He via Phabricator via llvm-commits]

wenlei marked 11 inline comments as done.
wenlei added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleContextTracker.cpp:61-65
+ContextTrieNode *
+ContextTrieNode::getOrCreateChildContext(const LineLocation &CallSite,
+                                         StringRef CalleeName) {
+  return getOrCreateChildContext(CallSite, CalleeName, true);
+}
----------------
wmi wrote:
> Make param AllowCreate default to true so we don't need this wrapper?
Good catch, changed.


================
Comment at: llvm/lib/Transforms/IPO/SampleContextTracker.cpp:373
+
+ContextTrieNode *SampleContextTracker::getContextFor(const DILocation *DIL) {
+  assert(DIL);
----------------
wmi wrote:
> A question, the context in SampleContextTracker includes not only inline stack but also call stack. S vector below only contains the inline stack at the DIL location. How can it match with the full stack starting from RootContext?  
When we decided to not inline a call site, context profile will be promoted to root, so what remains in context tracker should reflect the accurate remainder context profile. E.g. if we start with A->(call) B->(inline) C in context tracker. At some point if we're looking at B->C from DIL, there're two scenarios:

1. If A inlined B, in this case, we wouldn't be able to match B->C from DIL to anything in context tracker. But this is intentional and desired, because The remainder/base profile for B, or the context profile B->C shouldn't have anything if A->B inline happened. 
2. If A not inlined B, in this case, B->C should be moved/promoted from child of A to be under root. Then we would be able to match B->C from DIL to B->C (under root) in context tracker. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90125