[PATCH] D14845: [Support] Fix SaturatingMultiply<T>() to be correct (and fast), Re-enable Unit Tests

Richard Smith via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 20 11:39:50 PST 2015


rsmith added inline comments.

================
Comment at: unittests/Support/MathExtrasTest.cpp:220
@@ +219,3 @@
+  EXPECT_EQ(Max, SaturatingMultiply(Max, Max));
+}
+
----------------
It'd be useful to also test cases like `((1 << A) - 1) * ((1 << B) + K)`, for `K` in [-1, 0, 1] and `A + B == numeric_limits<T>::digits`, as those are interesting transition cases for the new algorithm (you get overflow iff `A > B` and `K = 1`). More generally, testcases where overflow //almost// happens or //only just// happens would be good here.

Also, coverage for `0 * Max` and `0 * 0` would be good.


http://reviews.llvm.org/D14845





More information about the llvm-commits mailing list