<div dir="ltr">Hi Roman,<div><br></div><div>Either this patch or your other NFCI one have caused the ConstantRangeTest to start failing on PPC: <a href="http://lab.llvm.org:8011/builders/clang-ppc64le-linux-lnt/builds/27058">http://lab.llvm.org:8011/builders/clang-ppc64le-linux-lnt/builds/27058</a></div><div><br></div><div><pre style="font-family:"Courier New",courier,monotype,monospace;color:rgb(0,0,0);font-size:medium"><span class="gmail-stdout">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 <</span><span class="gmail-stdout"><a href="http://bugzilla.redhat.com/bugzilla">http://bugzilla.redhat.com/bugzilla</a>> for instructions.
Preprocessed source stored into /tmp/ccHOOMsN.out file, please attach this to your bugreport.</span></pre><pre style="font-family:"Courier New",courier,monotype,monospace;color:rgb(0,0,0);font-size:medium"><span class="gmail-stdout"><br></span></pre><pre style="color:rgb(0,0,0)"><font face="arial, sans-serif" style="">Please take a look!</font></pre><pre style="color:rgb(0,0,0)"><font face="arial, sans-serif" style=""><br></font></pre><pre style="color:rgb(0,0,0)"><font face="arial, sans-serif" style="">Thanks,</font></pre><pre style="color:rgb(0,0,0)"><font face="arial, sans-serif" style="">Matt</font></pre></div><div><br></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Sep 24, 2020 at 2:37 PM Roman Lebedev via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Author: Roman Lebedev<br>
Date: 2020-09-25T00:36:42+03:00<br>
New Revision: 9bcf7b1c7a139a455400df109d81c638b9e75150<br>
<br>
URL: <a href="https://github.com/llvm/llvm-project/commit/9bcf7b1c7a139a455400df109d81c638b9e75150" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/9bcf7b1c7a139a455400df109d81c638b9e75150</a><br>
DIFF: <a href="https://github.com/llvm/llvm-project/commit/9bcf7b1c7a139a455400df109d81c638b9e75150.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/9bcf7b1c7a139a455400df109d81c638b9e75150.diff</a><br>
<br>
LOG: [NFCI][IR] ConstantRangeTest: add basic scaffolding for next-gen precision/correctness testing<br>
<br>
I have long complained that while we have exhaustive tests<br>
for ConstantRange, they are, uh, not good.<br>
<br>
The approach of groking our own constant range<br>
via exhaustive enumeration is, mysterious.<br>
<br>
It neither tells us without doubt that the result is<br>
conservatively correct, nor the precise match to the ConstantRange<br>
result tells us that the result is precise.<br>
But yeah, it's fast, i give it that.<br>
<br>
In short, there are three things that we need to check:<br>
1. That ConstantRange result is conservatively correct<br>
2. That ConstantRange range is reasonable<br>
3. That ConstantRange result is reasonably precise<br>
<br>
So let's not just check the middle one, but all three.<br>
<br>
This provides precision test coverage for D88178.<br>
<br>
Added: <br>
<br>
<br>
Modified: <br>
llvm/unittests/IR/ConstantRangeTest.cpp<br>
<br>
Removed: <br>
<br>
<br>
<br>
################################################################################<br>
diff --git a/llvm/unittests/IR/ConstantRangeTest.cpp b/llvm/unittests/IR/ConstantRangeTest.cpp<br>
index 5e8a98e61f85..6e574dc2192e 100644<br>
--- a/llvm/unittests/IR/ConstantRangeTest.cpp<br>
+++ b/llvm/unittests/IR/ConstantRangeTest.cpp<br>
@@ -59,6 +59,12 @@ static void ForeachNumInConstantRange(const ConstantRange &CR, Fn TestFn) {<br>
}<br>
}<br>
<br>
+unsigned GetNumValuesInConstantRange(const ConstantRange &CR) {<br>
+ unsigned NumValues = 0;<br>
+ ForeachNumInConstantRange(CR, [&NumValues](const APInt &) { ++NumValues; });<br>
+ return NumValues;<br>
+}<br>
+<br>
struct OpRangeGathererBase {<br>
void account(const APInt &N);<br>
ConstantRange getRange();<br>
@@ -107,6 +113,79 @@ struct SignedOpRangeGatherer : public OpRangeGathererBase {<br>
}<br>
};<br>
<br>
+struct AccumulatedPrecisionData {<br>
+ unsigned NumActualValues;<br>
+ unsigned NumValuesInActualCR;<br>
+ unsigned NumValuesInExactCR;<br>
+<br>
+ // If NumValuesInActualCR and NumValuesInExactCR are identical, and are not<br>
+ // equal to the NumActualValues, then the implementation is<br>
+ // overly conservatively correct, i.e. imprecise.<br>
+<br>
+ void reset() {<br>
+ NumActualValues = 0;<br>
+ NumValuesInActualCR = 0;<br>
+ NumValuesInExactCR = 0;<br>
+ }<br>
+};<br>
+<br>
+template <typename OpRangeGathererTy, typename Fn1, typename Fn2><br>
+static void TestUnaryOpExhaustive(Fn1 RangeFn, Fn2 IntFn,<br>
+ AccumulatedPrecisionData &Total) {<br>
+ Total.reset();<br>
+<br>
+ constexpr unsigned Bits = 4;<br>
+<br>
+ EnumerateConstantRanges(Bits, [&](const ConstantRange &CR) {<br>
+ // We'll want to record each true new value, for precision testing.<br>
+ SmallDenseSet<APInt, 1 << Bits> ExactValues;<br>
+<br>
+ // What constant range does ConstantRange method return?<br>
+ ConstantRange ActualCR = RangeFn(CR);<br>
+<br>
+ // We'll want to sanity-check the ActualCR, so this will build our own CR.<br>
+ OpRangeGathererTy ExactR(CR.getBitWidth());<br>
+<br>
+ // Let's iterate for each value in the original constant range.<br>
+ ForeachNumInConstantRange(CR, [&](const APInt &N) {<br>
+ // For this singular value, what is the true new value?<br>
+ const APInt NewN = IntFn(N);<br>
+<br>
+ // Constant range provided by ConstantRange method must be conservatively<br>
+ // correct, it must contain the true new value.<br>
+ EXPECT_TRUE(ActualCR.contains(NewN));<br>
+<br>
+ // Record this true new value in our own constant range.<br>
+ ExactR.account(NewN);<br>
+<br>
+ // And record the new true value itself.<br>
+ ExactValues.insert(NewN);<br>
+ });<br>
+<br>
+ // So, what range did we grok by exhaustively looking over each value?<br>
+ ConstantRange ExactCR = ExactR.getRange();<br>
+<br>
+ // So, how many new values are there actually, and as per the ranges?<br>
+ unsigned NumActualValues = ExactValues.size();<br>
+ unsigned NumValuesInExactCR = GetNumValuesInConstantRange(ExactCR);<br>
+ unsigned NumValuesInActualCR = GetNumValuesInConstantRange(ActualCR);<br>
+<br>
+ // Ranges should contain at least as much values as there actually was,<br>
+ // but it is possible they will contain extras.<br>
+ EXPECT_GE(NumValuesInExactCR, NumActualValues);<br>
+ EXPECT_GE(NumValuesInActualCR, NumActualValues);<br>
+<br>
+ // We expect that OpRangeGathererTy produces the exactly identical range<br>
+ // to what the ConstantRange method does.<br>
+ EXPECT_EQ(ExactR.getRange(), ActualCR);<br>
+<br>
+ // For precision testing, accumulate the overall numbers.<br>
+ Total.NumActualValues += NumActualValues;<br>
+ Total.NumValuesInActualCR += NumValuesInActualCR;<br>
+ Total.NumValuesInExactCR += NumValuesInExactCR;<br>
+ });<br>
+}<br>
+<br>
template <typename Fn1, typename Fn2><br>
static void TestUnsignedUnaryOpExhaustive(Fn1 RangeFn, Fn2 IntFn,<br>
bool SkipSignedIntMin = false) {<br>
@@ -2400,9 +2479,16 @@ TEST_F(ConstantRangeTest, binaryXor) {<br>
}<br>
<br>
TEST_F(ConstantRangeTest, binaryNot) {<br>
- TestUnsignedUnaryOpExhaustive(<br>
+ AccumulatedPrecisionData Precision;<br>
+<br>
+ TestUnaryOpExhaustive<UnsignedOpRangeGatherer>(<br>
[](const ConstantRange &CR) { return CR.binaryNot(); },<br>
- [](const APInt &N) { return ~N; });<br>
+ [](const APInt &N) { return ~N; }, Precision);<br>
+ // FIXME: the implementation is not precise.<br>
+ EXPECT_EQ(Precision.NumActualValues, 1936u);<br>
+ EXPECT_EQ(Precision.NumValuesInActualCR, 2496u);<br>
+ EXPECT_EQ(Precision.NumValuesInExactCR, 2496u);<br>
+<br>
TestUnsignedUnaryOpExhaustive(<br>
[](const ConstantRange &CR) {<br>
return CR.binaryXor(<br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>