[polly] r279815 - Improve documentation and testing for isl_valFromAPInt

Tobias Grosser via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 26 05:01:08 PDT 2016


Author: grosser
Date: Fri Aug 26 07:01:07 2016
New Revision: 279815

URL: http://llvm.org/viewvc/llvm-project?rev=279815&view=rev
Log:
Improve documentation and testing for isl_valFromAPInt

The recent unit tests we gained made clear that the semantics of
isl_valFromAPInt are not clear, due to missing documentation. In this change we
document both the calling interface as well as the implementation of
isl_valFromAPInt.

We also make the implementation easier to read by removing integer wrappig in
abs() when passing in the minimal integer value for a given bitwidth. Even
though wrapping and subsequently interpreting the result as unsigned value gives
the correct result, this is far from obvious.  Instead, we explicitly add one
more bit to the input type to ensure that abs will never wrap. This change did
not uncover a bug in the old implementation, but was introduced to increase
readability.

We update the tests to add a test case for this special case and use this
opportunity to also test a number larger than 64 bit. Finally, we order the
arguments of the test cases to make sure the expected output is first. This
helps readability in case of failing test cases as gtest assumes the first value
to be the exected value.

Reviewed-by: Michael Kruse <llvm at meinersbur.de>
Differential Revision: https://reviews.llvm.org/D23917

Modified:
    polly/trunk/include/polly/Support/GICHelper.h
    polly/trunk/lib/Support/GICHelper.cpp
    polly/trunk/unittests/Isl/IslTest.cpp

