| Message ID | 4D5ACEFF.8090405@net-b.de |
|---|---|
| State | New |
| Headers | show |
Hi, On Tue, Feb 15, 2011 at 13:07, Tobias Burnus <burnus@net-b.de> wrote: > Dear Sebastian and Tobias, hello all, > > I think GCC should automatically link libisl ("-lisl") if one configures GCC > to use cloog-isl. That's the first patch at > http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01272.html , which remains > unreviewed. (The second part about "-lpwl" looks a bit odd.) > > The issue came also up today at #gfortran - and the proposed solutions was > the same. > > I think automatically linking libisl makes sense and is also in line with > PPL, which is linked via the toplevel configure.ac (cf. second - bogus - > part of the linked patch or simply the file itself). > > The patch I am talking about is: > > Index: config/cloog.m4 > =================================================================== > --- config/cloog.m4 (revision 166641) > +++ config/cloog.m4 (working copy) > @@ -143,7 +143,7 @@ > ;; > "ISL") > clooginc="${clooginc} ${_cloogorginc}" > - clooglibs="${clooglibs} -lcloog-isl" > + clooglibs="${clooglibs} -lcloog-isl -lisl" I think that this change makes sense but I cannot approve it. Paolo, could you please review this as well? Thanks, Sebastian > cloog_org=yes > ;; > "PPL") > > > > Tobias >
* Sebastian Pop wrote on Tue, Feb 15, 2011 at 08:38:28PM CET: > On Tue, Feb 15, 2011 at 13:07, Tobias Burnus wrote: > > --- config/cloog.m4 (revision 166641) > > +++ config/cloog.m4 (working copy) > > @@ -143,7 +143,7 @@ > > ;; > > "ISL") > > clooginc="${clooginc} ${_cloogorginc}" > > - clooglibs="${clooglibs} -lcloog-isl" > > + clooglibs="${clooglibs} -lcloog-isl -lisl" > > I think that this change makes sense but I cannot approve it. > Paolo, could you please review this as well? I'm not Paolo. But the patch is OK if libisl is always needed for this configuration. Does GCC call functions from libisl directly? If yes, then adding -lisl is unconditionally a good thing. If not, then this should primarily help for static linking. I assume that GCC cares less about the odd extra library linked against than a broken link (witness the ppllibs in configure.ac). > > cloog_org=yes > > ;; > > "PPL") Thanks, Ralf
> I'm not Paolo. But the patch is OK if libisl is always needed for this > configuration. Thanks Ralf, I will regstrap the patch and commit it later today. > Does GCC call functions from libisl directly? If yes, then adding -lisl > is unconditionally a good thing. If not, then this should primarily > help for static linking. In 4.6 there are no ISL functions called directly from GCC. In 4.7 we plan to use some of ISL's functionality (array data flow analysis) to check (or replace) the same analysis of GCC. > I assume that GCC cares less about the odd > extra library linked against than a broken link (witness the ppllibs in > configure.ac). The watchdog library is required only when the PPL is configured with --enable-watchdog. This is not a requirement of GCC, but could help to fix the linking problem reported. Is the second patch of http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01272.html ok for trunk as well? Thanks, Sebastian
* Sebastian Pop wrote on Tue, Feb 15, 2011 at 10:19:32PM CET: > The watchdog library is required only when the PPL is configured with > --enable-watchdog. This is not a requirement of GCC, but could help > to fix the linking problem reported. Is the second patch of > http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01272.html > ok for trunk as well? Where is the bug report for this? Is it possible to find out whether PPL was configured with --enable-watchdog? Does PPL install a pkg-config .pc file or a libtool .la file from which one could gather the required information automatically? Is libpwl even built and installed from PPL if --enable-watchdog was not passed to its configure script (or even --disable-watchdog was passed)? Are the two libraries distributed in the same package on all common distributions (or, if in two packages, does the one with PPL routinely depend on the pwl one)? Thanks, Ralf
In message <20110215213906.GI24361@gmx.de>
Ralf Wildenhues <Ralf.Wildenhues@gmx.de> wrote:
> * Sebastian Pop wrote on Tue, Feb 15, 2011 at 10:19:32PM CET:
> > The watchdog library is required only when the PPL is configured with
> > --enable-watchdog. This is not a requirement of GCC, but could help
> > to fix the linking problem reported. Is the second patch of
> > http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01272.html
> > ok for trunk as well?
>
> Where is the bug report for this? Is it possible to find out whether
> PPL was configured with --enable-watchdog? Does PPL install a
> pkg-config .pc file or a libtool .la file from which one could gather
> the required information automatically?
>
> Is libpwl even built and installed from PPL if --enable-watchdog was not
> passed to its configure script (or even --disable-watchdog was passed)?
> Are the two libraries distributed in the same package on all common
> distributions (or, if in two packages, does the one with PPL routinely
> depend on the pwl one)?
No bug report but see http://gcc.gnu.org/ml/gcc-patches/2011-01/msg01946.html
especially http://gcc.gnu.org/ml/gcc-patches/2011-01/msg02177.html
Looks like PPL's pkg-config needs fixing.
John.
On Feb 15, 2011, Tobias Burnus <burnus@net-b.de> wrote: > - clooglibs="${clooglibs} -lcloog-isl" > + clooglibs="${clooglibs} -lcloog-isl -lisl" This is ok, thanks.
* John Tytgat wrote on Tue, Feb 15, 2011 at 11:16:16PM CET: > In message <20110215213906.GI24361@gmx.de> > Ralf Wildenhues wrote: > > * Sebastian Pop wrote on Tue, Feb 15, 2011 at 10:19:32PM CET: > > > The watchdog library is required only when the PPL is configured with > > > --enable-watchdog. This is not a requirement of GCC, but could help > > > to fix the linking problem reported. Is the second patch of > > > http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01272.html > > > ok for trunk as well? > > > > Where is the bug report for this? Is it possible to find out whether > > PPL was configured with --enable-watchdog? Does PPL install a > > pkg-config .pc file or a libtool .la file from which one could gather > > the required information automatically? > > > > Is libpwl even built and installed from PPL if --enable-watchdog was not > > passed to its configure script (or even --disable-watchdog was passed)? > > Are the two libraries distributed in the same package on all common > > distributions (or, if in two packages, does the one with PPL routinely > > depend on the pwl one)? > > No bug report but see http://gcc.gnu.org/ml/gcc-patches/2011-01/msg01946.html > especially http://gcc.gnu.org/ml/gcc-patches/2011-01/msg02177.html In that case the patch is not OK. > Looks like PPL's pkg-config needs fixing. I guess. For GCC, a more acceptable alternative than the proposed patch would be to actually do a link test for libpwl (using AC_CHECK_LIB for example) and only adding it based on the result. Such a change wouldn't require a newer PPL with fixed pkg-config data. Thanks, Ralf
On Tue, Feb 15, 2011 at 08:07:43PM +0100, Tobias Burnus wrote: > Dear Sebastian and Tobias, hello all, > > I think GCC should automatically link libisl ("-lisl") if one configures > GCC to use cloog-isl. That's the first patch at > http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01272.html , which remains > unreviewed. (The second part about "-lpwl" looks a bit odd.) > > The issue came also up today at #gfortran - and the proposed solutions > was the same. > > I think automatically linking libisl makes sense and is also in line > with PPL, which is linked via the toplevel configure.ac (cf. second - > bogus - part of the linked patch or simply the file itself). > > The patch I am talking about is: > > Index: config/cloog.m4 > =================================================================== > --- config/cloog.m4 (revision 166641) > +++ config/cloog.m4 (working copy) > @@ -143,7 +143,7 @@ > ;; > "ISL") > clooginc="${clooginc} ${_cloogorginc}" > - clooglibs="${clooglibs} -lcloog-isl" > + clooglibs="${clooglibs} -lcloog-isl -lisl" > cloog_org=yes > ;; > "PPL") > > > > Tobias Tobias, Are we certain that it is essential to explicitly link in -lisl? This change has caused an unnecessary rebuild of FSF gcc when upgrading cloog.org from 0.16.1 to 0.16.2 due to the soversion bump on libisl. Do we know of any direct calls from graphite into libisl? If all accesses are done through the API in libcloog-isl, which hasn't been version bumped, the extra linkage on -lisl would appear to be unnecessary. Jack
* Jack Howarth wrote on Thu, Apr 07, 2011 at 12:20:00AM CEST: > On Tue, Feb 15, 2011 at 08:07:43PM +0100, Tobias Burnus wrote: > > --- config/cloog.m4 (revision 166641) > > +++ config/cloog.m4 (working copy) > > @@ -143,7 +143,7 @@ > > ;; > > "ISL") > > clooginc="${clooginc} ${_cloogorginc}" > > - clooglibs="${clooglibs} -lcloog-isl" > > + clooglibs="${clooglibs} -lcloog-isl -lisl" > > cloog_org=yes > > ;; > > "PPL") > Are we certain that it is essential to explicitly link in -lisl? > This change has caused an unnecessary rebuild of FSF gcc when upgrading > cloog.org from 0.16.1 to 0.16.2 due to the soversion bump on libisl. > Do we know of any direct calls from graphite into libisl? If all accesses > are done through the API in libcloog-isl, which hasn't been version bumped, > the extra linkage on -lisl would appear to be unnecessary. What about static only links? Cheers, Ralf
Index: config/cloog.m4 =================================================================== --- config/cloog.m4 (revision 166641) +++ config/cloog.m4 (working copy) @@ -143,7 +143,7 @@ ;; "ISL") clooginc="${clooginc} ${_cloogorginc}" - clooglibs="${clooglibs} -lcloog-isl" + clooglibs="${clooglibs} -lcloog-isl -lisl" cloog_org=yes ;; "PPL")
Dear Sebastian and Tobias, hello all, I think GCC should automatically link libisl ("-lisl") if one configures GCC to use cloog-isl. That's the first patch at http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01272.html , which remains unreviewed. (The second part about "-lpwl" looks a bit odd.) The issue came also up today at #gfortran - and the proposed solutions was the same. I think automatically linking libisl makes sense and is also in line with PPL, which is linked via the toplevel configure.ac (cf. second - bogus - part of the linked patch or simply the file itself). The patch I am talking about is: Tobias