[llvm] 9bcf7b1 - [NFCI][IR] ConstantRangeTest: add basic scaffolding for next-gen precision/correctness testing

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 24 23:50:56 PDT 2020


Roman, please put up a differential revision for this change.

Thank you,
Nikita

On Fri, Sep 25, 2020 at 8:22 AM Roman Lebedev via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Sorry about that, and thanks for the revert!
>
> Roman
>
>
> On Fri, Sep 25, 2020 at 2:16 AM Matt Morehouse <mascasa at google.com> wrote:
> >
> > Hi Roman,
> >
> > Either this patch or your other NFCI one have caused the
> ConstantRangeTest to start failing on PPC:
> http://lab.llvm.org:8011/builders/clang-ppc64le-linux-lnt/builds/27058
> >
> > FAILED: unittests/IR/CMakeFiles/IRTests.dir/ConstantRangeTest.cpp.o
> > /usr/bin/c++  -DGTEST_HAS_RTTI=0 -DGTEST_HAS_TR1_TUPLE=0 -D_DEBUG
> -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS
> -D__STDC_LIMIT_MACROS -Iunittests/IR
> -I/home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/llvm/llvm/unittests/IR
> -Iinclude
> -I/home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/llvm/llvm/include
> -I/home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/llvm/llvm/utils/unittest/googletest/include
> -I/home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/llvm/llvm/utils/unittest/googlemock/include
> -fPIC -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra
> -Wno-unused-parameter -Wwrite-strings -Wcast-qual
> -Wno-missing-field-initializers -pedantic -Wno-long-long
> -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess
> -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment
> -fdiagnostics-color -ffunction-sections -fdata-sections -O3
>  -Wno-variadic-macros -fno-exceptions -fno-rtti -UNDEBUG
> -Wno-suggest-override -std=c++14 -MD -MT
> unittests/IR/CMakeFiles/IRTests.dir/ConstantRangeTest.cpp.o -MF
> unittests/IR/CMakeFiles/IRTests.dir/ConstantRangeTest.cpp.o.d -o
> unittests/IR/CMakeFiles/IRTests.dir/ConstantRangeTest.cpp.o -c
> /home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/llvm/llvm/unittests/IR/ConstantRangeTest.cpp
> >
> /home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/llvm/llvm/unittests/IR/ConstantRangeTest.cpp:
> In lambda function:
> >
> /home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/llvm/llvm/unittests/IR/ConstantRangeTest.cpp:234:12:
> warning: suggest explicit braces to avoid ambiguous ‘else’ [-Wdangling-else]
> >          if (Exact.isEmptySet())
> >             ^
> >
> /home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/llvm/llvm/unittests/IR/ConstantRangeTest.cpp:
> In instantiation of ‘void {anonymous}::TestUnaryOpExhaustive(Fn1, Fn2,
> {anonymous}::AccumulatedPrecisionData&) [with OpRangeGathererTy =
> {anonymous}::UnsignedOpRangeGatherer; Fn1 =
> {anonymous}::ConstantRangeTest_binaryNot_Test::TestBody()::<lambda(const
> llvm::ConstantRange&)>; Fn2 =
> {anonymous}::ConstantRangeTest_binaryNot_Test::TestBody()::<lambda(const
> llvm::APInt&)>]’:
> >
> /home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/llvm/llvm/unittests/IR/ConstantRangeTest.cpp:2486:51:
>  required from here
> >
> /home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/llvm/llvm/unittests/IR/ConstantRangeTest.cpp:141:31:
> internal compiler error: in lookup_template_class_1, at cp/pt.c:9459
> >      SmallDenseSet<APInt, 1 << Bits> ExactValues;
> >                                ^~~~
> > Please submit a full bug report,
> > with preprocessed source if appropriate.
> > See <http://bugzilla.redhat.com/bugzilla> for instructions.
> > Preprocessed source stored into /tmp/ccHOOMsN.out file, please attach
> this to your bugreport.
> >
> >
> > Please take a look!
> >
> >
> > Thanks,
> >
> > Matt
> >
> >
> >
> >
> > On Thu, Sep 24, 2020 at 2:37 PM Roman Lebedev via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
> >>
> >>
> >> Author: Roman Lebedev
> >> Date: 2020-09-25T00:36:42+03:00
> >> New Revision: 9bcf7b1c7a139a455400df109d81c638b9e75150
> >>
> >> URL:
> https://github.com/llvm/llvm-project/commit/9bcf7b1c7a139a455400df109d81c638b9e75150
> >> DIFF:
> https://github.com/llvm/llvm-project/commit/9bcf7b1c7a139a455400df109d81c638b9e75150.diff
> >>
> >> LOG: [NFCI][IR] ConstantRangeTest: add basic scaffolding for next-gen
> precision/correctness testing
> >>
> >> I have long complained that while we have exhaustive tests
> >> for ConstantRange, they are, uh, not good.
> >>
> >> The approach of groking our own constant range
> >> via exhaustive enumeration is, mysterious.
> >>
> >> It neither tells us without doubt that the result is
> >> conservatively correct, nor the precise match to the ConstantRange
> >> result tells us that the result is precise.
> >> But yeah, it's fast, i give it that.
> >>
> >> In short, there are three things that we need to check:
> >> 1. That ConstantRange result is conservatively correct
> >> 2. That ConstantRange range is reasonable
> >> 3. That ConstantRange result is reasonably precise
> >>
> >> So let's not just check the middle one, but all three.
> >>
> >> This provides precision test coverage for D88178.
> >>
> >> Added:
> >>
> >>
> >> Modified:
> >>     llvm/unittests/IR/ConstantRangeTest.cpp
> >>
> >> Removed:
> >>
> >>
> >>
> >>
> ################################################################################
> >> diff  --git a/llvm/unittests/IR/ConstantRangeTest.cpp
> b/llvm/unittests/IR/ConstantRangeTest.cpp
> >> index 5e8a98e61f85..6e574dc2192e 100644
> >> --- a/llvm/unittests/IR/ConstantRangeTest.cpp
> >> +++ b/llvm/unittests/IR/ConstantRangeTest.cpp
> >> @@ -59,6 +59,12 @@ static void ForeachNumInConstantRange(const
> ConstantRange &CR, Fn TestFn) {
> >>    }
> >>  }
> >>
> >> +unsigned GetNumValuesInConstantRange(const ConstantRange &CR) {
> >> +  unsigned NumValues = 0;
> >> +  ForeachNumInConstantRange(CR, [&NumValues](const APInt &) {
> ++NumValues; });
> >> +  return NumValues;
> >> +}
> >> +
> >>  struct OpRangeGathererBase {
> >>    void account(const APInt &N);
> >>    ConstantRange getRange();
> >> @@ -107,6 +113,79 @@ struct SignedOpRangeGatherer : public
> OpRangeGathererBase {
> >>    }
> >>  };
> >>
> >> +struct AccumulatedPrecisionData {
> >> +  unsigned NumActualValues;
> >> +  unsigned NumValuesInActualCR;
> >> +  unsigned NumValuesInExactCR;
> >> +
> >> +  // If NumValuesInActualCR and NumValuesInExactCR are identical, and
> are not
> >> +  // equal to the NumActualValues, then the implementation is
> >> +  // overly conservatively correct, i.e. imprecise.
> >> +
> >> +  void reset() {
> >> +    NumActualValues = 0;
> >> +    NumValuesInActualCR = 0;
> >> +    NumValuesInExactCR = 0;
> >> +  }
> >> +};
> >> +
> >> +template <typename OpRangeGathererTy, typename Fn1, typename Fn2>
> >> +static void TestUnaryOpExhaustive(Fn1 RangeFn, Fn2 IntFn,
> >> +                                  AccumulatedPrecisionData &Total) {
> >> +  Total.reset();
> >> +
> >> +  constexpr unsigned Bits = 4;
> >> +
> >> +  EnumerateConstantRanges(Bits, [&](const ConstantRange &CR) {
> >> +    // We'll want to record each true new value, for precision testing.
> >> +    SmallDenseSet<APInt, 1 << Bits> ExactValues;
> >> +
> >> +    // What constant range does ConstantRange method return?
> >> +    ConstantRange ActualCR = RangeFn(CR);
> >> +
> >> +    // We'll want to sanity-check the ActualCR, so this will build our
> own CR.
> >> +    OpRangeGathererTy ExactR(CR.getBitWidth());
> >> +
> >> +    // Let's iterate for each value in the original constant range.
> >> +    ForeachNumInConstantRange(CR, [&](const APInt &N) {
> >> +      // For this singular value, what is the true new value?
> >> +      const APInt NewN = IntFn(N);
> >> +
> >> +      // Constant range provided by ConstantRange method must be
> conservatively
> >> +      // correct, it must contain the true new value.
> >> +      EXPECT_TRUE(ActualCR.contains(NewN));
> >> +
> >> +      // Record this true new value in our own constant range.
> >> +      ExactR.account(NewN);
> >> +
> >> +      // And record the new true value itself.
> >> +      ExactValues.insert(NewN);
> >> +    });
> >> +
> >> +    // So, what range did we grok by exhaustively looking over each
> value?
> >> +    ConstantRange ExactCR = ExactR.getRange();
> >> +
> >> +    // So, how many new values are there actually, and as per the
> ranges?
> >> +    unsigned NumActualValues = ExactValues.size();
> >> +    unsigned NumValuesInExactCR = GetNumValuesInConstantRange(ExactCR);
> >> +    unsigned NumValuesInActualCR =
> GetNumValuesInConstantRange(ActualCR);
> >> +
> >> +    // Ranges should contain at least as much values as there actually
> was,
> >> +    // but it is possible they will contain extras.
> >> +    EXPECT_GE(NumValuesInExactCR, NumActualValues);
> >> +    EXPECT_GE(NumValuesInActualCR, NumActualValues);
> >> +
> >> +    // We expect that OpRangeGathererTy produces the exactly identical
> range
> >> +    // to what the ConstantRange method does.
> >> +    EXPECT_EQ(ExactR.getRange(), ActualCR);
> >> +
> >> +    // For precision testing, accumulate the overall numbers.
> >> +    Total.NumActualValues += NumActualValues;
> >> +    Total.NumValuesInActualCR += NumValuesInActualCR;
> >> +    Total.NumValuesInExactCR += NumValuesInExactCR;
> >> +  });
> >> +}
> >> +
> >>  template <typename Fn1, typename Fn2>
> >>  static void TestUnsignedUnaryOpExhaustive(Fn1 RangeFn, Fn2 IntFn,
> >>                                            bool SkipSignedIntMin =
> false) {
> >> @@ -2400,9 +2479,16 @@ TEST_F(ConstantRangeTest, binaryXor) {
> >>  }
> >>
> >>  TEST_F(ConstantRangeTest, binaryNot) {
> >> -  TestUnsignedUnaryOpExhaustive(
> >> +  AccumulatedPrecisionData Precision;
> >> +
> >> +  TestUnaryOpExhaustive<UnsignedOpRangeGatherer>(
> >>        [](const ConstantRange &CR) { return CR.binaryNot(); },
> >> -      [](const APInt &N) { return ~N; });
> >> +      [](const APInt &N) { return ~N; }, Precision);
> >> +  // FIXME: the implementation is not precise.
> >> +  EXPECT_EQ(Precision.NumActualValues, 1936u);
> >> +  EXPECT_EQ(Precision.NumValuesInActualCR, 2496u);
> >> +  EXPECT_EQ(Precision.NumValuesInExactCR, 2496u);
> >> +
> >>    TestUnsignedUnaryOpExhaustive(
> >>        [](const ConstantRange &CR) {
> >>          return CR.binaryXor(
> >>
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://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/20200925/18355e3d/attachment.html>


More information about the llvm-commits mailing list