diff mbox

Allow new ISL/CLooG versions

Message ID 20130114160341.GA15254@bromo.med.uc.edu
State New
Headers show

Commit Message

Jack Howarth Jan. 14, 2013, 4:03 p.m. UTC 
On Mon, Jan 14, 2013 at 04:44:04PM +0100, Tobias Grosser wrote:
> On 01/14/2013 03:29 PM, Richard Biener wrote:
>>
>> This makes us accept the CLooG 0.18.0 and ISL 0.11.1 combo.
>>
>> It's probably not the best stage to move the version checks to
>> gcc/ where we can rely on built in-tree ISL/CLooG, so this avoids
>> it with the caveat that in-tree CLooG 0.18.0 will fail the
>> version check (they no longer ship built version.h but only
>> version.h.in).
>>
>> I verified all GRAPHITE tests pass with 0.18.0/0.11.1.
>>
>> Ok for trunk?
>>
>> Or do people prefer to move CLooG/ISL checks to gcc/ configure
>> time to fix the in-tree use of 0.18.0 and also do version checks
>> of in-tree ISL at all (they don't have a version.h).
>
> Hi Richi,
>
> I think this is a good thing. But this probably requires some config  
> guys to approve it.
>
> All the best,
> Tobi

Tnis change has already been committed at...

r195150 | rguenth | 2013-01-14 10:01:13 -0500 (Mon, 14 Jan 2013) | 5 lines

2013-01-14  Richard Biener  <rguenther@suse.de>

        * configure.ac (cloog/isl): Also allow ISL 0.11.x and CLooG 0.18.0.
        * configure: Re-generate


This change is broken in several ways. The current commit doesn't handle
isl 0.11.1 because it omits teaching config/isl.m4 how to handle revsions
with...


like config/cloog.m4 does. Also the current commit leaves the legacy...

   ISL_CHECK_VERSION(0,10)
+  if test "${gcc_cv_isl}" = no ; then
+    ISL_CHECK_VERSION(0,11)
+  fi

test on isl 0.10 present insuring that it will fail for any out of tree
isl version.
         Jack

Comments

Jack Howarth Jan. 14, 2013, 4:29 p.m. UTC | #1 
On Mon, Jan 14, 2013 at 11:03:41AM -0500, Jack Howarth wrote:
> On Mon, Jan 14, 2013 at 04:44:04PM +0100, Tobias Grosser wrote:
> > On 01/14/2013 03:29 PM, Richard Biener wrote:
> >>
> >> This makes us accept the CLooG 0.18.0 and ISL 0.11.1 combo.
> >>
> >> It's probably not the best stage to move the version checks to
> >> gcc/ where we can rely on built in-tree ISL/CLooG, so this avoids
> >> it with the caveat that in-tree CLooG 0.18.0 will fail the
> >> version check (they no longer ship built version.h but only
> >> version.h.in).
> >>
> >> I verified all GRAPHITE tests pass with 0.18.0/0.11.1.
> >>
> >> Ok for trunk?

Richard,
   Did you see my patch proposal in the gcc mailing list  before you crafted your patch?

http://gcc.gnu.org/ml/gcc/2012-12/msg00195.html

            Jack

> >>
> >> Or do people prefer to move CLooG/ISL checks to gcc/ configure
> >> time to fix the in-tree use of 0.18.0 and also do version checks
> >> of in-tree ISL at all (they don't have a version.h).
> >
> > Hi Richi,
> >
> > I think this is a good thing. But this probably requires some config  
> > guys to approve it.
> >
> > All the best,
> > Tobi
> 
> Tnis change has already been committed at...
> 
> r195150 | rguenth | 2013-01-14 10:01:13 -0500 (Mon, 14 Jan 2013) | 5 lines
> 
> 2013-01-14  Richard Biener  <rguenther@suse.de>
> 
>         * configure.ac (cloog/isl): Also allow ISL 0.11.x and CLooG 0.18.0.
>         * configure: Re-generate
> 
> 
> This change is broken in several ways. The current commit doesn't handle
> isl 0.11.1 because it omits teaching config/isl.m4 how to handle revsions
> with...
> 
> Index: config/isl.m4
> ===================================================================
> --- config/isl.m4	(revision 194744)
> +++ config/isl.m4	(working copy)
> @@ -89,13 +89,13 @@ AC_DEFUN([ISL_REQUESTED],
>  ]
>  )
>  
> -# _ISL_CHECK_CT_PROG(MAJOR, MINOR)
> +# _ISL_CHECK_CT_PROG(MAJOR, MINOR, REVISION)
>  # --------------------------------------------
>  # Helper for verifying ISL compile time version.
>  m4_define([_ISL_CHECK_CT_PROG],[AC_LANG_PROGRAM(
>    [#include <isl/version.h>
>     #include <string.h>],
> -  [if (strncmp (isl_version (), "isl-$1.$2", strlen ("isl-$1.$2")) != 0)
> +  [if (strncmp (isl_version (), "isl-$1.$2.$3", strlen ("isl-$1.$2.$3")) != 0)
>       return 1;
>     ])])
>  
> @@ -115,9 +115,9 @@ AC_DEFUN([ISL_CHECK_VERSION],
>      LIBS="${_isl_saved_LIBS} -lisl"
>      echo $CFLAGS
>  
> -    AC_CACHE_CHECK([for version $1.$2 of ISL],
> +    AC_CACHE_CHECK([for version $1.$2.$3 of ISL],
>        [gcc_cv_isl],
> -      [AC_RUN_IFELSE([_ISL_CHECK_CT_PROG($1,$2)],
> +      [AC_RUN_IFELSE([_ISL_CHECK_CT_PROG($1,$2,$3)],
>  	[gcc_cv_isl=yes],
>  	[gcc_cv_isl=no],
>  	[gcc_cv_isl=yes])])
> 
> like config/cloog.m4 does. Also the current commit leaves the legacy...
> 
>    ISL_CHECK_VERSION(0,10)
> +  if test "${gcc_cv_isl}" = no ; then
> +    ISL_CHECK_VERSION(0,11)
> +  fi
> 
> test on isl 0.10 present insuring that it will fail for any out of tree
> isl version.
>          Jack
> 
>
Richard Biener Jan. 14, 2013, 6:22 p.m. UTC | #2 
Jack Howarth <howarth@bromo.med.uc.edu> wrote:

>On Mon, Jan 14, 2013 at 11:03:41AM -0500, Jack Howarth wrote:
>> On Mon, Jan 14, 2013 at 04:44:04PM +0100, Tobias Grosser wrote:
>> > On 01/14/2013 03:29 PM, Richard Biener wrote:
>> >>
>> >> This makes us accept the CLooG 0.18.0 and ISL 0.11.1 combo.
>> >>
>> >> It's probably not the best stage to move the version checks to
>> >> gcc/ where we can rely on built in-tree ISL/CLooG, so this avoids
>> >> it with the caveat that in-tree CLooG 0.18.0 will fail the
>> >> version check (they no longer ship built version.h but only
>> >> version.h.in).
>> >>
>> >> I verified all GRAPHITE tests pass with 0.18.0/0.11.1.
>> >>
>> >> Ok for trunk?
>
>Richard,
>Did you see my patch proposal in the gcc mailing list  before you
>crafted your patch?
>
>http://gcc.gnu.org/ml/gcc/2012-12/msg00195.html
>
Yes I did.  The patch is not broken, it restricts compare to major and minor.

Richard.
>            Jack
>
>> >>
>> >> Or do people prefer to move CLooG/ISL checks to gcc/ configure
>> >> time to fix the in-tree use of 0.18.0 and also do version checks
>> >> of in-tree ISL at all (they don't have a version.h).
>> >
>> > Hi Richi,
>> >
>> > I think this is a good thing. But this probably requires some
>config  
>> > guys to approve it.
>> >
>> > All the best,
>> > Tobi
>> 
>> Tnis change has already been committed at...
>> 
>> r195150 | rguenth | 2013-01-14 10:01:13 -0500 (Mon, 14 Jan 2013) | 5
>lines
>> 
>> 2013-01-14  Richard Biener  <rguenther@suse.de>
>> 
>>         * configure.ac (cloog/isl): Also allow ISL 0.11.x and CLooG
>0.18.0.
>>         * configure: Re-generate
>> 
>> 
>> This change is broken in several ways. The current commit doesn't
>handle
>> isl 0.11.1 because it omits teaching config/isl.m4 how to handle
>revsions
>> with...
>> 
>> Index: config/isl.m4
>> ===================================================================
>> --- config/isl.m4	(revision 194744)
>> +++ config/isl.m4	(working copy)
>> @@ -89,13 +89,13 @@ AC_DEFUN([ISL_REQUESTED],
>>  ]
>>  )
>>  
>> -# _ISL_CHECK_CT_PROG(MAJOR, MINOR)
>> +# _ISL_CHECK_CT_PROG(MAJOR, MINOR, REVISION)
>>  # --------------------------------------------
>>  # Helper for verifying ISL compile time version.
>>  m4_define([_ISL_CHECK_CT_PROG],[AC_LANG_PROGRAM(
>>    [#include <isl/version.h>
>>     #include <string.h>],
>> -  [if (strncmp (isl_version (), "isl-$1.$2", strlen ("isl-$1.$2"))
>!= 0)
>> +  [if (strncmp (isl_version (), "isl-$1.$2.$3", strlen
>("isl-$1.$2.$3")) != 0)
>>       return 1;
>>     ])])
>>  
>> @@ -115,9 +115,9 @@ AC_DEFUN([ISL_CHECK_VERSION],
>>      LIBS="${_isl_saved_LIBS} -lisl"
>>      echo $CFLAGS
>>  
>> -    AC_CACHE_CHECK([for version $1.$2 of ISL],
>> +    AC_CACHE_CHECK([for version $1.$2.$3 of ISL],
>>        [gcc_cv_isl],
>> -      [AC_RUN_IFELSE([_ISL_CHECK_CT_PROG($1,$2)],
>> +      [AC_RUN_IFELSE([_ISL_CHECK_CT_PROG($1,$2,$3)],
>>  	[gcc_cv_isl=yes],
>>  	[gcc_cv_isl=no],
>>  	[gcc_cv_isl=yes])])
>> 
>> like config/cloog.m4 does. Also the current commit leaves the
>legacy...
>> 
>>    ISL_CHECK_VERSION(0,10)
>> +  if test "${gcc_cv_isl}" = no ; then
>> +    ISL_CHECK_VERSION(0,11)
>> +  fi
>> 
>> test on isl 0.10 present insuring that it will fail for any out of
>tree
>> isl version.
>>          Jack
>> 
>>
Jack Howarth Jan. 14, 2013, 8:04 p.m. UTC | #3 
On Mon, Jan 14, 2013 at 07:22:03PM +0100, Richard Biener wrote:
> Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> 
> >On Mon, Jan 14, 2013 at 11:03:41AM -0500, Jack Howarth wrote:
> >> On Mon, Jan 14, 2013 at 04:44:04PM +0100, Tobias Grosser wrote:
> >> > On 01/14/2013 03:29 PM, Richard Biener wrote:
> >> >>
> >> >> This makes us accept the CLooG 0.18.0 and ISL 0.11.1 combo.
> >> >>
> >> >> It's probably not the best stage to move the version checks to
> >> >> gcc/ where we can rely on built in-tree ISL/CLooG, so this avoids
> >> >> it with the caveat that in-tree CLooG 0.18.0 will fail the
> >> >> version check (they no longer ship built version.h but only
> >> >> version.h.in).
> >> >>
> >> >> I verified all GRAPHITE tests pass with 0.18.0/0.11.1.
> >> >>
> >> >> Ok for trunk?
> >
> >Richard,
> >Did you see my patch proposal in the gcc mailing list  before you
> >crafted your patch?
> >
> >http://gcc.gnu.org/ml/gcc/2012-12/msg00195.html
> >
> Yes I did.  The patch is not broken, it restricts compare to major and minor.

How exactly did you test this? I am using isl 0.11.1 and cloog 0.18.0 from fink
installed in /sw out of tree. This fails in config.log as...

configure:5836: checking for version 0.10 of ISL
configure:5858: gcc -o conftest -g -O2 -I/sw/include -I/sw/include -I/sw/include    -L/sw/lib conftest.c  -lisl >&5
configure:5858: $? = 0
configure:5858: ./conftest
configure:5858: $? = 1
configure: program exited with status 1
configure: failed program was:
| /* confdefs.h */
| #define PACKAGE_NAME ""
| #define PACKAGE_TARNAME ""
| #define PACKAGE_VERSION ""
| #define PACKAGE_STRING ""
| #define PACKAGE_BUGREPORT ""
| #define PACKAGE_URL ""
| #define LT_OBJDIR ".libs/"
| /* end confdefs.h.  */
| #include <isl/version.h>
|    #include <string.h>
| int
| main ()
| {
| if (strncmp (isl_version (), "isl-0.10", strlen ("isl-0.10")) != 0)
|      return 1;
| 
|   ;
|   return 0;
| }
configure:5868: result: no
configure:5889: checking for version 0.11 of ISL
configure:5921: result: no
configure:5956: error: Unable to find a usable ISL.  See config.log for details.

This is with gcc trunk configured as...

 $ ../gcc-4.8-20130114/configure --prefix=/sw --prefix=/sw/lib/gcc4.8 --mandir=/sw/share/man --infodir=/sw/lib/gcc4.8/info --enable-languages=c,c++,fortran,lto,objc,obj-c++,java --with-gmp=/sw --with-libiconv-prefix=/sw --with-isl=/sw --with-cloog=/sw --with-mpc=/sw --with-system-zlib --enable-checking=yes --x-includes=/usr/X11R6/include --x-libraries=/usr/X11R6/lib --program-suffix=-fsf-4.8


> 
> Richard.
> >            Jack
> >
> >> >>
> >> >> Or do people prefer to move CLooG/ISL checks to gcc/ configure
> >> >> time to fix the in-tree use of 0.18.0 and also do version checks
> >> >> of in-tree ISL at all (they don't have a version.h).
> >> >
> >> > Hi Richi,
> >> >
> >> > I think this is a good thing. But this probably requires some
> >config  
> >> > guys to approve it.
> >> >
> >> > All the best,
> >> > Tobi
> >> 
> >> Tnis change has already been committed at...
> >> 
> >> r195150 | rguenth | 2013-01-14 10:01:13 -0500 (Mon, 14 Jan 2013) | 5
> >lines
> >> 
> >> 2013-01-14  Richard Biener  <rguenther@suse.de>
> >> 
> >>         * configure.ac (cloog/isl): Also allow ISL 0.11.x and CLooG
> >0.18.0.
> >>         * configure: Re-generate
> >> 
> >> 
> >> This change is broken in several ways. The current commit doesn't
> >handle
> >> isl 0.11.1 because it omits teaching config/isl.m4 how to handle
> >revsions
> >> with...
> >> 
> >> Index: config/isl.m4
> >> ===================================================================
> >> --- config/isl.m4	(revision 194744)
> >> +++ config/isl.m4	(working copy)
> >> @@ -89,13 +89,13 @@ AC_DEFUN([ISL_REQUESTED],
> >>  ]
> >>  )
> >>  
> >> -# _ISL_CHECK_CT_PROG(MAJOR, MINOR)
> >> +# _ISL_CHECK_CT_PROG(MAJOR, MINOR, REVISION)
> >>  # --------------------------------------------
> >>  # Helper for verifying ISL compile time version.
> >>  m4_define([_ISL_CHECK_CT_PROG],[AC_LANG_PROGRAM(
> >>    [#include <isl/version.h>
> >>     #include <string.h>],
> >> -  [if (strncmp (isl_version (), "isl-$1.$2", strlen ("isl-$1.$2"))
> >!= 0)
> >> +  [if (strncmp (isl_version (), "isl-$1.$2.$3", strlen
> >("isl-$1.$2.$3")) != 0)
> >>       return 1;
> >>     ])])
> >>  
> >> @@ -115,9 +115,9 @@ AC_DEFUN([ISL_CHECK_VERSION],
> >>      LIBS="${_isl_saved_LIBS} -lisl"
> >>      echo $CFLAGS
> >>  
> >> -    AC_CACHE_CHECK([for version $1.$2 of ISL],
> >> +    AC_CACHE_CHECK([for version $1.$2.$3 of ISL],
> >>        [gcc_cv_isl],
> >> -      [AC_RUN_IFELSE([_ISL_CHECK_CT_PROG($1,$2)],
> >> +      [AC_RUN_IFELSE([_ISL_CHECK_CT_PROG($1,$2,$3)],
> >>  	[gcc_cv_isl=yes],
> >>  	[gcc_cv_isl=no],
> >>  	[gcc_cv_isl=yes])])
> >> 
> >> like config/cloog.m4 does. Also the current commit leaves the
> >legacy...
> >> 
> >>    ISL_CHECK_VERSION(0,10)
> >> +  if test "${gcc_cv_isl}" = no ; then
> >> +    ISL_CHECK_VERSION(0,11)
> >> +  fi
> >> 
> >> test on isl 0.10 present insuring that it will fail for any out of
> >tree
> >> isl version.
> >>          Jack
> >> 
> >> 
>
diff mbox

Patch

Index: config/isl.m4
===================================================================
--- config/isl.m4	(revision 194744)
+++ config/isl.m4	(working copy)
@@ -89,13 +89,13 @@  AC_DEFUN([ISL_REQUESTED],
 ]
 )
 
-# _ISL_CHECK_CT_PROG(MAJOR, MINOR)
+# _ISL_CHECK_CT_PROG(MAJOR, MINOR, REVISION)
 # --------------------------------------------
 # Helper for verifying ISL compile time version.
 m4_define([_ISL_CHECK_CT_PROG],[AC_LANG_PROGRAM(
   [#include <isl/version.h>
    #include <string.h>],
-  [if (strncmp (isl_version (), "isl-$1.$2", strlen ("isl-$1.$2")) != 0)
+  [if (strncmp (isl_version (), "isl-$1.$2.$3", strlen ("isl-$1.$2.$3")) != 0)
      return 1;
    ])])
 
@@ -115,9 +115,9 @@  AC_DEFUN([ISL_CHECK_VERSION],
     LIBS="${_isl_saved_LIBS} -lisl"
     echo $CFLAGS
 
-    AC_CACHE_CHECK([for version $1.$2 of ISL],
+    AC_CACHE_CHECK([for version $1.$2.$3 of ISL],
       [gcc_cv_isl],
-      [AC_RUN_IFELSE([_ISL_CHECK_CT_PROG($1,$2)],
+      [AC_RUN_IFELSE([_ISL_CHECK_CT_PROG($1,$2,$3)],
 	[gcc_cv_isl=yes],
 	[gcc_cv_isl=no],
 	[gcc_cv_isl=yes])])