[PATCH] Clang changes for indirect call target profiling
David
davidxl at google.com
Wed May 20 13:12:12 PDT 2015
================
Comment at: lib/CodeGen/CodeGenPGO.cpp:811
@@ +810,3 @@
+ Sum = 0;
+ OutputData.clear();
+ for (unsigned I = 0; I < IndirectTargetResults.size(); ++I) {
----------------
Why is clear needed here? Better asserting.
================
Comment at: lib/CodeGen/CodeGenPGO.cpp:812
@@ +811,3 @@
+ OutputData.clear();
+ for (unsigned I = 0; I < IndirectTargetResults.size(); ++I) {
+ llvm::InstrProfValueRecord *Current = &IndirectTargetResults[I];
----------------
Is there a better way than linear search here?
================
Comment at: lib/CodeGen/CodeGenPGO.cpp:858
@@ +857,3 @@
+ if (PGOReader && haveRegionCounts()) {
+ // We record the top most called five function at each call site.
+ // Profile metadata contains "indirect_call_targets" string identifying
----------------
5 is too large as the default value. I suggest change the default to 2 targets and also make it controllable via an internal option.
================
Comment at: lib/CodeGen/CodeGenPGO.cpp:867
@@ +866,3 @@
+ std::multimap<uint64_t, std::string>::reverse_iterator I =
+ IndirectCallSiteData.rbegin(), E = IndirectCallSiteData.rend();
+
----------------
Should you bypass the rest when the result map is empty or sum is zero ?
================
Comment at: lib/CodeGen/CodeGenPGO.cpp:871
@@ +870,3 @@
+ SmallVector<llvm::Metadata*, 2> Vals;
+ Vals.push_back(MDHelper.createString("indirect_call_targets"));
+ Vals.push_back(MDHelper.createConstant(
----------------
Perhaps using shorter string to save memory usage.
================
Comment at: lib/CodeGen/CodeGenPGO.cpp:874
@@ +873,3 @@
+ llvm::ConstantInt::get(llvm::Type::getInt64Ty(Ctx), Sum)));
+ for (unsigned Count = 0; (I != E) && (Count < 5); ++I, ++Count) {
+ Vals.push_back(MDHelper.createString(I->second));
----------------
Do not use hard coded number 5 here.
http://reviews.llvm.org/D8940
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the cfe-commits
mailing list