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

[Hal Finkel via llvm-dev]

On 01/15/2018 10:52 AM, Michael Kruse via llvm-dev wrote:
> Dear community,
>
> for our goal to make polyhedral optimization available in the main
> LLVM source, we will need the Integer Set Library (isl)[1]. It is the
> main dependency of Polly, but would be required even if we do not
> directly import Polly.
>
> I already prepared a patch [2], unfortunately without feedback on the
> general approach. As Philip suggested in the review, I am reaching out
> with an RFC with a wider audience on llvm-dev.

If this is ready to be reviewed, I recommend removing the "[WIP]" from
the title. Normally I interpret "[WIP]" (which I presume stands for
"work in progress") as meaning "I'm looking for early feedback on this
approach, perhaps primarily from a limited audience, but the patch
itself is not really ready for review (e.g., still known not to build,
to crash, miscompile, and so on).

What you have below sounds generally good to me (a couple of questions
below).
>
> As for the main motivation on why to import the entire source of isl at all:
> Polly interacts relative tightly with isl which provide the main
> optimization algorithms. For instance, Polly's regression tests
> compare the string output of set representation, which unfortunately
> can change between versions. For instance, if a newer version
> implements a simpler string description of the same set. That means
> that tests would fail if a different version is installed on the host
> system. This is significant for buildbots that would need regular
> manual action to install a newer version of isl on the system.
>
> An alternative would be to only store the git sha1 and a CMake script
> would download that version when building. However, it would also be
> more difficult to both sources in-sync in the checkout (think of git
> bisect).
>
> I suppose these were also the reasons why Polly imported the isl
> source in commit r228193 instead of requiring users to call
> utils/checkout_isl.sh manually.
>
>
> The other design decisions and rationales are mentioned in the review
> summary [2]. I repeat them here so they can be referred to in replies.
>
> * The library is named LLVMISL and contained in the lib/ISL folder to
> work best with LLVM's component system. The component's name "ISL" was
> chosen over "isl" as it matches the capitalization of other
> two/three-letter-acronym components.
>
> * Isl's headers are installed into $PREFIX/include/llvm/ISL/isl. This
> is to not conflict with potentially user/system-installed isl headers.
> Please note that this means that MIT-licensed headers land on the
> target system.
>
> * The source of isl itself is added to the contrib/isl directory. This
> is to keep the source in one directory rather than spreading it into
> the lib and include directories, facilitating integration of upstream
> changes.
>  There is a script "isl-merge.py" that automates this. It
> currently requires LLVM being checked out using git. The new subfolder
> "contrib" should make it clear that this is downstream code and maybe
> one shouldn't run clang-format over it which would make the next merge
> difficult.
>
> * There is an additional "GIT_HEAD_ID" file containing the upstream's
> revision. It is used by isl-merge.py to determine the common ancestor
> and CMakeLists.txt to determine the version. Normally isl's make dist
> would generate this file, it is done manually here to not requiring a
> configure/make/make dist cycle when merging the latest isl. Due to
> different Autotools versions being installed on the different systems,
> this might lead to spurious conflicts with the generated configure
> etc. files. However, due to the need of an isl-noexeceptions.h, I may
> need to include such a cycle in isl-merge.py anyway.

Can you clarify when/if autotools are required and under what circumstances.

>
> * Isl has its own unittest "isl_test.c". It is put into bin/ (but not
> installed) and run by llvm-lit on llvm-check. This was just the
> simplest to do since it doesn't require any additional lit.site.cfg in
> order to find the isl_test in a different directory.

Can it end up in the unittests subdirectory with the other unit tests?

Thanks again,
Hal

>
>
> Looking forward for your feedback.
>
> Michael
>
>
> [1] http://isl.gforge.inria.fr/
> [2] https://reviews.llvm.org/D40122
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory