Patchwork [2/2] document ISL requirement for GCC installation

login
register
mail settings
Submitter Sebastian Pop
Date Aug. 12, 2011, 3:16 p.m.
Message ID <1313162165-12071-2-git-send-email-sebpop@gmail.com>
Download mbox | patch
Permalink /patch/109854/
State New
Headers show

Comments

Sebastian Pop - Aug. 12, 2011, 3:16 p.m.
---
 gcc/doc/install.texi |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)
Sven Verdoolaege - Aug. 12, 2011, 3:25 p.m.
On Fri, Aug 12, 2011 at 10:16:05AM -0500, Sebastian Pop wrote:
> ---
>  gcc/doc/install.texi |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
> index 368221f..f2b2fd9 100644
> --- a/gcc/doc/install.texi
> +++ b/gcc/doc/install.texi
> @@ -368,6 +368,11 @@ It can be downloaded from @uref{http://www.cs.unipr.it/ppl/Download/}.
>  The configure option @option{--with-ppl} should be used if PPL is not
>  installed in your default library search path.
>  
> +@item Integer Set Library (ISL) version 0.08

Are you going to wait until 0.08 is available before
applying these patches?

> +Necessary to build GCC with the Graphite loop optimizations.
> +It can be downloaded from @uref{http://www.kotnet.org/~skimo/isl/}.

Please use http://freshmeat.net/projects/isl/ instead.

skimo
Joseph S. Myers - Aug. 12, 2011, 5:02 p.m.
On Fri, 12 Aug 2011, Sebastian Pop wrote:

> ---
>  gcc/doc/install.texi |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
> index 368221f..f2b2fd9 100644
> --- a/gcc/doc/install.texi
> +++ b/gcc/doc/install.texi
> @@ -368,6 +368,11 @@ It can be downloaded from @uref{http://www.cs.unipr.it/ppl/Download/}.
>  The configure option @option{--with-ppl} should be used if PPL is not
>  installed in your default library search path.
>  
> +@item Integer Set Library (ISL) version 0.08
> +
> +Necessary to build GCC with the Graphite loop optimizations.
> +It can be downloaded from @uref{http://www.kotnet.org/~skimo/isl/}.

So have things changed relative to what was said in 
<http://gcc.gnu.org/ml/gcc-patches/2010-08/msg00873.html> about ISL being 
included in CLooG-ISL?

Please propose all changes to libraries for building GCC in separate 
threads on the gcc@ list rather than just in patches on gcc-patches - the 
principle of what would be needed should be clearly understood before 
things get to the point of patch posting.  Make very clear what different 
configurations have been tested, including both static and shared 
libraries, different hosts (as many as possible among the primary and 
secondary platforms) and Canadian cross configurations, and solicit 
assistance to test other platforms as needed.  See what was done for the 
original inclusion of Graphite and the associated use of PPL.  Make clear 
whether versions of the libraries in question might affect the code 
generated, and if so then default to version checks tight enough to avoid 
such dependence.
Sven Verdoolaege - Aug. 12, 2011, 5:35 p.m.
On Fri, Aug 12, 2011 at 05:02:18PM +0000, Joseph S. Myers wrote:
> On Fri, 12 Aug 2011, Sebastian Pop wrote:
> > +@item Integer Set Library (ISL) version 0.08
> > +
> > +Necessary to build GCC with the Graphite loop optimizations.
> > +It can be downloaded from @uref{http://www.kotnet.org/~skimo/isl/}.
> 
> So have things changed relative to what was said in 
> <http://gcc.gnu.org/ml/gcc-patches/2010-08/msg00873.html> about ISL being 
> included in CLooG-ISL?

isl is still included in CLooG, but Sebastian now wants to use isl
itself in graphite.

skimo
Jack Howarth - Aug. 12, 2011, 5:50 p.m.
On Fri, Aug 12, 2011 at 07:35:24PM +0200, Sven Verdoolaege wrote:
> On Fri, Aug 12, 2011 at 05:02:18PM +0000, Joseph S. Myers wrote:
> > On Fri, 12 Aug 2011, Sebastian Pop wrote:
> > > +@item Integer Set Library (ISL) version 0.08
> > > +
> > > +Necessary to build GCC with the Graphite loop optimizations.
> > > +It can be downloaded from @uref{http://www.kotnet.org/~skimo/isl/}.
> > 
> > So have things changed relative to what was said in 
> > <http://gcc.gnu.org/ml/gcc-patches/2010-08/msg00873.html> about ISL being 
> > included in CLooG-ISL?
> 
> isl is still included in CLooG, but Sebastian now wants to use isl
> itself in graphite.
> 
> skimo

   Will graphite be totally converted to isl such that ppl can be deprecated
in time for the gcc 4.7 release?
                  Jack
Joseph S. Myers - Aug. 12, 2011, 6:56 p.m.
On Fri, 12 Aug 2011, Sven Verdoolaege wrote:

> On Fri, Aug 12, 2011 at 05:02:18PM +0000, Joseph S. Myers wrote:
> > On Fri, 12 Aug 2011, Sebastian Pop wrote:
> > > +@item Integer Set Library (ISL) version 0.08
> > > +
> > > +Necessary to build GCC with the Graphite loop optimizations.
> > > +It can be downloaded from @uref{http://www.kotnet.org/~skimo/isl/}.
> > 
> > So have things changed relative to what was said in 
> > <http://gcc.gnu.org/ml/gcc-patches/2010-08/msg00873.html> about ISL being 
> > included in CLooG-ISL?
> 
> isl is still included in CLooG, but Sebastian now wants to use isl
> itself in graphite.

I don't see why that should make any difference to the build requirements.  
If CLooG-ISL builds and installs a library libisl.a as well as 
libcloog-isl.a (as config/cloog.m4 thinks it does at present), why should 
someone need to download and install a separate ISL package rather than 
getting libisl.a from CLooG-ISL?
Sven Verdoolaege - Aug. 12, 2011, 7:13 p.m.
On Fri, Aug 12, 2011 at 06:56:38PM +0000, Joseph S. Myers wrote:
> I don't see why that should make any difference to the build requirements.  
> If CLooG-ISL builds and installs a library libisl.a as well as 
> libcloog-isl.a (as config/cloog.m4 thinks it does at present), why should 
> someone need to download and install a separate ISL package rather than 
> getting libisl.a from CLooG-ISL?

The one that comes with CLooG may not be recent enough.

skimo
Joseph S. Myers - Aug. 12, 2011, 7:16 p.m.
On Fri, 12 Aug 2011, Sven Verdoolaege wrote:

> On Fri, Aug 12, 2011 at 06:56:38PM +0000, Joseph S. Myers wrote:
> > I don't see why that should make any difference to the build requirements.  
> > If CLooG-ISL builds and installs a library libisl.a as well as 
> > libcloog-isl.a (as config/cloog.m4 thinks it does at present), why should 
> > someone need to download and install a separate ISL package rather than 
> > getting libisl.a from CLooG-ISL?
> 
> The one that comes with CLooG may not be recent enough.

Do you mean there is not only a requirement to build both libraries, but 
there is a requirement to build CLooG *first*, then ISL, so that ISL's 
libisl.a overwrites CLooG's rather than the other way round (supposing 
that they are installed in the same prefix)?  That's clearly not a 
sensible approach.  If you can't make the version included in CLooG the 
right one for GCC, then stop including it in CLooG altogether (like GMP 
stopped including MPFR some years ago).
Sven Verdoolaege - Aug. 12, 2011, 7:22 p.m.
On Fri, Aug 12, 2011 at 07:16:55PM +0000, Joseph S. Myers wrote:
> Do you mean there is not only a requirement to build both libraries, but 
> there is a requirement to build CLooG *first*, then ISL, so that ISL's 
> libisl.a overwrites CLooG's rather than the other way round (supposing 
> that they are installed in the same prefix)?

No.  You build isl first and configure CLooG to use that isl.

skimo
Sebastian Pop - Aug. 12, 2011, 7:26 p.m.
On Fri, Aug 12, 2011 at 12:50, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> On Fri, Aug 12, 2011 at 07:35:24PM +0200, Sven Verdoolaege wrote:
>> On Fri, Aug 12, 2011 at 05:02:18PM +0000, Joseph S. Myers wrote:
>> > On Fri, 12 Aug 2011, Sebastian Pop wrote:
>> > > +@item Integer Set Library (ISL) version 0.08
>> > > +
>> > > +Necessary to build GCC with the Graphite loop optimizations.
>> > > +It can be downloaded from @uref{http://www.kotnet.org/~skimo/isl/}.
>> >
>> > So have things changed relative to what was said in
>> > <http://gcc.gnu.org/ml/gcc-patches/2010-08/msg00873.html> about ISL being
>> > included in CLooG-ISL?
>>
>> isl is still included in CLooG, but Sebastian now wants to use isl
>> itself in graphite.
>>
>> skimo
>
>   Will graphite be totally converted to isl such that ppl can be deprecated
> in time for the gcc 4.7 release?
>                  Jack

Yes, having GCC only depend on ISL and CLooG-ISL (and not depend
anymore on PPL) is our plan for 4.7.

The rationale behind this is that with PPL it would be very difficult to fix
bugs due to wrap around unsigned types, like this one:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47594
With ISL it is possible to represent the wrapping types and fix this bug.

Sebastian
Joseph S. Myers - Aug. 12, 2011, 7:28 p.m.
On Fri, 12 Aug 2011, Sven Verdoolaege wrote:

> On Fri, Aug 12, 2011 at 07:16:55PM +0000, Joseph S. Myers wrote:
> > Do you mean there is not only a requirement to build both libraries, but 
> > there is a requirement to build CLooG *first*, then ISL, so that ISL's 
> > libisl.a overwrites CLooG's rather than the other way round (supposing 
> > that they are installed in the same prefix)?
> 
> No.  You build isl first and configure CLooG to use that isl.

Will CLooG give an error if you configure it in such a way that its isl 
would overwrite one previously installed?  If not, this seems too 
error-prone - there's no obvious way for CLooG to know whether a 
previously installed isl comes from a previous installation of CLooG 
(should be overwritten) or was installed on its own (so CLooG should be 
built to use it).

The requirements for how CLooG is configured need to be clearly documented 
in GCC's documentation.  But, first, all these proposals (starting with 
the one to use CLooG-ISL instead of CLooG-PPL) need to be raised in their 
own threads on the gcc list, making clear exactly what is proposed, how it 
has been tested on different hosts and how many versions the requirements 
are expected to be stable for - patch submission should be the very last 
stage after sufficient discussion (and notice to the many GCC builders who 
may not follow gcc-patches) on the gcc list.
Jack Howarth - Aug. 12, 2011, 7:30 p.m.
On Fri, Aug 12, 2011 at 09:22:04PM +0200, Sven Verdoolaege wrote:
> On Fri, Aug 12, 2011 at 07:16:55PM +0000, Joseph S. Myers wrote:
> > Do you mean there is not only a requirement to build both libraries, but 
> > there is a requirement to build CLooG *first*, then ISL, so that ISL's 
> > libisl.a overwrites CLooG's rather than the other way round (supposing 
> > that they are installed in the same prefix)?
> 
> No.  You build isl first and configure CLooG to use that isl.
> 
> skimo

Skimo,
   Currently we don't have any checks for the minimal isl version required.
Since the proposed patches only include a requirement for cloog 0.16.3, this
clearly is insufficent to force the use of the correct isl version. Will isl
continue to be bundled with cloog such that the required version of both isl
and cloog will be determined from the cloog version numbering?
         Jack
Sven Verdoolaege - Aug. 12, 2011, 7:38 p.m.
On Fri, Aug 12, 2011 at 07:28:52PM +0000, Joseph S. Myers wrote:
> On Fri, 12 Aug 2011, Sven Verdoolaege wrote:
> 
> > On Fri, Aug 12, 2011 at 07:16:55PM +0000, Joseph S. Myers wrote:
> > > Do you mean there is not only a requirement to build both libraries, but 
> > > there is a requirement to build CLooG *first*, then ISL, so that ISL's 
> > > libisl.a overwrites CLooG's rather than the other way round (supposing 
> > > that they are installed in the same prefix)?
> > 
> > No.  You build isl first and configure CLooG to use that isl.
> 
> Will CLooG give an error if you configure it in such a way that its isl 
> would overwrite one previously installed?  If not, this seems too 
> error-prone - there's no obvious way for CLooG to know whether a 
> previously installed isl comes from a previous installation of CLooG 
> (should be overwritten) or was installed on its own (so CLooG should be 
> built to use it).

If CLooG is told to use a previously built isl, it won't even compile
the bundled isl.

skimo
Sven Verdoolaege - Aug. 12, 2011, 7:41 p.m.
On Fri, Aug 12, 2011 at 03:30:25PM -0400, Jack Howarth wrote:
> Skimo,
>    Currently we don't have any checks for the minimal isl version required.

I assume they will be added at some point.
AFAIU, Sebastian just started working on this.
It will take some time for him to finish the transition.

Anyway, I'm not involved in graphite development.
I was just asked to review some patches on their use of isl.

skimo
Joseph S. Myers - Aug. 13, 2011, 3:32 p.m.
On Fri, 12 Aug 2011, Sven Verdoolaege wrote:

> On Fri, Aug 12, 2011 at 07:28:52PM +0000, Joseph S. Myers wrote:
> > On Fri, 12 Aug 2011, Sven Verdoolaege wrote:
> > 
> > > On Fri, Aug 12, 2011 at 07:16:55PM +0000, Joseph S. Myers wrote:
> > > > Do you mean there is not only a requirement to build both libraries, but 
> > > > there is a requirement to build CLooG *first*, then ISL, so that ISL's 
> > > > libisl.a overwrites CLooG's rather than the other way round (supposing 
> > > > that they are installed in the same prefix)?
> > > 
> > > No.  You build isl first and configure CLooG to use that isl.
> > 
> > Will CLooG give an error if you configure it in such a way that its isl 
> > would overwrite one previously installed?  If not, this seems too 
> > error-prone - there's no obvious way for CLooG to know whether a 
> > previously installed isl comes from a previous installation of CLooG 
> > (should be overwritten) or was installed on its own (so CLooG should be 
> > built to use it).
> 
> If CLooG is told to use a previously built isl, it won't even compile
> the bundled isl.

I advise either removing the option for CLooG to use bundled ISL, or 
making the bundled version the recommended version for GCC.  Having too 
many ways to configure things is bad.
Sebastian Pop - Aug. 13, 2011, 4:02 p.m.
On Sat, Aug 13, 2011 at 10:32, Joseph S. Myers <joseph@codesourcery.com> wrote:
> I advise either removing the option for CLooG to use bundled ISL, or
> making the bundled version the recommended version for GCC.  Having too
> many ways to configure things is bad.

I would prefer using the ISL bundled with CLooG and not have
to provide a way to configure GCC with ISL.

Sebastian
Jack Howarth - Aug. 13, 2011, 4:18 p.m.
On Sat, Aug 13, 2011 at 11:02:40AM -0500, Sebastian Pop wrote:
> On Sat, Aug 13, 2011 at 10:32, Joseph S. Myers <joseph@codesourcery.com> wrote:
> > I advise either removing the option for CLooG to use bundled ISL, or
> > making the bundled version the recommended version for GCC.  Having too
> > many ways to configure things is bad.
> 
> I would prefer using the ISL bundled with CLooG and not have
> to provide a way to configure GCC with ISL.

Sebastian,
   On a related issue, only isl git...

http://repo.or.cz/w/isl.git

seems to have these newer changes. These changes need to be synchronized
with cloog.org git in order to test your proposed patches.
             Jack

> 
> Sebastian
Matthias Klose - Aug. 13, 2011, 4:26 p.m.
On 08/13/2011 06:02 PM, Sebastian Pop wrote:
> On Sat, Aug 13, 2011 at 10:32, Joseph S. Myers <joseph@codesourcery.com> wrote:
>> I advise either removing the option for CLooG to use bundled ISL, or
>> making the bundled version the recommended version for GCC.  Having too
>> many ways to configure things is bad.
> 
> I would prefer using the ISL bundled with CLooG and not have
> to provide a way to configure GCC with ISL.

which would be exactly the way no distribution would use it. So please just
don't bundle ISL with CLoog.

  Matthias
Sebastian Pop - Aug. 13, 2011, 4:30 p.m.
On Sat, Aug 13, 2011 at 11:26, Matthias Klose <doko@debian.org> wrote:
> On 08/13/2011 06:02 PM, Sebastian Pop wrote:
>> On Sat, Aug 13, 2011 at 10:32, Joseph S. Myers <joseph@codesourcery.com> wrote:
>>> I advise either removing the option for CLooG to use bundled ISL, or
>>> making the bundled version the recommended version for GCC.  Having too
>>> many ways to configure things is bad.
>>
>> I would prefer using the ISL bundled with CLooG and not have
>> to provide a way to configure GCC with ISL.
>
> which would be exactly the way no distribution would use it. So please just
> don't bundle ISL with CLoog.

Sven also pointed out that it would be a mistake to use the ISL
bundled with CLooG.
So, I will prepare a patch for GCC to use a configure option --with-isl.

Sebastian
Richard Guenther - Aug. 14, 2011, 9:30 a.m.
On Sat, Aug 13, 2011 at 6:26 PM, Matthias Klose <doko@debian.org> wrote:
> On 08/13/2011 06:02 PM, Sebastian Pop wrote:
>> On Sat, Aug 13, 2011 at 10:32, Joseph S. Myers <joseph@codesourcery.com> wrote:
>>> I advise either removing the option for CLooG to use bundled ISL, or
>>> making the bundled version the recommended version for GCC.  Having too
>>> many ways to configure things is bad.
>>
>> I would prefer using the ISL bundled with CLooG and not have
>> to provide a way to configure GCC with ISL.
>
> which would be exactly the way no distribution would use it. So please just
> don't bundle ISL with CLoog.

Well, I would simply have linked the bundled ISL statically into libcloog.

Richard.
Michael Matz - Aug. 15, 2011, 2:17 p.m.
Hi,

On Sun, 14 Aug 2011, Richard Guenther wrote:

> > which would be exactly the way no distribution would use it. So please 
> > just don't bundle ISL with CLoog.
> 
> Well, I would simply have linked the bundled ISL statically into 
> libcloog.

Which would still require not exporting the (bundled) libisl symbols from 
libcloog.  Looking at the cloog git repo it doesn't seem that it knows 
anything about symbol versions (and it wouldn't help static libs anyway).

No, the only even barely sane way is for the cloog version GCC should use 
to not include a builtin copy of some stale version of isl at all.


Ciao,
Michael.

Patch

diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 368221f..f2b2fd9 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -368,6 +368,11 @@  It can be downloaded from @uref{http://www.cs.unipr.it/ppl/Download/}.
 The configure option @option{--with-ppl} should be used if PPL is not
 installed in your default library search path.
 
+@item Integer Set Library (ISL) version 0.08
+
+Necessary to build GCC with the Graphite loop optimizations.
+It can be downloaded from @uref{http://www.kotnet.org/~skimo/isl/}.
+
 @item CLooG-ISL 0.16
 
 Necessary to build GCC with the Graphite loop optimizations.  It is
@@ -1620,7 +1625,8 @@  a cross compiler, they will not be used to configure target libraries.
 @itemx --with-cloog=@var{pathname}
 @itemx --with-cloog-include=@var{pathname}
 @itemx --with-cloog-lib=@var{pathname}
-If you do not have PPL (the Parma Polyhedra Library) and the CLooG
+If you do not have ISL (the Integer Set Library),
+PPL (the Parma Polyhedra Library), and the CLooG (the Chunky Loop Generator)
 libraries installed in a standard location and you want to build GCC,
 you can explicitly specify the directory where they are installed
 (@samp{--with-ppl=@/@var{pplinstalldir}},