[patch][graphite] Check cloog revision when configure.

[Li Feng-2]

Hi,

In polyhedral benchmark, aermod is miscompiled because of the
older cloog revision number.

Here is the bug and discussion:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41924
http://groups.google.com/group/gcc-graphite/browse_thread/thread/ee4485e5fd1e4302

To avoid this happen again from both user and develper,
I would like to check cloog revision when configuring.
Sebastian has proposed the same thing some time ago:
http://gcc.gnu.org/ml/gcc-patches/2009-07/msg01839.html

Here is the patch:

diff --git a/configure.ac b/configure.ac
index 48fa580..5fab23a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1593,7 +1593,7 @@ if test "${ENABLE_CLOOG_CHECK}" = "yes"; then
   CFLAGS="$CFLAGS $clooginc $gmpinc $pplinc"
   AC_MSG_CHECKING([for correct version of CLooG])
   AC_TRY_COMPILE([#include "cloog/cloog.h"],[
-  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
+  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
CLOOG_VERSION_REVISION < 4
   choke me
   #endif
   ], [AC_MSG_RESULT([yes])], [AC_MSG_RESULT([no]); clooglibs= ; clooginc= ])

Ok for trunk?

Li

[Tobias Grosser-3]

On Fri, 2009-11-06 at 11:05 +0800, Li Feng wrote:

> Hi,
>
> In polyhedral benchmark, aermod is miscompiled because of the
> older cloog revision number.
>
> Here is the bug and discussion:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41924
> http://groups.google.com/group/gcc-graphite/browse_thread/thread/ee4485e5fd1e4302
>
> To avoid this happen again from both user and develper,
> I would like to check cloog revision when configuring.
> Sebastian has proposed the same thing some time ago:
> http://gcc.gnu.org/ml/gcc-patches/2009-07/msg01839.html
>
> Here is the patch:
>
> diff --git a/configure.ac b/configure.ac
> index 48fa580..5fab23a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1593,7 +1593,7 @@ if test "${ENABLE_CLOOG_CHECK}" = "yes"; then
>    CFLAGS="$CFLAGS $clooginc $gmpinc $pplinc"
>    AC_MSG_CHECKING([for correct version of CLooG])
>    AC_TRY_COMPILE([#include "cloog/cloog.h"],[
> -  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
> +  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
> CLOOG_VERSION_REVISION < 4

I personally believe moving "||" to the next line and identing the new
row would look better.

>    choke me
>    #endif
>    ], [AC_MSG_RESULT([yes])], [AC_MSG_RESULT([no]); clooglibs= ; clooginc= ])
>
> Ok for trunk?

You still need to add a ChangeLog entry and to update the "configure"
file using autotools.

Otherwise I believe the patch is useful and required since a while, I
just did not do it. So thanks for your affords.

If there are no concerns until Sunday it should be fine to commit,
however I would like to see the updated patch, before saying yes.

Tobi

[Li Feng-2]

Hi,
On Fri, Nov 6, 2009 at 11:46 AM, Tobias Grosser
<grosser@...> wrote:

> On Fri, 2009-11-06 at 11:05 +0800, Li Feng wrote:
>> Hi,
>>
>> In polyhedral benchmark, aermod is miscompiled because of the
>> older cloog revision number.
>>
>> Here is the bug and discussion:
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41924
>> http://groups.google.com/group/gcc-graphite/browse_thread/thread/ee4485e5fd1e4302
>>
>> To avoid this happen again from both user and develper,
>> I would like to check cloog revision when configuring.
>> Sebastian has proposed the same thing some time ago:
>> http://gcc.gnu.org/ml/gcc-patches/2009-07/msg01839.html
>>
>> Here is the patch:
>>
>> diff --git a/configure.ac b/configure.ac
>> index 48fa580..5fab23a 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -1593,7 +1593,7 @@ if test "${ENABLE_CLOOG_CHECK}" = "yes"; then
>>    CFLAGS="$CFLAGS $clooginc $gmpinc $pplinc"
>>    AC_MSG_CHECKING([for correct version of CLooG])
>>    AC_TRY_COMPILE([#include "cloog/cloog.h"],[
>> -  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
>> +  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
>> CLOOG_VERSION_REVISION < 4
>
> I personally believe moving "||" to the next line and identing the new
> row would look better.

Because of the #if directives, we should put them in one line. Otherwise,
it will not checking this.

>
>>    choke me
>>    #endif
>>    ], [AC_MSG_RESULT([yes])], [AC_MSG_RESULT([no]); clooglibs= ; clooginc= ])
>>
>> Ok for trunk?
>
> You still need to add a ChangeLog entry and to update the "configure"
> file using autotools.
>

