Patchwork Move Graphite from using PPL over to ISL

login
register
mail settings
Submitter Richard Guenther
Date July 3, 2012, 8:28 a.m.
Message ID <alpine.LNX.2.00.1207031022200.17233@jbgna.fhfr.qr>
Download mbox | patch
Permalink /patch/168731/
State New
Headers show

Comments

Richard Guenther - July 3, 2012, 8:28 a.m.
On Mon, 2 Jul 2012, Nenad Vukicevic wrote:

> On 6/27/2012 8:06 AM, Richard Guenther wrote:
> > This merges from the graphite branch the move of PPL to ISL,
> > and completes it where it was lacking - thanks to Micha.
> > It leaves unmerged the addition of a pluto-like ISL optimizer
> > as well as a bugfix for stride > 1 which did not come with
> > a testcase.
> > 
> > With this patch (ontop of the one requiring ClooG 0.17.0)
> > we will require ISL 0.10 for enabling Graphite.
> > 
> > I've bootstrapped and built various combinations with in-tree
> > and out-of-tree cloog and ISL, so I'm pretty confident that
> > this works.
> > 
> > With out-of-tree ClooG and ISL a slightly older patch ontop of its
> > prerequesite passed bootstrap and testing on x86_64-unknown-linux-gnu.
> > 
> > Currently re-bootstrapping and testing on x86_64-unknown-linux-gnu.
> > 
> > Ok for trunk?
> 
> After trying to build from the trunk I got this error on x86_64 platform:
> 
> make[3]: Entering directory `/eng/upc/dev/nenad/bart/bld-trunk/cloog/test'
>   CC     generate_test.o
> cd ..; make  libcloog-isl.la
> make[4]: Entering directory `/eng/upc/dev/nenad/bart/bld-trunk/cloog'
>   CC     libcloog_isl_la-domain.lo
> In file included from
> ../../gcc-trunk/cloog/include/cloog/isl/constraintset.h:4:0,
>                  from ../../gcc-trunk/cloog/include/cloog/isl/cloog.h:9,
>                  from ../../gcc-trunk/cloog/source/isl/domain.c:6:
> ../../gcc-trunk/cloog/include/cloog/isl/backend.h:4:28: fatal error:

^^^

This means that cloog was not able to build against ISL 0.10.  You
should see in its configury that it uses the ISL version installed
on the system (and picks up the one just built from the drop-in
in the source tree).

That is, cloog is configured via

host_modules= { module= cloog; lib_path=.libs; bootstrap=true;
                extra_configure_flags='--disable-shared 
--with-gmp-library=$$r/$(HOST_SUBDIR)/gmp/.libs 
--with-gmp-include=$$r/$(HOST_SUBDIR)/gmp --with-bits=gmp 
--with-isl=system';
                extra_exports='CPPFLAGS="-I$$r/$(HOST_SUBDIR)/isl/include 
-I$$s/isl/include ${CPPFLAGS}"; export CPPFLAGS; 
LDFLAGS="-L$$r/$(HOST_SUBDIR)/isl/.libs $$LDFLAGS"; export LDFLAGS; ';
                extra_make_flags='CPPFLAGS="${CPPFLAGS}" 
LDFLAGS="$$LDFLAGS"';
                no_install= true; };

thus using --with-isl=system, via CPPFLAGS it should be able to pick up
the just built ISL.  You need to debug why that is not working.

I suppose for installed cloog / ISL we could strengthen the configure
check or re-order the includes so that the ISL include dir comes before
the cloog one (if you happen to build against two different ISL versions).

Thus,

$(OUTPUT_OPTION)

but the question is whether this will hide real issues that will
pop up when cloog is built against a different ISL version than
0.10 (for example against the version that is included within the
cloog tarball).

So - can you double-check what happens?  I did test both in-tree
and out-of-tree builds, with wrong versions and correct versions.

Thanks,
Richard.
Richard Guenther - July 3, 2012, 8:48 a.m.
On Tue, 3 Jul 2012, Richard Guenther wrote:

