[llvm] r253497 - [llvm-profdata] Add SaturatingAdd/SaturatingMultiply Helper Functions (2nd try)
Xinliang David Li via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 18 21:48:39 PST 2015
probably
cmake -DLLVM_USE_SANITIZER=Undefined ...
David
On Wed, Nov 18, 2015 at 9:37 PM, Sean Silva via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> What is the configure line for this bot?
>
> On Wed, Nov 18, 2015 at 4:28 PM, Bruno Cardoso Lopes via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Hi Nathan,
>>
>> Ubsan is detecting some undefined behavior after your commit:
>> http://lab.llvm.org:8080/green/job/clang-stage2-cmake-RgSan_check/746/
>>
>> ******************** TEST 'LLVM-Unit ::
>> Support/SupportTests/MathExtras.SaturatingMultiply' FAILED
>> ********************
>> Note: Google Test filter = MathExtras.SaturatingMultiply
>> [==========] Running 1 test from 1 test case.
>> [----------] Global test environment set-up.
>> [----------] 1 test from MathExtras
>> [ RUN ] MathExtras.SaturatingMultiply
>> /Users/buildslave/jenkins/sharedspace/clang-stage2-cmake-RgSan at 2
>> /llvm/include/llvm/Support/MathExtras.h:675:11:
>> runtime error: signed integer overflow: 65535 * 65535 cannot be
>> represented in type 'int'
>> SUMMARY: AddressSanitizer: undefined-behavior
>> /Users/buildslave/jenkins/sharedspace/clang-stage2-cmake-RgSan at 2
>> /llvm/include/llvm/Support/MathExtras.h:675:11
>> in
>> 0 SupportTests 0x0000000105b92545
>> llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 277
>> 1 SupportTests 0x0000000105b90400
>> llvm::sys::RunSignalHandlers() + 448
>> 2 SupportTests 0x0000000105b937d3
>> SignalHandler(int) + 611
>> 3 libsystem_platform.dylib 0x00007fff8d239f1a _sigtramp + 26
>> 4 libsystem_platform.dylib 0x0000000000000003 _sigtramp +
>> 1927045379
>> 5 libsystem_c.dylib 0x00007fff88edd9b3 abort + 129
>> 6 libclang_rt.asan_osx_dynamic.dylib 0x00000001060c0f76
>> __sanitizer::Abort() + 6
>>
>>
>>
>> On Wed, Nov 18, 2015 at 12:40 PM, Nathan Slingerland via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>> > Author: slingn
>> > Date: Wed Nov 18 14:40:41 2015
>> > New Revision: 253497
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=253497&view=rev
>> > Log:
>> > [llvm-profdata] Add SaturatingAdd/SaturatingMultiply Helper Functions
>> (2nd try)
>> >
>> > 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.
>> >
>> > 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/ProfileData/InstrProfTest.cpp
>> > 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=253497&r1=253496&r2=253497&view=diff
>> >
>> ==============================================================================
>> > --- llvm/trunk/include/llvm/ProfileData/InstrProf.h (original)
>> > +++ llvm/trunk/include/llvm/ProfileData/InstrProf.h Wed Nov 18 14:40:41
>> 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=253497&r1=253496&r2=253497&view=diff
>> >
>> ==============================================================================
>> > --- llvm/trunk/include/llvm/ProfileData/SampleProf.h (original)
>> > +++ llvm/trunk/include/llvm/ProfileData/SampleProf.h Wed Nov 18
>> 14:40:41 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=253497&r1=253496&r2=253497&view=diff
>> >
>> ==============================================================================
>> > --- llvm/trunk/include/llvm/Support/MathExtras.h (original)
>> > +++ llvm/trunk/include/llvm/Support/MathExtras.h Wed Nov 18 14:40:41
>> 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/ProfileData/InstrProfTest.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ProfileData/InstrProfTest.cpp?rev=253497&r1=253496&r2=253497&view=diff
>> >
>> ==============================================================================
>> > --- llvm/trunk/unittests/ProfileData/InstrProfTest.cpp (original)
>> > +++ llvm/trunk/unittests/ProfileData/InstrProfTest.cpp Wed Nov 18
>> 14:40:41 2015
>> > @@ -350,6 +350,38 @@ TEST_F(InstrProfTest, get_icall_data_mer
>> > ASSERT_EQ(2U, VD_4[2].Count);
>> > }
>> >
>> > +TEST_F(InstrProfTest, get_icall_data_merge1_saturation) {
>> > + const uint64_t Max = std::numeric_limits<uint64_t>::max();
>> > +
>> > + InstrProfRecord Record1("caller", 0x1234, {1});
>> > + InstrProfRecord Record2("caller", 0x1234, {1});
>> > + InstrProfRecord Record3("callee1", 0x1235, {3, 4});
>> > +
>> > + Record1.reserveSites(IPVK_IndirectCallTarget, 1);
>> > + InstrProfValueData VD1[] = {{(uint64_t) "callee1", 1}};
>> > + Record1.addValueData(IPVK_IndirectCallTarget, 0, VD1, 1, nullptr);
>> > +
>> > + Record2.reserveSites(IPVK_IndirectCallTarget, 1);
>> > + InstrProfValueData VD2[] = {{(uint64_t) "callee1", Max}};
>> > + Record2.addValueData(IPVK_IndirectCallTarget, 0, VD2, 1, nullptr);
>> > +
>> > + Writer.addRecord(std::move(Record1));
>> > + Writer.addRecord(std::move(Record2));
>> > + Writer.addRecord(std::move(Record3));
>> > +
>> > + auto Profile = Writer.writeBuffer();
>> > + readProfile(std::move(Profile));
>> > +
>> > + // Verify saturation of counts.
>> > + ErrorOr<InstrProfRecord> R = Reader->getInstrProfRecord("caller",
>> 0x1234);
>> > + ASSERT_TRUE(NoError(R.getError()));
>> > + ASSERT_EQ(1U, R.get().getNumValueSites(IPVK_IndirectCallTarget));
>> > + std::unique_ptr<InstrProfValueData[]> VD =
>> > + R.get().getValueForSite(IPVK_IndirectCallTarget, 0);
>> > + ASSERT_EQ(StringRef("callee1"), StringRef((const char *)VD[0].Value,
>> 7));
>> > + ASSERT_EQ(Max, VD[0].Count);
>> > +}
>> > +
>> > TEST_F(InstrProfTest, get_max_function_count) {
>> > InstrProfRecord Record1("foo", 0x1234, {1ULL << 31, 2});
>> > InstrProfRecord Record2("bar", 0, {1ULL << 63});
>> >
>> > Modified: llvm/trunk/unittests/Support/MathExtrasTest.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/MathExtrasTest.cpp?rev=253497&r1=253496&r2=253497&view=diff
>> >
>> ==============================================================================
>> > --- llvm/trunk/unittests/Support/MathExtrasTest.cpp (original)
>> > +++ llvm/trunk/unittests/Support/MathExtrasTest.cpp Wed Nov 18 14:40:41
>> 2015
>> > @@ -190,4 +190,40 @@ TEST(MathExtras, RoundUpToAlignment) {
>> > EXPECT_EQ(552u, RoundUpToAlignment(321, 255, 42));
>> > }
>> >
>> > +template<typename T>
>> > +void SaturatingAddTestHelper()
>> > +{
>> > + const T Max = std::numeric_limits<T>::max();
>> > + EXPECT_EQ(T(3), SaturatingAdd(T(1), T(2)));
>> > + EXPECT_EQ(Max, SaturatingAdd(Max, T(1)));
>> > + EXPECT_EQ(Max, SaturatingAdd(T(1), Max));
>> > + EXPECT_EQ(Max, SaturatingAdd(Max, Max));
>> > +}
>> > +
>> > +TEST(MathExtras, SaturatingAdd) {
>> > + SaturatingAddTestHelper<uint8_t>();
>> > + SaturatingAddTestHelper<uint16_t>();
>> > + SaturatingAddTestHelper<uint32_t>();
>> > + 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>();
>> > +}
>> > +
>> > }
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>>
>> --
>> Bruno Cardoso Lopes
>> http://www.brunocardoso.cc
>> _______________________________________________
>> 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/20151118/7cabf846/attachment.html>
More information about the llvm-commits
mailing list