[PATCH] D14547: [llvm-profdata] Add support for weighted merge of profile data

[Nathan Slingerland via llvm-commits]

slingn marked 6 inline comments as done.

================
Comment at: lib/ProfileData/InstrProfWriter.cpp:143
@@ +142,3 @@
+                     Dest.Counts.begin(),
+                     std::bind1st(std::multiplies<uint64_t>(), Weight));
+    }
----------------
silvas wrote:
> Just use a lambda. Or I think you might be able to get away with this which is shorter:
> 
> ```
> for (auto &Count : Dest.Counts)
>   Count *= Weight;
> ```
> 
> Also, it seems like there are multiple places that the weight comes into play. Can you please ensure there is a Single Point Of Truth for where the weight is applied?
I have refactored things in the next patch set.

================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:61
@@ -56,1 +60,3 @@
+    const auto &Filename = *InputItr;
+    uint64_t Weight = *WeightItr;
     auto ReaderOrErr = InstrProfReader::create(Filename);
----------------
davidxl wrote:
> You can directly fetch the weight from the augmented file name.
It's still better in my mind to parse the weight in a separate method since that can be shared between instr and sampled merge - and is a clearer separation of concerns.

================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:69
@@ +68,3 @@
+      // Scale record counts by weight (if needed).
+      if (Weight > 1) {
+        std::transform(I.Counts.begin(), I.Counts.end(),
----------------
davidxl wrote:
> Why doing weight here?
Fixed. Yes - this approach was left over from a previous revision.


http://reviews.llvm.org/D14547