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

[Jake VanAdrighem via llvm-commits]

jakev added a comment.

Just a couple nits from me.


================
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 "
----------------
Does this need ZeroOrMore?

================
Comment at: lib/Transforms/Instrumentation/PGOInstrumentation.cpp:122
@@ +121,3 @@
+    "comdat-append-hash", cl::init(true), cl::Hidden, cl::ZeroOrMore,
+    cl::desc("Append functin hash to the name of COMDAT function to avoid "
+             "function hash mismatch due to the preinliner"));
----------------
functin -> function

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

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

================
Comment at: lib/Transforms/Instrumentation/PGOInstrumentation.cpp:888
@@ -804,1 +887,3 @@
 
+// Collect the set of members for each comdat.in module M and store
+// in ComdatMembers.
----------------
comdat.in -> comdat in


https://reviews.llvm.org/D22600