Ok.

> Otherwise I believe the patch is useful and required since a while, I
> just did not do it. So thanks for your affords.
>
> If there are no concerns until Sunday it should be fine to commit,
> however I would like to see the updated patch, before saying yes.
>
> Tobi
>
>
here is the patch:

diff --git a/ChangeLog b/ChangeLog
index 293407a..d6c000f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
[phone]  Li Feng  <nemokingdom@...>
+
+ * configure.ac: Check if the right cloog revison.
+ * configure: Regenerate.
+
 2009-11-05  Joern Rennecke <amylaar@...>

  * MAINTAINERS (Write After Approval): Add entry for my INRIA work.
diff --git a/configure b/configure
index 7b068a1..58aa1e9 100755
--- a/configure
+++ b/configure
@@ -5906,7 +5906,7 @@ int
 main ()
 {

-  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
+  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
CLOOG_VERSION_REVISION < 4
   choke me
   #endif

diff --git a/configure.ac b/configure.ac
index 36774a4..728efbe 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1611,7 +1611,7 @@ if test "x$with_cloog" != "xno" -a
"${ENABLE_CLOOG_CHECK}" = "yes"; then
   CFLAGS="$CFLAGS $clooginc $gmpinc $pplinc"
   AC_MSG_CHECKING([for correct version of CLooG])
   AC_TRY_COMPILE([#include "cloog/cloog.h"],[
-  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
+  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
CLOOG_VERSION_REVISION < 4
   choke me
   #endif
   ], [AC_MSG_RESULT([yes])], [AC_MSG_RESULT([no]); clooglibs= ; clooginc= ])

Li

[Tobias Grosser-3]

On Fri, 2009-11-06 at 18:15 +0800, Li Feng wrote:

> Hi,
> On Fri, Nov 6, 2009 at 11:46 AM, Tobias Grosser
> <grosser@...> wrote:
> > On Fri, 2009-11-06 at 11:05 +0800, Li Feng wrote:
> >> Hi,
> >>
> >> In polyhedral benchmark, aermod is miscompiled because of the
> >> older cloog revision number.
> >>
> >> Here is the bug and discussion:
> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41924
> >> http://groups.google.com/group/gcc-graphite/browse_thread/thread/ee4485e5fd1e4302
> >>
> >> To avoid this happen again from both user and develper,
> >> I would like to check cloog revision when configuring.
> >> Sebastian has proposed the same thing some time ago:
> >> http://gcc.gnu.org/ml/gcc-patches/2009-07/msg01839.html
> >>
> >> Here is the patch:
> >>
> >> diff --git a/configure.ac b/configure.ac
> >> index 48fa580..5fab23a 100644
> >> --- a/configure.ac
> >> +++ b/configure.ac
> >> @@ -1593,7 +1593,7 @@ if test "${ENABLE_CLOOG_CHECK}" = "yes"; then
> >>    CFLAGS="$CFLAGS $clooginc $gmpinc $pplinc"
> >>    AC_MSG_CHECKING([for correct version of CLooG])
> >>    AC_TRY_COMPILE([#include "cloog/cloog.h"],[
> >> -  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
> >> +  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
> >> CLOOG_VERSION_REVISION < 4
> >
> > I personally believe moving "||" to the next line and identing the new
> > row would look better.
>
> Because of the #if directives, we should put them in one line. Otherwise,
> it will not checking this.
>
> >
> >>    choke me
> >>    #endif
> >>    ], [AC_MSG_RESULT([yes])], [AC_MSG_RESULT([no]); clooglibs= ; clooginc= ])
> >>
> >> Ok for trunk?
> >
> > You still need to add a ChangeLog entry and to update the "configure"
> > file using autotools.
> >
> Ok.
>
> > Otherwise I believe the patch is useful and required since a while, I
> > just did not do it. So thanks for your affords.
> >
> > If there are no concerns until Sunday it should be fine to commit,
> > however I would like to see the updated patch, before saying yes.
> >
> > Tobi
> >
> >
> here is the patch:

It is fine with me. Please commit if bootstrapped (or configured) and
there are no concerns until Sunday.

Tobi

[Sebastian Pop-2]

On Fri, Nov 6, 2009 at 05:33, Tobias Grosser <grosser@...> wrote:

> On Fri, 2009-11-06 at 18:15 +0800, Li Feng wrote:
>> Hi,
>> On Fri, Nov 6, 2009 at 11:46 AM, Tobias Grosser
>> <grosser@...> wrote:
>> > On Fri, 2009-11-06 at 11:05 +0800, Li Feng wrote:
>> >> Hi,
>> >>
>> >> In polyhedral benchmark, aermod is miscompiled because of the
>> >> older cloog revision number.
>> >>
>> >> Here is the bug and discussion:
>> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41924
>> >> http://groups.google.com/group/gcc-graphite/browse_thread/thread/ee4485e5fd1e4302
>> >>
>> >> To avoid this happen again from both user and develper,
>> >> I would like to check cloog revision when configuring.
>> >> Sebastian has proposed the same thing some time ago:
>> >> http://gcc.gnu.org/ml/gcc-patches/2009-07/msg01839.html
>> >>
>> >> Here is the patch:
>> >>
>> >> diff --git a/configure.ac b/configure.ac
>> >> index 48fa580..5fab23a 100644
>> >> --- a/configure.ac
>> >> +++ b/configure.ac
>> >> @@ -1593,7 +1593,7 @@ if test "${ENABLE_CLOOG_CHECK}" = "yes"; then
>> >>    CFLAGS="$CFLAGS $clooginc $gmpinc $pplinc"
>> >>    AC_MSG_CHECKING([for correct version of CLooG])
>> >>    AC_TRY_COMPILE([#include "cloog/cloog.h"],[
>> >> -  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
>> >> +  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
>> >> CLOOG_VERSION_REVISION < 4
>> >
>> > I personally believe moving "||" to the next line and identing the new
>> > row would look better.
>>
>> Because of the #if directives, we should put them in one line. Otherwise,
>> it will not checking this.
>>
>> >
>> >>    choke me
>> >>    #endif
>> >>    ], [AC_MSG_RESULT([yes])], [AC_MSG_RESULT([no]); clooglibs= ; clooginc= ])
>> >>
>> >> Ok for trunk?
>> >
>> > You still need to add a ChangeLog entry and to update the "configure"
>> > file using autotools.
>> >
>> Ok.
>>
>> > Otherwise I believe the patch is useful and required since a while, I
>> > just did not do it. So thanks for your affords.
>> >
>> > If there are no concerns until Sunday it should be fine to commit,
>> > however I would like to see the updated patch, before saying yes.
>> >
>> > Tobi
>> >
>> >
>> here is the patch:
>
> It is fine with me. Please commit if bootstrapped (or configured) and
> there are no concerns until Sunday.
>

From the original discussion at
http://gcc.gnu.org/ml/gcc-patches/2009-07/msg01839.html
Richi was concerned by such a patch, so I think that we should get
an approval from Richi and from a configure maintainer before
committing this patch to trunk.

Sebastian

[Richard Guenther-3]

On Fri, 6 Nov 2009, Sebastian Pop wrote:

> On Fri, Nov 6, 2009 at 05:33, Tobias Grosser <grosser@...> wrote:
> > On Fri, 2009-11-06 at 18:15 +0800, Li Feng wrote:
> >> Hi,
> >> On Fri, Nov 6, 2009 at 11:46 AM, Tobias Grosser
> >> <grosser@...> wrote:
> >> > On Fri, 2009-11-06 at 11:05 +0800, Li Feng wrote:
> >> >> Hi,
> >> >>
> >> >> In polyhedral benchmark, aermod is miscompiled because of the
> >> >> older cloog revision number.
> >> >>
> >> >> Here is the bug and discussion:
> >> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41924
> >> >> http://groups.google.com/group/gcc-graphite/browse_thread/thread/ee4485e5fd1e4302
> >> >>
> >> >> To avoid this happen again from both user and develper,
> >> >> I would like to check cloog revision when configuring.
> >> >> Sebastian has proposed the same thing some time ago:
> >> >> http://gcc.gnu.org/ml/gcc-patches/2009-07/msg01839.html
> >> >>
> >> >> Here is the patch:
> >> >>
> >> >> diff --git a/configure.ac b/configure.ac
> >> >> index 48fa580..5fab23a 100644
> >> >> --- a/configure.ac
> >> >> +++ b/configure.ac
> >> >> @@ -1593,7 +1593,7 @@ if test "${ENABLE_CLOOG_CHECK}" = "yes"; then
> >> >>    CFLAGS="$CFLAGS $clooginc $gmpinc $pplinc"
> >> >>    AC_MSG_CHECKING([for correct version of CLooG])
> >> >>    AC_TRY_COMPILE([#include "cloog/cloog.h"],[
> >> >> -  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
> >> >> +  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
> >> >> CLOOG_VERSION_REVISION < 4
> >> >
> >> > I personally believe moving "||" to the next line and identing the new
> >> > row would look better.
> >>
> >> Because of the #if directives, we should put them in one line. Otherwise,
> >> it will not checking this.
> >>
> >> >
> >> >>    choke me
> >> >>    #endif
> >> >>    ], [AC_MSG_RESULT([yes])], [AC_MSG_RESULT([no]); clooglibs= ; clooginc= ])
> >> >>
> >> >> Ok for trunk?
> >> >
> >> > You still need to add a ChangeLog entry and to update the "configure"
> >> > file using autotools.
> >> >
> >> Ok.
> >>
> >> > Otherwise I believe the patch is useful and required since a while, I
> >> > just did not do it. So thanks for your affords.
> >> >
> >> > If there are no concerns until Sunday it should be fine to commit,
> >> > however I would like to see the updated patch, before saying yes.
> >> >
> >> > Tobi
> >> >
> >> >
> >> here is the patch:
> >
> > It is fine with me. Please commit if bootstrapped (or configured) and
> > there are no concerns until Sunday.
> >
>
> From the original discussion at
> http://gcc.gnu.org/ml/gcc-patches/2009-07/msg01839.html
> Richi was concerned by such a patch, so I think that we should get
> an approval from Richi and from a configure maintainer before
> committing this patch to trunk.

Well, as GCC doesn't bootstrap with the older versions the check is
ok with me.  But you still need a configure maintainer to approve
the patch.

Richard.

[Tobias Grosser-3]

On Fri, 2009-11-06 at 18:00 +0100, Richard Guenther wrote:

> On Fri, 6 Nov 2009, Sebastian Pop wrote:
>
> > On Fri, Nov 6, 2009 at 05:33, Tobias Grosser <grosser@...> wrote:
> > > On Fri, 2009-11-06 at 18:15 +0800, Li Feng wrote:
> > >> Hi,
> > >> On Fri, Nov 6, 2009 at 11:46 AM, Tobias Grosser
> > >> <grosser@...> wrote:
> > >> > On Fri, 2009-11-06 at 11:05 +0800, Li Feng wrote:
> > >> >> Hi,
> > >> >>
> > >> >> In polyhedral benchmark, aermod is miscompiled because of the
> > >> >> older cloog revision number.
> > >> >>
> > >> >> Here is the bug and discussion:
> > >> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41924
> > >> >> http://groups.google.com/group/gcc-graphite/browse_thread/thread/ee4485e5fd1e4302
> > >> >>
> > >> >> To avoid this happen again from both user and develper,
> > >> >> I would like to check cloog revision when configuring.
> > >> >> Sebastian has proposed the same thing some time ago:
> > >> >> http://gcc.gnu.org/ml/gcc-patches/2009-07/msg01839.html
> > >> >>
> > >> >> Here is the patch:
> > >> >>
> > >> >> diff --git a/configure.ac b/configure.ac
> > >> >> index 48fa580..5fab23a 100644
> > >> >> --- a/configure.ac
> > >> >> +++ b/configure.ac
> > >> >> @@ -1593,7 +1593,7 @@ if test "${ENABLE_CLOOG_CHECK}" = "yes"; then
> > >> >>    CFLAGS="$CFLAGS $clooginc $gmpinc $pplinc"
> > >> >>    AC_MSG_CHECKING([for correct version of CLooG])
> > >> >>    AC_TRY_COMPILE([#include "cloog/cloog.h"],[
> > >> >> -  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
> > >> >> +  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
> > >> >> CLOOG_VERSION_REVISION < 4
> > >> >
> > >> > I personally believe moving "||" to the next line and identing the new
> > >> > row would look better.
> > >>
> > >> Because of the #if directives, we should put them in one line. Otherwise,
> > >> it will not checking this.
> > >>
> > >> >
> > >> >>    choke me
> > >> >>    #endif
> > >> >>    ], [AC_MSG_RESULT([yes])], [AC_MSG_RESULT([no]); clooglibs= ; clooginc= ])
> > >> >>
> > >> >> Ok for trunk?
> > >> >
> > >> > You still need to add a ChangeLog entry and to update the "configure"
> > >> > file using autotools.
> > >> >
> > >> Ok.
> > >>
> > >> > Otherwise I believe the patch is useful and required since a while, I
> > >> > just did not do it. So thanks for your affords.
> > >> >
> > >> > If there are no concerns until Sunday it should be fine to commit,
> > >> > however I would like to see the updated patch, before saying yes.
> > >> >
> > >> > Tobi
> > >> >
> > >> >
> > >> here is the patch:
> > >
> > > It is fine with me. Please commit if bootstrapped (or configured) and
> > > there are no concerns until Sunday.
> > >
> >
> > From the original discussion at
> > http://gcc.gnu.org/ml/gcc-patches/2009-07/msg01839.html
> > Richi was concerned by such a patch, so I think that we should get
> > an approval from Richi and from a configure maintainer before
> > committing this patch to trunk.
>
> Well, as GCC doesn't bootstrap with the older versions the check is
> ok with me.  But you still need a configure maintainer to approve
> the patch.

Actually it does bootstrap, but there are some bugs in spec and
polyhedron. The most problematic is fortran code, as it uses a different
loop structure that tends to trigger this cloog bug. (It uses a != 100
instead of a < 100 to limit loop iteration count)

The other option would be to not handle the problematic constructs, but
this would almost completely disable Fortran support.

So we will be waiting for some configure maintainer.

Tobi

[Paolo Bonzini-2]

If this is the patch:

-  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
+  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
CLOOG_VERSION_REVISION < 5
    choke me
    #endif

it is fine as is, however consider future proofing it by allowing 0.16
and 1.0 to pass the test.

Paolo

[Sebastian Pop-2]

On Fri, Nov 6, 2009 at 12:59, Paolo Bonzini <bonzini@...> wrote:

> If this is the patch:
>
> -  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
> +  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
> CLOOG_VERSION_REVISION < 5
>   choke me
>   #endif
>
> it is fine as is, however consider future proofing it by allowing 0.16 and
> 1.0 to pass the test.

I do not think that CLooG 0.16 will be compatible with CLooG-PPL 0.15.
At least the changes that I saw in the patches that Tobias prepared
would simply not compile with the code of Graphite that is in trunk
and branch right now.  So please leave the hard checks for major
and minor versions.

Thanks,
Sebastian

[Paolo Bonzini-2]

On 11/06/2009 08:56 PM, Sebastian Pop wrote:
> I do not think that CLooG 0.16 will be compatible with CLooG-PPL 0.15.
> At least the changes that I saw in the patches that Tobias prepared
> would simply not compile with the code of Graphite that is in trunk
> and branch right now.  So please leave the hard checks for major
> and minor versions.

I'm not going to touch that test. :-)

Paolo

[Tobias Grosser-3]

On Fri, 2009-11-06 at 13:56 -0600, Sebastian Pop wrote:

> On Fri, Nov 6, 2009 at 12:59, Paolo Bonzini <bonzini@...> wrote:
> > If this is the patch:
> >
> > -  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
> > +  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
> > CLOOG_VERSION_REVISION < 5
> >   choke me
> >   #endif
> >
> > it is fine as is, however consider future proofing it by allowing 0.16 and
> > 1.0 to pass the test.
>
> I do not think that CLooG 0.16 will be compatible with CLooG-PPL 0.15.
> At least the changes that I saw in the patches that Tobias prepared
> would simply not compile with the code of Graphite that is in trunk
> and branch right now.  So please leave the hard checks for major
> and minor versions.

Yep, as we want to unify the interface of our cloog and the CLooG of
cloog.org and we decided to adapt the cloog.org interface there will not
be any compatibility. However these changes will come with the next gcc
release.

Tobi

[Li Feng-2]

On Sat, Nov 7, 2009 at 2:59 AM, Paolo Bonzini <bonzini@...> wrote:
> If this is the patch:
>
> -  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
> +  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
> CLOOG_VERSION_REVISION < 5
>   choke me
>   #endif

Em, this is the patch.

-  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
+  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
CLOOG_VERSION_REVISION < 4

 I have noticed in Sebastian's last patch, the
CLOOG_VERSION_REVISION will not allow revision 4.

@Sebastian, is there some special reason for this? I have tested
cloog 0.15.4, and it works (at least) with aermod in polyhedral benchmark.

>
> it is fine as is, however consider future proofing it by allowing 0.16 and
> 1.0 to pass the test.
>
> Paolo
>

Thanks,
Li

[Tobias Grosser-3]

On Sat, 2009-11-07 at 09:40 +0800, Li Feng wrote:

> On Sat, Nov 7, 2009 at 2:59 AM, Paolo Bonzini <bonzini@...> wrote:
> > If this is the patch:
> >
> > -  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
> > +  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
> > CLOOG_VERSION_REVISION < 5
> >   choke me
> >   #endif
>
> Em, this is the patch.
>
> -  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
> +  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
> CLOOG_VERSION_REVISION < 4
>
>  I have noticed in Sebastian's last patch, the
> CLOOG_VERSION_REVISION will not allow revision 4.
>
> @Sebastian, is there some special reason for this? I have tested
> cloog 0.15.4, and it works (at least) with aermod in polyhedral benchmark.
>

.4 is enough to fix this problem.

There exist newer CLooG versions that fix other problems:

.5 - Compilation with -Wc++-compat PR40103
.6 - Add --with-host-libstdcxx to allow usage of static compiled CLooG
with static or dynamic libstdcxx
.7 (not yet released) - Fixes some memory leaks

I believe fixing .4 is required and probably .5 will also hit a lot of
people so it would be helpful to warn them at the beginning.

For .6 and .7 this does only affect a few people, who will find the
solution on the mailinglists. Forcing all people to upgrade CLooG just
to make life for a few a little easier does not sound useful to me.

However I do not have a strong opinion about .6 and .7

Tobi

[Paolo Bonzini-2]

On 11/07/2009 03:09 AM, Tobias Grosser wrote:

> .4 is enough to fix this problem.
>
> There exist newer CLooG versions that fix other problems:
>
> .5 - Compilation with -Wc++-compat PR40103
> .6 - Add --with-host-libstdcxx to allow usage of static compiled CLooG
> with static or dynamic libstdcxx
> .7 (not yet released) - Fixes some memory leaks
>
> I believe fixing .4 is required and probably .5 will also hit a lot of
> people so it would be helpful to warn them at the beginning.
>
> For .6 and .7 this does only affect a few people, who will find the
> solution on the mailinglists. Forcing all people to upgrade CLooG just
> to make life for a few a little easier does not sound useful to me.
>
> However I do not have a strong opinion about .6 and .7

Whatever. :-)  You decide on the last digit. :-)

Paolo

[Sebastian Pop-2]

On Fri, Nov 6, 2009 at 20:09, Tobias Grosser <grosser@...> wrote:

> On Sat, 2009-11-07 at 09:40 +0800, Li Feng wrote:
>> On Sat, Nov 7, 2009 at 2:59 AM, Paolo Bonzini <bonzini@...> wrote:
>> > If this is the patch:
>> >
>> > -  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
>> > +  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
>> > CLOOG_VERSION_REVISION < 5
>> >   choke me
>> >   #endif
>>
>> Em, this is the patch.
>>
>> -  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
>> +  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
>> CLOOG_VERSION_REVISION < 4
>>
>>  I have noticed in Sebastian's last patch, the
>> CLOOG_VERSION_REVISION will not allow revision 4.
>>
>> @Sebastian, is there some special reason for this? I have tested
>> cloog 0.15.4, and it works (at least) with aermod in polyhedral benchmark.
>>
>
> .4 is enough to fix this problem.
>
> There exist newer CLooG versions that fix other problems:
>
> .5 - Compilation with -Wc++-compat PR40103
> .6 - Add --with-host-libstdcxx to allow usage of static compiled CLooG
> with static or dynamic libstdcxx
> .7 (not yet released) - Fixes some memory leaks
>
> I believe fixing .4 is required and probably .5 will also hit a lot of
> people so it would be helpful to warn them at the beginning.
>
> For .6 and .7 this does only affect a few people, who will find the
> solution on the mailinglists. Forcing all people to upgrade CLooG just
> to make life for a few a little easier does not sound useful to me.
>
> However I do not have a strong opinion about .6 and .7

