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@meinersbur.de>
Differential Revision: https://reviews.llvm.org/D23917

llvm-svn: 279815
This commit is contained in:
Tobias Grosser 2016-08-26 12:01:07 +00:00
parent 0a1986790c
commit 437200089d
3 changed files with 94 additions and 5 deletions

View File

@ -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);

View File

@ -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;

View File

@ -19,6 +19,43 @@ namespace {
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);
@ -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);
}