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

Wenlei He via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 25 20:40:25 PDT 2020


wenlei added a comment.

Great - thanks for making the changes. Looks good with two minor comments.



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


================
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)) {
----------------
hoist `Remapper = Reader->getRemapper()` out of the loop?


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