Let's say that CLooG-PPL 0.15.7 or later revision is required for gcc4.5.

Sebastian

[Richard Guenther-3]

On Fri, 6 Nov 2009, Sebastian Pop wrote:

> On Fri, Nov 6, 2009 at 20:09, Tobias Grosser <grosser@...> wrote:
> > On Sat, 2009-11-07 at 09:40 +0800, Li Feng wrote:
> >> On Sat, Nov 7, 2009 at 2:59 AM, Paolo Bonzini <bonzini@...> wrote:
> >> > If this is the patch:
> >> >
> >> > -  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
> >> > +  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
> >> > CLOOG_VERSION_REVISION < 5
> >> >   choke me
> >> >   #endif
> >>
> >> Em, this is the patch.
> >>
> >> -  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15
> >> +  #if CLOOG_VERSION_MAJOR != 0 || CLOOG_VERSION_MINOR != 15 ||
> >> CLOOG_VERSION_REVISION < 4
> >>
> >>  I have noticed in Sebastian's last patch, the
> >> CLOOG_VERSION_REVISION will not allow revision 4.
> >>
> >> @Sebastian, is there some special reason for this? I have tested
> >> cloog 0.15.4, and it works (at least) with aermod in polyhedral benchmark.
> >>
> >
> > .4 is enough to fix this problem.
> >
> > There exist newer CLooG versions that fix other problems:
> >
> > .5 - Compilation with -Wc++-compat PR40103
> > .6 - Add --with-host-libstdcxx to allow usage of static compiled CLooG
> > with static or dynamic libstdcxx
> > .7 (not yet released) - Fixes some memory leaks
> >
> > I believe fixing .4 is required and probably .5 will also hit a lot of
> > people so it would be helpful to warn them at the beginning.
> >
> > For .6 and .7 this does only affect a few people, who will find the
> > solution on the mailinglists. Forcing all people to upgrade CLooG just
> > to make life for a few a little easier does not sound useful to me.
> >
> > However I do not have a strong opinion about .6 and .7
>
> Let's say that CLooG-PPL 0.15.7 or later revision is required for gcc4.5.

Please restrict the requirement to .5 (otherwise bootstrap fails).

Thanks,
Richard.