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

Sean Silva via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 10 22:27:47 PST 2015


silvas added inline comments.

================
Comment at: include/llvm/ProfileData/SampleProf.h:194
@@ +193,3 @@
+  /// Optionally specify a sample weight \p Weight.
+    void merge(const SampleRecord &Other, uint64_t Weight = 1) {
+    addSamples(Other.getSamples(), Weight);
----------------
Why did this suddenly become unaligned with the comment? (I suggest setting up clang-format).

================
Comment at: lib/ProfileData/InstrProfWriter.cpp:143
@@ +142,3 @@
+                     Dest.Counts.begin(),
+                     std::bind1st(std::multiplies<uint64_t>(), Weight));
+    }
----------------
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?


http://reviews.llvm.org/D14547





More information about the llvm-commits mailing list