> On Mon, 2 Jul 2012, Nenad Vukicevic wrote:
> 
> > On 6/27/2012 8:06 AM, Richard Guenther wrote:
> > > This merges from the graphite branch the move of PPL to ISL,
> > > and completes it where it was lacking - thanks to Micha.
> > > It leaves unmerged the addition of a pluto-like ISL optimizer
> > > as well as a bugfix for stride > 1 which did not come with
> > > a testcase.
> > > 
> > > With this patch (ontop of the one requiring ClooG 0.17.0)
> > > we will require ISL 0.10 for enabling Graphite.
> > > 
> > > I've bootstrapped and built various combinations with in-tree
> > > and out-of-tree cloog and ISL, so I'm pretty confident that
> > > this works.
> > > 
> > > With out-of-tree ClooG and ISL a slightly older patch ontop of its
> > > prerequesite passed bootstrap and testing on x86_64-unknown-linux-gnu.
> > > 
> > > Currently re-bootstrapping and testing on x86_64-unknown-linux-gnu.
> > > 
> > > Ok for trunk?
> > 
> > After trying to build from the trunk I got this error on x86_64 platform:
> > 
> > make[3]: Entering directory `/eng/upc/dev/nenad/bart/bld-trunk/cloog/test'
> >   CC     generate_test.o
> > cd ..; make  libcloog-isl.la
> > make[4]: Entering directory `/eng/upc/dev/nenad/bart/bld-trunk/cloog'
> >   CC     libcloog_isl_la-domain.lo
> > In file included from
> > ../../gcc-trunk/cloog/include/cloog/isl/constraintset.h:4:0,
> >                  from ../../gcc-trunk/cloog/include/cloog/isl/cloog.h:9,
> >                  from ../../gcc-trunk/cloog/source/isl/domain.c:6:
> > ../../gcc-trunk/cloog/include/cloog/isl/backend.h:4:28: fatal error:

Looking again this is from the cloog build.  If I repeat your tests
(I suppose I didn't check non-bootstrap in-tree builds) it works just
fine for me.  The cloog/config.log should contain something like

configure:11964: checking which isl to use
configure:11966: result: system

and all gcc invocations in the configury should have include paths
set up to have the built isl/include first, then the source isl/include:

configure:12073: gcc -c -g -O2 -I/abuild/rguenther/obj/./isl/include 
-I/space/rguenther/src/svn/trunk/isl/include  conftest.c >&5

Did you really unpack isl into the source tree as well?

Ah, I see what might happen - if you forget to have isl inside the
tree we disable graphite but cloog still gets built, but will fail.
I'll see to fix that.

Thanks,
Richard.
Richard Guenther - July 3, 2012, 8:55 a.m.
On Tue, 3 Jul 2012, Richard Guenther wrote:

> On Tue, 3 Jul 2012, Richard Guenther wrote:
> 
> > On Mon, 2 Jul 2012, Nenad Vukicevic wrote:
> > 
> > > On 6/27/2012 8:06 AM, Richard Guenther wrote:
> > > > This merges from the graphite branch the move of PPL to ISL,
> > > > and completes it where it was lacking - thanks to Micha.
> > > > It leaves unmerged the addition of a pluto-like ISL optimizer
> > > > as well as a bugfix for stride > 1 which did not come with
> > > > a testcase.
> > > > 
> > > > With this patch (ontop of the one requiring ClooG 0.17.0)
> > > > we will require ISL 0.10 for enabling Graphite.
> > > > 
> > > > I've bootstrapped and built various combinations with in-tree
> > > > and out-of-tree cloog and ISL, so I'm pretty confident that
> > > > this works.
> > > > 
> > > > With out-of-tree ClooG and ISL a slightly older patch ontop of its
> > > > prerequesite passed bootstrap and testing on x86_64-unknown-linux-gnu.
> > > > 
> > > > Currently re-bootstrapping and testing on x86_64-unknown-linux-gnu.
> > > > 
> > > > Ok for trunk?
> > > 
> > > After trying to build from the trunk I got this error on x86_64 platform:
> > > 
> > > make[3]: Entering directory `/eng/upc/dev/nenad/bart/bld-trunk/cloog/test'
> > >   CC     generate_test.o
> > > cd ..; make  libcloog-isl.la
> > > make[4]: Entering directory `/eng/upc/dev/nenad/bart/bld-trunk/cloog'
> > >   CC     libcloog_isl_la-domain.lo
> > > In file included from
> > > ../../gcc-trunk/cloog/include/cloog/isl/constraintset.h:4:0,
> > >                  from ../../gcc-trunk/cloog/include/cloog/isl/cloog.h:9,
> > >                  from ../../gcc-trunk/cloog/source/isl/domain.c:6:
> > > ../../gcc-trunk/cloog/include/cloog/isl/backend.h:4:28: fatal error:
> 
> Looking again this is from the cloog build.  If I repeat your tests
> (I suppose I didn't check non-bootstrap in-tree builds) it works just
> fine for me.  The cloog/config.log should contain something like
> 
> configure:11964: checking which isl to use
> configure:11966: result: system
> 
> and all gcc invocations in the configury should have include paths
> set up to have the built isl/include first, then the source isl/include:
> 
> configure:12073: gcc -c -g -O2 -I/abuild/rguenther/obj/./isl/include 
> -I/space/rguenther/src/svn/trunk/isl/include  conftest.c >&5
> 
> Did you really unpack isl into the source tree as well?
> 
> Ah, I see what might happen - if you forget to have isl inside the
> tree we disable graphite but cloog still gets built, but will fail.
> I'll see to fix that.

Fixed with the following.  I'll test it with some combinations of
valid/invalid cloog/isl versions before installing.

Thanks for the report!
Richard.

2012-07-03  Richard Guenther  <rguenther@suse.de>

	config/
	* cloog.m4: Remove debugging print.

	* configure.ac: If either the ISL or the CLooG check failed
	do not try to build in-tree versions.
	* configure: Regenerated.

Index: config/cloog.m4
===================================================================
*** config/cloog.m4	(revision 189158)
--- config/cloog.m4	(working copy)
*************** AC_DEFUN([CLOOG_INIT_FLAGS],
*** 67,73 ****
    dnl source, set up flags to use that.
    if test "x${clooginc}" == x && test "x${clooglibs}" == x \
       && test -d ${srcdir}/cloog; then
-      echo FooBar
       clooglibs='-L$$r/$(HOST_SUBDIR)/cloog/'"$lt_cv_objdir"' '
       clooginc='-I$$r/$(HOST_SUBDIR)/cloog/include -I$$s/cloog/include -I'${srcdir}'/cloog/include '
    fi
--- 67,72 ----
Index: configure.ac
===================================================================
*** configure.ac	(revision 189158)
--- configure.ac	(working copy)
*************** fi
*** 1535,1541 ****
  dnl Provide configure switches and initialize clooginc & clooglibs
  dnl with user input.
  CLOOG_INIT_FLAGS
! if test "x$with_cloog" != "xno"; then
    dnl The minimal version of CLooG required for Graphite.
    dnl
    dnl If we use CLooG-Legacy, the provided version information is
--- 1535,1544 ----
  dnl Provide configure switches and initialize clooginc & clooglibs
  dnl with user input.
  CLOOG_INIT_FLAGS
! if test "x$isllibs" = x && test "x$islinc" = x; then
!   clooglibs=
!   clooginc=
! elif test "x$with_cloog" != "xno"; then
    dnl The minimal version of CLooG required for Graphite.
    dnl
    dnl If we use CLooG-Legacy, the provided version information is
*************** if test "x$with_cloog" != "xno"; then
*** 1547,1552 ****
--- 1550,1561 ----
      AC_MSG_ERROR([Unable to find a usable CLooG.  See config.log for details.])])
  fi
  
+ # If either the ISL or the CLooG check failed, disable builds of in-tree
+ # variants of both
+ if test "x$clooglibs" = x && test "x$clooginc" = x; then
+   noconfigdirs="$noconfigdirs cloog isl"
+ fi
+ 
  # Check for LTO support.
  AC_ARG_ENABLE(lto,
  [AS_HELP_STRING([--enable-lto], [enable link time optimization support])],
Richard Guenther - July 3, 2012, 9:41 a.m.
On Tue, 3 Jul 2012, Richard Guenther wrote:

> On Tue, 3 Jul 2012, Richard Guenther wrote:
> 
> > On Tue, 3 Jul 2012, Richard Guenther wrote:
> > 
> > > On Mon, 2 Jul 2012, Nenad Vukicevic wrote:
> > > 
> > > > On 6/27/2012 8:06 AM, Richard Guenther wrote:
> > > > > This merges from the graphite branch the move of PPL to ISL,
> > > > > and completes it where it was lacking - thanks to Micha.
> > > > > It leaves unmerged the addition of a pluto-like ISL optimizer
> > > > > as well as a bugfix for stride > 1 which did not come with
> > > > > a testcase.
> > > > > 
> > > > > With this patch (ontop of the one requiring ClooG 0.17.0)
> > > > > we will require ISL 0.10 for enabling Graphite.
> > > > > 
> > > > > I've bootstrapped and built various combinations with in-tree
> > > > > and out-of-tree cloog and ISL, so I'm pretty confident that
> > > > > this works.
> > > > > 
> > > > > With out-of-tree ClooG and ISL a slightly older patch ontop of its
> > > > > prerequesite passed bootstrap and testing on x86_64-unknown-linux-gnu.
> > > > > 
> > > > > Currently re-bootstrapping and testing on x86_64-unknown-linux-gnu.
> > > > > 
> > > > > Ok for trunk?
> > > > 
> > > > After trying to build from the trunk I got this error on x86_64 platform:
> > > > 
> > > > make[3]: Entering directory `/eng/upc/dev/nenad/bart/bld-trunk/cloog/test'
> > > >   CC     generate_test.o
> > > > cd ..; make  libcloog-isl.la
> > > > make[4]: Entering directory `/eng/upc/dev/nenad/bart/bld-trunk/cloog'
> > > >   CC     libcloog_isl_la-domain.lo
> > > > In file included from
> > > > ../../gcc-trunk/cloog/include/cloog/isl/constraintset.h:4:0,
> > > >                  from ../../gcc-trunk/cloog/include/cloog/isl/cloog.h:9,
> > > >                  from ../../gcc-trunk/cloog/source/isl/domain.c:6:
> > > > ../../gcc-trunk/cloog/include/cloog/isl/backend.h:4:28: fatal error:
> > 
> > Looking again this is from the cloog build.  If I repeat your tests
> > (I suppose I didn't check non-bootstrap in-tree builds) it works just
> > fine for me.  The cloog/config.log should contain something like
> > 
> > configure:11964: checking which isl to use
> > configure:11966: result: system
> > 
> > and all gcc invocations in the configury should have include paths
> > set up to have the built isl/include first, then the source isl/include:
> > 
> > configure:12073: gcc -c -g -O2 -I/abuild/rguenther/obj/./isl/include 
> > -I/space/rguenther/src/svn/trunk/isl/include  conftest.c >&5
> > 
> > Did you really unpack isl into the source tree as well?
> > 
> > Ah, I see what might happen - if you forget to have isl inside the
> > tree we disable graphite but cloog still gets built, but will fail.
> > I'll see to fix that.
> 
> Fixed with the following.  I'll test it with some combinations of
> valid/invalid cloog/isl versions before installing.

Hum.  Ok, I installed the following, but in-tree builds seem to no
longer work for some reason.  CPPFLAGS get dropped in some way,
even as they are present properly ...

Richard.

2012-07-03  Richard Guenther  <rguenther@suse.de>

	config/
	* cloog.m4: Remove debugging print.

	* Makefile.def (cloog): Add V=1 to extra_make_flags.
	* configure.ac: If either the ISL or the CLooG check failed
	do not try to build in-tree versions.
	* Makefile.in: Regenerated.
	* configure: Regenerated.

Index: config/cloog.m4
===================================================================
*** config/cloog.m4	(revision 189158)
--- config/cloog.m4	(working copy)
*************** AC_DEFUN([CLOOG_INIT_FLAGS],
*** 67,73 ****
    dnl source, set up flags to use that.
    if test "x${clooginc}" == x && test "x${clooglibs}" == x \
       && test -d ${srcdir}/cloog; then
-      echo FooBar
       clooglibs='-L$$r/$(HOST_SUBDIR)/cloog/'"$lt_cv_objdir"' '
       clooginc='-I$$r/$(HOST_SUBDIR)/cloog/include -I$$s/cloog/include -I'${srcdir}'/cloog/include '
    fi
--- 67,72 ----
Index: configure.ac
===================================================================
*** configure.ac	(revision 189158)
--- configure.ac	(working copy)
*************** fi
*** 1535,1541 ****
  dnl Provide configure switches and initialize clooginc & clooglibs
  dnl with user input.
  CLOOG_INIT_FLAGS
! if test "x$with_cloog" != "xno"; then
    dnl The minimal version of CLooG required for Graphite.
    dnl
    dnl If we use CLooG-Legacy, the provided version information is
--- 1535,1544 ----
  dnl Provide configure switches and initialize clooginc & clooglibs
  dnl with user input.
  CLOOG_INIT_FLAGS
! if test "x$isllibs" = x && test "x$islinc" = x; then
!   clooglibs=
!   clooginc=
! elif test "x$with_cloog" != "xno"; then
    dnl The minimal version of CLooG required for Graphite.
    dnl
    dnl If we use CLooG-Legacy, the provided version information is
*************** if test "x$with_cloog" != "xno"; then
*** 1547,1552 ****
--- 1550,1561 ----
      AC_MSG_ERROR([Unable to find a usable CLooG.  See config.log for details.])])
  fi
  
+ # If either the ISL or the CLooG check failed, disable builds of in-tree
+ # variants of both
+ if test "x$clooglibs" = x && test "x$clooginc" = x; then
+   noconfigdirs="$noconfigdirs cloog isl"
+ fi
+ 
  # Check for LTO support.
  AC_ARG_ENABLE(lto,
  [AS_HELP_STRING([--enable-lto], [enable link time optimization support])],
Index: Makefile.def
===================================================================
*** Makefile.def	(revision 189203)
--- Makefile.def	(working copy)
*************** host_modules= { module= isl; lib_path=.l
*** 69,75 ****
  host_modules= { module= cloog; lib_path=.libs; bootstrap=true;
  		extra_configure_flags='--disable-shared --with-gmp-library=$$r/$(HOST_SUBDIR)/gmp/.libs --with-gmp-include=$$r/$(HOST_SUBDIR)/gmp --with-bits=gmp --with-isl=system';
  		extra_exports='CPPFLAGS="-I$$r/$(HOST_SUBDIR)/isl/include -I$$s/isl/include ${CPPFLAGS}"; export CPPFLAGS; LDFLAGS="-L$$r/$(HOST_SUBDIR)/isl/.libs $$LDFLAGS"; export LDFLAGS; ';
! 		extra_make_flags='CPPFLAGS="${CPPFLAGS}" LDFLAGS="$$LDFLAGS"';
  		no_install= true; };
  host_modules= { module= libelf; lib_path=.libs; bootstrap=true;
  		extra_configure_flags='--disable-shared';
--- 69,75 ----
  host_modules= { module= cloog; lib_path=.libs; bootstrap=true;
  		extra_configure_flags='--disable-shared --with-gmp-library=$$r/$(HOST_SUBDIR)/gmp/.libs --with-gmp-include=$$r/$(HOST_SUBDIR)/gmp --with-bits=gmp --with-isl=system';
  		extra_exports='CPPFLAGS="-I$$r/$(HOST_SUBDIR)/isl/include -I$$s/isl/include ${CPPFLAGS}"; export CPPFLAGS; LDFLAGS="-L$$r/$(HOST_SUBDIR)/isl/.libs $$LDFLAGS"; export LDFLAGS; ';
! 		extra_make_flags='CPPFLAGS="${CPPFLAGS}" LDFLAGS="$$LDFLAGS" V=1';
  		no_install= true; };
  host_modules= { module= libelf; lib_path=.libs; bootstrap=true;
  		extra_configure_flags='--disable-shared';

Patch

Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in     (revision 189158)
+++ gcc/Makefile.in     (working copy)
@@ -1057,7 +1057,7 @@  BUILD_ERRORS = build/errors.o
 INCLUDES = -I. -I$(@D) -I$(srcdir) -I$(srcdir)/$(@D) \
           -I$(srcdir)/../include @INCINTL@ \
           $(CPPINC) $(GMPINC) $(DECNUMINC) \
-          $(CLOOGINC) $(ISLINC)
+          $(ISLINC) $(CLOOGINC)
 
 .c.o:
        $(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $<