[PATCH] Change APInt comparison with uint64_t.

[Duncan P. N. Exon Smith]

> On 2015 Jun 23, at 08:14, Paweł Bylica <chfast at gmail.com> wrote:
> 
> Hi chandlerc,
> 
> This patch changes the way APInt is compared with a value of type uint64_t.
> Before the uint64_t value was truncated to the size of APInt before comparison.
> Now the comparison takes into account full 64-bit precision.
> 
> http://reviews.llvm.org/D10655
> 
> Files:
>  include/llvm/ADT/APInt.h
>  unittests/ADT/APIntTest.cpp
> 

You never got a response from your llvmdev post.  There are two ways to
go here:

 1. Assert that the value is in the range of BitWidth.
 2. Extend this to 64-bits and compare.

I'm inclined to agree that (2) is more useful -- developers can opt-in
to the old behaviour by hand-constructing an `APInt()` -- but I'd like
to hear from someone else before this is committed.

In the meantime, review inline below.

> Index: include/llvm/ADT/APInt.h
> ===================================================================
> --- include/llvm/ADT/APInt.h
> +++ include/llvm/ADT/APInt.h
> @@ -1038,7 +1038,9 @@
>    /// the validity of the less-than relationship.
>    ///
>    /// \returns true if *this < RHS when considered unsigned.
> -  bool ult(uint64_t RHS) const { return ult(APInt(getBitWidth(), RHS)); }
> +  bool ult(uint64_t RHS) const {
> +    return getActiveBits() > 64 ? false : getZExtValue() < RHS;
> +  }
>  
>    /// \brief Signed less than comparison
>    ///
> @@ -1054,7 +1056,9 @@
>    /// the validity of the less-than relationship.
>    ///
>    /// \returns true if *this < RHS when considered signed.
> -  bool slt(uint64_t RHS) const { return slt(APInt(getBitWidth(), RHS)); }
> +  bool slt(int64_t RHS) const {
> +    return getMinSignedBits() >= 64 ? isNegative() : getSExtValue() < RHS;

Shouldn't this be `> 64`?

Consider `this` equal to 0x8000000000000000:
  - getMinSignedBits() => 64
  - isNegative() => false
  - this->slt(0x8000000000000001) => false
  - this->slt(0x8000000000000000) => false
  - this->slt(0x7FFFFFFFFFFFFFFF) => false (??)

> +  }
>  
>    /// \brief Unsigned less or equal comparison
>    ///
> @@ -1070,7 +1074,7 @@
>    /// the validity of the less-or-equal relationship.
>    ///
>    /// \returns true if *this <= RHS when considered unsigned.
> -  bool ule(uint64_t RHS) const { return ule(APInt(getBitWidth(), RHS)); }
> +  bool ule(uint64_t RHS) const { return ult(RHS) || *this == RHS; }

It's kind of gross to be calling into `ult()` and `==`, since those are
both non-trivial.

IMO, you should implement these directly:
  - sgt
  - slt
  - ugt
  - ult

And implement these trivially in terms of those:
  - sge
  - sle
  - uge
  - ule

>  
>    /// \brief Signed less or equal comparison
>    ///