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

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/ProfileData/InstrProfWriter.cpp:108
@@ -104,3 +107,3 @@
   for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind) {
     if (std::error_code EC = Dest.mergeValueProfData(Kind, Source))
       return EC;
----------------
value Profile data needs to be weight aware too.

================
Comment at: lib/ProfileData/InstrProfWriter.cpp:140
@@ +139,3 @@
+    if (Weight > 1) {
+      // multiply each count in Dest by Weight
+      std::transform(Dest.Counts.begin(), Dest.Counts.end(),
----------------
nit: Multiply ... by Weight.

================
Comment at: test/tools/llvm-profdata/weight-sample.test:42
@@ +41,3 @@
+3- Bad merge: foo and bar profiles with too few weight values
+RUN: not llvm-profdata merge --sample --text -weights=2 %p/Inputs/weight-sample-bar.proftext %p/Inputs/weight-sample-foo.proftext -o %t.out 2>&1 | FileCheck %s --check-prefix=ERROR1
+ERROR1: error: Profile weight must be specified for each input file.
----------------
No need for such tests if the weight is always attached to the input file.

================
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);
----------------
You can directly fetch the weight from the augmented file name.

================
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(),
----------------
Why doing weight here?

================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:74
@@ -62,2 +73,3 @@
+      }
       if (std::error_code EC = Writer.addRecord(std::move(I)))
         errs() << Filename << ": " << I.Name << ": " << EC.message() << "\n";
----------------
Pass the weight to addRecord method instead.


http://reviews.llvm.org/D14547