[PATCH] D86332: [SampleFDO] Enhance profile remapping support for inline instance

[Wei Mi via Phabricator via llvm-commits]

wmi added inline comments.


================
Comment at: llvm/lib/ProfileData/SampleProf.cpp:218-222
+  // with equivalent CalleeName using remapping rules. To avoid mixing
+  // profile inline instances at one callsite with inline instances at
+  // other callsites, the CalleeRemapper has to be cleared everytime
+  // before it is used, however the remapping table containing the
+  // remapping rules in the CalleeRemapper will be kept in clearing.
----------------
wenlei wrote:
> I guess I didn't fully understand the clearing part - if a function (inlinee) is renamed, why do we not want to apply the remapping globally? E.g. for A->B, and C->B inlined profile, if B is renamed to D, we would want to match D to B for both A->D profile and C->D path, right? Then a global remapper including all names should suffice.
> 
> Could you share a concrete example as to why each function needs its own temp remapper for all its inlinees? 
> 
Right, given two names B and D, ideally we want the remapper to tell us whether they are identical by returning true or false for remapper.equivalent(B, D). However, remapper doesn't work like this. To check whether B and D are identical, remapper needs to insert B into a foldingset first by parsing it, then check whether D has already existed in it by parsing D and trying to insert D (but not truely insert it). 

Similarly, if we want to check whether an equivalent name exists in a nameset, remapper need to insert alll the names in the nameset before it can answer the query. So to answer the query whether the callee's name matches an inline instance profile at a specific callsite, it needs to clear names in the remapper inserted at other callsites before it inserts the callee's name at the current callsite into remapper.  

Existing remapper doesn't have to be cleared throughout the sample profile loading pass because it insert all the names of outline instance in the sample profile, and the question it needs to answer in the whole pass is whether a function name has a match in the outline instances. So because of the way how remapper works, we need different remappers to answer different types of remapping queries. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D86332