[PATCH] D14786: [llvm-profdata] Add merge() to InstrProfRecord
Vedant Kumar via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 18 14:03:51 PST 2015
vsk added a subscriber: vsk.
vsk added a comment.
Thanks, some inline comments --
================
Comment at: include/llvm/ProfileData/InstrProf.h:323
@@ +322,3 @@
+ instrprof_error mergeValueProfData(uint32_t ValueKind,
+ InstrProfRecord &Src) {
+ uint32_t ThisNumValueSites = getNumValueSites(ValueKind);
----------------
clang-format?
================
Comment at: include/llvm/ProfileData/InstrProf.h:332
@@ +331,3 @@
+ Src.getValueSitesForKind(ValueKind);
+ for (uint32_t I = 0; I < ThisNumValueSites; I++)
+ ThisSiteRecords[I].mergeValueData(OtherSiteRecords[I]);
----------------
Can you rangify this loop while we're at it?
================
Comment at: include/llvm/ProfileData/InstrProf.h:417
@@ +416,3 @@
+
+ for (size_t I = 0, E = Other.Counts.size(); I < E; ++I) {
+ if (Counts[I] + Other.Counts[I] < Counts[I])
----------------
Rangify this one too.
================
Comment at: lib/ProfileData/InstrProfWriter.cpp:107
@@ -132,7 +106,3 @@
// We've never seen a function with this name and hash, add it.
- ProfileDataMap[I.Hash] = I;
-
- // We keep track of the max function count as we go for simplicity.
- if (I.Counts[0] > MaxFunctionCount)
- MaxFunctionCount = I.Counts[0];
- return instrprof_error::success;
+ Where = ProfileDataMap.insert(std::make_pair(I.Hash, I)).first;
+ } else {
----------------
Is there a need to keep the iterator object around, since we have the value?
http://reviews.llvm.org/D14786
More information about the llvm-commits
mailing list