[PATCH] D14845: [Support] Fix SaturatingMultiply<T>() to never cause overflow, Re-enable Unit Tests

[Nathan Slingerland via llvm-commits]

slingn created this revision.
slingn added reviewers: davidxl, silvas.
slingn added a subscriber: llvm-commits.

This change improves the SaturatingMultiply<T>() function template to not trigger unsigned overflow.

This has the benefit of working around the issue of -fsanitizer=undefined detecting signed integer overflow for SaturatingMultiply() with T=uint16_t.
For this reason the SaturatingMultiply unit tests disabled in r253539 are also re-enabled as part of this change.

Tested with -DLLVM_USE_SANITIZER="Undefined".

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,12 @@
 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;
+  const T Max = std::numeric_limits<T>::max();
+  if (Y != 0 && X >= Max / Y) {
+    return Max;
+  } else {
+    return X * Y;
+  }
 }
 
 extern const float huge_valf;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D14845.40709.patch
Type: text/x-patch
Size: 1541 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151119/0ce1b562/attachment.bin>