[PATCH] D14786: [llvm-profdata] Add merge() to InstrProfRecord

[Nathan Slingerland via llvm-commits]

slingn marked 3 inline comments as done.

================
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]);
----------------
As vsk commented separately, a for-range loop would be awkward to use here (accessing two vectors in parallel by index).

================
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])
----------------
Same issue here with parallel vector access by index.

================
Comment at: lib/ProfileData/InstrProfWriter.cpp:104
@@ -129,3 +103,3 @@
 
   auto Where = ProfileDataMap.find(I.Hash);
   if (Where == ProfileDataMap.end()) {
----------------
davidxl wrote:
> Better to do insert here and check if insertion actually happens or not.
Yes - much better. And also helps with addressing vsk's comment below.


http://reviews.llvm.org/D14786