[llvm] r253497 - [llvm-profdata] Add SaturatingAdd/SaturatingMultiply Helper Functions (2nd try)
Daniel Sanders via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 20 02:14:04 PST 2015
Hi,
I've reverted r253497 and r253539 in an attempt to fix the clang-cmake-mips buildbot since it's the only profile related change. If it doesn't fix the bot I'll re-commit it.
It caused link errors of the form:
InstrProfiling.c:(.text.__llvm_profile_instrument_target+0x1c0): undefined reference to `__sync_fetch_and_add_8'
We had a planned network outage at the time of the commit so the first build to show a problem is http://lab.llvm.org:8011/builders/clang-cmake-mips/builds/10827
> -----Original Message-----
> From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On Behalf
> Of Nathan Slingerland via llvm-commits
> Sent: 18 November 2015 20:41
> To: llvm-commits at lists.llvm.org
> Subject: [llvm] r253497 - [llvm-profdata] Add
> SaturatingAdd/SaturatingMultiply Helper Functions (2nd try)
>
> 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=253
> 496&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=253
> 496&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
More information about the llvm-commits
mailing list