[llvm] r253497 - [llvm-profdata] Add SaturatingAdd/SaturatingMultiply Helper Functions (2nd try)

Sean Silva via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 18 22:01:15 PST 2015


On Wed, Nov 18, 2015 at 9:48 PM, Xinliang David Li <xinliangli at gmail.com>
wrote:

> probably
>
> cmake -DLLVM_USE_SANITIZER=Undefined ...
>

That's what one would think, but it may be something custom, such as
something that turns on the unsigned integer checks (which are not
undefined behavior, but we still have an opt-in option for ubsan to emit an
error on them).

Unfortunately it is hard to tell from the green dragon jenkins pages what
the actual build command was used. Clicking "view as plain text" shows
http://lab.llvm.org:8080/green/job/clang-stage2-cmake-RgSan_check/746/consoleText
which calls `python /opt/llvm/zorg/zorg/jenkins//build.py cmake testlong`,
but looking at `zorg/jenkins//build.py` there is no mention of
`LLVM_USE_SANITIZER`
or `-fsanitize`.

-- Sean Silva


>
> 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/881f217b/attachment.html>


More information about the llvm-commits mailing list