Modified: polly/trunk/include/polly/Support/GICHelper.h
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/include/polly/Support/GICHelper.h?rev=279815&r1=279814&r2=279815&view=diff
==============================================================================
--- polly/trunk/include/polly/Support/GICHelper.h (original)
+++ polly/trunk/include/polly/Support/GICHelper.h Fri Aug 26 07:01:07 2016
@@ -37,6 +37,36 @@ class Value;
 } // namespace llvm
 
 namespace polly {
+
+/// @brief Translate an llvm::APInt to an isl_val.
+///
+/// Translate the bitsequence without sign information as provided by APInt into
+/// a signed isl_val type. Depending on the value of @p IsSigned @p Int is
+/// interpreted as unsigned value or as signed value in two's complement
+/// representation.
+///
+/// Input IsSigned                 Output
+///
+///     0        0           ->    0
+///     1        0           ->    1
+///    00        0           ->    0
+///    01        0           ->    1
+///    10        0           ->    2
+///    11        0           ->    3
+///
+///     0        1           ->    0
+///     1        1           ->   -1
+///    00        1           ->    0
+///    01        1           ->    1
+///    10        1           ->   -2
+///    11        1           ->   -1
+///
+/// @param Ctx      The isl_ctx to create the isl_val in.
+/// @param Int      The integer value to translate.
+/// @param IsSigned If the APInt should be interpreted as signed or unsigned
+///                 value.
+///
+/// @return The isl_val corresponding to @p Int.
 __isl_give isl_val *isl_valFromAPInt(isl_ctx *Ctx, const llvm::APInt Int,
                                      bool IsSigned);
 

Modified: polly/trunk/lib/Support/GICHelper.cpp
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Support/GICHelper.cpp?rev=279815&r1=279814&r2=279815&view=diff
==============================================================================
--- polly/trunk/lib/Support/GICHelper.cpp (original)
+++ polly/trunk/lib/Support/GICHelper.cpp Fri Aug 26 07:01:07 2016
@@ -30,8 +30,19 @@ __isl_give isl_val *polly::isl_valFromAP
   APInt Abs;
   isl_val *v;
 
+  // As isl is interpreting the input always as unsigned value, we need some
+  // additional pre and post processing to import signed values. The approach
+  // we take is to first obtain the absolute value of Int and then negate the
+  // value after it has been imported to isl.
+  //
+  // It should be noted that the smallest integer value represented in two's
+  // complement with a certain amount of bits does not have a corresponding
+  // positive representation in two's complement representation with the same
+  // number of bits. E.g. 110 (-2) does not have a corresponding value for (2).
+  // To ensure that there is always a corresponding value available we first
+  // sign-extend the input by one bit and only then take the absolute value.
   if (IsSigned)
-    Abs = Int.abs();
+    Abs = Int.sext(Int.getBitWidth() + 1).abs();
   else
     Abs = Int;
 

Modified: polly/trunk/unittests/Isl/IslTest.cpp
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/unittests/Isl/IslTest.cpp?rev=279815&r1=279814&r2=279815&view=diff
==============================================================================
--- polly/trunk/unittests/Isl/IslTest.cpp (original)
+++ polly/trunk/unittests/Isl/IslTest.cpp Fri Aug 26 07:01:07 2016
@@ -20,6 +20,43 @@ TEST(Isl, APIntToIslVal) {
   isl_ctx *IslCtx = isl_ctx_alloc();
 
   {
+    APInt APZero(1, 0, true);
+    auto *IslZero = isl_valFromAPInt(IslCtx, APZero, true);
+    EXPECT_EQ(isl_bool_true, isl_val_is_zero(IslZero));
+    isl_val_free(IslZero);
+  }
+
+  {
+    APInt APNOne(1, -1, true);
+    auto *IslNOne = isl_valFromAPInt(IslCtx, APNOne, true);
+    EXPECT_EQ(isl_bool_true, isl_val_is_negone(IslNOne));
+    isl_val_free(IslNOne);
+  }
+
+  {
+    APInt APZero(1, 0, false);
+    auto *IslZero = isl_valFromAPInt(IslCtx, APZero, false);
+    EXPECT_EQ(isl_bool_true, isl_val_is_zero(IslZero));
+    isl_val_free(IslZero);
+  }
+
+  {
+    APInt APOne(1, 1, false);
+    auto *IslOne = isl_valFromAPInt(IslCtx, APOne, false);
+    EXPECT_EQ(isl_bool_true, isl_val_is_one(IslOne));
+    isl_val_free(IslOne);
+  }
+
+  {
+    APInt APNTwo(2, -2, true);
+    auto *IslNTwo = isl_valFromAPInt(IslCtx, APNTwo, true);
+    auto *IslNTwoCmp = isl_val_int_from_si(IslCtx, -2);
+    EXPECT_EQ(isl_bool_true, isl_val_eq(IslNTwo, IslNTwoCmp));
+    isl_val_free(IslNTwo);
+    isl_val_free(IslNTwoCmp);
+  }
+
+  {
     APInt APNOne(32, -1, true);
     auto *IslNOne = isl_valFromAPInt(IslCtx, APNOne, true);
     EXPECT_EQ(isl_bool_true, isl_val_is_negone(IslNOne));
@@ -29,21 +66,21 @@ TEST(Isl, APIntToIslVal) {
   {
     APInt APZero(32, 0, false);
     auto *IslZero = isl_valFromAPInt(IslCtx, APZero, false);
-    EXPECT_EQ(isl_val_is_zero(IslZero), isl_bool_true);
+    EXPECT_EQ(isl_bool_true, isl_val_is_zero(IslZero));
     isl_val_free(IslZero);
   }
 
   {
     APInt APOne(32, 1, false);
     auto *IslOne = isl_valFromAPInt(IslCtx, APOne, false);
-    EXPECT_EQ(isl_val_is_one(IslOne), isl_bool_true);
+    EXPECT_EQ(isl_bool_true, isl_val_is_one(IslOne));
     isl_val_free(IslOne);
   }
 
   {
     APInt APTwo(32, 2, false);
     auto *IslTwo = isl_valFromAPInt(IslCtx, APTwo, false);
-    EXPECT_EQ(isl_val_cmp_si(IslTwo, 2), 0);
+    EXPECT_EQ(0, isl_val_cmp_si(IslTwo, 2));
     isl_val_free(IslTwo);
   }
 
@@ -51,11 +88,22 @@ TEST(Isl, APIntToIslVal) {
     APInt APNOne(32, (1ull << 32) - 1, false);
     auto *IslNOne = isl_valFromAPInt(IslCtx, APNOne, false);
     auto *IslRef = isl_val_int_from_ui(IslCtx, (1ull << 32) - 1);
-    EXPECT_EQ(isl_val_eq(IslNOne, IslRef), isl_bool_true);
+    EXPECT_EQ(isl_bool_true, isl_val_eq(IslNOne, IslRef));
     isl_val_free(IslNOne);
     isl_val_free(IslRef);
   }
 
+  {
+    APInt APLarge(130, 2, false);
+    APLarge = APLarge.shl(70);
+    auto *IslLarge = isl_valFromAPInt(IslCtx, APLarge, false);
+    auto *IslRef = isl_val_int_from_ui(IslCtx, 71);
+    IslRef = isl_val_2exp(IslRef);
+    EXPECT_EQ(isl_bool_true, isl_val_eq(IslLarge, IslRef));
+    isl_val_free(IslLarge);
+    isl_val_free(IslRef);
+  }
+
   isl_ctx_free(IslCtx);
 }
 




More information about the llvm-commits mailing list