[PATCH] D22600: [PGO] Fix profile mismatch in Comdat function with pre-inliner

Rong Xu via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 21 11:23:44 PDT 2016


xur marked 5 inline comments as done.

================
Comment at: lib/Transforms/Instrumentation/PGOInstrumentation.cpp:121
@@ +120,3 @@
+static cl::opt<bool> ComdatAppendHash(
+    "comdat-append-hash", cl::init(true), cl::Hidden, cl::ZeroOrMore,
+    cl::desc("Append functin hash to the name of COMDAT function to avoid "
----------------
jakev wrote:
> Does this need ZeroOrMore?
Should be Optional. Removed ZeroOrMore.

================
Comment at: lib/Transforms/Instrumentation/PGOInstrumentation.cpp:286
@@ -269,1 +285,3 @@
     computeCFGHash();
+    if (ComdatAppendHash && ComdatMembers.size())
+      renameComdatFunction();
----------------
jakev wrote:
> Can this just be ComdatMembers.size()?
good suggestion. changed.

================
Comment at: lib/Transforms/Instrumentation/PGOInstrumentation.cpp:325
@@ +324,3 @@
+// Check if we can safely rename this Comdat function.
+static bool canComdatRename(
+    Function &F,
----------------
jakev wrote:
> Can this be called 'canRenameComdat'?
changed.

================
Comment at: lib/Transforms/Instrumentation/PGOInstrumentation.cpp:988
@@ -881,2 +987,3 @@
   std::vector<Function *> ColdFunctions;
+  InstrProfSummaryBuilder Builder(ProfileSummaryBuilder::DefaultCutoffs);
   for (auto &F : M) {
----------------
silvas wrote:
> Is this variable `Builder` needed?
This is from a leftover of earlier merge. It should not be here. Thanks for catching this.


https://reviews.llvm.org/D22600





More information about the llvm-commits mailing list