llvm dev - Jan 2018 - RFC: Import of Integer Set Library into LLVM source tree

On Fri, Jan 19, 2018, at 05:47, Chris Lattner via llvm-dev
wrote:
> 
> 
> > On Jan 18, 2018, at 6:02 AM, Michael Kruse <llvmdev at
meinersbur.de> wrote:
> > 
> > 2018-01-18 6:40 GMT+01:00 Chris Lattner <clattner at
nondot.org>:
> >> 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.
> > 
> >> 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.
> > 
> > This would require buildbot admins to regularly install new versions
of isl.
> 
> It seems like you can choose how frequently to update.  Updating every 6 
> to 12 months doesn’t seem overly onerous, and still gives the benefit of 
> updates from the upstream project.
We tend to update isl at certain times more often and prefer to have a fast
turnaround in updates. The reason is that if a new LLVM change exposes a bug we 
would like to fix the bug in isl immediately and want to get this fix out as
fast as possible.  Should we in this case ask all buildbot admins to update?
What about the debian/ubuntu builds?

Best,
Tobias

On 19 January 2018 at 08:13, Tobias Grosser via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> On Fri, Jan 19, 2018, at 05:47, Chris Lattner via llvm-dev wrote:
>>
>>
>> > On Jan 18, 2018, at 6:02 AM, Michael Kruse <llvmdev at
meinersbur.de> wrote:
>> >
>> > 2018-01-18 6:40 GMT+01:00 Chris Lattner <clattner at
nondot.org>:
>> >> 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.
>> >
>> >> 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.
>> >
>> > This would require buildbot admins to regularly install new
versions of isl.
>>
>> It seems like you can choose how frequently to update.  Updating every
6
>> to 12 months doesn’t seem overly onerous, and still gives the benefit
of
>> updates from the upstream project.
>
> We tend to update isl at certain times more often and prefer to have a fast
turnaround in updates. The reason is that if a new LLVM change exposes a bug we 
would like to fix the bug in isl immediately and want to get this fix out as
fast as possible.  Should we in this case ask all buildbot admins to update?
What about the debian/ubuntu builds?
Maybe it's worth separating the issues here somewhat:

First issue: For a user building LLVM with Polly support, is it
expected they would build against a specifically chosen version of ISL
rather than whatever their distro happens to have packaged. Is it
expected that a distributed LLVM build would be packaged with a
compiled version of the ISL library? It sounds like the answer is yes
to both. Or am I misinterpreting your comments about rapidly updating
ISL alongside Polly?

Second issue: Should the ISL source be bundled with LLVM? This
discussion is clearly _strongly_ influenced by the previous issue.
Including ISL in LLVM might be the first time an external dependency
of this size has been imported. On the other hand, having on ISL as an
external dependency could also be the first time LLVM has such a
tightly coupled dependency (due to versioning requirements).

Best,

Alex

On Fri, Jan 19, 2018, at 14:40, Alex Bradbury via llvm-dev
wrote:
> On 19 January 2018 at 08:13, Tobias Grosser via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
> > On Fri, Jan 19, 2018, at 05:47, Chris Lattner via llvm-dev wrote:
> >>
> >>
> >> > On Jan 18, 2018, at 6:02 AM, Michael Kruse <llvmdev at
meinersbur.de> wrote:
> >> >
> >> > 2018-01-18 6:40 GMT+01:00 Chris Lattner <clattner at
nondot.org>:
> >> >> 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.
> >> >
> >> >> 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.
> >> >
> >> > This would require buildbot admins to regularly install new
versions of isl.
> >>
> >> It seems like you can choose how frequently to update.  Updating
every 6
> >> to 12 months doesn’t seem overly onerous, and still gives the
benefit of
> >> updates from the upstream project.
> >
> > We tend to update isl at certain times more often and prefer to have a
fast turnaround in updates. The reason is that if a new LLVM change exposes a
bug we  would like to fix the bug in isl immediately and want to get this fix
out as fast as possible.  Should we in this case ask all buildbot admins to
update? What about the debian/ubuntu builds?
> 
> Maybe it's worth separating the issues here somewhat:
> 
> First issue: For a user building LLVM with Polly support, is it
> expected they would build against a specifically chosen version of ISL
> rather than whatever their distro happens to have packaged. Is it
> expected that a distributed LLVM build would be packaged with a
> compiled version of the ISL library? It sounds like the answer is yes
> to both. Or am I misinterpreting your comments about rapidly updating
> ISL alongside Polly?
The answer is yes to both at the moment.

I understand that looser coupling is what we want and we indeed successfully
reduced the tight coupling at many places by defining "canonical"
representations and ensuring isl canonicalizes wherever reasonable. This
significantly reduced our test-case deltas on isl version updates.
Unfortunately, it is not always possible to do so and even less if we want to
allow isl to evolve.

We tried both over multiple years, having isl as a somehow flexible external
dependency and as a fixed version-locked dependency. Both is theoretically
feasible, but the latter has at least from my perspective shown to be more
reliable as well as user and support friendly.
 
> Second issue: Should the ISL source be bundled with LLVM? This
> discussion is clearly _strongly_ influenced by the previous issue.
> Including ISL in LLVM might be the first time an external dependency
> of this size has been imported. On the other hand, having on ISL as an
> external dependency could also be the first time LLVM has such a
> tightly coupled dependency (due to versioning requirements).
Right.  As an additional data point (which obviously should not cement the
outcome of this discussion): Today we successfully ship isl with Polly, isl is
included in LLVM releases, and debian / ubuntu ship Polly with a version-locked
bundled isl.

Another data point. A LLVM bare clone has 311M, a bare clone of isl is 6.2M, all
of isl zipped without history is 1.1M. As we plan to not import history, the
1.1M data added gives a repository increase of 0.36%.

Best,
Tobias