Comments

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.
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

On 11/12/2010 05:14 PM, Tobias Grosser wrote:
> 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)
Yes, the bug that Jack found is not a major problem.
The main problem is the different generated code; in particular, it's
not about failures, _any_ assembly code difference resulting from the
same configure options is problematic. Can you make a patch to ensure
that only one backend is sought for, depending on the user's choice
between --enable-cloog-backend=ppl and --enable-cloog-backend=isl (you
choose the default)?
To some extent, this is unavoidable when we are delegating optimization
choices to an external library, but we should minimize the differences.
It is probably necessary to have the version of cloog (and its
dependencies whenever applicable) printed as part of the output of "gcc
--version".
Still, the patch is okay. :)
Paolo