[PATCH] D28416: [PGO] Turn off comdat renaming in IR PGO by default

David Li via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 10 09:58:07 PST 2017


davidxl added inline comments.


================
Comment at: lib/ProfileData/InstrProf.cpp:839
+    return false;
+  if (!needsComdatForCounter(F, *(F.getParent())))
+    return false;
----------------
Given the name of the function, it should probably check address taken bit as well.


================
Comment at: lib/Transforms/Instrumentation/InstrProfiling.cpp:286
+  uint64_t FuncHash = Inc->getHash()->getZExtValue();
+  // No hash: don't appent it to the profile variable.
+  if (FuncHash == 0)
----------------
appent --> append


================
Comment at: lib/Transforms/Instrumentation/InstrProfiling.cpp:286
+  uint64_t FuncHash = Inc->getHash()->getZExtValue();
+  // No hash: don't appent it to the profile variable.
+  if (FuncHash == 0)
----------------
davidxl wrote:
> appent --> append
why there is on hash? or it is hash of a trivial function?


================
Comment at: lib/Transforms/Instrumentation/PGOInstrumentation.cpp:410
     std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers) {
-  if (F.getName().empty())
+  if (!canRenameComdatFunc(F))
     return false;
----------------
It is clearer to check DoComdatRenaming here as well.


https://reviews.llvm.org/D28416





More information about the llvm-commits mailing list