[PATCH] D48105: [llvm][Instrumentation] Add Call Graph Profile pass
David Li via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 21 19:48:27 PDT 2018
davidxl added a comment.
I am with Chandler here. Adding this pass to the legacy PM only will slow down the eventual migration to the new PM. It is better to do the new PM or both.
================
Comment at: llvm/trunk/lib/Transforms/Instrumentation/CGProfile.cpp:52
+ for (auto &F : M) {
+ if (F.isDeclaration())
+ continue;
----------------
You can check the existence of function entry count to decide whether to skip function earlier.
================
Comment at: llvm/trunk/lib/Transforms/Instrumentation/CGProfile.cpp:61
+ for (const auto &I : BB) {
+ auto *CI = dyn_cast<CallInst>(&I);
+ if (!CI)
----------------
Use CallSite interface instead so that InvokeInst is handled as well:
CallSite CS(&I);
if (!CS) continue;
Chandler has plan to unify the CallInst and InvokeInst in the future, but for now, CallSIte is the way to go.
================
Comment at: llvm/trunk/lib/Transforms/Instrumentation/CGProfile.cpp:65
+ Function *CalledF = CI->getCalledFunction();
+ if (!CalledF || CalledF->isIntrinsic())
+ continue;
----------------
you may want to check TTI to see if isLoweredToCall returns true.
================
Comment at: llvm/trunk/lib/Transforms/Instrumentation/CGProfile.cpp:68
+
+ uint64_t &Count = Counts[std::make_pair(&F, CalledF)];
+ Count = SaturatingAdd(Count, *BBCount);
----------------
You may also check indirect callsite to see if it has value profile data (indirect call target). If it has, you can create edges to the targets as well.
(I assume this information is for function layout purpose by the linker).
Repository:
rL LLVM
https://reviews.llvm.org/D48105
More information about the llvm-commits
mailing list