[PATCH] D14845: [Support] Fix SaturatingMultiply<T>() to be correct (and fast), Re-enable Unit Tests
Nathan Slingerland via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 23 07:36:26 PST 2015
This revision was automatically updated to reflect the committed changes.
Closed by commit rL253870: [Support] Fix SaturatingMultiply<T>() to be correct (and fast), Re-enableā¦ (authored by slingn).
Changed prior to commit:
http://reviews.llvm.org/D14845?vs=40821&id=40932#toc
Repository:
rL LLVM
http://reviews.llvm.org/D14845
Files:
llvm/trunk/include/llvm/Support/MathExtras.h
llvm/trunk/unittests/Support/MathExtrasTest.cpp
Index: llvm/trunk/unittests/Support/MathExtrasTest.cpp
===================================================================
--- llvm/trunk/unittests/Support/MathExtrasTest.cpp
+++ llvm/trunk/unittests/Support/MathExtrasTest.cpp
@@ -207,4 +207,52 @@
SaturatingAddTestHelper<uint64_t>();
}
+template<typename T>
+void SaturatingMultiplyTestHelper()
+{
+ const T Max = std::numeric_limits<T>::max();
+
+ // Test basic multiplication.
+ EXPECT_EQ(T(6), SaturatingMultiply(T(2), T(3)));
+ EXPECT_EQ(T(6), SaturatingMultiply(T(3), T(2)));
+
+ // Test multiplication by zero.
+ EXPECT_EQ(T(0), SaturatingMultiply(T(0), T(0)));
+ EXPECT_EQ(T(0), SaturatingMultiply(T(1), T(0)));
+ EXPECT_EQ(T(0), SaturatingMultiply(T(0), T(1)));
+ EXPECT_EQ(T(0), SaturatingMultiply(Max, T(0)));
+ EXPECT_EQ(T(0), SaturatingMultiply(T(0), Max));
+
+ // Test multiplication by maximum value.
+ EXPECT_EQ(Max, SaturatingMultiply(Max, T(2)));
+ EXPECT_EQ(Max, SaturatingMultiply(T(2),Max));
+ EXPECT_EQ(Max, SaturatingMultiply(Max, Max));
+
+ // Test interesting boundary conditions for algorithm -
+ // ((1 << A) - 1) * ((1 << B) + K) for K in [-1, 0, 1]
+ // and A + B == std::numeric_limits<T>::digits.
+ // We expect overflow iff A > B and K = 1.
+ const int Digits = std::numeric_limits<T>::digits;
+ for (int A = 1, B = Digits - 1; B >= 1; ++A, --B) {
+ for (int K = -1; K <= 1; ++K) {
+ T X = (T(1) << A) - T(1);
+ T Y = (T(1) << B) + K;
+ bool OverflowExpected = A > B && K == 1;
+
+ if(OverflowExpected) {
+ EXPECT_EQ(Max, SaturatingMultiply(X, Y));
+ } else {
+ EXPECT_EQ(X * Y, SaturatingMultiply(X, Y));
+ }
+ }
+ }
+}
+
+TEST(MathExtras, SaturatingMultiply) {
+ SaturatingMultiplyTestHelper<uint8_t>();
+ SaturatingMultiplyTestHelper<uint16_t>();
+ SaturatingMultiplyTestHelper<uint32_t>();
+ SaturatingMultiplyTestHelper<uint64_t>();
+}
+
}
Index: llvm/trunk/include/llvm/Support/MathExtras.h
===================================================================
--- llvm/trunk/include/llvm/Support/MathExtras.h
+++ llvm/trunk/include/llvm/Support/MathExtras.h
@@ -671,12 +671,30 @@
template <typename T>
typename std::enable_if<std::is_unsigned<T>::value, T>::type
SaturatingMultiply(T X, T Y) {
- // Hacker's Delight, p. 30
- T Z = X * Y;
- if (Y != 0 && Z / Y != X)
- return std::numeric_limits<T>::max();
- else
- return Z;
+ // Hacker's Delight, p. 30 has a different algorithm, but we don't use that
+ // because it fails for uint16_t (where multiplication can have undefined
+ // behavior due to promotion to int), and requires a division in addition
+ // to the multiplication.
+
+ // Log2(Z) would be either Log2Z or Log2Z + 1.
+ // Special case: if X or Y is 0, Log2_64 gives -1, and Log2Z
+ // will necessarily be less than Log2Max as desired.
+ int Log2Z = Log2_64(X) + Log2_64(Y);
+ const T Max = std::numeric_limits<T>::max();
+ int Log2Max = Log2_64(Max);
+ if (Log2Z < Log2Max)
+ return X * Y;
+ if (Log2Z > Log2Max)
+ return Max;
+
+ // We're going to use the top bit, and maybe overflow one
+ // bit past it. Multiply all but the bottom bit then add
+ // that on at the end.
+ T Z = (X >> 1) * Y;
+ if (Z & ~(Max >> 1))
+ return Max;
+ Z <<= 1;
+ return (X & 1) ? SaturatingAdd(Z, Y) : Z;
}
extern const float huge_valf;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D14845.40932.patch
Type: text/x-patch
Size: 3350 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151123/76431c61/attachment.bin>
More information about the llvm-commits
mailing list