[PATCH] D58832: [SampleFDO] add suffix elision control for fcn names

[Wei Mi via Phabricator via llvm-commits]

wmi added inline comments.


================
Comment at: include/llvm/ProfileData/SampleProf.h:420-429
+    } else if (Attr == "selected") {
+      StringRef Suffix(".llvm.");
+      StringRef Cand(F.getName());
+      auto It = Cand.rfind(Suffix);
+      if (It == StringRef::npos)
+        return Cand;
+      auto Dit = Cand.rfind('.');
----------------
It may be better to use a suffix array here so that it is easier for others to add new suffix later.


================
Comment at: include/llvm/ProfileData/SampleProf.h:433
+    } else {
+      assert(false);
+    }
----------------
Add an error message here: assert(false && "xxx")


================
Comment at: include/llvm/ProfileData/SampleProfReader.h:289-290
   FunctionSamples *getSamplesFor(const Function &F) {
     // The function name may have been updated by adding suffix. In sample
     // profile, the function names are all stripped, so we need to strip
     // the function name suffix before matching with profile.
----------------
Now that the function names are not necessarily all stripped, we want to update the comment accordingly. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58832