From 437200089d5712d3714880ee2eb64d855dfc60a6 Mon Sep 17 00:00:00 2001 From: Tobias Grosser Date: Fri, 26 Aug 2016 12:01:07 +0000 Subject: [PATCH] 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 Differential Revision: https://reviews.llvm.org/D23917 llvm-svn: 279815 --- polly/include/polly/Support/GICHelper.h | 30 ++++++++++++++++++ polly/lib/Support/GICHelper.cpp | 13 +++++++- polly/unittests/Isl/IslTest.cpp | 56 ++++++++++++++++++++++++++++++--- 3 files changed, 94 insertions(+), 5 deletions(-) diff --git a/polly/include/polly/Support/GICHelper.h b/polly/include/polly/Support/GICHelper.h index 4686b6c..e4861eb 100644 --- a/polly/include/polly/Support/GICHelper.h +++ b/polly/include/polly/Support/GICHelper.h @@ -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); diff --git a/polly/lib/Support/GICHelper.cpp b/polly/lib/Support/GICHelper.cpp index 48b6013..e3a5999 100644 --- a/polly/lib/Support/GICHelper.cpp +++ b/polly/lib/Support/GICHelper.cpp @@ -30,8 +30,19 @@ __isl_give isl_val *polly::isl_valFromAPInt(isl_ctx *Ctx, const APInt Int, 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; diff --git a/polly/unittests/Isl/IslTest.cpp b/polly/unittests/Isl/IslTest.cpp index 6b3fc03..5dccaba 100644 --- a/polly/unittests/Isl/IslTest.cpp +++ b/polly/unittests/Isl/IslTest.cpp @@ -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); } -- 2.7.4