diff mbox

Support official CLooG.org versions.

Message ID 20101112040945.GA23026@bromo.med.uc.edu
State New
Headers show

Commit Message

Jack Howarth Nov. 12, 2010, 4:09 a.m. UTC
On Fri, Nov 12, 2010 at 02:29:58AM +0000, Joseph S. Myers wrote:
> On Thu, 11 Nov 2010, Jack Howarth wrote:
> 
> > On Thu, Nov 11, 2010 at 06:41:03PM -0500, Jack Howarth wrote:
> > > 
> > > Sebastian,
> > >    Do these new changes require cloog-ppl 0.15.10 rather than 0.15.9? I am
> > > finding the the x86_64-apple-darwin bootstrap fails with...
> > > 
> > > checking for version 0.10 (or later revision) of PPL... yes
> > > checking for installed CLooG... no
> > > configure: error: Unable to find a usable CLooG. See config.log for details.
> > > ### execution of /var/tmp/tmp.1.UpSRyE failed, exit code 1
> > > 
> > 
> > Sebastian,
> >    I finally caught on that the recent check-ins completely depreciate the use
> > of cloog-ppl. It would be nice if the configure error message alerted the user
> > to that fact. Installed cloog-org from the git using the recommended commands...
> 
> If the configure changes have the effect of deprecating something or 
> adjusting the required versions of something, there is a serious problem 
> and they should not have been approved: the documentation of prerequisites 
> in install.texi does not appear to have been changed and it still 
> specifies CLooG-PPL.  It is not acceptable to change the prerequisites 
> without including the documentation changes in the same commit.
> 

Joseph.
   Actually the problem with building against the legacy cloog-ppl 0.15.9 seems
to be that configure's test omits passing ${ppllibs} to LDFLAGS. The gross hack...


allows configure to detect the installed legacy cloog. This may need to be
adjusted to avoid passing ${ppllibs} to the cloog-isl test.
                Jack

> > Is there any documentation on how to build cloog-ppl from the cloog.org git?
> > Also what will be the recommended build configuration for gcc's graphite support
> > in gcc 4.6? Will it be ppl 0.11 and cloog-isl or cloog-ppl?
> 
> Regarding recommended configurations the same principle as before still 
> applies: the version of GCC and the configure options should determine the 
> code generated, so given any particular configure options it should not 
> accept different CLooG variants that may cause GCC to generate different 
> code, it should accept only the recommended version unless 
> sanity-check-disabling options such as --disable-cloog-version-check are 
> passed.
> 
> -- 
> Joseph S. Myers
> joseph@codesourcery.com

Comments

Paolo Bonzini Nov. 12, 2010, 8:26 a.m. UTC | #1
On 11/12/2010 05:09 AM, Jack Howarth wrote:
>> If the configure changes have the effect of deprecating something or
>> adjusting the required versions of something, there is a serious problem
>> and they should not have been approved: the documentation of prerequisites
>> in install.texi does not appear to have been changed and it still
>> specifies CLooG-PPL.  It is not acceptable to change the prerequisites
>> without including the documentation changes in the same commit.

That was not mentioned in the discussion of the patch.  In fact I 
suppose Cloog-PPL was simply not tested.  Andreas, did you try all 
possible configurations?

Also, if I understood correctly ISL and PPL are different ways to "do 
the same thing", and they should cause no differences in code 
generation.  I assumed this because the patch didn't require any 
testsuite adjustment.  Is this the case?  If so, we only need to 
document the new possible choice of Cloog libraries.

If not, however, as Joseph said we need: 1) documentation of the new 
prerequisites; 2) a --enable-cloog-ppl option to enable Cloog-PPL tests 
_and_ disable Cloog-ISL at the same time.  In this case, I suggest 
reverting the patch on trunk.

Paolo
Paolo Bonzini Nov. 12, 2010, 8:59 a.m. UTC | #2
On 11/12/2010 09:43 AM, Sven Verdoolaege wrote:
> On Fri, Nov 12, 2010 at 09:26:00AM +0100, Paolo Bonzini wrote:
>> Also, if I understood correctly ISL and PPL are different ways to
>> "do the same thing", and they should cause no differences in code
>> generation.  I assumed this because the patch didn't require any
>> testsuite adjustment.  Is this the case?
>
> Semantically, the results should be the same, but there may
> be syntactic differences.  Perhaps no such syntactic differences
> occur for the gcc testsuite.

What does "semantic" and "syntactic" mean?  I suppose you mean that the 
produced code should be correct in any case (of course) but the GCC 
assembly language output may change?  This is what we care about.

Paolo
Paolo Bonzini Nov. 12, 2010, 9:10 a.m. UTC | #3
On 11/12/2010 10:08 AM, Sven Verdoolaege wrote:
> On Fri, Nov 12, 2010 at 09:59:45AM +0100, Paolo Bonzini wrote:
>> On 11/12/2010 09:43 AM, Sven Verdoolaege wrote:
>>> On Fri, Nov 12, 2010 at 09:26:00AM +0100, Paolo Bonzini wrote:
>>>> Also, if I understood correctly ISL and PPL are different ways to
>>>> "do the same thing", and they should cause no differences in code
>>>> generation.  I assumed this because the patch didn't require any
>>>> testsuite adjustment.  Is this the case?
>>>
>>> Semantically, the results should be the same, but there may
>>> be syntactic differences.  Perhaps no such syntactic differences
>>> occur for the gcc testsuite.
>>
>> What does "semantic" and "syntactic" mean?  I suppose you mean that
>> the produced code should be correct in any case (of course) but the
>> GCC assembly language output may change?
>
> Exactly.

That's bad.  Sebastian, please revert the patch.  It would also be 
appreciated to compile SPEC with both backends, and see how many 
different decisions are taken.

Paolo
diff mbox

Patch

Index: configure
===================================================================
--- configure	(revision 166643)
+++ configure	(working copy)
@@ -5730,7 +5731,7 @@ 
 
     CFLAGS="${CFLAGS} ${clooginc} ${pplinc} ${gmpinc}"
   CPPFLAGS="${CPPFLAGS} ${_clooglegacyinc} ${_cloogorginc}"
-  LDFLAGS="${LDFLAGS} ${clooglibs}"
+  LDFLAGS="${LDFLAGS} ${clooglibs} ${ppllibs}"
 
   { $as_echo "$as_me:${as_lineno-$LINENO}: checking for installed CLooG" >&5
 $as_echo_n "checking for installed CLooG... " >&6; }