[PATCH] D34982: [Polly][WIP] Fully-Indexed static expansion

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: lib/Transform/MaximalStaticExpansion.cpp:307-308
+           "The upper bound is not a positive integer.");
+    assert(UpperBound.le(isl::val(CurrentAccessMap.get_ctx(),
+                                  std::numeric_limits<unsigned>::max())) &&
+           "The upper bound overflow a long.");
----------------
niosega wrote:
> Meinersbur wrote:
> > This assertion fails on Windows (and 32 bit platforms): The `isl::val` constructor takes a `long`, which is 32 bit this platforms. UINT_MAX exceeds its range.
> It's a mistake from my side to compare it with UINT_MAX.
> 
> If I replace 
> ```
> std::numeric_limits<unsigned>::max()
> ```
> 
> with 
> ```
> std::numeric_limits<long>::max()
> ```
> 
> it should work on every platform, right ?
Except that it is stored into a `std::vector<unsigned>`. Storing a `long` into as an `unsigned int` may get you another overflow.

My suggestion is to stick with the lowest common maximum: `std::numeric_limits<int>::max()`. I don't think you would want to allocate memory larger than that anyway.


https://reviews.llvm.org/D34982