<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 23, 2015 at 3:20 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Is there a reason you used overloading instead of default arguments? I.e. instead of just declaring the ResultOverflowed variable as `bool *ResultOverflowed = nullptr`.</div></blockquote><div><br></div><div>(I don't really care either way, but `bool *ResultOverflowed = nullptr` seems to be the more common way to do this)</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><span class=""><font color="#888888"><br><div><br></div><div>-- Sean Silva</div></font></span></div><div class=""><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 23, 2015 at 1:54 PM, Nathan Slingerland via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: slingn<br>
Date: Mon Nov 23 15:54:22 2015<br>
New Revision: 253921<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=253921&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=253921&view=rev</a><br>
Log:<br>
[Support] Add optional argument to SaturatingAdd() and SaturatingMultiply() to indicate that overflow occurred<br>
<br>
Summary: Adds the ability for callers to detect when saturation occurred on the result of saturating addition/multiplication.<br>
<br>
Reviewers: davidxl, silvas, rsmith<br>
<br>
Subscribers: llvm-commits<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D14931" rel="noreferrer" target="_blank">http://reviews.llvm.org/D14931</a><br>
<br>
Modified:<br>
llvm/trunk/include/llvm/Support/MathExtras.h<br>
llvm/trunk/unittests/Support/MathExtrasTest.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/Support/MathExtras.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/MathExtras.h?rev=253921&r1=253920&r2=253921&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/MathExtras.h?rev=253921&r1=253920&r2=253921&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/Support/MathExtras.h (original)<br>
+++ llvm/trunk/include/llvm/Support/MathExtras.h Mon Nov 23 15:54:22 2015<br>
@@ -655,46 +655,79 @@ inline int64_t SignExtend64(uint64_t X,<br>
<br>
/// \brief Add two unsigned integers, X and Y, of type T.<br>
/// Clamp the result to the maximum representable value of T on overflow.<br>
+/// ResultOverflowed indicates if the result is larger than the maximum<br>
+/// representable value of type T.<br>
template <typename T><br>
typename std::enable_if<std::is_unsigned<T>::value, T>::type<br>
-SaturatingAdd(T X, T Y) {<br>
+SaturatingAdd(T X, T Y, bool &ResultOverflowed) {<br>
// Hacker's Delight, p. 29<br>
T Z = X + Y;<br>
- if (Z < X || Z < Y)<br>
+ ResultOverflowed = (Z < X || Z < Y);<br>
+ if (ResultOverflowed)<br>
return std::numeric_limits<T>::max();<br>
else<br>
return Z;<br>
}<br>
<br>
+/// \brief Add two unsigned integers, X and Y, of type T.<br>
+/// Clamp the result to the maximum representable value of T on overflow.<br>
+template <typename T><br>
+typename std::enable_if<std::is_unsigned<T>::value, T>::type<br>
+SaturatingAdd(T X, T Y) {<br>
+ bool ResultOverflowed;<br>
+ return SaturatingAdd(X, Y, ResultOverflowed);<br>
+}<br>
+<br>
/// \brief Multiply two unsigned integers, X and Y, of type T.<br>
/// Clamp the result to the maximum representable value of T on overflow.<br>
+/// ResultOverflowed indicates if the result is larger than the maximum<br>
+/// representable value of type T.<br>
template <typename T><br>
typename std::enable_if<std::is_unsigned<T>::value, T>::type<br>
-SaturatingMultiply(T X, T Y) {<br>
+SaturatingMultiply(T X, T Y, bool &ResultOverflowed) {<br>
// Hacker's Delight, p. 30 has a different algorithm, but we don't use that<br>
// because it fails for uint16_t (where multiplication can have undefined<br>
// behavior due to promotion to int), and requires a division in addition<br>
// to the multiplication.<br>
<br>
+ ResultOverflowed = false;<br>
+<br>
// Log2(Z) would be either Log2Z or Log2Z + 1.<br>
// Special case: if X or Y is 0, Log2_64 gives -1, and Log2Z<br>
// will necessarily be less than Log2Max as desired.<br>
int Log2Z = Log2_64(X) + Log2_64(Y);<br>
const T Max = std::numeric_limits<T>::max();<br>
int Log2Max = Log2_64(Max);<br>
- if (Log2Z < Log2Max)<br>
+ if (Log2Z < Log2Max) {<br>
return X * Y;<br>
- if (Log2Z > Log2Max)<br>
+ }<br>
+ if (Log2Z > Log2Max) {<br>
+ ResultOverflowed = true;<br>
return Max;<br>
+ }<br>
<br>
// We're going to use the top bit, and maybe overflow one<br>
// bit past it. Multiply all but the bottom bit then add<br>
// that on at the end.<br>
T Z = (X >> 1) * Y;<br>
- if (Z & ~(Max >> 1))<br>
+ if (Z & ~(Max >> 1)) {<br>
+ ResultOverflowed = true;<br>
return Max;<br>
+ }<br>
Z <<= 1;<br>
- return (X & 1) ? SaturatingAdd(Z, Y) : Z;<br>
+ if (X & 1)<br>
+ return SaturatingAdd(Z, Y, ResultOverflowed);<br>
+<br>
+ return Z;<br>
+}<br>
+<br>
+/// \brief Multiply two unsigned integers, X and Y, of type T.<br>
+/// Clamp the result to the maximum representable value of T on overflow.<br>
+template <typename T><br>
+typename std::enable_if<std::is_unsigned<T>::value, T>::type<br>
+SaturatingMultiply(T X, T Y) {<br>
+ bool ResultOverflowed;<br>
+ return SaturatingMultiply(X, Y, ResultOverflowed);<br>
}<br>
<br>
extern const float huge_valf;<br>
<br>
Modified: llvm/trunk/unittests/Support/MathExtrasTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/MathExtrasTest.cpp?rev=253921&r1=253920&r2=253921&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/MathExtrasTest.cpp?rev=253921&r1=253920&r2=253921&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/Support/MathExtrasTest.cpp (original)<br>
+++ llvm/trunk/unittests/Support/MathExtrasTest.cpp Mon Nov 23 15:54:22 2015<br>
@@ -194,10 +194,27 @@ template<typename T><br>
void SaturatingAddTestHelper()<br>
{<br>
const T Max = std::numeric_limits<T>::max();<br>
+ bool ResultOverflowed;<br>
+<br>
EXPECT_EQ(T(3), SaturatingAdd(T(1), T(2)));<br>
+ EXPECT_EQ(T(3), SaturatingAdd(T(1), T(2), ResultOverflowed));<br>
+ EXPECT_FALSE(ResultOverflowed);<br>
+<br>
EXPECT_EQ(Max, SaturatingAdd(Max, T(1)));<br>
+ EXPECT_EQ(Max, SaturatingAdd(Max, T(1), ResultOverflowed));<br>
+ EXPECT_TRUE(ResultOverflowed);<br>
+<br>
+ EXPECT_EQ(Max, SaturatingAdd(T(1), T(Max - 1)));<br>
+ EXPECT_EQ(Max, SaturatingAdd(T(1), T(Max - 1), ResultOverflowed));<br>
+ EXPECT_FALSE(ResultOverflowed);<br>
+<br>
EXPECT_EQ(Max, SaturatingAdd(T(1), Max));<br>
+ EXPECT_EQ(Max, SaturatingAdd(T(1), Max, ResultOverflowed));<br>
+ EXPECT_TRUE(ResultOverflowed);<br>
+<br>
EXPECT_EQ(Max, SaturatingAdd(Max, Max));<br>
+ EXPECT_EQ(Max, SaturatingAdd(Max, Max, ResultOverflowed));<br>
+ EXPECT_TRUE(ResultOverflowed);<br>
}<br>
<br>
TEST(MathExtras, SaturatingAdd) {<br>
@@ -211,22 +228,50 @@ template<typename T><br>
void SaturatingMultiplyTestHelper()<br>
{<br>
const T Max = std::numeric_limits<T>::max();<br>
+ bool ResultOverflowed;<br>
<br>
// Test basic multiplication.<br>
EXPECT_EQ(T(6), SaturatingMultiply(T(2), T(3)));<br>
+ EXPECT_EQ(T(6), SaturatingMultiply(T(2), T(3), ResultOverflowed));<br>
+ EXPECT_FALSE(ResultOverflowed);<br>
+<br>
EXPECT_EQ(T(6), SaturatingMultiply(T(3), T(2)));<br>
+ EXPECT_EQ(T(6), SaturatingMultiply(T(3), T(2), ResultOverflowed));<br>
+ EXPECT_FALSE(ResultOverflowed);<br>
<br>
// Test multiplication by zero.<br>
EXPECT_EQ(T(0), SaturatingMultiply(T(0), T(0)));<br>
+ EXPECT_EQ(T(0), SaturatingMultiply(T(0), T(0), ResultOverflowed));<br>
+ EXPECT_FALSE(ResultOverflowed);<br>
+<br>
EXPECT_EQ(T(0), SaturatingMultiply(T(1), T(0)));<br>
+ EXPECT_EQ(T(0), SaturatingMultiply(T(1), T(0), ResultOverflowed));<br>
+ EXPECT_FALSE(ResultOverflowed);<br>
+<br>
EXPECT_EQ(T(0), SaturatingMultiply(T(0), T(1)));<br>
+ EXPECT_EQ(T(0), SaturatingMultiply(T(0), T(1), ResultOverflowed));<br>
+ EXPECT_FALSE(ResultOverflowed);<br>
+<br>
EXPECT_EQ(T(0), SaturatingMultiply(Max, T(0)));<br>
+ EXPECT_EQ(T(0), SaturatingMultiply(Max, T(0), ResultOverflowed));<br>
+ EXPECT_FALSE(ResultOverflowed);<br>
+<br>
EXPECT_EQ(T(0), SaturatingMultiply(T(0), Max));<br>
+ EXPECT_EQ(T(0), SaturatingMultiply(T(0), Max, ResultOverflowed));<br>
+ EXPECT_FALSE(ResultOverflowed);<br>
<br>
// Test multiplication by maximum value.<br>
EXPECT_EQ(Max, SaturatingMultiply(Max, T(2)));<br>
- EXPECT_EQ(Max, SaturatingMultiply(T(2),Max));<br>
+ EXPECT_EQ(Max, SaturatingMultiply(Max, T(2), ResultOverflowed));<br>
+ EXPECT_TRUE(ResultOverflowed);<br>
+<br>
+ EXPECT_EQ(Max, SaturatingMultiply(T(2), Max));<br>
+ EXPECT_EQ(Max, SaturatingMultiply(T(2), Max, ResultOverflowed));<br>
+ EXPECT_TRUE(ResultOverflowed);<br>
+<br>
EXPECT_EQ(Max, SaturatingMultiply(Max, Max));<br>
+ EXPECT_EQ(Max, SaturatingMultiply(Max, Max, ResultOverflowed));<br>
+ EXPECT_TRUE(ResultOverflowed);<br>
<br>
// Test interesting boundary conditions for algorithm -<br>
// ((1 << A) - 1) * ((1 << B) + K) for K in [-1, 0, 1]<br>
@@ -241,8 +286,12 @@ void SaturatingMultiplyTestHelper()<br>
<br>
if(OverflowExpected) {<br>
EXPECT_EQ(Max, SaturatingMultiply(X, Y));<br>
+ EXPECT_EQ(Max, SaturatingMultiply(X, Y, ResultOverflowed));<br>
+ EXPECT_TRUE(ResultOverflowed);<br>
} else {<br>
EXPECT_EQ(X * Y, SaturatingMultiply(X, Y));<br>
+ EXPECT_EQ(X * Y, SaturatingMultiply(X, Y, ResultOverflowed));<br>
+ EXPECT_FALSE(ResultOverflowed);<br>
}<br>
}<br>
}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>