[PATCH] D14845: [Support] Fix SaturatingMultiply<T>() to never cause overflow, Re-enable Unit Tests
Nathan Slingerland via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 20 10:48:43 PST 2015
slingn updated this revision to Diff 40803.
slingn added a comment.
Updated with Richard Smith's patch for SaturatingMultiply().
http://reviews.llvm.org/D14845
Files:
include/llvm/Support/MathExtras.h
unittests/Support/MathExtrasTest.cpp
Index: unittests/Support/MathExtrasTest.cpp
===================================================================
--- unittests/Support/MathExtrasTest.cpp
+++ unittests/Support/MathExtrasTest.cpp
@@ -207,4 +207,23 @@
SaturatingAddTestHelper<uint64_t>();
}
+template<typename T>
+void SaturatingMultiplyTestHelper()
+{
+ const T Max = std::numeric_limits<T>::max();
+ EXPECT_EQ(T(0), SaturatingMultiply(T(1), T(0)));
+ EXPECT_EQ(T(0), SaturatingMultiply(T(0), T(1)));
+ EXPECT_EQ(T(6), SaturatingMultiply(T(2), T(3)));
+ EXPECT_EQ(Max, SaturatingMultiply(Max, T(2)));
+ EXPECT_EQ(Max, SaturatingMultiply(T(2),Max));
+ EXPECT_EQ(Max, SaturatingMultiply(Max, Max));
+}
+
+TEST(MathExtras, SaturatingMultiply) {
+ SaturatingMultiplyTestHelper<uint8_t>();
+ SaturatingMultiplyTestHelper<uint16_t>();
+ SaturatingMultiplyTestHelper<uint32_t>();
+ SaturatingMultiplyTestHelper<uint64_t>();
+}
+
}
Index: include/llvm/Support/MathExtras.h
===================================================================
--- include/llvm/Support/MathExtras.h
+++ 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.40803.patch
Type: text/x-patch
Size: 2314 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151120/413e4e27/attachment.bin>
More information about the llvm-commits
mailing list