[llvm] r253412 - [llvm-profdata] Add SaturatingAdd/SaturatingMultiply Helper Functions
Sean Silva via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 18 19:56:07 PST 2015
On Wed, Nov 18, 2015 at 12:42 PM, Nathan Slingerland <slingn at gmail.com>
wrote:
> Thanks Sean.
>
> I have updated the test case based on your comments.
>
FYI, in LLVM-land we usually include the revision number when making
comments like this. I also noticed a missing revision number for r253474 in
your response to Justin in the post-commit thread for r253384.
(the reason for this convention is that it makes "links" between different
things so it is easier to search for stuff and see the chain of events
leading to things)
-- Sean Silva
>
> On Tue, Nov 17, 2015 at 8:04 PM, Sean Silva <chisophugis at gmail.com> wrote:
>
>>
>>
>> On Tue, Nov 17, 2015 at 4:52 PM, Nathan Slingerland via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Author: slingn
>>> Date: Tue Nov 17 18:52:43 2015
>>> New Revision: 253412
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=253412&view=rev
>>> Log:
>>> [llvm-profdata] Add SaturatingAdd/SaturatingMultiply Helper Functions
>>>
>>> Summary:
>>> This change adds MathExtras helper functions for handling unsigned,
>>> saturating addition and multiplication. It also updates the instrumentation
>>> and sample profile merge implementations to use them.
>>>
>>> No functional changes.
>>>
>>> Reviewers: dnovillo, bogner, davidxl
>>>
>>> Subscribers: davidxl, llvm-commits
>>>
>>> Differential Revision: http://reviews.llvm.org/D14720
>>>
>>> Modified:
>>> llvm/trunk/include/llvm/ProfileData/InstrProf.h
>>> llvm/trunk/include/llvm/ProfileData/SampleProf.h
>>> llvm/trunk/include/llvm/Support/MathExtras.h
>>> llvm/trunk/unittests/Support/MathExtrasTest.cpp
>>>
>>> Modified: llvm/trunk/include/llvm/ProfileData/InstrProf.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/InstrProf.h?rev=253412&r1=253411&r2=253412&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/ProfileData/InstrProf.h (original)
>>> +++ llvm/trunk/include/llvm/ProfileData/InstrProf.h Tue Nov 17 18:52:43
>>> 2015
>>> @@ -226,7 +226,7 @@ struct InstrProfValueSiteRecord {
>>> while (I != IE && I->Value < J->Value)
>>> ++I;
>>> if (I != IE && I->Value == J->Value) {
>>> - I->Count += J->Count;
>>> + I->Count = SaturatingAdd(I->Count, J->Count);
>>> ++I;
>>> continue;
>>> }
>>>
>>> Modified: llvm/trunk/include/llvm/ProfileData/SampleProf.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/SampleProf.h?rev=253412&r1=253411&r2=253412&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/ProfileData/SampleProf.h (original)
>>> +++ llvm/trunk/include/llvm/ProfileData/SampleProf.h Tue Nov 17 18:52:43
>>> 2015
>>> @@ -173,10 +173,7 @@ public:
>>> /// Sample counts accumulate using saturating arithmetic, to avoid
>>> wrapping
>>> /// around unsigned integers.
>>> void addSamples(uint64_t S) {
>>> - if (NumSamples <= std::numeric_limits<uint64_t>::max() - S)
>>> - NumSamples += S;
>>> - else
>>> - NumSamples = std::numeric_limits<uint64_t>::max();
>>> + NumSamples = SaturatingAdd(NumSamples, S);
>>> }
>>>
>>> /// Add called function \p F with samples \p S.
>>> @@ -185,10 +182,7 @@ public:
>>> /// around unsigned integers.
>>> void addCalledTarget(StringRef F, uint64_t S) {
>>> uint64_t &TargetSamples = CallTargets[F];
>>> - if (TargetSamples <= std::numeric_limits<uint64_t>::max() - S)
>>> - TargetSamples += S;
>>> - else
>>> - TargetSamples = std::numeric_limits<uint64_t>::max();
>>> + TargetSamples = SaturatingAdd(TargetSamples, S);
>>> }
>>>
>>> /// Return true if this sample record contains function calls.
>>>
>>> Modified: llvm/trunk/include/llvm/Support/MathExtras.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/MathExtras.h?rev=253412&r1=253411&r2=253412&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/Support/MathExtras.h (original)
>>> +++ llvm/trunk/include/llvm/Support/MathExtras.h Tue Nov 17 18:52:43 2015
>>> @@ -653,6 +653,32 @@ inline int64_t SignExtend64(uint64_t X,
>>> return int64_t(X << (64 - B)) >> (64 - B);
>>> }
>>>
>>> +/// \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) {
>>> + // Hacker's Delight, p. 29
>>> + T Z = X + Y;
>>> + if (Z < X || Z < Y)
>>> + return std::numeric_limits<T>::max();
>>> + else
>>> + 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) {
>>> + // Hacker's Delight, p. 30
>>> + T Z = X * Y;
>>> + if (Y != 0 && Z / Y != X)
>>> + return std::numeric_limits<T>::max();
>>> + else
>>> + return Z;
>>> +}
>>> +
>>> extern const float huge_valf;
>>> } // End llvm namespace
>>>
>>>
>>> Modified: llvm/trunk/unittests/Support/MathExtrasTest.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/MathExtrasTest.cpp?rev=253412&r1=253411&r2=253412&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/unittests/Support/MathExtrasTest.cpp (original)
>>> +++ llvm/trunk/unittests/Support/MathExtrasTest.cpp Tue Nov 17 18:52:43
>>> 2015
>>> @@ -190,4 +190,52 @@ TEST(MathExtras, RoundUpToAlignment) {
>>> EXPECT_EQ(552u, RoundUpToAlignment(321, 255, 42));
>>> }
>>>
>>> +template<typename T>
>>> +void SaturatingAddTestHelper()
>>> +{
>>> + EXPECT_EQ(static_cast<T>(3),
>>> + SaturatingAdd(static_cast<T>(1), static_cast<T>(2)));
>>> + EXPECT_EQ(std::numeric_limits<T>::max(),
>>> + SaturatingAdd(std::numeric_limits<T>::max(),
>>> static_cast<T>(1)));
>>> + EXPECT_EQ(std::numeric_limits<T>::max(),
>>> + SaturatingAdd(static_cast<T>(1),
>>> std::numeric_limits<T>::max()));
>>> + EXPECT_EQ(std::numeric_limits<T>::max(),
>>> + SaturatingAdd(std::numeric_limits<T>::max(),
>>> + std::numeric_limits<T>::max()));
>>> +}
>>> +
>>> +TEST(MathExtras, SaturatingAdd) {
>>> + SaturatingAddTestHelper<uint8_t>();
>>> + SaturatingAddTestHelper<uint16_t>();
>>> + SaturatingAddTestHelper<uint32_t>();
>>> + SaturatingAddTestHelper<uint64_t>();
>>> +}
>>> +
>>> +template<typename T>
>>> +void SaturatingMultiplyTestHelper()
>>> +{
>>> + EXPECT_EQ(static_cast<T>(0),
>>> + SaturatingMultiply(static_cast<T>(1), static_cast<T>(0)));
>>> + EXPECT_EQ(static_cast<T>(0),
>>> + SaturatingMultiply(static_cast<T>(0), static_cast<T>(1)));
>>> + EXPECT_EQ(static_cast<T>(6),
>>> + SaturatingMultiply(static_cast<T>(2), static_cast<T>(3)));
>>> + EXPECT_EQ(std::numeric_limits<T>::max(),
>>> + SaturatingMultiply(std::numeric_limits<T>::max(),
>>> + static_cast<T>(2)));
>>> + EXPECT_EQ(std::numeric_limits<T>::max(),
>>> + SaturatingMultiply(static_cast<T>(2),
>>> + std::numeric_limits<T>::max()));
>>> + EXPECT_EQ(std::numeric_limits<T>::max(),
>>> + SaturatingMultiply(std::numeric_limits<T>::max(),
>>> + std::numeric_limits<T>::max()));
>>>
>>
>> I think adding a helper:
>>
>> T Max = std::numeric_limits<T>::max();
>>
>> would help clean these up. Also, I think instead of `static_cast<T>(1)`
>> you can do just `T(1)`.
>>
>> So for example that second-to-last one becomes:
>>
>> EXPECT_EQ(Max, SaturatingMultiply(T(2), Max))
>>
>> which seems like a nice improvement.
>>
>> -- Sean Silva
>>
>>
>>> +}
>>> +
>>> +TEST(MathExtras, SaturatingMultiply) {
>>> + SaturatingMultiplyTestHelper<uint8_t>();
>>> + SaturatingMultiplyTestHelper<uint16_t>();
>>> + SaturatingMultiplyTestHelper<uint32_t>();
>>> + SaturatingMultiplyTestHelper<uint64_t>();
>>> +}
>>> +
>>> }
>>>
>>>
>>> _______________________________________________
>>> 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/20151118/027db16d/attachment.html>
More information about the llvm-commits
mailing list