[polly] r279734 - Introduce unittests.

[Tobias Grosser via llvm-commits]

On Thu, Aug 25, 2016, at 09:44 PM, Michael Kruse via llvm-commits wrote:
> Hi Tobias,
> 
> depends whether the caller always treats the number as a signed value.
> The only caller IslExprBuilder::createInt most likely calls sext on
> it, therefore it is correct (assuming that also the entire
> IslExprBuilder chain assumed signed values). Ie we can remove the
> fixme.

Yes, at the moment the full IslExprBuilder chain assumes signed values
(or at least it should). We could potentially save some bits here and
there by sometimes assuming unsigned values, but the complexity increase
does not seem to be  justified before we don't have a clear case where
these individual bits would improve performance. Also, this requires us
to get precise knowledge about the range of the generated values, which
by itself requires some work -- even though Sven, Matthias and me are
working on this ATM.
 
> Could we maybe add documentation to this function that points it out?

Does this make it more clear:

https://reviews.llvm.org/D23910

Best,
Tobias
 
> Michael
> 
> 
> 
> 2016-08-25 20:26 GMT+02:00 Tobias Grosser <tobias at grosser.es>:
> > Hi Michael,
> >
> > I just looked into this FIXME below myself, and it seems the code might
> > actually be correct, as we always generate the smallest APInt that is
> > large enough to model a certain value. For -1, this APInt has a size of
> > 1 bit. If interpreted as signed,
> > it can contain the values 0 and -1, with 0 modeled as '0' and -1 modeled
> > as '1' in two's complement representation. If interpreted as unsigned,
> > these values map to '0' and '1'. Hence, the test below seems to be
> > correct to me:
> >
> >> +  {
> >> +    auto *IslNOne = isl_val_int_from_si(IslCtx, -1);
> >> +    auto APNOne = APIntFromVal(IslNOne);
> >> +    // APInt has no sign bit, so never equals to a negative number.
> >> +    // FIXME: The canonical representation of a negative APInt is two's
> >> +    // complement.
> >> +    EXPECT_EQ(APNOne, 1);
> >> +  }
> >
> > It seems we just missed that APInt is of size one bit. What are your
> > thoughts?
> >
> > Best,
> > Tobias
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits