This is the mail archive of the crossgcc@sourceware.org mailing list for the crossgcc project.

See crosstool-NG for lots more information.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add support for latest CLooG and ISL required by GCC 4.8


Dmitry, All,

[Please, do not post as HTML]

On Thursday 14 March 2013 Plotnikov Dmitry wrote:
> Current GCC trunk requires CLooG version 0.17.0 and later which is now 
> using libisl instead of PPL.  New Graphite also requires libisl 
> (http://gcc.gnu.org/wiki/Graphite-4.8).  This patch adds an ability to 
> build current GCC trunk by adding support for the new CLooG versions and 
> libisl to the companion libraries.  I'm not sure that this is the 
> most beautiful way to implement this, but I hope that this patch will be 
> useful for the community.

Next time, please post the patch in the mail body, it is easier to review
and comment. You can use 'hg email' to send patches in properly-formatted
emails.

Also, you need to add your Signed-off-by line to each of your patches. See:
    http://elinux.org/Developer_Certificate_Of_Origin

> diff -rupd ct-ng-1.18-clean/lib/ct-ng.1.18.0/config/cc/gcc.in.2 ct-ng-1.18/lib/ct-ng.1.18.0/config/cc/gcc.in.2
> --- ct-ng-1.18-clean/lib/ct-ng.1.18.0/config/cc/gcc.in.2        2013-02-01 00:07:52.000000000 +0400
> +++ ct-ng-1.18/lib/ct-ng.1.18.0/config/cc/gcc.in.2      2013-03-14 14:45:06.985719092 +0400
> @@ -85,6 +85,16 @@ config CC_GCC_USE_GRAPHITE
> 
>        On some systems (eg. Cygwin), PPL and/or CLooG may not
>        build properly (yet), so you'll have to say 'N' here.
> 
> +config CC_GCC_GRAPHITE_ISL
> +    bool
> +    prompt "Enable ISL for GRAPHITE"
> +    default n
> +    depends on CC_GCC_USE_GRAPHITE
> +    select ISL_NEEDED
> +    help
> +      GCC trunk versions later than 14 Jan 2013 requires ISL
> +      library for GRAPHITE

If ISL is needed, the selection should be automatic, not left to the user
to decide.

Use the "CC_GCC_X_Y_or_later" semantics (with X.Y the gcc version),
that selects the required libs. See how it is done for, eg. gcc 4.7
with the symbol "CC_GCC_4_7_or_later".

Please, can you confirm the following (I think I read that somewhere,
can't find it again, though...):
  - even if CLooG now requires ISL and not PPL, PPL is still needed
    directly by gcc, right?
(so, gcc still needs to "select CC_GCC_NEEDS_PPL")

>  # The way LTO works is a bit twisted.
>  # See: http://gcc.gnu.org/wiki/LinkTimeOptimization#Requirements
> 
>  # Basically:
> diff -rupd ct-ng-1.18-clean/lib/ct-ng.1.18.0/config/companion_libs/cloog.in ct-ng-1.18/lib/ct-ng.1.18.0/config/companion_libs/cloog.in
> --- ct-ng-1.18-clean/lib/ct-ng.1.18.0/config/companion_libs/cloog.in   2013-02-01 00:07:52.000000000 +0400
> +++ ct-ng-1.18/lib/ct-ng.1.18.0/config/companion_libs/cloog.in  2013-03-14 15:21:08.309806972 +0400
[--SNIP--]
> @@ -38,6 +53,9 @@ config CLOOG_VERSION
> 
>      string
>  # Don't remove next line
>  # CT_INSERT_VERSION_STRING_BELOW
> +    default "0.18.0" if CLOOG_V_0_18_0
> +    default "0.18.0" if CLOOG_V_0_16_2
> +    default "0.18.0" if CLOOG_V_0_16_1

Err... be careful with the version strings, they all are 0.18.0, even the
0.16.x and 0.17.x ones.

Are the 0.16.x and 0.17.x really needed? Can't we just bump to 0.18.0?

> @@ -45,9 +63,17 @@ config CLOOG_VERSION
>      default "0.15.7" if CLOOG_V_0_15_7
>      default "0.15.6" if CLOOG_V_0_15_6

> +config CLOOG_0_16_OR_LATER
> +    bool

In all the other "_or_later" symbols, the "-Or_later" is lower-case, not
upper-case.

(Note: if not addind 0.16.x and 0.17.x, and only adding 0.18.0, rename
this symbol to "CLOOG_0_18_or_later").

> +    select CLOOG_WITHOUT_PPL
> +
[--SNIP--]
> +config CLOOG_WITHOUT_PPL
> +    bool
> +

I'd rather we use positive semantics, rather than negative. What about:

config CLOOG_0_18_or_later
    bool

config CLOOG_NEEDS_PPL
    def_bool y
    depends on ! CLOOG_0_16_OR_LATER

And then, propagate the "CLOOG_NEEDS_PPL" down to where it is meaningfull.

[--SNIP--]
> diff -rupd ct-ng-1.18-clean/lib/ct-ng.1.18.0/scripts/build/cc/gcc.sh ct-ng-1.18/lib/ct-ng.1.18.0/scripts/build/cc/gcc.sh
> --- ct-ng-1.18-clean/lib/ct-ng.1.18.0/scripts/build/cc/gcc.sh   2013-02-01 00:07:53.000000000 +0400
> +++ ct-ng-1.18/lib/ct-ng.1.18.0/scripts/build/cc/gcc.sh 2013-03-14 15:18:54.957801549 +0400
> @@ -289,6 +289,9 @@ do_cc_core_backend() {
>              host_libstdcxx_flags+=("-lpwl")
>          fi
>          extra_config+=("--with-cloog=${complibs}")
> +        if [ "${CT_CLOOG_WITHOUT_PPL}" = "y" ]; then

Again, use positive semantics, something like:
    if [ "${CT_CLOOG_WITH_PPL}" = "y" ]; then

> +            extra_config+=("--with-isl=${complibs}")
> +        fi
>      elif [ "${CT_CC_GCC_HAS_GRAPHITE}" = "y" ]; then
>          extra_config+=("--with-ppl=no")
>          extra_config+=("--with-cloog=no")
> @@ -695,6 +705,9 @@ do_cc_backend() {
>              host_libstdcxx_flags+=("-lpwl")
>          fi
>          extra_config+=("--with-cloog=${complibs}")
> +        if [ "${CT_CLOOG_WITHOUT_PPL}" = "y" ]; then

Ditto.

> +            extra_config+=("--with-isl=${complibs}")
> +        fi
>      elif [ "${CT_CC_GCC_HAS_GRAPHITE}" = "y" ]; then
>          extra_config+=("--with-ppl=no")
>          extra_config+=("--with-cloog=no")
> diff -rupd ct-ng-1.18-clean/lib/ct-ng.1.18.0/scripts/build/companion_libs/130-cloog.s h ct-ng-1.18/lib/ct-ng.1.18.0/scripts/build/companion_libs/130-cloog.sh
> --- ct-ng-1.18-clean/lib/ct-ng.1.18.0/scripts/build/companion_libs/130-cloog.s h 2013-02-01 00:07:53.000000000 +0400
> +++ ct-ng-1.18/lib/ct-ng.1.18.0/scripts/build/companion_libs/130-cloog.sh     2013-03-14 15:17:17.981797607 +0400
> @@ -10,9 +10,15 @@ do_cloog_for_host() { :; }
> # Overide functions depending on configuration
> if [ "${CT_CLOOG}" = "y" ]; then
> 
> +PPL_PREFIX="-ppl"
> +
> +if [ "${CT_CLOOG_WITHOUT_PPL}" = "y" ]; then
> +  PPL_PREFIX=""
> +fi

if [ "${CT_CLOOG_WITH_PPL}" = "y" ]; then
    PPL_PREFIX=
else
    PPL_PREFIX="-ppl"
fi

[--SNIP--]
> @@ -114,7 +120,7 @@ do_cloog_backend() {
>          --enable-static
>      
>      CT_DoLog EXTRA "Building CLooG/ppl"
> -    CT_DoExecLog ALL make ${JOBSFLAGS} libcloog.la
> +    CT_DoExecLog ALL make ${JOBSFLAGS} all

Did you check this still worked with previous versions of GLooG?
Why do you want to build all instead of just the library?

We just need the library, and IIRC the change was done on purpose; but
revision ab0759ba292c is not very verbose as to why this change was
needed. Blame me... :-(

>      if [ "${CT_COMPLIBS_CHECK}" = "y" ]; then
>          CT_DoLog EXTRA "Checking CLooG/ppl"
> @@ -122,7 +128,7 @@ do_cloog_backend() {
>      fi
>      
>      CT_DoLog EXTRA "Installing CLooG/ppl"
> -    CT_DoExecLog ALL make install-libLTLIBRARIES install-pkgincludeHEADERS
> +    CT_DoExecLog ALL make install

Ditto. 

>  }
>  
>  fi # CT_CLOOG
> diff -rupd ct-ng-1.18-clean/lib/ct-ng.1.18.0/scripts/showSamples.sh
> ct-ng-1.18/lib/ct-ng.1.18.0/scripts/showSamples.sh ---
> ct-ng-1.18-clean/lib/ct-ng.1.18.0/scripts/showSamples.sh    2013-02-01
> 00:07:53.000000000 +0400 +++
> ct-ng-1.18/lib/ct-ng.1.18.0/scripts/showSamples.sh  2013-03-14
> 14:26:15.229673072 +0400 @@ -56,12 +56,14 @@ dump_single_sample() {
> 
>                   -o -n "${CT_MPFR}"             \
>                   -o -n "${CT_PPL}"              \
>                   -o -n "${CT_CLOOG}"            \
> +                 -o -n "${CT_ISL}"              \
>                   -o -n "${CT_MPC}"              \
>                   -o -n "${CT_LIBELF}"           \
>                   -o -n "${CT_GMP_TARGET}"       \
>                   -o -n "${CT_MPFR_TARGET}"      \
>                   -o -n "${CT_PPL_TARGET}"       \
>                   -o -n "${CT_CLOOG_TARGET}"     \
> +                 -o -n "${CT_ISL_TARGET}"       \

Hmm... Unrelated to your changes, but all these _TARGET should go,
they are never defined.

[--SNIP--]
> diff -rupdN ct-ng-1.18-clean/lib/ct-ng.1.18.0/scripts/build/companion_libs/135-isl.sh ct-ng-1.18/lib/ct-ng.1.18.0/scripts/build/companion_libs/135-isl.sh
> --- ct-ng-1.18-clean/lib/ct-ng.1.18.0/scripts/build/companion_libs/135-isl.sh   1970-01-01 03:00:00.000000000 +0300
> +++ ct-ng-1.18/lib/ct-ng.1.18.0/scripts/build/companion_libs/135-isl.sh 2013-03-13 16:26:17.122452838 +0400
> @@ -0,0 +1,122 @@
> +# This file adds the functions to build the ISL library
> +# Copyright 2009 Yann E. MORIN
> +# Licensed under the GPL v2. See COPYING in the root of this package
> +
> +do_isl_get() { :; }
> +do_isl_extract() { :; }
> +do_isl_for_build() { :; }
> +do_isl_for_host() { :; }
> +
> +# Overide functions depending on configuration
> +if [ "${CT_CLOOG}" = "y" ]; then

This is ISL, not GLooG --> if [ "${CT_ISL}" = "y" ]; then

> +# Download ISL
> +do_isl_get() {
> +    CT_GetFile "isl-0.11.1"  \
> +        ftp://gcc.gnu.org/pub/gcc/infrastructure

Instead of using gcc's copy of ISL, can't we get it directly form the
official ISL upstream?

> +}
> +
> +# Extract ISL
> +do_isl_extract() {
> +    local _t
> +
> +    CT_Extract "isl-0.11.1"
> +
> +    # Help the autostuff in case it thinks there are things to regenerate...
> +    CT_DoExecLog DEBUG mkdir -p "${CT_SRC_DIR}/isl-0.11.1/m4"

Is it really needed for isl? Looks like not, as you're not regenerating
the autotools stuff.

> +
> +#    if [ "${CT_CLOOG_NEEDS_AUTORECONF}" = "y" ]; then
> +#        CT_Pushd "${CT_SRC_DIR}/isl-0.11.1"
> +#        CT_DoExecLog CFG ./autogen.sh
> +#        CT_Popd
> +#    fi

Commented code: just remove it.

> +}
> +
> +# Build ISL for running on build
> +# - always build statically
> +# - we do not have build-specific CFLAGS
> +# - install in build-tools prefix
> +do_isl_for_build() {
> +    local -a cloog_opts

We're speaking about ISL here, not cloog: rename the variable.

[--SNIP--]
> +# Build ISL for running on host
> +do_isl_for_host() {
> +    local -a cloog_opts

Ditto.

Otherwise, this file looks good.

[--SNIP--]
> diff -rupdN ct-ng-1.18-clean/lib/ct-ng.1.18.0/config/companion_libs/isl.in ct-ng-1.18/lib/ct-ng.1.18.0/config/companion_libs/isl.in
> --- ct-ng-1.18-clean/lib/ct-ng.1.18.0/config/companion_libs/isl.in      1970-01-01 03:00:00.000000000 +0300
> +++ ct-ng-1.18/lib/ct-ng.1.18.0/config/companion_libs/isl.in    2013-03-14 14:38:24.325702720 +0400
[--SNIP--]

This file looks good, too.

Thanks for the submission! :-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

--
For unsubscribe information see http://sourceware.org/lists.html#faq


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]