[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/Transforms/IPO/SampleProfile.cpp:1065
+            auto MapName = Remapper->lookUpNameInProfile(F.getName());
+            if (MapName && CalleeFunctionName == *MapName)
+              continue;
----------------
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. 


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1939
+    // Insert the remapped names into SymbolMap.
+    if (auto Remapper = Reader->getRemapper()) {
+      if (auto MapName = Remapper->lookUpNameInProfile(OrigName)) {
----------------
wenlei wrote:
> hoist `Remapper = Reader->getRemapper()` out of the loop?
Good point.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D86332