[PATCH] D15306: [llvm-profdata] Add support for weighted merge of profile data (2nd try)

Diego Novillo via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 14 12:52:15 PST 2015


dnovillo added a comment.

I like the new way of specifying weighted input better.  Thanks!  This LGTM with a couple of changes below.


================
Comment at: include/llvm/ProfileData/SampleProf.h:144
@@ -137,1 +143,3 @@
+    if (Weight > 1)
+      S = SaturatingMultiply(S, Weight);
     TargetSamples = SaturatingAdd(TargetSamples, S);
----------------
We should be indicating overflows somehow here.  Could you for now assert that an overflow hasn't happened?  Adding a FIXME for doing a more principled handling is fine for now.

================
Comment at: include/llvm/ProfileData/SampleProf.h:189
@@ +188,3 @@
+      Num = SaturatingMultiply(Num, Weight);
+    TotalSamples += Num;
+  }
----------------
Since we're using satmul, might as well change the addition to a saturating addition.


http://reviews.llvm.org/D15306





More information about the llvm-commits mailing list