[PATCH] D23833: [Polly] Introduce unittests.

Michael Kruse via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 24 06:25:50 PDT 2016


Meinersbur marked an inline comment as done.

================
Comment at: unittests/Isl/IslTest.cpp:27
@@ +26,3 @@
+    isl_val_free(IslNOne);
+  }
+
----------------
grosser wrote:
>> 22		​  {
> 23		​    APInt APNOne(32, -1, true);
> 24		​    auto *IslNOne = isl_valFromAPInt(IslCtx, APNOne, false);
> 
> with 'false' in valFromAPInt might also be an interesting test case.
I am not sure what the result even should be as the test was specifically designed to get negative one. Currently it returns '4294967295'. If this is the indented result, we should test
```
APInt APNOne(32, 4294967295, false);
auto *IslNOne = isl_valFromAPInt(IslCtx, APNOne, false);
EXPECT_EQ(isl_val_cmp_si(IslNOne , 4294967295), 0);
```
The APInt is equal to `APInt(32, -1, true)`. The third argument only matters if first argument `>64`

================
Comment at: unittests/Isl/IslTest.cpp:61
@@ +60,3 @@
+    // FIXME: The canonical representation of a negative APInt is two's
+    // complement.
+    EXPECT_EQ(APNOne, 1);
----------------
grosser wrote:
> Very interesting observation. Do you have an idea why this is working today? Or does this indeed result in a bug that can be triggered externally?
I don't know.


https://reviews.llvm.org/D23833





More information about the llvm-commits mailing list