This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Support official CLooG.org versions.
On Fri, Nov 12, 2010 at 11:14:38AM -0500, Tobias Grosser wrote:
> On 11/12/2010 04:10 AM, Paolo Bonzini wrote:
>> 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.
>
> Hi,
>
> I just tested this again and Jack Howarth's analysis was correct. CLooG
> PPL is not detected, if PPL is not in the default path. This slipped
> through our tests. As long as PPL is installed in the system library
> path everything is all right.
Tobias,
Are you sure your analysis is correct? It seems to me that the problem
is actually due to the code section in config/cloog.m4 below...
# CLOOG_FIND_FLAGS ()
# ------------------
# Detect the used CLooG-backend and set clooginc/clooglibs/cloog_org.
# Preference: CLooG-PPL (Legacy) > CLooG-ISL > CLooG-PPL
AC_DEFUN([CLOOG_FIND_FLAGS],
[
AC_REQUIRE([CLOOG_INIT_FLAGS])
_cloog_saved_CFLAGS=$CFLAGS
_cloog_saved_CPPFLAGS=$CPPFLAGS
_cloog_saved_LDFLAGS=$LDFLAGS
_cloog_saved_LIBS=$LIBS
_clooglegacyinc="-DCLOOG_PPL_BACKEND"
_cloogorginc="-DCLOOG_INT_GMP -DCLOOG_ORG"
dnl clooglibs & clooginc may have been initialized by CLOOG_INIT_FLAGS.
CFLAGS="${CFLAGS} ${clooginc} ${pplinc} ${gmpinc}"
CPPFLAGS="${CPPFLAGS} ${_clooglegacyinc} ${_cloogorginc}"
LDFLAGS="${LDFLAGS} ${clooglibs}"
AC_CACHE_CHECK([for installed CLooG],
[gcc_cv_cloog_type],
[LIBS="-lcloog ${_cloog_saved_LIBS}"
AC_LINK_IFELSE([_CLOOG_PPL_LEGACY_PROG],
[gcc_cv_cloog_type="PPL Legacy"],
[LIBS="-lcloog-isl -lisl ${_cloog_saved_LIBS}"
AC_LINK_IFELSE([_CLOOG_ORG_PROG],
[gcc_cv_cloog_type=ISL],
[LIBS="-lcloog-ppl ${_cloog_saved_LIBS}"
AC_LINK_IFELSE([_CLOOG_ORG_PROG],
[gcc_cv_cloog_type=PPL],
[gcc_cv_cloog_type=no])])])])
case $gcc_cv_cloog_type in
"PPL Legacy")
clooginc="${clooginc} ${_clooglegacyinc}"
clooglibs="${clooglibs} -lcloog"
cloog_org=no
;;
"ISL")
clooginc="${clooginc} ${_cloogorginc}"
clooglibs="${clooglibs} -lcloog-isl"
cloog_org=yes
;;
"PPL")
clooginc="${clooginc} ${_cloogorginc}"
clooglibs="${clooglibs} -lcloog-ppl"
cloog_org=yes
;;
*)
clooglibs=
clooginc=
cloog_org=
;;
esac
LIBS=$_cloog_saved_LIBS
CFLAGS=$_cloog_saved_CFLAGS
CPPFLAGS=$_cloog_saved_CPPFLAGS
LDFLAGS=$_cloog_saved_LDFLAGS
]
)
In particular, the lines...
dnl clooglibs & clooginc may have been initialized by CLOOG_INIT_FLAGS.
CFLAGS="${CFLAGS} ${clooginc} ${pplinc} ${gmpinc}"
CPPFLAGS="${CPPFLAGS} ${_clooglegacyinc} ${_cloogorginc}"
LDFLAGS="${LDFLAGS} ${clooglibs}"
where ${pplinc} is blindly passed to all the tests on CFLAGS but
${ppllibs} is never passed to LDFLAGS for the legacy cloog test.
Since only the legacy cloog test directly makes a call into ppl...
# _CLOOG_PPL_LEGACY_PROG ()
# -------------------------
# Helper for detecting CLooG-Legacy (CLooG-PPL).
m4_define([_CLOOG_PPL_LEGACY_PROG], [AC_LANG_PROGRAM(
[#include <cloog/cloog.h>],
[ppl_version_major ()])])
...it would seem that ${pplinc} and ${ppllibs} only need to be
provided to CFLAGS and LDFLAGS respectively in that case.
Jack
> This is definitely a bug in the patch, as the patch was not intended to
> change the default behavior at all. I propose to fix this bug.
> A patch for this is attached (the one Jack proposed).
>
> Regarding the syntactic difference:
>
> CLooG isl will in some cases generate different code. This is why we
> currently have support for both CLooG versions. Our first tests did not
> show any new SPEC suite failures, but we still want to run more tests
> with different optimization flags and on even more platforms. We push
> that patch upstream, such that people like Jack can test CLooG with
> CLooG-isl on their platform. (Even if it was not intended they hit such
> a simple configure bug)
>
> I believe with the attached patch, the know CLooG configure bugs are
> fixed and configure should act as it was planned.
>
> The behavior is:
>
> configure will detect CLooG PPL if available in the default search paths
> or pointed to using the --with-cloog flag.
> Only in the case no CLooG PPL is available but a version of CLooG ISL is
> installed or pointed explicitly to using the --with-cloog flag, gcc will
> compile against cloog-isl.
>
> cloog-isl is currently unreleased (only available using git).
>
> Is this behavior OK for configure? The idea is to keep it like this for
> a couple of weeks, such that more people can try CLooG isl and help us
> finding the remaining bugs.
>
> The final goal for gcc 4.6 is to remove CLooG ppl support.
>
> Is this plan as explained and finally implemented (with the attached
> fix) OK, or do we need to change anything else?
>
> Cheers
> Tobi
> >From a8d97d865ab5e0af7e1a760ee6769dfb877231a1 Mon Sep 17 00:00:00 2001
> From: Tobias Grosser <grosser@fim.uni-passau.de>
> Date: Thu, 11 Nov 2010 22:54:55 -0500
> Subject: [PATCH 2/2] Pass PPL libraries to CLooG version check
>
> * config/cloog.m4: Pass ppl libraries to the CLooG version check.
> * configure: Regenerate.
> ---
> config/cloog.m4 | 4 ++--
> configure | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/config/cloog.m4 b/config/cloog.m4
> index d24b5ac..87ff50a 100644
> --- a/config/cloog.m4
> +++ b/config/cloog.m4
> @@ -122,7 +122,7 @@ AC_DEFUN([CLOOG_FIND_FLAGS],
> dnl clooglibs & clooginc may have been initialized by CLOOG_INIT_FLAGS.
> CFLAGS="${CFLAGS} ${clooginc} ${pplinc} ${gmpinc}"
> CPPFLAGS="${CPPFLAGS} ${_clooglegacyinc} ${_cloogorginc}"
> - LDFLAGS="${LDFLAGS} ${clooglibs}"
> + LDFLAGS="${LDFLAGS} ${clooglibs} ${ppllibs}"
>
> AC_CACHE_CHECK([for installed CLooG],
> [gcc_cv_cloog_type],
> @@ -206,7 +206,7 @@ AC_DEFUN([CLOOG_CHECK_VERSION],
> _cloog_saved_LDFLAGS=$LDFLAGS
>
> CFLAGS="${_cloog_saved_CFLAGS} ${clooginc} ${pplinc} ${gmpinc}"
> - LDFLAGS="${_cloog_saved_LDFLAGS} ${clooglibs}"
> + LDFLAGS="${_cloog_saved_LDFLAGS} ${clooglibs} ${ppllibs}"
>
> if test "${cloog_org}" = yes ; then
> AC_CACHE_CHECK([for verison $1.$2.$3 of CLooG],
> diff --git a/configure b/configure
> index 3f8c26a..a85874e 100755
> --- a/configure
> +++ b/configure
> @@ -5730,7 +5730,7 @@ if test "x$with_cloog" != "xno"; then
>
> 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; }
> @@ -5835,7 +5835,7 @@ $as_echo "$gcc_cv_cloog_type" >&6; }
> _cloog_saved_LDFLAGS=$LDFLAGS
>
> CFLAGS="${_cloog_saved_CFLAGS} ${clooginc} ${pplinc} ${gmpinc}"
> - LDFLAGS="${_cloog_saved_LDFLAGS} ${clooglibs}"
> + LDFLAGS="${_cloog_saved_LDFLAGS} ${clooglibs} ${ppllibs}"
>
> if test "${cloog_org}" = yes ; then
> { $as_echo "$as_me:${as_lineno-$LINENO}: checking for verison 0.14.0 of CLooG" >&5
> --
> 1.7.1
>