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

[Chris Lattner via llvm-dev]

> On Jan 17, 2018, at 12:14 AM, Tobias Grosser <tobias.grosser at inf.ethz.ch> wrote:
>> 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 understand. If needed we can make the compilation of isl optional.
> 
> However, as pointed out earlier in this thread it seems preferable to lock LLVM with a very specific version of isl to make sure version differences don't cause spurious bugs. This at least has shown to be very useful in Polly.

Great, I think that that would be a fine approach: you can have the cmake logic detect which version of isl is installed and fail if it is the wrong version.  This would address my concern.

>> What other options have you considered?  
> 
> There are no other license compatible alternatives for the full set of functionalities we are using.

I’m not familiar with this technology or how much of it you’re using, but… how much code is this really?  Have you considered designing a from scratch implementation in the LLVM style?  This would presumably be much more efficient, it would be something you could control, and you’d be able to focus it on a specific requirement.

>> Why should this be included with every LLVM distribution instead of 
>> being conditionally compiled?  If the major issue is the polly 
>> regression tests, why not fix the tests?
> 
> We currently don't have any issue. isl is today shipped as part of the Polly repository.
> 
> As Polly matured over time, all license issues have been resolved, we want to better integrate Polly also with the new pass manager, and alternative uses of parts of isl arose, we propose (see other thread) to move Polly into the LLVM core repository. This would allow also for a  closer integration of polyhedral operations with parts of LLVM. This patch focuses on where/how to integrate isl.

Right, but I understand that you’re interested in getting polly integrated with LLVM (something I don’t yet fully understand the implications of).  That is a very different situation than including ISL in Polly when Polly is out of tree.

The motivation explained in the original email seemed to hinge around the fact that Polly’s unit tests depend on accidental details of ISL that change across releases.  If you can fix that, then it seems more plausible to unbundle it.

If you can’t fix that, and can’t replace ISL, then the best approach seems to have CMake detect and reject incompatible versions.

-Chris