[llvm] b38d897 - [ConstantRange] binaryXor(): special-case binary complement case - the result is precise

[Nikita Popov via llvm-commits]

On Tue, Sep 22, 2020 at 9:48 PM Nikita Popov <nikita.ppv at gmail.com> wrote:

> On Tue, Sep 22, 2020 at 8:52 PM Roman Lebedev <lebedev.ri at gmail.com>
> wrote:
>
>> On Tue, Sep 22, 2020 at 9:47 PM Nikita Popov <nikita.ppv at gmail.com>
>> wrote:
>> >
>> > On Tue, Sep 22, 2020 at 8:38 PM Roman Lebedev via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>> >>
>> >>
>> >> Author: Roman Lebedev
>> >> Date: 2020-09-22T21:37:29+03:00
>> >> New Revision: b38d897e802664034c7e6e4654328256ed370a61
>> >>
>> >> URL:
>> https://github.com/llvm/llvm-project/commit/b38d897e802664034c7e6e4654328256ed370a61
>> >> DIFF:
>> https://github.com/llvm/llvm-project/commit/b38d897e802664034c7e6e4654328256ed370a61.diff
>> >>
>> >> LOG: [ConstantRange] binaryXor(): special-case binary complement case
>> - the result is precise
>> >>
>> >> Use the fact that `~X` is equivalent to `-1 - X`, which gives us
>> >> fully-precise answer, and we only need to special-handle the wrapped
>> case.
>> >>
>> >> This fires ~16k times for vanilla llvm test-suite + RawSpeed.
>> >>
>> >> Added:
>> >>
>> >>
>> >> Modified:
>> >>     llvm/include/llvm/IR/ConstantRange.h
>> >>     llvm/lib/IR/ConstantRange.cpp
>> >>     llvm/unittests/IR/ConstantRangeTest.cpp
>> >>
>> >> Removed:
>> >>
>> >>
>> >>
>> >>
>> ################################################################################
>> >> diff  --git a/llvm/include/llvm/IR/ConstantRange.h
>> b/llvm/include/llvm/IR/ConstantRange.h
>> >> index 318532b24e83..494a14a10cdb 100644
>> >> --- a/llvm/include/llvm/IR/ConstantRange.h
>> >> +++ b/llvm/include/llvm/IR/ConstantRange.h
>> >> @@ -409,6 +409,11 @@ class LLVM_NODISCARD ConstantRange {
>> >>    /// value in \p Other.
>> >>    ConstantRange srem(const ConstantRange &Other) const;
>> >>
>> >> +  /// Return a new range representing the possible values resulting
>> from
>> >> +  /// a binary-xor of a value in this range by an all-one value,
>> >> +  /// aka bitwise complement operation.
>> >> +  ConstantRange binaryNot() const;
>> >> +
>> >>    /// Return a new range representing the possible values resulting
>> >>    /// from a binary-and of a value in this range by a value in \p
>> Other.
>> >>    ConstantRange binaryAnd(const ConstantRange &Other) const;
>> >>
>> >> diff  --git a/llvm/lib/IR/ConstantRange.cpp
>> b/llvm/lib/IR/ConstantRange.cpp
>> >> index 396c39b5b3a8..7b8dd66b993c 100644
>> >> --- a/llvm/lib/IR/ConstantRange.cpp
>> >> +++ b/llvm/lib/IR/ConstantRange.cpp
>> >> @@ -1240,6 +1240,16 @@ ConstantRange ConstantRange::srem(const
>> ConstantRange &RHS) const {
>> >>    return ConstantRange(std::move(Lower), std::move(Upper));
>> >>  }
>> >>
>> >> +ConstantRange ConstantRange::binaryNot() const {
>> >> +  if (isEmptySet())
>> >> +    return getEmpty();
>> >> +
>> >> +  if (isWrappedSet())
>> >> +    return getFull();
>> >
>> >
>> > Probably don't need those checks, as you're forwarding to the sub()
>> implementation anyway?
>> It is needed if we want to give the right answer.
>>
>
> Would you mind elaborating on that? It's not immediately obvious to me why
> the X ^ -1 to -1 - X transform would not hold for wrapped ranges.
>
> Possibly it's just a testing issue? TestUnsignedUnaryOpExhaustive tests
> for the unsigned envelope, so it will report false positives if the result
> is a (correct) wrapped range. Unfortunately, I don't think we have a helper
> for the general case right now.
>

For future reference, I've submitted https://reviews.llvm.org/D88178 to
address this issue.

Nikita
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200924/fb8b8cee/attachment.html>