[llvm] r253921 - [Support] Add optional argument to SaturatingAdd() and SaturatingMultiply() to indicate that overflow occurred
Sean Silva via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 23 15:41:06 PST 2015
On Mon, Nov 23, 2015 at 3:31 PM, Nathan Slingerland <slingn at gmail.com>
wrote:
> Right - I tried it both ways. Using a pointer made the
> SaturatingMultiply() code a bit harder to read since it needs to check for
> non-null at each point where `ResultOverflowed` should be updated.
> Otherwise the two ways seem equivalent.
>
This kind of pattern avoids it:
bool Dummy;
bool &Overflowed = ResultOverflowed ? ResultOverflowed : &Dummy;
or using a lambda:
auto setOverflowed = [&]() { if (ResultOverflowed) *ResultOverflowed =
true; };
Either pattern seems preferable to duplicating the template/enable_if stuff.
-- Sean Silva
>
>
> On Mon, Nov 23, 2015 at 3:30 PM, Xinliang David Li <xinliangli at gmail.com>
> wrote:
>
>> One reason I can see is that using default arguments will resulting in
>> bigger code -- as all the updates need to check nullness.
>>
>> David
>>
>> On Mon, Nov 23, 2015 at 3:20 PM, Sean Silva via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Is there a reason you used overloading instead of default arguments?
>>> I.e. instead of just declaring the ResultOverflowed variable as `bool
>>> *ResultOverflowed = nullptr`.
>>>
>>> -- Sean Silva
>>>
>>> On Mon, Nov 23, 2015 at 1:54 PM, Nathan Slingerland via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>> Author: slingn
>>>> Date: Mon Nov 23 15:54:22 2015
>>>> New Revision: 253921
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=253921&view=rev
>>>> Log:
>>>> [Support] Add optional argument to SaturatingAdd() and
>>>> SaturatingMultiply() to indicate that overflow occurred
>>>>
>>>> Summary: Adds the ability for callers to detect when saturation
>>>> occurred on the result of saturating addition/multiplication.
>>>>
>>>> Reviewers: davidxl, silvas, rsmith
>>>>
>>>> Subscribers: llvm-commits
>>>>
>>>> Differential Revision: http://reviews.llvm.org/D14931
>>>>
>>>> Modified:
>>>> llvm/trunk/include/llvm/Support/MathExtras.h
>>>> llvm/trunk/unittests/Support/MathExtrasTest.cpp
>>>>
>>>> Modified: llvm/trunk/include/llvm/Support/MathExtras.h
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/MathExtras.h?rev=253921&r1=253920&r2=253921&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/include/llvm/Support/MathExtras.h (original)
>>>> +++ llvm/trunk/include/llvm/Support/MathExtras.h Mon Nov 23 15:54:22
>>>> 2015
>>>> @@ -655,46 +655,79 @@ inline int64_t SignExtend64(uint64_t X,
>>>>
>>>> /// \brief Add two unsigned integers, X and Y, of type T.
>>>> /// Clamp the result to the maximum representable value of T on
>>>> overflow.
>>>> +/// ResultOverflowed indicates if the result is larger than the maximum
>>>> +/// representable value of type T.
>>>> template <typename T>
>>>> typename std::enable_if<std::is_unsigned<T>::value, T>::type
>>>> -SaturatingAdd(T X, T Y) {
>>>> +SaturatingAdd(T X, T Y, bool &ResultOverflowed) {
>>>> // Hacker's Delight, p. 29
>>>> T Z = X + Y;
>>>> - if (Z < X || Z < Y)
>>>> + ResultOverflowed = (Z < X || Z < Y);
>>>> + if (ResultOverflowed)
>>>> return std::numeric_limits<T>::max();
>>>> else
>>>> return Z;
>>>> }
>>>>
>>>> +/// \brief Add two unsigned integers, X and Y, of type T.
>>>> +/// Clamp the result to the maximum representable value of T on
>>>> overflow.
>>>> +template <typename T>
>>>> +typename std::enable_if<std::is_unsigned<T>::value, T>::type
>>>> +SaturatingAdd(T X, T Y) {
>>>> + bool ResultOverflowed;
>>>> + return SaturatingAdd(X, Y, ResultOverflowed);
>>>> +}
>>>> +
>>>> /// \brief Multiply two unsigned integers, X and Y, of type T.
>>>> /// Clamp the result to the maximum representable value of T on
>>>> overflow.
>>>> +/// ResultOverflowed indicates if the result is larger than the maximum
>>>> +/// representable value of type T.
>>>> template <typename T>
>>>> typename std::enable_if<std::is_unsigned<T>::value, T>::type
>>>> -SaturatingMultiply(T X, T Y) {
>>>> +SaturatingMultiply(T X, T Y, bool &ResultOverflowed) {
>>>> // 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.
>>>>
>>>> + ResultOverflowed = false;
>>>> +
>>>> // 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)
>>>> + if (Log2Z < Log2Max) {
>>>> return X * Y;
>>>> - if (Log2Z > Log2Max)
>>>> + }
>>>> + if (Log2Z > Log2Max) {
>>>> + ResultOverflowed = true;
>>>> 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))
>>>> + if (Z & ~(Max >> 1)) {
>>>> + ResultOverflowed = true;
>>>> return Max;
>>>> + }
>>>> Z <<= 1;
>>>> - return (X & 1) ? SaturatingAdd(Z, Y) : Z;
>>>> + if (X & 1)
>>>> + return SaturatingAdd(Z, Y, ResultOverflowed);
>>>> +
>>>> + return Z;
>>>> +}
>>>> +
>>>> +/// \brief Multiply two unsigned integers, X and Y, of type T.
>>>> +/// Clamp the result to the maximum representable value of T on
>>>> overflow.
>>>> +template <typename T>
>>>> +typename std::enable_if<std::is_unsigned<T>::value, T>::type
>>>> +SaturatingMultiply(T X, T Y) {
>>>> + bool ResultOverflowed;
>>>> + return SaturatingMultiply(X, Y, ResultOverflowed);
>>>> }
>>>>
>>>> extern const float huge_valf;
>>>>
>>>> Modified: llvm/trunk/unittests/Support/MathExtrasTest.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/MathExtrasTest.cpp?rev=253921&r1=253920&r2=253921&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/unittests/Support/MathExtrasTest.cpp (original)
>>>> +++ llvm/trunk/unittests/Support/MathExtrasTest.cpp Mon Nov 23 15:54:22
>>>> 2015
>>>> @@ -194,10 +194,27 @@ template<typename T>
>>>> void SaturatingAddTestHelper()
>>>> {
>>>> const T Max = std::numeric_limits<T>::max();
>>>> + bool ResultOverflowed;
>>>> +
>>>> EXPECT_EQ(T(3), SaturatingAdd(T(1), T(2)));
>>>> + EXPECT_EQ(T(3), SaturatingAdd(T(1), T(2), ResultOverflowed));
>>>> + EXPECT_FALSE(ResultOverflowed);
>>>> +
>>>> EXPECT_EQ(Max, SaturatingAdd(Max, T(1)));
>>>> + EXPECT_EQ(Max, SaturatingAdd(Max, T(1), ResultOverflowed));
>>>> + EXPECT_TRUE(ResultOverflowed);
>>>> +
>>>> + EXPECT_EQ(Max, SaturatingAdd(T(1), T(Max - 1)));
>>>> + EXPECT_EQ(Max, SaturatingAdd(T(1), T(Max - 1), ResultOverflowed));
>>>> + EXPECT_FALSE(ResultOverflowed);
>>>> +
>>>> EXPECT_EQ(Max, SaturatingAdd(T(1), Max));
>>>> + EXPECT_EQ(Max, SaturatingAdd(T(1), Max, ResultOverflowed));
>>>> + EXPECT_TRUE(ResultOverflowed);
>>>> +
>>>> EXPECT_EQ(Max, SaturatingAdd(Max, Max));
>>>> + EXPECT_EQ(Max, SaturatingAdd(Max, Max, ResultOverflowed));
>>>> + EXPECT_TRUE(ResultOverflowed);
>>>> }
>>>>
>>>> TEST(MathExtras, SaturatingAdd) {
>>>> @@ -211,22 +228,50 @@ template<typename T>
>>>> void SaturatingMultiplyTestHelper()
>>>> {
>>>> const T Max = std::numeric_limits<T>::max();
>>>> + bool ResultOverflowed;
>>>>
>>>> // Test basic multiplication.
>>>> EXPECT_EQ(T(6), SaturatingMultiply(T(2), T(3)));
>>>> + EXPECT_EQ(T(6), SaturatingMultiply(T(2), T(3), ResultOverflowed));
>>>> + EXPECT_FALSE(ResultOverflowed);
>>>> +
>>>> EXPECT_EQ(T(6), SaturatingMultiply(T(3), T(2)));
>>>> + EXPECT_EQ(T(6), SaturatingMultiply(T(3), T(2), ResultOverflowed));
>>>> + EXPECT_FALSE(ResultOverflowed);
>>>>
>>>> // Test multiplication by zero.
>>>> EXPECT_EQ(T(0), SaturatingMultiply(T(0), T(0)));
>>>> + EXPECT_EQ(T(0), SaturatingMultiply(T(0), T(0), ResultOverflowed));
>>>> + EXPECT_FALSE(ResultOverflowed);
>>>> +
>>>> EXPECT_EQ(T(0), SaturatingMultiply(T(1), T(0)));
>>>> + EXPECT_EQ(T(0), SaturatingMultiply(T(1), T(0), ResultOverflowed));
>>>> + EXPECT_FALSE(ResultOverflowed);
>>>> +
>>>> EXPECT_EQ(T(0), SaturatingMultiply(T(0), T(1)));
>>>> + EXPECT_EQ(T(0), SaturatingMultiply(T(0), T(1), ResultOverflowed));
>>>> + EXPECT_FALSE(ResultOverflowed);
>>>> +
>>>> EXPECT_EQ(T(0), SaturatingMultiply(Max, T(0)));
>>>> + EXPECT_EQ(T(0), SaturatingMultiply(Max, T(0), ResultOverflowed));
>>>> + EXPECT_FALSE(ResultOverflowed);
>>>> +
>>>> EXPECT_EQ(T(0), SaturatingMultiply(T(0), Max));
>>>> + EXPECT_EQ(T(0), SaturatingMultiply(T(0), Max, ResultOverflowed));
>>>> + EXPECT_FALSE(ResultOverflowed);
>>>>
>>>> // Test multiplication by maximum value.
>>>> EXPECT_EQ(Max, SaturatingMultiply(Max, T(2)));
>>>> - EXPECT_EQ(Max, SaturatingMultiply(T(2),Max));
>>>> + EXPECT_EQ(Max, SaturatingMultiply(Max, T(2), ResultOverflowed));
>>>> + EXPECT_TRUE(ResultOverflowed);
>>>> +
>>>> + EXPECT_EQ(Max, SaturatingMultiply(T(2), Max));
>>>> + EXPECT_EQ(Max, SaturatingMultiply(T(2), Max, ResultOverflowed));
>>>> + EXPECT_TRUE(ResultOverflowed);
>>>> +
>>>> EXPECT_EQ(Max, SaturatingMultiply(Max, Max));
>>>> + EXPECT_EQ(Max, SaturatingMultiply(Max, Max, ResultOverflowed));
>>>> + EXPECT_TRUE(ResultOverflowed);
>>>>
>>>> // Test interesting boundary conditions for algorithm -
>>>> // ((1 << A) - 1) * ((1 << B) + K) for K in [-1, 0, 1]
>>>> @@ -241,8 +286,12 @@ void SaturatingMultiplyTestHelper()
>>>>
>>>> if(OverflowExpected) {
>>>> EXPECT_EQ(Max, SaturatingMultiply(X, Y));
>>>> + EXPECT_EQ(Max, SaturatingMultiply(X, Y, ResultOverflowed));
>>>> + EXPECT_TRUE(ResultOverflowed);
>>>> } else {
>>>> EXPECT_EQ(X * Y, SaturatingMultiply(X, Y));
>>>> + EXPECT_EQ(X * Y, SaturatingMultiply(X, Y, ResultOverflowed));
>>>> + EXPECT_FALSE(ResultOverflowed);
>>>> }
>>>> }
>>>> }
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151123/68c2805e/attachment.html>
More information about the llvm-commits
mailing list