[llvm-dev] RFC: Import of Integer Set Library into LLVM source tree

[Michael Kruse via llvm-dev]

Hi Chris,

thanks for your feedback. I have some additions to what Tobias already
mentioned.

2018-01-17 7:22 GMT+01:00 Chris Lattner <clattner at nondot.org>:
> I have significant concerns about including ISL into LLVM.  LLVM intentionally takes a very conservative approach to bundling third party code with itself: we prefer instead of have them be external dependencies that are conditionally configured in on demand.
>
> I’m not an expert in ISL in particular, but it looks like it is something of an odd dependency to include: it isn’t structured at all like LLVM, and it (apparently, I could definitely be wrong) depends on the GMP library which is LGPL.

isl has three arbitrary-length integer backends: gmp, imath
(https://github.com/creachadair/imath, MIT licensed), imath32 (32 bit
integer, falls back to imath on overflow).

The imath support has been added because of the licensing issue when
included into Polly.

> Beyond licensing, it has an old-school C-style API which appears to be extremely inefficient compared to things like APInt that avoid allocations for 64-bit integers and less.

The imath32 backend was written because of this. We are constantly
improving the performance.

> Why should this be included with every LLVM distribution instead of being conditionally compiled?

The patch https://reviews.llvm.org/D40122 compiles isl conditionally
(LLVM_INCLUDE_ISL, off by default).

>  If the major issue is the polly regression tests, why not fix the tests?

The regression tests verify that the LLVM IR is correctly represented
in isl (affine expressions, iteration domains, dependencies, etc),
which in turn depends on isl's string output. The only way I see to
avoid this issue is to not check those at all. Only (input IR ->
generated IR) tests remain. It is like testing ScalarEvolution without
being able to dump an llvm::SCEV.


Michael