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

Wei Mi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 25 22:03:54 PDT 2020


wmi added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1065
+            auto MapName = Remapper->lookUpNameInProfile(F.getName());
+            if (MapName && CalleeFunctionName == *MapName)
+              continue;
----------------
wenlei wrote:
> wmi wrote:
> > wenlei wrote:
> > > Do we actually need this extra check? Renaming won't change wether a call is self-recursive, so the check on line 1059 should be good enough?
> > Line 1059 only checks whether an inline instance at the callsite has the identical name with the parent function F. It is possible that the inline instance has a different but equivalent name as F. In that case, SymbolMap may map the name of the inline instance back to the definition of F and use F as the indirect call promotion target. 
> > 
> > I can add one more check "R->getValue()->getName() != F.getName()" to the if (R != SymbolMap.end() ...) below and simplify the code a little bit. 
> I see. Or `R->getValue() != &F` would work too?
Yes, that is better.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D86332



More information about the llvm-commits mailing list