[llvm] 9bcf7b1 - [NFCI][IR] ConstantRangeTest: add basic scaffolding for next-gen precision/correctness testing
Roman Lebedev via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 24 23:21:25 PDT 2020
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
More information about the llvm-commits
mailing list