[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